Subject: Re: [RFC bpf-next 07/23] ice: Support RX hash XDP hint

From: Larysa Zaremba <[email-masked]>
Date: Thu, 14 Sep 2023 18:59:27 +0200

On Thu, Sep 14, 2023 at 06:54:21PM +0200, Alexander Lobakin wrote:
> From: Larysa Zaremba <[email-masked]>
> Date: Thu, 24 Aug 2023 21:26:46 +0200
> 
> > RX hash XDP hint requests both hash value and type.
> > Type is XDP-specific, so we need a separate way to map
> > these values to the hardware ptypes, so create a lookup table.
> > 
> > Instead of creating a new long list, reuse contents
> > of ice_decode_rx_desc_ptype[] through preprocessor.
> > 
> > Current hash type enum does not contain ICMP packet type,
> > but ice devices support it, so also add a new type into core code.
> > 
> > Then use previously refactored code and create a function
> > that allows XDP code to read RX hash.
> > 
> > Signed-off-by: Larysa Zaremba <[email-masked]>
> 
> [...]
> 
> >  	/* unused entries */
> > -	[154 ... 1023] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 }
> > +	[ICE_NUM_DEFINED_PTYPES ... 1023] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 }
> >  };
> >  
> >  static inline struct ice_rx_ptype_decoded ice_decode_rx_desc_ptype(u16 ptype)
> > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> > index 463d9e5cbe05..b11cfaedb81c 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> > @@ -567,6 +567,79 @@ static int ice_xdp_rx_hw_ts(const struct xdp_md *ctx, u64 *ts_ns)
> >  	return 0;
> >  }
> >  
> > +/* Define a ptype index -> XDP hash type lookup table.
> > + * It uses the same ptype definitions as ice_decode_rx_desc_ptype[],
> > + * avoiding possible copy-paste errors.
> > + */
> > +#undef ICE_PTT
> > +#undef ICE_PTT_UNUSED_ENTRY
> > +
> > +#define ICE_PTT(PTYPE, OUTER_IP, OUTER_IP_VER, OUTER_FRAG, T, TE, TEF, I, PL)\
> > +	[PTYPE] = XDP_RSS_L3_##OUTER_IP_VER | XDP_RSS_L4_##I | XDP_RSS_TYPE_##PL
> > +
> > +#define ICE_PTT_UNUSED_ENTRY(PTYPE) [PTYPE] = 0
> > +
> > +/* A few supplementary definitions for when XDP hash types do not coincide
> > + * with what can be generated from ptype definitions
> > + * by means of preprocessor concatenation.
> > + */
> > +#define XDP_RSS_L3_NONE		XDP_RSS_TYPE_NONE
> > +#define XDP_RSS_L4_NONE		XDP_RSS_TYPE_NONE
> > +#define XDP_RSS_TYPE_PAY2	XDP_RSS_TYPE_L2
> > +#define XDP_RSS_TYPE_PAY3	XDP_RSS_TYPE_NONE
> > +#define XDP_RSS_TYPE_PAY4	XDP_RSS_L4
> > +
> > +static const enum xdp_rss_hash_type
> > +ice_ptype_to_xdp_hash[ICE_NUM_DEFINED_PTYPES] = {
> > +	ICE_PTYPES
> > +};
> 
> Is there a big win in performance with this 600-byte static table
> comparing to having several instructions which would do
> to_parsed_ptype() and then build a return enum according to its fields?
> I believe that would cost only several instructions. Not that it's a
> disaster to consume 600 more bytes of rodata, but still.
>

It is not disasterous either way, I have added this table after a discussion 
with team members and would like not to throw this away now.

> Alternatively, you can look at how parsed ptype is compressed to 16 bit
> in libie and use those saved bits to encode complete XDP RSS hash enum
> directly there, so that ice_ptype_lkup[] would have both parsed ptype
> and XDP hash return value :D
> 
> > +
> > +#undef XDP_RSS_L3_NONE
> > +#undef XDP_RSS_L4_NONE
> > +#undef XDP_RSS_TYPE_PAY2
> > +#undef XDP_RSS_TYPE_PAY3
> > +#undef XDP_RSS_TYPE_PAY4
> 
> [...]
> 
> Thanks,
> Olek