Re: [lwip-devel] netifapi limitations

[Sylvain Rochet]

Hi Jonathan,


On Fri, Mar 16, 2018 at 02:23:02AM +0000, Jonathan Larmour wrote:
> Hi all, popping my head round the door after a bit of a while :-).
> 
> I've noticed a limitation in the netifapi (not the netif API!) : Many of the
> functions require a struct netif* argument, however the netifapi is intended
> to be used outside of the TCPIP thread.
> 
> The main problem is that there is no way to obtain a netif using the netifapi.
> You can convert to/from indexes and names, but can't get the netif itself,
> except arguably 'netif_default', but that won't help those with multiple
> interfaces. Perhaps we could add a function to look up and return a struct
> netif* based on a name/index? But that doesn't seem very safe to me given what
> the netifapi is for: resilience against netifs being added/removed in another
> thread.
> 
> Perhaps it could be argued that keeping lwIP simple means making compromises,
> and maybe this is a compromise to make. But overall IMHO, the netif API would
> be better off removing all mentions of struct netif* entirely. Instead each
> function should take an if index.
> 

> That brings me to a second issue, netif_find() assumes that the netif 
> name is exactly three chars long, the third being the if number from 
> 0-9.

Which version of lwIP are you using ? It doesn't seem to be the case.
http://git.savannah.nongnu.org/cgit/lwip.git/tree/src/core/netif.c?id=6070a7ef6a9c1fd4f9b7d2dd16b412110a466244#n1656


> The number is assigned in netif_add() by incrementing a local 
> static variable each time. The problem here is that this means you can 
> only ever have added 10 interfaces before the API breaks, even if you 
> have removed netifs in the meantime. Even if we parsed the name 
> argument as a normal NUL-terminated string, the index is a u8_t. If 
> there is a possibility that people can be adding/removing netif's 
> dynamically and repeatedly (think USB or PPP), this can't work long.

We take care of this issue in netif_add() by iterating over the netif 
list to find a free entry if necessary.
http://git.savannah.nongnu.org/cgit/lwip.git/tree/src/core/netif.c?id=6070a7ef6a9c1fd4f9b7d2dd16b412110a466244#n367


> The index probably wants to be u32_t for safety given how long 
> embedded devices can be up for, and netif_find() needs to accept a 
> normal NUL-terminated string and use strtoul() on the number 
> component.

Same here, this is exactly what netif_find() is doing.


> Next, if the numbers are part of the name, but are assigned by lwIP, how can
> netif API users even find an interface except in the most simple static cases,
> and making assumptions about interface initialisation order? It makes me think
> we need an iterator function in netifapi to report the next used index (which
> in practice just traverses the netif list). For example:
> 
>   err_t err = 0;
>   /* NETIFAPI_GET_FIRST_INDEX is a magic value to get index of first netif */
>   for ( index = NETIFAPI_GET_FIRST_INDEX;
>         ;
>         index = next_index )
>   {
>     err = netifapi_get_next_index(index, &next_index);
>     if ( err != 0 )
>       break;
>     ... Do stuff with interface 'next_index' ...
>   }
> 
> I don't know how official the netif api is, so whether it is okay to break the
> API this much, but I can't really see it being correct or particularly usable
> even at the moment.
> 
> Comments? I'm prepared to put in some work to make this happen.

Since netif struct is not allocated/freed by the stack itself, how is 
getting a netif pointer a problem at all ?


Sylvain

signature.asc
Description: Digital signature