[lwip-users] Re: [lwip] ARP bug (compare MAC instead of IP)

[leon . woestenberg]

Hello John,

you are correct.

Currently, without timeouts, however, it is not appropriate to store more
than one
(IP, MAC) mapping for each MAC, as the cache grows dirty over time. Which
results in miscommunication, or communication starvation.

Something I experienced myself. My own "solution" was to compare MAC
addresses,
as I (falsy) reasoned that any interface needs only one (IP,MAC) mapping in
order to
be reachable on the physical network. Later I added timeouts, without
rethinking the
stuff.

In short, both IP address comparison AND timeouts are necessary to ensure
correct
operation.

Regards,

Leon Woestenberg.



                                                                                
                                         
                    "John C. Toman"                                             
                                         
                    <address@hidden       To:     address@hidden                
                                       
                    -tech.com>                cc:                               
                                         
                    Sent by:                  Subject:     Re: [lwip] ARP bug 
(compare MAC instead of IP)                
                    address@hidden                                              
                                     
                                                                                
                                         
                                                                                
                                         
                    17-12-01 19:05                                              
                                         
                    Please respond to                                           
                                         
                    lwip                                                        
                                         
                                                                                
                                         
                                                                                
                                         




Hi Leon,

It is very possible for one MAC address to be configured for two or more
IP addresses! In my own office environment, this is done for various
reasons. Proxy ARP (often used for PPP and firewalls) is another case
where this is done. Thus it is appropriate to use IP addresses as a
comparison point, not MAC addresses.

With an ARP cache, and the scenario you mentioned (where an IP address
is changed), the old entry should quickly time out when it isn't used.
Your scenario is one of the key arguments for quick ARP cache timeouts.
Timeouts aren't in the current lwIP ARP implementation but will be at
some point. Another stack I've used sets ARP cache timeouts for five
minutes. Other than not implementing timeouts, I believe Adam's
implementation of this is correct.

Regards,

John

address@hidden wrote:

>Hello Adam,
>
>in the ARP code, when adding an (MAC, IP) entry, you use IP
>addresses to compare. You should, however, use the MAC
>addresses to find an old entry.
>
>----
>To see what goes wrong, using IP address compare, an example:
>
>Suppose client with MAC 01:02:03:04:05:06 has obtained
>IP address 192.168.0.2, and by whatever means, is configured
>to use 192.168.0.3 now.
>
>Adding this (MAC, IP) mapping will not find the old entry, and create
>a new entry in the ARP cache.  The ARP cache will now hold two
>mappings with the same MAC address.
>----
>
>static void
>add_arp_entry(struct ip_addr *ipaddr, struct eth_addr *ethaddr)
>{
>  u8_t i, j, k;
>  u8_t maxage;
>
>  /* Walk through the ARP mapping table and try to find an entry to
>     update. If none is found, the IP -> MAC address mapping is
>     inserted in the ARP table. */
>  for(i = 0; i < ARPTABLE_SIZE; ++i) {
>
>    /* Only check those entries that are actually in use. */
>    if(!ip_addr_isany(&arp_table[i].ipaddr)) {
>      /* Check if the source IP address of the incoming packet matches
>         the IP address in this ARP table entry. */
>      if(ip_addr_cmp(ipaddr, &arp_table[i].ipaddr)) {
><--------------------------- should test MAC bytes here, not IP address
>     /* An old entry found, update this and return. */
>
>[This message was sent through the lwip discussion list.]
>



[This message was sent through the lwip discussion list.]




[This message was sent through the lwip discussion list.]