patchlwIP - A Lightweight TCP/IP stack - Patches: patch #5824, gratuitous ARP

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #5824: gratuitous ARP

Submitter:  Frédéric Bernon <fbernon>
Submitted:  Mon 26 Mar 2007 08:35:44 PM UTC
   
 
Category:  None Priority:  3 - Low
Status:  Done Privacy:  Public
Assigned to:  fbernon Open/Closed:  Closed
Planned Release:  None

Jump to the original submission

Fri 30 Mar 2007 10:28:21 AM UTC, comment #12: 

Yes, no, perhaps... Because file name is ethernetif, I could say "yes", but this skeleton can be used as sample - with change - for non-ethernet devices and I think "no", perhaps add it with a comment would be good. I will do that...

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 10:19:30 AM UTC, comment #11: 

Just came across my mind:  do we need to mark netif with NETIF_FLAG_ETHARP in src/netif/ethernetif.c even throught it's just a sample?

Tai-hwa Liang <atliang>
Fri 30 Mar 2007 10:11:51 AM UTC, comment #10: 

Ah, you're right.  Thank you for the quick and correct fix. I was thinking about inverting the flag to NETIF_FLAG_NO_ETHARP... ;)

Tai-hwa Liang <atliang>
Fri 30 Mar 2007 09:23:07 AM UTC, comment #9: 

I think your patch is not good, because non-ethernet interfaces are not break, just because... NETIF_FLAG_ETHARP should only be used on ethernet devices (which can used etharp_ functions)!!! :)

loopif, slipif and ppp don't have to set (and don't set) NETIF_FLAG_ETHARP.

But this problem is more that

if ((netif->flags & NETIF_FLAG_ETHARP) == 0) in netif_set_up

should be

if ((netif->flags & NETIF_FLAG_ETHARP) != 0)

Thank for this fix...

Frédéric Bernon <fbernon>
Group Member
Fri 30 Mar 2007 09:16:26 AM UTC, comment #8: 

Hmm... this change breaks non-ethernet interfaces such like loopif, slipif and ppp since they did not have a valid netif->linkoutput().

System initialized.
TCP/IP initialized.
[New Thread 0x28210520 (LWP 100065)]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x28210520 (LWP 100065)]
0x00000000 in ?? ()
(gdb) where
#0  0x00000000 in ?? ()
#1  0x080588c1 in etharp_request (netif=0x806b440, ipaddr=0x806b444)
    at ../../../../../lwip/src/netif/etharp.c:913
#2  0x08058566 in etharp_query (netif=0x806b440, ipaddr=0x806b444, q=0x0)
    at ../../../../../lwip/src/netif/etharp.c:807
#3  0x0804ad05 in netif_set_up (netif=0x806b440)
    at ../../../../../lwip/src/core/netif.c:310
#4  0x08048fd2 in main_thread (arg=0x0) at simhost.c:378
#5  0x280dd701 in pthread_create () from /lib/libpthread.so.2
#6  0x281a9e73 in _ctx_start () from /lib/libc.so.7
(gdb) up                
#1  0x080588c1 in etharp_request (netif=0x806b440, ipaddr=0x806b444)
    at ../../../../../lwip/src/netif/etharp.c:913
913         result = netif->linkoutput(netif, p);
(gdb) print netif->linkoutput
$2 = (err_t ()(struct netif , struct pbuf *)) 0
(gdb)

The attached patch tries to mark these interfaces with NETIF_FLAG_ETHARP; however, I don't know if doing a NULL validation in etharp.c is better:

etharp_request()
{
  .
  .
  if (netif->linkoutput != NULL)
    netif->linkoutput(netif, p);
  .
  .
}

 or, simply provide a dummy linkoutput() for these interfaces.

