[En-Nut-Discussion] Duplicate ARPs

[Harald Kipp]

Hi Dusan,

Yes, it is quite obvious, that the ATF_COM check and the the
request are in wrong order. So obvious, that I start banging
my head on the table. :-)

But as a coding purist, I see two things in your solution:

- 'fl_send' contains information, which is already contained
   in nb.

- 'else' isn't required if the 'if' part returns to the caller.
   (Yeah...OK...that a philosophical issue, but it points me
    to some additional restructuring).

How about this (yet untested) one?

-------------------- snip -----------------------
int NutArpCacheQuery(NUTDEVICE * dev, CONST u_long ip, u_char * mac)
{
     volatile ARPENTRY *entry;
     NETBUF *nb = 0;
     int rc = -1;
     u_char retries = 4;

     /*
      * Search a matching entry. If we none exists, create a new
      * incomplete entry and a request packet.
      */
     if ((entry = NutArpCacheLookup(dev->dev_icb, ip)) == 0) {
         if ((entry = NutArpCacheNew(dev->dev_icb, ip, 0, 0)) == 0) {
             return -1;
         }
         if ((nb = NutArpAllocNetBuf(ARPOP_REQUEST, ip, 0)) == 0) {
             return -1;
         }
     }

     /*
      * We enter a loop, which will send ARP requests on fixed
      * time intervals until our ARP entry gets completed.
      */
     while (retries--) {
         if (entry->ae_flags & ATF_COM) {
             memcpy(mac, (void *)entry->ae_ha, 6);
             rc = 0;
             break;
         }
         if (nb && NutArpOutput(dev, nb)) {
             break;
         }
         /*
          * Sleep until woken up by a new ARP
          * response from the net or on timeout.
          */
         NutEventWait(&entry->ae_tq, 500);
     }
     if (nb) {
         NutNetBufFree(nb);
     }
     return rc;
}
-------------------- snap -----------------------

Harald


At 12:54 13.01.2005 +0100, you wrote:
>Hi Harald,
>
>best questions are those with answers attached. Here is another one 
>(arpcache.c w/o this bug attached) published by courtesy of us :-)
>
>Why there are 2 ARPS when a packet is originating from Nut/OS and there is 
>no entry in ARP cache ?
>
>The code loop in NutArpCacheQuery unrolled (current code in CVS):
>- send ARP request
>- check if ATF_COM was set by ARP response (rxi5 thread)
>         (no other thread run, so no reason why flag can be set)
>- wait for ARP cache entry event
>- send ARP request
>         (here we are with the 2nd ARP)
>- check if ATF_COM was set
>         (of course it is set now, but in the meantime 2nd ARP request was 
> sent)
>
>So we exchanged the order of ATF_COM check and ARP request sending. Voila. 
>It looks like solved.
>
>We also eliminated code that is not really used in NutArpCacheNew().
>
>If you approve this, it can be commited to CVS. RH stands for Rostislav 
>Hlebak from Embedded Technologies.
>
>---
>BTW 1-2.5 ms is the time between ARP response packet entered RTL ethernet 
>side, was processed and new ARP request was generated.