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. 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. 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.
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.