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 <[email protected]>
>> ---
>> 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
