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
