[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lwip-devel] netifapi limitations
From: |
Sylvain Rochet |
Subject: |
Re: [lwip-devel] netifapi limitations |
Date: |
Fri, 16 Mar 2018 11:46:32 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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