netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: <netfilter-devel@vger.kernel.org>
Cc: lorenzo@kernel.org, <netdev@vger.kernel.org>,
	Florian Westphal <fw@strlen.de>
Subject: [PATCH nf-next 5/8] netfilter: nf_tables: reject flowtable hw offload for same device
Date: Tue, 21 Nov 2023 13:27:48 +0100	[thread overview]
Message-ID: <20231121122800.13521-6-fw@strlen.de> (raw)
In-Reply-To: <20231121122800.13521-1-fw@strlen.de>

The existing check is not sufficient, as it only considers flowtables
within the same table.

In case of HW offload, we need check all flowtables that exist in
the same net namespace.

We can skip flowtables that are slated for removal (not active
in next gen).

Ideally this check would supersede the existing one, but this is
probably too risky and might prevent existing configs from working.

As is, you can do all of the following:

table ip t { flowtable f { devices = { lo  } } }
table ip6 t { flowtable f { devices = { lo  } } }
table inet t { flowtable f { devices = { lo  } } }

... but IMO this should not be possible in the first place.

Disable this for HW offload.

This is related to XDP flowtable work, the idea is to keep a small
hashtable that has a 'struct net_device := struct nf_flowtable' map.

This mapping must be unique.  The idea is to add a "XDP OFFLOAD"
flag to nftables api and then have this function run for 'xdp offload'
case too.

This is useful, because it would permit the "xdp offload" hashtable
to tolerate duplicate keys -- they would only occur during transactional
updates, e.g. a flush of the current table combined with atomic reload.

Without this change, the nf_flowtable core cannot tell when flowtable
is a real duplicate, or just a temporary artefact of the
two-phase-commit protocol (i.e., the clashing entry is queued for removal).

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 36 +++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index e779e275d694..7437b997ca7e 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8189,6 +8189,37 @@ static void nft_unregister_flowtable_net_hooks(struct net *net,
 	__nft_unregister_flowtable_net_hooks(net, hook_list, false);
 }
 
+static bool nft_flowtable_offload_clash(struct net *net,
+					const struct nft_hook *hook,
+					struct nft_flowtable *flowtable)
+{
+	const struct nftables_pernet *nft_net;
+	struct nft_flowtable *existing_ft;
+	const struct nft_table *table;
+
+	/* No offload requested, no need to validate */
+	if (!nf_flowtable_hw_offload(flowtable->ft))
+		return false;
+
+	nft_net = nft_pernet(net);
+
+	list_for_each_entry(table, &nft_net->tables, list) {
+		list_for_each_entry(existing_ft, &table->flowtables, list) {
+			const struct nft_hook *hook2;
+
+			if (!nft_is_active_next(net, existing_ft))
+				continue;
+
+			list_for_each_entry(hook2, &existing_ft->hook_list, list) {
+				if (hook->ops.dev == hook2->ops.dev)
+					return true;
+			}
+		}
+	}
+
+	return false;
+}
+
 static int nft_register_flowtable_net_hooks(struct net *net,
 					    struct nft_table *table,
 					    struct list_head *hook_list,
@@ -8199,6 +8230,11 @@ static int nft_register_flowtable_net_hooks(struct net *net,
 	int err, i = 0;
 
 	list_for_each_entry(hook, hook_list, list) {
+		if (nft_flowtable_offload_clash(net, hook, flowtable)) {
+			err = -EEXIST;
+			goto err_unregister_net_hooks;
+		}
+
 		list_for_each_entry(ft, &table->flowtables, list) {
 			if (!nft_is_active_next(net, ft))
 				continue;
-- 
2.41.0


  parent reply	other threads:[~2023-11-21 12:28 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-21 12:27 [PATCH nf-next 0/8] netfilter: make nf_flowtable lifetime differ from container struct Florian Westphal
2023-11-21 12:27 ` [PATCH nf-next 1/8] netfilter: flowtable: move nf_flowtable out of container structures Florian Westphal
2023-11-23 13:52   ` Simon Horman
2023-11-23 14:10     ` Florian Westphal
2023-11-25  8:26       ` Simon Horman
2023-11-25  8:36         ` Simon Horman
2023-11-21 12:27 ` [PATCH nf-next 2/8] netfilter: nf_flowtable: replace init callback with a create one Florian Westphal
2023-11-21 12:27 ` [PATCH nf-next 3/8] netfilter: nf_flowtable: make free a real free function Florian Westphal
2023-11-21 12:27 ` [PATCH nf-next 4/8] netfilter: nf_flowtable: delay flowtable release a second time Florian Westphal
2023-11-21 12:27 ` Florian Westphal [this message]
2023-11-21 12:27 ` [PATCH nf-next 6/8] netfilter: nf_tables: add xdp offload flag Florian Westphal
2023-11-21 12:27 ` [PATCH nf-next 7/8] netfilter: nf_tables: add flowtable map for xdp offload Florian Westphal
2023-11-21 14:25   ` Lorenzo Bianconi
2023-11-24 10:59   ` Toke Høiland-Jørgensen
2023-11-30 13:53     ` Florian Westphal
2023-11-30 14:17       ` Toke Høiland-Jørgensen
2023-11-21 12:27 ` [PATCH nf-next 8/8] netfilter: nf_tables: permit duplicate flowtable mappings Florian Westphal
2023-11-24  9:50 ` [PATCH nf-next 0/8] netfilter: make nf_flowtable lifetime differ from container struct Pablo Neira Ayuso
2023-11-24  9:55   ` Florian Westphal
2023-11-24 10:10     ` Pablo Neira Ayuso
2023-11-24 10:16       ` Florian Westphal
2023-11-24 10:48   ` Toke Høiland-Jørgensen

Reply instructions:

You may reply publicly 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=20231121122800.13521-6-fw@strlen.de \
    --to=fw@strlen.de \
    --cc=lorenzo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).