Re: [net-next 16/16] net/mlx5: Support multiport eswitch mode

From: Jakub Kicinski <kuba@kernel.org>
To: Saeed Mahameed <saeedm@nvidia.com>
Cc: Saeed Mahameed <saeed@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, Eli Cohen <elic@nvidia.com>,
	Mark Bloch <mbloch@nvidia.com>
Subject: Re: [net-next 16/16] net/mlx5: Support multiport eswitch mode
Date: Wed, 18 May 2022 21:36:52 -0700	[thread overview]
Message-ID: <20220518213652.312ffb2e@kernel.org> (raw)
In-Reply-To: <20220519042628.e2yqi37ceyks6rbv@sx1>

On Wed, 18 May 2022 21:26:28 -0700 Saeed Mahameed wrote:
> On 18 May 17:21, Jakub Kicinski wrote:
> >On Tue, 17 May 2022 23:49:38 -0700 Saeed Mahameed wrote:  
> >> From: Eli Cohen <elic@nvidia.com>
> >>
> >> Multiport eswitch mode is a LAG mode that allows to add rules that
> >> forward traffic to a specific physical port without being affected by LAG
> >> affinity configuration.
> >>
> >> This mode of operation is mutual exclusive with the other LAG modes used
> >> by multipath and bonding.
> >>
> >> To make the transition between the modes, we maintain a counter on the
> >> number of rules specifying one of the uplink representors as the target
> >> of mirred egress redirect action.
> >>
> >> An example of such rule would be:
> >>
> >> $ tc filter add dev enp8s0f0_0 prot all root flower dst_mac \
> >>   00:11:22:33:44:55 action mirred egress redirect dev enp8s0f0
> >>
> >> If the reference count just grows to one and LAG is not in use, we
> >> create the LAG in multiport eswitch mode. Other mode changes are not
> >> allowed while in this mode. When the reference count reaches zero, we
> >> destroy the LAG and let other modes be used if needed.
> >>
> >> logic also changed such that if forwarding to some uplink destination
> >> cannot be guaranteed, we fail the operation so the rule will eventually
> >> be in software and not in hardware.
> >>
> >> Signed-off-by: Eli Cohen <elic@nvidia.com>
> >> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> >> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>  
> >
> >GCC 12 also points out that:
> >
> >drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c: In function ‘mlx5_do_bond’:
> >drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c:786:28: warning: ‘tracker’ is used uninitialized [-Wuninitialized]
> >  786 |         struct lag_tracker tracker;
> >      |                            ^~~~~~~
> >drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c:786:28: note: ‘tracker’ declared here
> >  786 |         struct lag_tracker tracker;
> >      |                            ^~~~~~~  
> 
> it's a false alarm, anyway clang and gcc12 are happy on my machine:

Yeah, looks like a false alarm.

> $ KCFLAGS="-Wall" make W=1 drivers/net/ethernet/mellanox/mlx5/core/lag/lag.o

No extra flags, basic allmodconfig build here (well, with WERROR=n).

> CALL    scripts/checksyscalls.sh
> CALL    scripts/atomic/check-atomics.sh
> DESCEND objtool
> CC      drivers/net/ethernet/mellanox/mlx5/core/lag/lag.o
> 
> $ gcc --version
> gcc (GCC) 12.1.1 20220507 (Red Hat 12.1.1-1)

Same:

gcc (GCC) 12.1.1 20220507 (Red Hat 12.1.1-1)