netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Joseph Huang <Joseph.Huang@garmin.com>
Cc: netdev@vger.kernel.org,
	Joseph Huang <joseph.huang.2024@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	David Ahern <dsahern@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>,
	Kuniyuki Iwashima <kuniyu@google.com>,
	Ahmed Zaki <ahmed.zaki@intel.com>,
	Alexander Lobakin <aleksander.lobakin@intel.com>,
	linux-kernel@vger.kernel.org, bridge@lists.linux.dev
Subject: Re: [PATCH net] net: bridge: Trigger host query on v6 addr valid
Date: Sat, 13 Sep 2025 21:23:18 +0300	[thread overview]
Message-ID: <aMW2lvRboW_oPyyP@shredder> (raw)
In-Reply-To: <20250912223937.1363559-1-Joseph.Huang@garmin.com>

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().

6. Please extend bridge_mld.sh with a test case in a separate patch. You
can look at xstats to see if queries were sent or not. See for example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aea45363e29dd16050e6ce333ce0d3696ac3b5a9

Thanks

  reply	other threads:[~2025-09-13 18:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-12 22:39 [PATCH net] net: bridge: Trigger host query on v6 addr valid Joseph Huang
2025-09-13 18:23 ` Ido Schimmel [this message]
2025-09-15 22:41   ` Huang, Joseph
2025-09-17 11:30     ` Ido Schimmel
2025-10-04 14:27       ` Linus Lüssing
2025-10-06 15:43         ` Huang, Joseph
2025-10-08 12:28           ` Ido Schimmel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aMW2lvRboW_oPyyP@shredder \
    --to=idosch@nvidia.com \
    --cc=Joseph.Huang@garmin.com \
    --cc=ahmed.zaki@intel.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=bridge@lists.linux.dev \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=joseph.huang.2024@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=sdf@fomichev.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).