(file #12343)

Tai-hwa Liang <atliang>
Wed 28 Mar 2007 09:24:48 AM UTC, comment #7: 

Ok, commited with Simon's request...

Frédéric Bernon <fbernon>
Group Member
Wed 28 Mar 2007 07:31:31 AM UTC, comment #6: 

ok for  LWIP_ARP. Just, I don't understand "Including non-relevant comments in the patch"? Can you tell what part is? The /* LWIP_SNMP */ (and co)? If you confirm, I will try to avoid in patch file next time (but are you agree to commit like this, to avoid to have new changelog with "just add comments on #endif"?).

Frédéric Bernon <fbernon>
Group Member
Wed 28 Mar 2007 07:05:09 AM UTC, comment #5: 

Including non-relevant comments in the patch makes it more difficult to read, I think. The 'core' of the patch seems OK. Only it would be good to #ifdef LWIP_ETHARP (or what it is called now?) the inclusion of etharp.h and the call to etharp_query(). If that's included, I'm happy with the patch.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 27 Mar 2007 07:21:49 PM UTC, comment #4: 

Patch attach. Ok to commit?

"2007-03-27 Frédéric Bernon

  • netif.h, netif.c: A new NETIF_FLAG_ETHARP flag is defined in netif.h, to allow to initialize a network interface's flag with. It tell this interface is an ethernet device, and we can use ARP with it to do a "gratuitous ARP" (RFC 3220 "IP Mobility Support for IPv4" section 4.6) when interface is "up" with netif_set_up()."


http://www.faqs.org/rfcs/rfc3220.html


(file #12312)

Frédéric Bernon <fbernon>
Group Member
Tue 27 Mar 2007 01:55:04 PM UTC, comment #3: 

No, gratuitous ARP is just to update all subnetwork devices that this host is inline (and to update their ARP cache, by example, when you change your LAN router by another one, it is very useful to avoid to wait ARP cache timeout). It is not used to detect any IP conflict (BTW, is someone got an Autoip/APIPA module?).

About netif_set_up, I'm agree with you (more, during dhcp_release, netif_set_ipaddr is called with IP_ADDR_ANY).

Ok to commit that?

Frédéric Bernon <fbernon>
Group Member
Tue 27 Mar 2007 01:45:04 PM UTC, comment #2: 

Hm, is gratuitous ARP used also to detect duplicate IP addresses? If so, we're not waiting for an answer...

Anyway, I think this would better fit into the netif_set_up, or am I wrong?

Simon Goldschmidt <goldsimon>
Group administrator
Tue 27 Mar 2007 01:05:14 PM UTC, comment #1: 

Some comments about that?

Frédéric Bernon <fbernon>
Group Member
Mon 26 Mar 2007 08:35:44 PM UTC, original submission:  

I would like to reactivate the "gratuitous ARP". In netif.c, in netif_set_ipaddr(), we can see :

#if 0 /* only allowed for Ethernet interfaces TODO: how can we check? */
  /** For Ethernet network interfaces, we would like to send a
   *  "gratuitous ARP"; this is an ARP packet sent by a node in order
   *  to spontaneously cause other nodes to update an entry in their
   *  ARP cache. From RFC 3220 "IP Mobility Support for IPv4" section 4.6.
   */
  etharp_query(netif, ipaddr, NULL);
#endif

So, my idea will be to add in netif.h a new flag like this :

/** if set, the netif is an device using ARP */
#define NETIF_FLAG_ETHARP 0x20U

Each driver would have to declare if it can do "gratuitous ARP" when prepare the netif (like for the NETIF_FLAG_BROADCAST flag).

So, we could check if we have to send a "gratuitous ARP" like that :

if ((netif->flags & NETIF_FLAG_ETHARP) == 0)

Note that, of course, etharp_init have to be called before...


Frédéric Bernon <fbernon>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #12343:  non-eth-arp.patch added by atliang (1KiB - application/octet-stream)
file #12312:  netif.etharp.flag.patch added by fbernon (3KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by atliang (Updated the item)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by fbernon (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-03-30 atliang Attached File- Added non-eth-arp.patch, #12343
    2007-03-28 fbernon StatusWorks For Me Done
        Open/ClosedOpen Closed
    2007-03-27 fbernon Attached File- Added netif.etharp.flag.patch, #12312
        StatusNone Works For Me

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code