Re: [PATCH net-next,v3 0/4] flow_offload: update mangle action representation

[Edward Cree <ecree@solarflare.com>]

On 06/09/2019 14:14, Pablo Neira Ayuso wrote:
> OK, I can document this semantics, I need just _time_ to write that
> documentation. I was expecting this patch description is enough by now
> until I can get to finish that documentation.
I think for two structs with apparently the same contents but different
 semantics (one has the mask bitwise complemented) it's best to hold up
 the code change until the comment is ready to come with it, because
 until then it's a dangerously confusing situation.

>> And you can't just coalesce all consecutive mangles, because if you
>>  mangle two consecutive fields (e.g. UDP sport and dport) the driver
>>  still needs to disentangle that if it works on a 'fields' (rather
>>  than 'u32s') level.
> This infrastructure is _not_ coalescing two consecutive field, e.g.
> UDP sport and dport is _not_ coalesced. The coalesce routine does
> _not_ handle multiple tc pedit ex actions.
So an IPv6 address mangle only comes as a single action if it's from
 netfilter, not if it's coming from TC pedit.  Therefore drivers still
 need to handle an IPv6 or MAC address mangle coming in multiple
 actions, therefore your driver simplifications are invalid.  No?

> The model you propose would still need this code for tc pedit to
> adjust offset/length and coalesce u32 fields.
Yes, but we don't add code/features to the kernel based on what we
 *could* use it for later; every submission has to be self-contained
 in providing something of demonstrable value.  So either implement
 "the model I propose" (which to be clear I'm *not* proposing, I want
 the u32 pedit left as it is; it's just that it's a better model than
 what you're implementing here), or leave well alone.

-Ed