netdev - Re: [PATCHv3 net-next 3/3] selftests: bonding: add test for LACP actor port priority

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] [day] [month] [year] [list]

Message-ID: <aKcpXt_OLrvf4XDN@fedora>
Date: Thu, 21 Aug 2025 14:12:46 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, Jay Vosburgh <jv@...sburgh.net>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Simon Horman <horms@...nel.org>, Shuah Khan <shuah@...nel.org>,
	Jonathan Corbet <corbet@....net>, Petr Machata <petrm@...dia.com>,
	Amit Cohen <amcohen@...dia.com>,
	Vladimir Oltean <vladimir.oltean@....com>,
	Stephen Hemminger <stephen@...workplumber.org>,
	David Ahern <dsahern@...il.com>,
	Jonas Gorski <jonas.gorski@...il.com>, linux-doc@...r.kernel.org,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCHv3 net-next 3/3] selftests: bonding: add test for LACP
 actor port priority

On Thu, Aug 21, 2025 at 11:14:34AM +0200, Paolo Abeni wrote:
> On 8/18/25 11:23 AM, Hangbin Liu wrote:
> > +# Trigger link state change to reselect the aggregator
> > +ip -n "${c_ns}" link set eth1 down
> > +sleep 1
> > +ip -n "${c_ns}" link set eth1 up
> > +# the active agg should be connect to switch
> > +bond_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show bond0" ".[].linkinfo.info_data.ad_info.aggregator")
> > +eth0_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show eth0" ".[].linkinfo.info_slave_data.ad_aggregator_id")
> > +[ "${bond_agg_id}" -ne "${eth0_agg_id}" ] && RET=1
> 
> A few lines above exceed 100 chars, it would be better to wrap them
> 
> > +log_test "bond 802.3ad" "actor_port_prio select"
> > +
> > +# Change the actor port prio and re-test
> > +ip -n "${c_ns}" link set eth0 type bond_slave actor_port_prio 10
> > +ip -n "${c_ns}" link set eth2 type bond_slave actor_port_prio 1000
> > +# Trigger link state change to reselect the aggregator
> > +ip -n "${c_ns}" link set eth1 down
> > +sleep 1
> > +ip -n "${c_ns}" link set eth1 up
> > +# now the active agg should be connect to backup switch
> > +bond_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show bond0" ".[].linkinfo.info_data.ad_info.aggregator")
> > +eth2_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show eth2" ".[].linkinfo.info_slave_data.ad_aggregator_id")
> > +# shellcheck disable=SC2034
> > +if [ "${bond_agg_id}" -ne "${eth2_agg_id}" ]; then
> > +	RET=1
> > +fi
> > +log_test "bond 802.3ad" "actor_port_prio switch"
> 
> The test above is basically the same of the previous one, with a
> slightly different style, it would be better to factor the whole
> status cycling, data fetching and comparison in an helper to avoid code
> duplication and the mentioned confusing difference.
> 

OK, I will fix it.

Regards
Hangbin