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 at least when hw offlaod is requested.
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 so
that xdp program can obtain the flowtable that the net_device is
part of.
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).
NB: This also disables ip and ip6 family for same device, plan would be
to support 'xdp offload' only for NFPROTO_INET for simplicity.
CC: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
---
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 3c1fd8283bf4..0c3485410efc 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8183,6 +8183,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->data))
+ 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,
@@ -8193,6 +8224,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
