[lwip-devel] [bug #20409] Add netif callback function for link change event

[Frédéric Bernon]

Follow-up Comment #38, bug #20409 (project lwip):

>If we give the user "master" powers over a netif, then he expects the stack
to obey them

If you want to power off an netif, the right thing to do is the down it and
to remove it

About the patch file:

>netif->driver_callback = NULL;

Can you confirm that driver_callback is to provide the information to the
application? In this case, link_status will be a better name.

>#if LWIP_NETIF_LINK_CALLBACK
> if ( netif->driver_callback )
> (netif->driver_callback)( netif );
>#endif

It's not specific to this callback, but to avoid such block, I propose to
change with at the beginning of the file:

#if LWIP_NETIF_LINK_CALLBACK
#define NETIF_LINK_CALLBACK(n) if (n->link_callback)
                               (n->link_callback)(n)
#else
#define NETIF_LINK_CALLBACK(n)
#define   
#endif /* LWIP_NETIF_LINK_CALLBACK */

And place use it in all file where the callback is invoked (same for
status_callback. It's more a good pratice we should do in most file (in fact,
I think this is already the case). Perhaps one callback for both cases, with a
"netif_change" parameter will be better (a flag saying what is the event)?

About the place to do the "gratuitous ARP", I think netif_set_up stay better
(see https://savannah.nongnu.org/patch/index.php?5824, comment#2 &
comment#3). But since it's only two lines, ok to call it when the link become
up (like you do), but not in netif_set_ipaddr.

>netif_set_down
I think it's wrong, since down the netif doesn't have to stop the dhcp client
by example. It's another reason to let the module do the job...

>netif_link_change

I prefer two functions (netif_set_link_up/down). It will also set/rest the
NETIF_FLAG_LINK_UP flag. About calling dhcp_link_change/autoip_link_change, I
always thing that do the job inside the module timer is better (direct calls
from netif could cause circular references problems). Last you forget IGMP
module.


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?20409>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/