Re: [PATCH net 2/2] nfp: flower: fix hardware offload for the transfer layer port

[Louis Peens <louis.peens@corigine.com>]

On Mon, Dec 11, 2023 at 07:08:49PM -0800, Jakub Kicinski wrote:
> On Fri,  8 Dec 2023 08:59:56 +0200 Louis Peens wrote:
> > +		if (mangle_action->mangle.offset == offsetof(struct tcphdr, source)) {
> > +			mangle_action->mangle.val =
> > +				(__force u32)cpu_to_be32(mangle_action->mangle.val << 16);
> > +			mangle_action->mangle.mask =
> > +				(__force u32)cpu_to_be32(mangle_action->mangle.mask << 16 | 0xFFFF);
> 
> This a bit odd. Here you fill in the "other half" of the mask with Fs...
> 
> > +		}
> > +		if (mangle_action->mangle.offset == offsetof(struct tcphdr, dest)) {
> > +			mangle_action->mangle.offset = 0;
> > +			mangle_action->mangle.val =
> > +				(__force u32)cpu_to_be32(mangle_action->mangle.val);
> > +			mangle_action->mangle.mask =
> > +				(__force u32)cpu_to_be32(mangle_action->mangle.mask);
> > +		}
> 
> .. but here you just let it be zero.
> 
> If it's correct it'd be good to explain in the commit msg why.

Thanks for asking, it does indeed look a bit strange. It has to do with
act_ct using the inverse value of the mask, basically requiring a
rotate-left operation for the source field. It can definitely do with a
better explanation. Will submit a v2 doing so.

> -- 
> pw-bot: cr