On Thu, Aug 29, 2019 at 06:54:48PM -0700, Jakub Kicinski wrote: > On Fri, 30 Aug 2019 02:53:32 +0200, Pablo Neira Ayuso wrote: > > * Offsets do not need to be on the 32-bits boundaries anymore. This > > patchset adds front-end code to adjust the offset and length coming > > from the tc pedit representation, so drivers get an exact header field > > offset and length. > > But drivers use offsetof(start of field) to match headers, and I don't > see you changing that. So how does this work then? Drivers can only use offsetof() for fields that are on the 32-bits boundary. Before this patchset, if you want to mangle the destination port, then the driver needs to refer to the source port offset and the length is 4 bytes, so the mask is telling what needs to be mangled. After this patchset, the offset is set to the destination port, the length is set to 2-bytes, and the mask is telling what bytes of the destination port field you specifically want to update. It's just 100 LOC of preprocessing that is simplifying driver codebase. > Say - I want to change the second byte of an IPv4 address. Then, the front-end sets the offset to IPv4 address header field, and the mask tells what to update. > > * The front-end coalesces consecutive pedit actions into one single > > word, so drivers can mangle IPv6 and ethernet address fields in one > > single go. > > You still only coalesce up to 16 bytes, no? You only have to rise FLOW_ACTION_MANGLE_MAXLEN coming in this patch if you need more. I don't know of any packet field larger than 16 bytes. If there is a use-case for this, it should be easy to rise that definition. > As I said previously drivers will continue to implement mangle action > merge code if that's the case. It'd be nice if core did the coalescing, > and mark down first and last action, in case there is a setup cost for > rewrite group. In this patchset, the core front-end is doing the coalescing.