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

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

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@intel.com>
> > Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.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.

> ...