Re: [PATCHv2 net] bonding: add ns target multicast address to slave device

[Hangbin Liu <liuhangbin@gmail.com>]

On Wed, Oct 30, 2024 at 05:21:05PM +0100, Jay Vosburgh wrote:
> Hangbin Liu <liuhangbin@gmail.com> wrote:
> 
> >Commit 4598380f9c54 ("bonding: fix ns validation on backup slaves")
> >tried to resolve the issue where backup slaves couldn't be brought up when
> >receiving IPv6 Neighbor Solicitation (NS) messages. However, this fix only
> >worked for drivers that receive all multicast messages, such as the veth
> >interface.
> >
> >For standard drivers, the NS multicast message is silently dropped because
> >the slave device is not a member of the NS target multicast group.
> >
> >To address this, we need to make the slave device join the NS target
> >multicast group, ensuring it can receive these IPv6 NS messages to validate
> >the slave’s status properly.
> >
> >There are three policies before joining the multicast group:
> >1. All settings must be under active-backup mode (alb and tlb do not support
> >   arp_validate), with backup slaves and slaves supporting multicast.
> >2. We can add or remove multicast groups when arp_validate changes.
> >3. Other operations, such as enslaving, releasing, or setting NS targets,
> >   need to be guarded by arp_validate.
> >
> >Fixes: 4e24be018eb9 ("bonding: add new parameter ns_targets")
> >Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> >---
> >v2: only add/del mcast group on backup slaves when arp_validate is set (Jay Vosburgh)
> 
> 	Sorry for the delay in responding, I've been traveling.
> 
> 	For the above, I suspect I wasn't sufficiently clear in my
> commentary; what I meant wasn't just checking arp_validate being
> enabled, but that the implementation could be much less complex if it
> simply kept all of the multicast addresses added to the backup interface
> (in addition to the active interface) when arp_validate is enabled.
> 
> 	I suspect the set of multicast addresses involved is likely to
> be small in the usual case, so the question then is whether the
> presumably small amount of traffic that inadvertently passes the filter
> (and is then thrown away by the kernel RX logic) is worth the complexity
> added here.

Hi Jan,

Apologies for troubling you so many times with the same issue. Recently, we
discovered another corner case related to IPv6 NS target validation.

Previously, I mainly focused on backup validation when arp_validate is set
to 2, 3, or 6. However, if arp_validate is set to 0, bond_rcv_validate()
updates last_rx directly upon receiving any packet. The problem occurs when
the backup slave only receives IPv6 NS messages sent by the active slave,
these messages are dropped because the backup slave hasn't joined the NS
multicast group.

So, should we remove the limitation that restricts joining the NS multicast
group only when arp_validate is set?

By the way, another question unrelated to this topic. Does target_last_arp_rx
have any usage? I couldn't find any references to it being used anywhere.

Thanks
Hangbin