netdev - Re: [PATCH net] bonding: show saner speed for broadcast mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Aug 2020 22:29:56 -0700
From:   Jay Vosburgh <jay.vosburgh@...onical.com>
To:     Jarod Wilson <jarod@...hat.com>
cc:     linux-kernel@...r.kernel.org, Veaceslav Falico <vfalico@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net] bonding: show saner speed for broadcast mode

Jarod Wilson <jarod@...hat.com> wrote:

>Broadcast mode bonds transmit a copy of all traffic simultaneously out of
>all interfaces, so the "speed" of the bond isn't really the aggregate of
>all interfaces, but rather, the speed of the lowest active interface.

	Did you mean "slowest" here?

>Also, the type of the speed field is u32, not unsigned long, so adjust
>that accordingly, as required to make min() function here without
>complaining about mismatching types.
>
>Fixes: bb5b052f751b ("bond: add support to read speed and duplex via ethtool")
>CC: Jay Vosburgh <j.vosburgh@...il.com>
>CC: Veaceslav Falico <vfalico@...il.com>
>CC: Andy Gospodarek <andy@...yhouse.net>
>CC: "David S. Miller" <davem@...emloft.net>
>CC: netdev@...r.kernel.org
>Signed-off-by: Jarod Wilson <jarod@...hat.com>

	Did you notice this by inspection, or did it come up in use
somewhere?  I can't recall ever hearing of anyone using broadcast mode,
so I'm curious if there is a use for it, but this change seems
reasonable enough regardless.

	-J

Acked-by: Jay Vosburgh <jay.vosburgh@...onical.com>


>---
> drivers/net/bonding/bond_main.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 5ad43aaf76e5..c853ca67058c 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -4552,13 +4552,23 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
> 	return ret;
> }
> 
>+static u32 bond_mode_bcast_speed(struct slave *slave, u32 speed)
>+{
>+	if (speed == 0 || speed == SPEED_UNKNOWN)
>+		speed = slave->speed;
>+	else
>+		speed = min(speed, slave->speed);
>+
>+	return speed;
>+}
>+
> static int bond_ethtool_get_link_ksettings(struct net_device *bond_dev,
> 					   struct ethtool_link_ksettings *cmd)
> {
> 	struct bonding *bond = netdev_priv(bond_dev);
>-	unsigned long speed = 0;
> 	struct list_head *iter;
> 	struct slave *slave;
>+	u32 speed = 0;
> 
> 	cmd->base.duplex = DUPLEX_UNKNOWN;
> 	cmd->base.port = PORT_OTHER;
>@@ -4570,8 +4580,13 @@ static int bond_ethtool_get_link_ksettings(struct net_device *bond_dev,
> 	 */
> 	bond_for_each_slave(bond, slave, iter) {
> 		if (bond_slave_can_tx(slave)) {
>-			if (slave->speed != SPEED_UNKNOWN)
>-				speed += slave->speed;
>+			if (slave->speed != SPEED_UNKNOWN) {
>+				if (BOND_MODE(bond) == BOND_MODE_BROADCAST)
>+					speed = bond_mode_bcast_speed(slave,
>+								      speed);
>+				else
>+					speed += slave->speed;
>+			}
> 			if (cmd->base.duplex == DUPLEX_UNKNOWN &&
> 			    slave->duplex != DUPLEX_UNKNOWN)
> 				cmd->base.duplex = slave->duplex;
>-- 
>2.20.1
>