[U-Boot,v2,3/3] net: Work-around for brain-damaged Cisco equipment with arp-proxy

Cisco's arp-proxy feature fails to ignore the link-local address range
This means that a link-local device on a network with this Cisco
equipment will reply to ARP requests for our device (in addition to
our reply).
If we happen to reply first, the requester's ARP table will be
populated with our MAC address, and one packet will be sent to us...
shortly following this, the requester will get an ARP reply from the
Cisco equipment telling the requester to send packets their way
instead of to our device from now on.
This work-around detects this link-local condition and will delay
replying to the ARP request for 5ms so that the first packet is sent
to the Cisco equipment and all following packets are sent to our
device.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Mike Frysinger <vapier@gentoo.org>
---
Changes for v2:
   - Guard with #ifdef CONFIG_CMD_LINK_LOCAL

 net/arp.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

Hi Joe,

On Tue, Mar 27, 2012 at 4:43 PM, Joe Hershberger <joe.hershberger@ni.com> wrote:
> Cisco's arp-proxy feature fails to ignore the link-local address range
> This means that a link-local device on a network with this Cisco
> equipment will reply to ARP requests for our device (in addition to
> our reply).
> If we happen to reply first, the requester's ARP table will be
> populated with our MAC address, and one packet will be sent to us...
> shortly following this, the requester will get an ARP reply from the
> Cisco equipment telling the requester to send packets their way
> instead of to our device from now on.
> This work-around detects this link-local condition and will delay
> replying to the ARP request for 5ms so that the first packet is sent
> to the Cisco equipment and all following packets are sent to our
> device.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Mike Frysinger <vapier@gentoo.org>
> ---
> Changes for v2:
>   - Guard with #ifdef CONFIG_CMD_LINK_LOCAL
>
>  net/arp.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/net/arp.c b/net/arp.c
> index 7599d30..56c5da8 100644
> --- a/net/arp.c
> +++ b/net/arp.c
> @@ -169,6 +169,20 @@ void ArpReceive(struct Ethernet_hdr *et, struct IP_UDP_hdr *ip, int len)
>                NetCopyIP(&arp->ar_tpa, &arp->ar_spa);
>                memcpy(&arp->ar_sha, NetOurEther, ARP_HLEN);
>                NetCopyIP(&arp->ar_spa, &NetOurIP);
> +
> +#ifdef CONFIG_CMD_LINK_LOCAL
> +               /*
> +                * Work-around for brain-damaged Cisco equipment with
> +                *   arp-proxy enabled.
> +                *
> +                *   If the requesting IP is not on our subnet, wait 5ms to
> +                *   reply to ARP request so that our reply will overwrite
> +                *   the arp-proxy's instead of the other way around.
> +                */
> +               if ((NetReadIP(&arp->ar_tpa) & NetOurSubnetMask) !=
> +                   (NetReadIP(&arp->ar_spa) & NetOurSubnetMask))
> +                       udelay(5000);
> +#endif

I'm sure this solves the problem, but is 5ms enough, and should we
make this a CONFIG option so it can be turned off if needed?

>                NetSendPacket((uchar *)et, eth_hdr_size + ARP_HDR_SIZE);
>                return;
>
> --
> 1.6.0.2
>

Regards,
Simon

Hi Simon,

On Fri, Apr 20, 2012 at 9:44 PM, Simon Glass <sjg@chromium.org> wrote:
>> +#ifdef CONFIG_CMD_LINK_LOCAL
>> +               /*
>> +                * Work-around for brain-damaged Cisco equipment with
>> +                *   arp-proxy enabled.
>> +                *
>> +                *   If the requesting IP is not on our subnet, wait 5ms to
>> +                *   reply to ARP request so that our reply will overwrite
>> +                *   the arp-proxy's instead of the other way around.
>> +                */
>> +               if ((NetReadIP(&arp->ar_tpa) & NetOurSubnetMask) !=
>> +                   (NetReadIP(&arp->ar_spa) & NetOurSubnetMask))
>> +                       udelay(5000);
>> +#endif
>
> I'm sure this solves the problem, but is 5ms enough, and should we
> make this a CONFIG option so it can be turned off if needed?

In my experience it is enough.  I think I'd rather keep it simple
until someone sees an environment where it needs configurability... in
which case it should probably be an env variable.

-Joe