netdev - Re: [iwl-next v1 4/8] ice: control default Tx rule in lag

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]

Message-ID: <ZbtCom/grznFpesc@mev-dev>
Date: Thu, 1 Feb 2024 08:05:06 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Simon Horman <horms@...nel.org>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	marcin.szycik@...el.com, wojciech.drewek@...el.com,
	sridhar.samudrala@...el.com, przemyslaw.kitszel@...el.com,
	Marcin Szycik <marcin.szycik@...ux.intel.com>
Subject: Re: [iwl-next v1 4/8] ice: control default Tx rule in lag

On Mon, Jan 29, 2024 at 10:55:41AM +0000, Simon Horman wrote:
> On Thu, Jan 25, 2024 at 01:53:10PM +0100, Michal Swiatkowski wrote:
> > Tx rule in switchdev was changed to use PF instead of additional control
> > plane VSI. Because of that during lag we should control it. Control
> > means to add and remove the default Tx rule during lag active/inactive
> > switching.
> > 
> > It can be done the same way as default Rx rule.
> 
> Hi Michal,
> 
> Can I confirm that LAG TX/RX works both before and after this patch?
> 

Hi Simon,

This part of LAG code is related to the LAG + switchdev feature (it
isn't chaning LAG core code). Hope that normal LAG also works well. This
is the scenario when you have PF in switchdev, bond created of two PFs
connected to the bridge with representors. Switching between interfaces
from bond needs to add default Rx rule, and after my changes also
default Tx rule.

Do you think I should add this description to commit message?

Thanks,
Michal

> > 
> > Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
> > Reviewed-by: Marcin Szycik <marcin.szycik@...ux.intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_lag.c | 39 ++++++++++++++++++------
> >  drivers/net/ethernet/intel/ice/ice_lag.h |  3 +-
> >  2 files changed, 32 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
> 
> ...
> 
> > @@ -266,9 +274,22 @@ ice_lag_cfg_dflt_fltr(struct ice_lag *lag, bool add)
> >  {
> >  	u32 act = ICE_SINGLE_ACT_VSI_FORWARDING |
> >  		ICE_SINGLE_ACT_VALID_BIT | ICE_SINGLE_ACT_LAN_ENABLE;
> > +	int err;
> > +
> > +	err = ice_lag_cfg_fltr(lag, act, lag->pf_recipe, &lag->pf_rx_rule_id,
> > +			       ICE_FLTR_RX, add);
> > +	if (err)
> > +		return err;
> > +
> > +	err = ice_lag_cfg_fltr(lag, act, lag->pf_recipe, &lag->pf_tx_rule_id,
> > +			       ICE_FLTR_TX, add);
> > +	if (err) {
> > +		ice_lag_cfg_fltr(lag, act, lag->pf_recipe, &lag->pf_rx_rule_id,
> > +				 ICE_FLTR_RX, !add);
> > +		return err;
> > +	}
> >  
> > -	return ice_lag_cfg_fltr(lag, act, lag->pf_recipe,
> > -				&lag->pf_rule_id, add);
> > +	return 0;
> >  }
> 
> nit: perhaps this could be more idiomatically written using a
>      goto to unwind on error.
>
Thanks, I will rewrite it.

> ...