netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: Hangbin Liu <liuhangbin@gmail.com>
Cc: netdev@vger.kernel.org
Subject: Re: [Issue] Bonding can't show correct speed if lower interface is bond 802.3ad
Date: Fri, 28 Apr 2023 09:06:40 -0700	[thread overview]
Message-ID: <15524.1682698000@famine> (raw)
In-Reply-To: <ZEt3hvyREPVdbesO@Laptop-X1>

Hangbin Liu <liuhangbin@gmail.com> wrote:

>A user reported a bonding issue that if we put an active-back bond on top of a
>802.3ad bond interface. When the 802.3ad bond's speed/duplex changed
>dynamically. The upper bonding interface's speed/duplex can't be changed at
>the same time.
>
>This seems not easy to fix since we update the speed/duplex only
>when there is a failover(except 802.3ad mode) or slave netdev change.
>But the lower bonding interface doesn't trigger netdev change when the speed
>changed as ethtool get bonding speed via bond_ethtool_get_link_ksettings(),
>which not affect bonding interface itself.

	Well, this gets back into the intermittent discussion on whether
or not being able to nest bonds is useful or not, and thus whether it
should be allowed or not.  It's at best a niche use case (I don't recall
the example configurations ever being anything other than 802.3ad under
active-backup), and was broken for a number of years without much
uproar.

	In this particular case, nesting two LACP (802.3ad) bonds inside
an active-backup bond provides no functional benefit as far as I'm aware
(maybe gratuitous ARP?), as 802.3ad mode will correctly handle switching
between multiple aggregators.  The "ad_select" option provides a few
choices on the criteria for choosing the active aggregator.

	Is there a reason the user in your case doesn't use 802.3ad mode
directly?

>Here is a reproducer:
>
>```
>#!/bin/bash
>s_ns="s"
>c_ns="c"
>
>ip netns del ${c_ns} &> /dev/null
>ip netns del ${s_ns} &> /dev/null
>sleep 1
>ip netns add ${c_ns}
>ip netns add ${s_ns}
>
>ip -n ${c_ns} link add bond0 type bond mode 802.3ad miimon 100
>ip -n ${s_ns} link add bond0 type bond mode 802.3ad miimon 100
>ip -n ${s_ns} link add bond1 type bond mode active-backup miimon 100
>
>for i in $(seq 0 2); do
>        ip -n ${c_ns} link add eth${i} type veth peer name eth${i} netns ${s_ns}
>        [ $i -eq 2 ] && break
>        ip -n ${c_ns} link set eth${i} master bond0
>        ip -n ${s_ns} link set eth${i} master bond0
>done
>
>ip -n ${c_ns} link set eth2 up
>ip -n ${c_ns} link set bond0 up
>
>ip -n ${s_ns} link set bond0 master bond1
>ip -n ${s_ns} link set bond1 up
>
>sleep 5
>
>ip netns exec ${s_ns} ethtool bond0 | grep Speed
>ip netns exec ${s_ns} ethtool bond1 | grep Speed
>```
>
>When run the reproducer directly, you will see:
># ./bond_topo_lacp.sh
>        Speed: 20000Mb/s
>        Speed: 10000Mb/s
>
>So do you have any thoughts about how to fix it?

	Maybe it's time to disable nesting of bonds, update the
documentation to note that it's disabled and that 802.3ad mode is smart
enough to do multiple aggregators, and then see if anyone has some other
use case and complains.

	In the past, I've been against doing this, but only because it
might break existing configurations.  If nested configurations are going
to misbehave and require complicated shenanigans to fix, then perhaps
it's time to push users into a configuration that works without the
nesting.

	The only thing I can think of that active-backup over 802.3ad
gets is the gratuitous ARP / NS on failover.  If that's the key feature
for nesting, then I'd rather add the grat ARP to 802.3ad aggregator
selection and disable nesting.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

  reply	other threads:[~2023-04-28 16:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-28  7:36 [Issue] Bonding can't show correct speed if lower interface is bond 802.3ad Hangbin Liu
2023-04-28 16:06 ` Jay Vosburgh [this message]
2023-05-08  9:26   ` Hangbin Liu
2023-05-08 18:32     ` Jay Vosburgh
2023-05-09  3:16       ` Hangbin Liu
2023-05-10  7:50       ` Hangbin Liu
2023-05-10 16:57         ` Andrew J. Schorr
2023-05-10 17:14           ` Andrew J. Schorr
2023-05-12  1:38             ` Jay Vosburgh
2023-05-12 14:44               ` Andrew J. Schorr
2023-05-16 15:11                 ` Andrew J. Schorr

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=15524.1682698000@famine \
    --to=jay.vosburgh@canonical.com \
    --cc=liuhangbin@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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).