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

Larysa Zaremba
2023-09-14T17:05:48Z

On Thu, Sep 14, 2023 at 06:54:21PM +0200, Alexander Lobakin wrote:
> From: Larysa Zaremba <larysa.zaremba@intel.com>
> 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 <larysa.zaremba@intel.com>
>
> [...]
>
> > /* 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