Re: [PATCH net] net: bridge: Trigger host query on v6 addr valid

["Huang, Joseph" <joseph.huang.at.garmin@gmail.com>]

On 9/13/2025 2:23 PM, Ido Schimmel wrote:
 > On Fri, Sep 12, 2025 at 06:39:30PM -0400, Joseph Huang wrote:
 >> Trigger the bridge to (re)start sending out Queries to the Host once
 >> IPv6 address becomes valid.
 >>
 >> In current implementation, once the bridge (interface) is brought up,
 >> the bridge will start trying to send v4 and v6 Queries to the Host
 >> immediately. However, at that time most likely the IPv6 address of
 >> the bridge interface is not valid yet, and thus the send (actually
 >> the alloc) operation will fail. So the first v6 Startup Query is
 >> always missed.
 >>
 >> This caused a ripple effect on the timing of Querier Election. In
 >> current implementation, :: always wins the election. In order for
 >> the "real" election to take place, the bridge would have to first
 >> select itself (this happens when a v6 Query is successfully sent
 >> to the Host), and then do the real address comparison when the next
 >> Query is received. In worst cast scenario, the bridge would have to
 >> wait for [Startup Query Interval] seconds (for the second Query to
 >> be sent to the Host) plus [Query Interval] seconds (for the real
 >> Querier to send the next Query) before it can recognize the real
 >> Querier.
 >>
 >> This patch adds a new notification NETDEV_NEWADDR when IPv6 address
 >> becomes valid. When the bridge receives the notification, it will
 >> restart the Startup Queries (much like how the bridge handles port
 >> NETDEV_CHANGE events today).
 >>
 >> Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
 >> ---
 >>   include/linux/netdevice.h |  1 +
 >>   net/bridge/br.c           |  5 +++++
 >>   net/bridge/br_multicast.c | 16 ++++++++++++++++
 >>   net/bridge/br_private.h   |  1 +
 >>   net/core/dev.c            | 10 +++++-----
 >>   net/ipv6/addrconf.c       |  3 +++
 >>   6 files changed, 31 insertions(+), 5 deletions(-)
 >
 > A few comments:
 >
 > 1. The confidentiality footer needs to be removed.
 >
 > 2. Patches targeted at net need to have a Fixes tag. If you cannot
 > identify a commit before which this worked correctly (i.e., it's not a
 > regression), then target the patch at net-next instead.
 >
 > 3. The commit message needs to describe the user visible changes. My
 > understanding is as follows: When the bridge is brought administratively
 > up it will try to send a General Query which requires an IPv6 link-local
 > address to be configured on the bridge device. Because of DAD, such an
 > address might not exist right away, which means that the first General
 > Query will be sent after "mcast_startup_query_interval" seconds.
 >
 > During this time the bridge will be unaware of multicast listeners that
 > joined before the creation of the bridge. Therefore, the bridge will
 > either unnecessarily flood multicast traffic to all the bridge ports or
 > just to those marked as router ports.
 >
 > The patch aims to reduce this time period and send a General Query as
 > soon as the bridge is assigned an IPv6 link-local address.
 >
 > 4. Use imperative mood:
 > 
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
 >
 > 5. There is already a notification chain that notifies about addition /
 > deletion of IPv6 addresses. See register_inet6addr_notifier().
 >

It seems that inet6addr_notifier_call_chain() can be called when the 
address is still tentative, which means br_ip6_multicast_alloc_query() 
is still going to fail (br_ip6_multicast_alloc_query() calls 
ipv6_dev_get_saddr(), which calls __ipv6_dev_get_saddr(), which does not 
consider tentative source addresses).

What the bridge needs really is a notification after DAD is completed, 
but I couldn't find such notification. Or did you mean reusing the same 
notification inet6addr_notifier_call_chain() but with a new event after 
DAD is completed?

Thanks,
Joseph