Re: [lwip-users] ARP ETHARP_TRY_HARD

[Leon Woestenberg]

Hello Paul,

Paul Clarke wrote:

> Hi Leon.
>
> The reason why line 494 needs to be for_us = 1;
> A few lines later we have
>
>  case ARP_REPLY:
>    /* ARP reply. We already updated the ARP cache earlier. */
>    LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("etharp_arp_input: incoming ARP
> reply\n"));
> #if (LWIP_DHCP && DHCP_DOES_ARP_CHECK)
>    /* DHCP wants to know about ARP replies to our wanna-have-address */
>    if (for_us) dhcp_arp_reply(netif, &sipaddr);
> #endif
>    break;
>
> The other option is to change the code there to
>
> #if (LWIP_DHCP && DHCP_DOES_ARP_CHECK)
>    /* DHCP wants to know about ARP replies to our wanna-have-address */
>    if (for_us || (netif->ip_addr.addr == 0)) dhcp_arp_reply(netif,
> &sipaddr);
> #endif
>
> Which is better?
>  
The first is not OK as the for_us also works for all ARP traffic, 
requests as well. Setting for_us = 1
for a request when the interface is not configured would be bad.

The last one is better, although I now spot a subtle bug in the current 
code:

If the interface in non-configured (0.0.0.0), for_us will be 0 and the 
dhcp_arp_reply() will not be
called.

This would be correct:

#if (LWIP_DHCP && DHCP_DOES_ARP_CHECK)
   /* When unconfigured, DHCP wants to know about ARP replies from the
    * address offered to us, as that means someone else uses it already! */
   if (netif->ip_addr.addr == 0) dhcp_arp_reply(netif, &sipaddr);
#endif

Changed in CVS.

Regards,

Leon.