Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Edward Cree <ecree@solarflare.com>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: <netfilter-devel@vger.kernel.org>, <davem@davemloft.net>,
	<netdev@vger.kernel.org>, <jakub.kicinski@netronome.com>,
	<jiri@resnulli.us>, <saeedm@mellanox.com>, <vishal@chelsio.com>,
	<vladbu@mellanox.com>
Subject: Re: [PATCH net-next,v3 0/4] flow_offload: update mangle action representation
Date: Fri, 6 Sep 2019 14:37:16 +0100
Message-ID: <35ac21be-ff2f-a9cd-dd71-28bc37e8a51b@solarflare.com> (raw)
In-Reply-To: <20190906131457.7olkal45kkdtbevo@salvia>

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

  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 [this message]
2019-09-06 14:50           ` Pablo Neira Ayuso
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=35ac21be-ff2f-a9cd-dd71-28bc37e8a51b@solarflare.com \
    --to=ecree@solarflare.com \
    --cc=davem@davemloft.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=saeedm@mellanox.com \
    --cc=vishal@chelsio.com \
    --cc=vladbu@mellanox.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 \
		netfilter-devel@vger.kernel.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