Re: [Intel-wired-lan] [PATCH net-next v2 4/4] ice: use src VSI instead of src MAC in slow-path

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

On Tue, Apr 04, 2023 at 12:30:42PM +0200, Alexander Lobakin wrote:
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Date: Tue,  4 Apr 2023 09:28:33 +0200
> 
> > The use of a source  MAC to direct packets from the VF to the
> > corresponding port representor is only ok if there is only one
> > MAC on a VF. To support this functionality when the number
> > of MACs on a VF is greater, it is necessary to match a source
> > VSI instead of a source MAC.
> 
> [...]
> 
> > @@ -32,11 +31,9 @@
> >  	if·(!list)
> >  		return·-ENOMEM;
> >
> > -	list[0].type·=·ICE_MAC_OFOS;
> > -	ether_addr_copy(list[0].h_u.eth_hdr.src_addr,·mac);
> > -	eth_broadcast_addr(list[0].m_u.eth_hdr.src_addr);
> > +	ice_rule_add_src_vsi_metadata(&list[0]);
> 
> &list[0] == list.
> 

Will do

> > -	rule_info.sw_act.flag·|=·ICE_FLTR_TX;
> > +	rule_info.sw_act.flag·=·ICE_FLTR_TX;
> >  	rule_info.sw_act.vsi_handle·=·ctrl_vsi->idx;
> 
> [...]
> 
> > @@ -269,10 +235,18 @@ static int ice_eswitch_setup_reprs(struct ice_pf *pf)
> >  			goto err;
> >  		}
> >  
> > +		if (ice_eswitch_add_vf_sp_rule(pf, vf)) {
> > +			ice_fltr_add_mac_and_broadcast(vsi,
> > +						       vf->hw_lan_addr,
> 
> Fits into the previous line :p
>

Yeah, will move it.

> > +						       ICE_FWD_TO_VSI);
> > +			goto err;
> > +		}
> > +
> 
> [...]
> 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_protocol_type.h b/drivers/net/ethernet/intel/ice/ice_protocol_type.h
> > index ed0ab8177c61..664e2f45e249 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_protocol_type.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_protocol_type.h
> > @@ -256,7 +256,9 @@ struct ice_nvgre_hdr {
> >   * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >   *
> >   * Source VSI = Source VSI of packet loopbacked in switch (for egress) (10b).
> > - *
> > + */
> > +#define ICE_MDID_SOURCE_VSI_MASK 0x3ff
> 
> GENMASK()?
>

Sorry, it should be there (Simon pointed it), but I forgot about
amending :( . Thanks for catching it.

> > +/*
> 
> A newline before this line maybe to improve readability a bit?
>

Will add

> >   * MDID 20
> >   * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >   * |A|B|C|D|E|F|R|R|G|H|I|J|K|L|M|N|
> 
> [...]
> 
> > --- a/drivers/net/ethernet/intel/ice/ice_repr.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_repr.h
> > @@ -13,9 +13,8 @@ struct ice_repr {
> >  	struct net_device *netdev;
> >  	struct metadata_dst *dst;
> >  #ifdef CONFIG_ICE_SWITCHDEV
> > -	/* info about slow path MAC rule  */
> > -	struct ice_rule_query_data *mac_rule;
> > -	u8 rule_added;
> > +	/* info about slow path rule  */
> 
> Two spaces after 'rule' here :s
>

Will fix

> > +	struct ice_rule_query_data sp_rule;
> >  #endif
> >  };
> [...]
> 
> Thanks,
> Olek

Thanks for the review
Michal