From: Pablo Neira Ayuso <email@example.com> To: Edward Cree <firstname.lastname@example.org> Cc: email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org Subject: Re: [PATCH net-next,v3 0/4] flow_offload: update mangle action representation Date: Fri, 6 Sep 2019 16:50:19 +0200 Message-ID: <20190906145019.2bggchaq43tcqdyc@salvia> (raw) In-Reply-To: <email@example.com> On Fri, Sep 06, 2019 at 02:37:16PM +0100, Edward Cree wrote: > 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. The idea is that flow rule API != tc front-end anymore. So the flow rule API can evolve regardless the front-end requirements. Before this update there was no other choice than using the tc pedit layout since it was exposed to the drivers, this is not the case anymore. > >> 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. Driver gets one single action from tc/netfilter (and potentially ethtool if it would support for this), it comes as a single action from both subsystems: front-end -----> flow_rule API -----> drivers Front-end need to translate their representation to this FLOW_ACTION_MANGLE layout. By front-end, I refer to ethtool/netfilter/tc. > Therefore drivers still need to handle an IPv6 or MAC address > mangle coming in multiple actions, therefore your driver > simplifications are invalid. No? No. IPv6 and MAC come as a single action. All subsystems use the same flow_rule API, they use the same layout. > > 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 This is already useful: Look at the cxgb driver in particular, it's way easier to read. Other existing drivers do not need to do things like: case round_down(offsetof(struct iphdr, tos), 4): and the play with masks to infer if the user wants to mangle the TOS field, they can just do: case offsetof(struct iphdr, tos): which is way more natural representation.
next prev parent reply index Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-09-06 0:03 Pablo Neira Ayuso 2019-09-06 0:04 ` [PATCH net-next,v3 1/4] net: flow_offload: flip mangle action mask Pablo Neira Ayuso 2019-09-06 0:04 ` [PATCH net-next,v3 2/4] net: flow_offload: bitwise AND on mangle action value field Pablo Neira Ayuso 2019-09-06 0:04 ` [PATCH net-next,v3 3/4] net: flow_offload: mangle action at byte level Pablo Neira Ayuso 2019-09-06 0:04 ` [PATCH net-next,v3 4/4] netfilter: nft_payload: packet mangling offload support Pablo Neira Ayuso 2019-09-06 10:02 ` [PATCH net-next,v3 0/4] flow_offload: update mangle action representation Edward Cree 2019-09-06 10:56 ` Pablo Neira Ayuso 2019-09-06 12:55 ` Edward Cree 2019-09-06 13:14 ` Pablo Neira Ayuso 2019-09-06 13:37 ` Edward Cree 2019-09-06 14:50 ` Pablo Neira Ayuso [this message] 2019-09-06 15:13 ` Edward Cree 2019-09-06 15:58 ` Pablo Neira Ayuso 2019-09-06 16:49 ` Edward Cree 2019-09-06 18:15 ` Pablo Neira Ayuso
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190906145019.2bggchaq43tcqdyc@salvia \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Netfilter-Devel Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \ firstname.lastname@example.org public-inbox-index netfilter-devel Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git