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
