Subject: Re: [PATCH net-2.6] bonding: drop frames received with master's source MAC

From: Nicolas de Pesloüan<[email-masked]>
Date: Sat, 26 Feb 2011 00:08:03 +0100

Le 25/02/2011 23:24, Andy Gospodarek a écrit :
> On Fri, Feb 25, 2011 at 11:04:27PM +0100, Nicolas de Pesloüan wrote:
>> Le 25/02/2011 22:13, Andy Gospodarek a écrit :
>>> I was looking at my system and wondering why I sometimes saw these
>>> DAD messages in my logs:
>>>
>>> bond0: IPv6 duplicate address fe80::21b:21ff:fe38:2ec4 detected!
>>>
>>> I traced it back and realized the IPv6 Neighbor Solicitations I was
>>> sending were also coming back into the stack on the slave(s) that did
>>> not transmit the frames.  I could not think of a compelling reason to
>>> notify the user that a NS we sent came back, so I set out to just drop
>>> the frame silently in ndisc_recv_ns drop.
>>>
>>> That seemed to work well, but when I thought about it I could not
>>> compelling reason to save any of these frames.  Dropping them as soon as
>>> we get them seems like a much better idea as it fixes other issues that
>>> may exist for more than just IPv6 DAD.
>>>
>>> I chose to check the incoming frame against the master's MAC address as
>>> that should be the MAC address used anytime a broadcast frame is sent by
>>> the bonding driver that had the chance to make its way back into one of
>>> the other devices.
>>
>> I think this could break the ARP monitoring. ARP monitoring rely on a
>> normal protocol handler, registered in bond_main.c.
>>
>> void bond_register_arp(struct bonding *bond)
>> {
>>          struct packet_type *pt =&bond->arp_mon_pt;
>>
>>          if (pt->type)
>>                  return;
>>
>>          pt->type = htons(ETH_P_ARP);
>>          pt->dev = bond->dev;
>>          pt->func = bond_arp_rcv;
>>          dev_add_pack(pt);
>> }
>>
>> For as far as I understand, some variants of arp_validate require the
>> backup interfaces to receive ARP requests sent from the master, through
>> the active interface, presumably with the master MAC as the source MAC.
>>
>> As this protocol handler is registered at the master level, the exact
>> match logic in __netif_receive_skb(), which apply at the slave level,
>> shouldn't deliver this skb to bond_arp_rcv().
>>
>> Can someone confirm ? Jay ?
>>
>> 	Nicolas.
>>
>
> I confirmed your suspicion, this breaks ARP monitoring.  I would still
> welcome other opinions though as I think it would be nice to fix this as
> low as possible.

Why do you want to fix it earlier that in ndisc_recv_ns drop? Your original idea of silently 
dropping the frame there seems perfect to me.

	Nicolas.