* [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support
@ 2023-07-12 9:59 Pablo Neira Ayuso
2023-07-12 11:05 ` Igor Raits
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2023-07-12 9:59 UTC (permalink / raw)
To: netfilter-devel; +Cc: fw, igor, phil
Add context structure to improve bridge among support which creates an
anonymous set. This context structure specifies the command and it
allows to optionally store a anonymous set.
Use this context to generate native bytecode only if this is an
add/insert/replace command.
This fixes a dangling anonymous set that is created on rule removal.
Fixes: 26753888720d ("nft: bridge: Rudimental among extension support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
This test passes tests/py and tests/shell.
iptables/nft-arp.c | 4 ++--
iptables/nft-bridge.c | 9 ++++----
iptables/nft-cmd.c | 6 ++++-
iptables/nft-ipv4.c | 6 ++---
iptables/nft-ipv6.c | 6 ++---
iptables/nft-shared.h | 5 ++--
iptables/nft.c | 54 ++++++++++++++++++++++++++++---------------
iptables/nft.h | 9 +++++---
8 files changed, 62 insertions(+), 37 deletions(-)
diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index 265de5f88cea..9868966a0368 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -40,8 +40,8 @@ static bool need_devaddr(struct arpt_devaddr_info *info)
return false;
}
-static int nft_arp_add(struct nft_handle *h, struct nftnl_rule *r,
- struct iptables_command_state *cs)
+static int nft_arp_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
+ struct nftnl_rule *r, struct iptables_command_state *cs)
{
struct arpt_entry *fw = &cs->arp;
uint32_t op;
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 6e50950774e6..391a8ab723c1 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -138,7 +138,8 @@ static int _add_action(struct nftnl_rule *r, struct iptables_command_state *cs)
static int
nft_bridge_add_match(struct nft_handle *h, const struct ebt_entry *fw,
- struct nftnl_rule *r, struct xt_entry_match *m)
+ struct nft_rule_ctx *ctx, struct nftnl_rule *r,
+ struct xt_entry_match *m)
{
if (!strcmp(m->u.user.name, "802_3") && !(fw->bitmask & EBT_802_3))
xtables_error(PARAMETER_PROBLEM,
@@ -152,10 +153,10 @@ nft_bridge_add_match(struct nft_handle *h, const struct ebt_entry *fw,
xtables_error(PARAMETER_PROBLEM,
"For IPv6 filtering the protocol must be specified as IPv6.");
- return add_match(h, r, m);
+ return add_match(h, ctx, r, m);
}
-static int nft_bridge_add(struct nft_handle *h,
+static int nft_bridge_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
struct nftnl_rule *r,
struct iptables_command_state *cs)
{
@@ -217,7 +218,7 @@ static int nft_bridge_add(struct nft_handle *h,
for (iter = cs->match_list; iter; iter = iter->next) {
if (iter->ismatch) {
- if (nft_bridge_add_match(h, fw, r, iter->u.match->m))
+ if (nft_bridge_add_match(h, fw, ctx, r, iter->u.match->m))
break;
} else {
if (add_target(r, iter->u.watcher->t))
diff --git a/iptables/nft-cmd.c b/iptables/nft-cmd.c
index 7b2fc3a59578..8a824586ad8c 100644
--- a/iptables/nft-cmd.c
+++ b/iptables/nft-cmd.c
@@ -14,12 +14,16 @@
#include <xtables.h>
#include "nft.h"
#include "nft-cmd.h"
+#include <libnftnl/set.h>
struct nft_cmd *nft_cmd_new(struct nft_handle *h, int command,
const char *table, const char *chain,
struct iptables_command_state *state,
int rulenum, bool verbose)
{
+ struct nft_rule_ctx ctx = {
+ .command = command,
+ };
struct nftnl_rule *rule;
struct nft_cmd *cmd;
@@ -33,7 +37,7 @@ struct nft_cmd *nft_cmd_new(struct nft_handle *h, int command,
cmd->verbose = verbose;
if (state) {
- rule = nft_rule_new(h, chain, table, state);
+ rule = nft_rule_new(h, &ctx, chain, table, state);
if (!rule) {
nft_cmd_free(cmd);
return NULL;
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index 2a5d25d8694e..2f10220edd50 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -26,8 +26,8 @@
#include "nft.h"
#include "nft-shared.h"
-static int nft_ipv4_add(struct nft_handle *h, struct nftnl_rule *r,
- struct iptables_command_state *cs)
+static int nft_ipv4_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
+ struct nftnl_rule *r, struct iptables_command_state *cs)
{
struct xtables_rule_match *matchp;
uint32_t op;
@@ -84,7 +84,7 @@ static int nft_ipv4_add(struct nft_handle *h, struct nftnl_rule *r,
add_compat(r, cs->fw.ip.proto, cs->fw.ip.invflags & XT_INV_PROTO);
for (matchp = cs->matches; matchp; matchp = matchp->next) {
- ret = add_match(h, r, matchp->match->m);
+ ret = add_match(h, ctx, r, matchp->match->m);
if (ret < 0)
return ret;
}
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index 658a4f201895..d53f87c1d26e 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -25,8 +25,8 @@
#include "nft.h"
#include "nft-shared.h"
-static int nft_ipv6_add(struct nft_handle *h, struct nftnl_rule *r,
- struct iptables_command_state *cs)
+static int nft_ipv6_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
+ struct nftnl_rule *r, struct iptables_command_state *cs)
{
struct xtables_rule_match *matchp;
uint32_t op;
@@ -70,7 +70,7 @@ static int nft_ipv6_add(struct nft_handle *h, struct nftnl_rule *r,
add_compat(r, cs->fw6.ipv6.proto, cs->fw6.ipv6.invflags & XT_INV_PROTO);
for (matchp = cs->matches; matchp; matchp = matchp->next) {
- ret = add_match(h, r, matchp->match->m);
+ ret = add_match(h, ctx, r, matchp->match->m);
if (ret < 0)
return ret;
}
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index a06b263d77c1..4f47058d2ec5 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -35,13 +35,14 @@
| FMT_NUMERIC | FMT_NOTABLE)
#define FMT(tab,notab) ((format) & FMT_NOTABLE ? (notab) : (tab))
+struct nft_rule_ctx;
struct xtables_args;
struct nft_handle;
struct xt_xlate;
struct nft_family_ops {
- int (*add)(struct nft_handle *h, struct nftnl_rule *r,
- struct iptables_command_state *cs);
+ int (*add)(struct nft_handle *h, struct nft_rule_ctx *ctx,
+ struct nftnl_rule *r, struct iptables_command_state *cs);
bool (*is_same)(const struct iptables_command_state *cs_a,
const struct iptables_command_state *cs_b);
void (*print_payload)(struct nftnl_expr *e,
diff --git a/iptables/nft.c b/iptables/nft.c
index 1cb104e75ccc..59e3fa7079c4 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1154,7 +1154,8 @@ gen_lookup(uint32_t sreg, const char *set_name, uint32_t set_id, uint32_t flags)
#define NFT_DATATYPE_ETHERADDR 9
static int __add_nft_among(struct nft_handle *h, const char *table,
- struct nftnl_rule *r, struct nft_among_pair *pairs,
+ struct nft_rule_ctx *ctx, struct nftnl_rule *r,
+ struct nft_among_pair *pairs,
int cnt, bool dst, bool inv, bool ip)
{
uint32_t set_id, type = NFT_DATATYPE_ETHERADDR, len = ETH_ALEN;
@@ -1235,7 +1236,7 @@ static int __add_nft_among(struct nft_handle *h, const char *table,
return 0;
}
-static int add_nft_among(struct nft_handle *h,
+static int add_nft_among(struct nft_handle *h, struct nft_rule_ctx *ctx,
struct nftnl_rule *r, struct xt_entry_match *m)
{
struct nft_among_data *data = (struct nft_among_data *)m->data;
@@ -1251,10 +1252,10 @@ static int add_nft_among(struct nft_handle *h,
}
if (data->src.cnt)
- __add_nft_among(h, table, r, data->pairs, data->src.cnt,
+ __add_nft_among(h, table, ctx, r, data->pairs, data->src.cnt,
false, data->src.inv, data->src.ip);
if (data->dst.cnt)
- __add_nft_among(h, table, r, data->pairs + data->src.cnt,
+ __add_nft_among(h, table, ctx, r, data->pairs + data->src.cnt,
data->dst.cnt, true, data->dst.inv,
data->dst.ip);
return 0;
@@ -1462,22 +1463,30 @@ static int add_nft_mark(struct nft_handle *h, struct nftnl_rule *r,
return 0;
}
-int add_match(struct nft_handle *h,
+int add_match(struct nft_handle *h, struct nft_rule_ctx *ctx,
struct nftnl_rule *r, struct xt_entry_match *m)
{
struct nftnl_expr *expr;
int ret;
- if (!strcmp(m->u.user.name, "limit"))
- return add_nft_limit(r, m);
- else if (!strcmp(m->u.user.name, "among"))
- return add_nft_among(h, r, m);
- else if (!strcmp(m->u.user.name, "udp"))
- return add_nft_udp(h, r, m);
- else if (!strcmp(m->u.user.name, "tcp"))
- return add_nft_tcp(h, r, m);
- else if (!strcmp(m->u.user.name, "mark"))
- return add_nft_mark(h, r, m);
+ switch (ctx->command) {
+ case NFT_COMPAT_RULE_APPEND:
+ case NFT_COMPAT_RULE_INSERT:
+ case NFT_COMPAT_RULE_REPLACE:
+ if (!strcmp(m->u.user.name, "limit"))
+ return add_nft_limit(r, m);
+ else if (!strcmp(m->u.user.name, "among"))
+ return add_nft_among(h, ctx, r, m);
+ else if (!strcmp(m->u.user.name, "udp"))
+ return add_nft_udp(h, r, m);
+ else if (!strcmp(m->u.user.name, "tcp"))
+ return add_nft_tcp(h, r, m);
+ else if (!strcmp(m->u.user.name, "mark"))
+ return add_nft_mark(h, r, m);
+ break;
+ default:
+ break;
+ }
expr = nftnl_expr_alloc("match");
if (expr == NULL)
@@ -1705,7 +1714,8 @@ void add_compat(struct nftnl_rule *r, uint32_t proto, bool inv)
}
struct nftnl_rule *
-nft_rule_new(struct nft_handle *h, const char *chain, const char *table,
+nft_rule_new(struct nft_handle *h, struct nft_rule_ctx *ctx,
+ const char *chain, const char *table,
struct iptables_command_state *cs)
{
struct nftnl_rule *r;
@@ -1718,7 +1728,7 @@ nft_rule_new(struct nft_handle *h, const char *chain, const char *table,
nftnl_rule_set_str(r, NFTNL_RULE_TABLE, table);
nftnl_rule_set_str(r, NFTNL_RULE_CHAIN, chain);
- if (h->ops->add(h, r, cs) < 0)
+ if (h->ops->add(h, ctx, r, cs) < 0)
goto err;
return r;
@@ -2878,6 +2888,9 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
{
struct iptables_command_state cs = {};
struct nftnl_rule *r, *new_rule;
+ struct nft_rule_ctx ctx = {
+ .command = NFT_COMPAT_RULE_ZERO,
+ };
struct nft_chain *c;
int ret = 0;
@@ -2896,7 +2909,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
h->ops->rule_to_cs(h, r, &cs);
cs.counters.pcnt = cs.counters.bcnt = 0;
- new_rule = nft_rule_new(h, chain, table, &cs);
+ new_rule = nft_rule_new(h, &ctx, chain, table, &cs);
h->ops->clear_cs(&cs);
if (!new_rule)
@@ -3274,6 +3287,9 @@ static int ebt_add_policy_rule(struct nftnl_chain *c, void *data)
.eb.bitmask = EBT_NOPROTO,
};
struct nftnl_udata_buf *udata;
+ struct nft_rule_ctx ctx = {
+ .command = NFT_COMPAT_RULE_APPEND,
+ };
struct nft_handle *h = data;
struct nftnl_rule *r;
const char *pname;
@@ -3301,7 +3317,7 @@ static int ebt_add_policy_rule(struct nftnl_chain *c, void *data)
command_jump(&cs, pname);
- r = nft_rule_new(h, nftnl_chain_get_str(c, NFTNL_CHAIN_NAME),
+ r = nft_rule_new(h, &ctx, nftnl_chain_get_str(c, NFTNL_CHAIN_NAME),
nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE), &cs);
ebt_cs_clean(&cs);
diff --git a/iptables/nft.h b/iptables/nft.h
index 1d18982dc8cf..5acbbf82e2c2 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -168,9 +168,11 @@ struct nftnl_set *nft_set_batch_lookup_byid(struct nft_handle *h,
/*
* Operations with rule-set.
*/
-struct nftnl_rule;
+struct nft_rule_ctx {
+ int command;
+};
-struct nftnl_rule *nft_rule_new(struct nft_handle *h, const char *chain, const char *table, struct iptables_command_state *cs);
+struct nftnl_rule *nft_rule_new(struct nft_handle *h, struct nft_rule_ctx *rule, const char *chain, const char *table, struct iptables_command_state *cs);
int nft_rule_append(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, struct nftnl_rule *ref, bool verbose);
int nft_rule_insert(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, int rulenum, bool verbose);
int nft_rule_check(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, bool verbose);
@@ -188,7 +190,8 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain, const char *
*/
int add_counters(struct nftnl_rule *r, uint64_t packets, uint64_t bytes);
int add_verdict(struct nftnl_rule *r, int verdict);
-int add_match(struct nft_handle *h, struct nftnl_rule *r, struct xt_entry_match *m);
+int add_match(struct nft_handle *h, struct nft_rule_ctx *ctx,
+ struct nftnl_rule *r, struct xt_entry_match *m);
int add_target(struct nftnl_rule *r, struct xt_entry_target *t);
int add_jumpto(struct nftnl_rule *r, const char *name, int verdict);
int add_action(struct nftnl_rule *r, struct iptables_command_state *cs, bool goto_set);
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support
2023-07-12 9:59 [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support Pablo Neira Ayuso
@ 2023-07-12 11:05 ` Igor Raits
2023-07-12 14:01 ` Pablo Neira Ayuso
2023-07-12 17:13 ` Pablo Neira Ayuso
2023-07-13 11:50 ` Phil Sutter
2 siblings, 1 reply; 7+ messages in thread
From: Igor Raits @ 2023-07-12 11:05 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, fw, phil
Hi Pablo,
Thanks for the patch!
On Wed, Jul 12, 2023 at 11:59 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Add context structure to improve bridge among support which creates an
> anonymous set. This context structure specifies the command and it
> allows to optionally store a anonymous set.
>
> Use this context to generate native bytecode only if this is an
> add/insert/replace command.
>
> This fixes a dangling anonymous set that is created on rule removal.
>
> Fixes: 26753888720d ("nft: bridge: Rudimental among extension support")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Reported-and-tested-by: Igor Raits <igor@gooddata.com>
> ---
> This test passes tests/py and tests/shell.
>
> iptables/nft-arp.c | 4 ++--
> iptables/nft-bridge.c | 9 ++++----
> iptables/nft-cmd.c | 6 ++++-
> iptables/nft-ipv4.c | 6 ++---
> iptables/nft-ipv6.c | 6 ++---
> iptables/nft-shared.h | 5 ++--
> iptables/nft.c | 54 ++++++++++++++++++++++++++++---------------
> iptables/nft.h | 9 +++++---
> 8 files changed, 62 insertions(+), 37 deletions(-)
>
> diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
> index 265de5f88cea..9868966a0368 100644
> --- a/iptables/nft-arp.c
> +++ b/iptables/nft-arp.c
> @@ -40,8 +40,8 @@ static bool need_devaddr(struct arpt_devaddr_info *info)
> return false;
> }
>
> -static int nft_arp_add(struct nft_handle *h, struct nftnl_rule *r,
> - struct iptables_command_state *cs)
> +static int nft_arp_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
> + struct nftnl_rule *r, struct iptables_command_state *cs)
> {
> struct arpt_entry *fw = &cs->arp;
> uint32_t op;
> diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
> index 6e50950774e6..391a8ab723c1 100644
> --- a/iptables/nft-bridge.c
> +++ b/iptables/nft-bridge.c
> @@ -138,7 +138,8 @@ static int _add_action(struct nftnl_rule *r, struct iptables_command_state *cs)
>
> static int
> nft_bridge_add_match(struct nft_handle *h, const struct ebt_entry *fw,
> - struct nftnl_rule *r, struct xt_entry_match *m)
> + struct nft_rule_ctx *ctx, struct nftnl_rule *r,
> + struct xt_entry_match *m)
> {
> if (!strcmp(m->u.user.name, "802_3") && !(fw->bitmask & EBT_802_3))
> xtables_error(PARAMETER_PROBLEM,
> @@ -152,10 +153,10 @@ nft_bridge_add_match(struct nft_handle *h, const struct ebt_entry *fw,
> xtables_error(PARAMETER_PROBLEM,
> "For IPv6 filtering the protocol must be specified as IPv6.");
>
> - return add_match(h, r, m);
> + return add_match(h, ctx, r, m);
> }
>
> -static int nft_bridge_add(struct nft_handle *h,
> +static int nft_bridge_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
> struct nftnl_rule *r,
> struct iptables_command_state *cs)
> {
> @@ -217,7 +218,7 @@ static int nft_bridge_add(struct nft_handle *h,
>
> for (iter = cs->match_list; iter; iter = iter->next) {
> if (iter->ismatch) {
> - if (nft_bridge_add_match(h, fw, r, iter->u.match->m))
> + if (nft_bridge_add_match(h, fw, ctx, r, iter->u.match->m))
> break;
> } else {
> if (add_target(r, iter->u.watcher->t))
> diff --git a/iptables/nft-cmd.c b/iptables/nft-cmd.c
> index 7b2fc3a59578..8a824586ad8c 100644
> --- a/iptables/nft-cmd.c
> +++ b/iptables/nft-cmd.c
> @@ -14,12 +14,16 @@
> #include <xtables.h>
> #include "nft.h"
> #include "nft-cmd.h"
> +#include <libnftnl/set.h>
>
> struct nft_cmd *nft_cmd_new(struct nft_handle *h, int command,
> const char *table, const char *chain,
> struct iptables_command_state *state,
> int rulenum, bool verbose)
> {
> + struct nft_rule_ctx ctx = {
> + .command = command,
> + };
> struct nftnl_rule *rule;
> struct nft_cmd *cmd;
>
> @@ -33,7 +37,7 @@ struct nft_cmd *nft_cmd_new(struct nft_handle *h, int command,
> cmd->verbose = verbose;
>
> if (state) {
> - rule = nft_rule_new(h, chain, table, state);
> + rule = nft_rule_new(h, &ctx, chain, table, state);
> if (!rule) {
> nft_cmd_free(cmd);
> return NULL;
> diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
> index 2a5d25d8694e..2f10220edd50 100644
> --- a/iptables/nft-ipv4.c
> +++ b/iptables/nft-ipv4.c
> @@ -26,8 +26,8 @@
> #include "nft.h"
> #include "nft-shared.h"
>
> -static int nft_ipv4_add(struct nft_handle *h, struct nftnl_rule *r,
> - struct iptables_command_state *cs)
> +static int nft_ipv4_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
> + struct nftnl_rule *r, struct iptables_command_state *cs)
> {
> struct xtables_rule_match *matchp;
> uint32_t op;
> @@ -84,7 +84,7 @@ static int nft_ipv4_add(struct nft_handle *h, struct nftnl_rule *r,
> add_compat(r, cs->fw.ip.proto, cs->fw.ip.invflags & XT_INV_PROTO);
>
> for (matchp = cs->matches; matchp; matchp = matchp->next) {
> - ret = add_match(h, r, matchp->match->m);
> + ret = add_match(h, ctx, r, matchp->match->m);
> if (ret < 0)
> return ret;
> }
> diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
> index 658a4f201895..d53f87c1d26e 100644
> --- a/iptables/nft-ipv6.c
> +++ b/iptables/nft-ipv6.c
> @@ -25,8 +25,8 @@
> #include "nft.h"
> #include "nft-shared.h"
>
> -static int nft_ipv6_add(struct nft_handle *h, struct nftnl_rule *r,
> - struct iptables_command_state *cs)
> +static int nft_ipv6_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
> + struct nftnl_rule *r, struct iptables_command_state *cs)
> {
> struct xtables_rule_match *matchp;
> uint32_t op;
> @@ -70,7 +70,7 @@ static int nft_ipv6_add(struct nft_handle *h, struct nftnl_rule *r,
> add_compat(r, cs->fw6.ipv6.proto, cs->fw6.ipv6.invflags & XT_INV_PROTO);
>
> for (matchp = cs->matches; matchp; matchp = matchp->next) {
> - ret = add_match(h, r, matchp->match->m);
> + ret = add_match(h, ctx, r, matchp->match->m);
> if (ret < 0)
> return ret;
> }
> diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
> index a06b263d77c1..4f47058d2ec5 100644
> --- a/iptables/nft-shared.h
> +++ b/iptables/nft-shared.h
> @@ -35,13 +35,14 @@
> | FMT_NUMERIC | FMT_NOTABLE)
> #define FMT(tab,notab) ((format) & FMT_NOTABLE ? (notab) : (tab))
>
> +struct nft_rule_ctx;
> struct xtables_args;
> struct nft_handle;
> struct xt_xlate;
>
> struct nft_family_ops {
> - int (*add)(struct nft_handle *h, struct nftnl_rule *r,
> - struct iptables_command_state *cs);
> + int (*add)(struct nft_handle *h, struct nft_rule_ctx *ctx,
> + struct nftnl_rule *r, struct iptables_command_state *cs);
> bool (*is_same)(const struct iptables_command_state *cs_a,
> const struct iptables_command_state *cs_b);
> void (*print_payload)(struct nftnl_expr *e,
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 1cb104e75ccc..59e3fa7079c4 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -1154,7 +1154,8 @@ gen_lookup(uint32_t sreg, const char *set_name, uint32_t set_id, uint32_t flags)
> #define NFT_DATATYPE_ETHERADDR 9
>
> static int __add_nft_among(struct nft_handle *h, const char *table,
> - struct nftnl_rule *r, struct nft_among_pair *pairs,
> + struct nft_rule_ctx *ctx, struct nftnl_rule *r,
> + struct nft_among_pair *pairs,
> int cnt, bool dst, bool inv, bool ip)
> {
> uint32_t set_id, type = NFT_DATATYPE_ETHERADDR, len = ETH_ALEN;
> @@ -1235,7 +1236,7 @@ static int __add_nft_among(struct nft_handle *h, const char *table,
> return 0;
> }
>
> -static int add_nft_among(struct nft_handle *h,
> +static int add_nft_among(struct nft_handle *h, struct nft_rule_ctx *ctx,
> struct nftnl_rule *r, struct xt_entry_match *m)
> {
> struct nft_among_data *data = (struct nft_among_data *)m->data;
> @@ -1251,10 +1252,10 @@ static int add_nft_among(struct nft_handle *h,
> }
>
> if (data->src.cnt)
> - __add_nft_among(h, table, r, data->pairs, data->src.cnt,
> + __add_nft_among(h, table, ctx, r, data->pairs, data->src.cnt,
> false, data->src.inv, data->src.ip);
> if (data->dst.cnt)
> - __add_nft_among(h, table, r, data->pairs + data->src.cnt,
> + __add_nft_among(h, table, ctx, r, data->pairs + data->src.cnt,
> data->dst.cnt, true, data->dst.inv,
> data->dst.ip);
> return 0;
> @@ -1462,22 +1463,30 @@ static int add_nft_mark(struct nft_handle *h, struct nftnl_rule *r,
> return 0;
> }
>
> -int add_match(struct nft_handle *h,
> +int add_match(struct nft_handle *h, struct nft_rule_ctx *ctx,
> struct nftnl_rule *r, struct xt_entry_match *m)
> {
> struct nftnl_expr *expr;
> int ret;
>
> - if (!strcmp(m->u.user.name, "limit"))
> - return add_nft_limit(r, m);
> - else if (!strcmp(m->u.user.name, "among"))
> - return add_nft_among(h, r, m);
> - else if (!strcmp(m->u.user.name, "udp"))
> - return add_nft_udp(h, r, m);
> - else if (!strcmp(m->u.user.name, "tcp"))
> - return add_nft_tcp(h, r, m);
> - else if (!strcmp(m->u.user.name, "mark"))
> - return add_nft_mark(h, r, m);
> + switch (ctx->command) {
> + case NFT_COMPAT_RULE_APPEND:
> + case NFT_COMPAT_RULE_INSERT:
> + case NFT_COMPAT_RULE_REPLACE:
> + if (!strcmp(m->u.user.name, "limit"))
> + return add_nft_limit(r, m);
> + else if (!strcmp(m->u.user.name, "among"))
> + return add_nft_among(h, ctx, r, m);
> + else if (!strcmp(m->u.user.name, "udp"))
> + return add_nft_udp(h, r, m);
> + else if (!strcmp(m->u.user.name, "tcp"))
> + return add_nft_tcp(h, r, m);
> + else if (!strcmp(m->u.user.name, "mark"))
> + return add_nft_mark(h, r, m);
> + break;
> + default:
> + break;
> + }
>
> expr = nftnl_expr_alloc("match");
> if (expr == NULL)
> @@ -1705,7 +1714,8 @@ void add_compat(struct nftnl_rule *r, uint32_t proto, bool inv)
> }
>
> struct nftnl_rule *
> -nft_rule_new(struct nft_handle *h, const char *chain, const char *table,
> +nft_rule_new(struct nft_handle *h, struct nft_rule_ctx *ctx,
> + const char *chain, const char *table,
> struct iptables_command_state *cs)
> {
> struct nftnl_rule *r;
> @@ -1718,7 +1728,7 @@ nft_rule_new(struct nft_handle *h, const char *chain, const char *table,
> nftnl_rule_set_str(r, NFTNL_RULE_TABLE, table);
> nftnl_rule_set_str(r, NFTNL_RULE_CHAIN, chain);
>
> - if (h->ops->add(h, r, cs) < 0)
> + if (h->ops->add(h, ctx, r, cs) < 0)
> goto err;
>
> return r;
> @@ -2878,6 +2888,9 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
> {
> struct iptables_command_state cs = {};
> struct nftnl_rule *r, *new_rule;
> + struct nft_rule_ctx ctx = {
> + .command = NFT_COMPAT_RULE_ZERO,
> + };
> struct nft_chain *c;
> int ret = 0;
>
> @@ -2896,7 +2909,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
>
> h->ops->rule_to_cs(h, r, &cs);
> cs.counters.pcnt = cs.counters.bcnt = 0;
> - new_rule = nft_rule_new(h, chain, table, &cs);
> + new_rule = nft_rule_new(h, &ctx, chain, table, &cs);
> h->ops->clear_cs(&cs);
>
> if (!new_rule)
> @@ -3274,6 +3287,9 @@ static int ebt_add_policy_rule(struct nftnl_chain *c, void *data)
> .eb.bitmask = EBT_NOPROTO,
> };
> struct nftnl_udata_buf *udata;
> + struct nft_rule_ctx ctx = {
> + .command = NFT_COMPAT_RULE_APPEND,
> + };
> struct nft_handle *h = data;
> struct nftnl_rule *r;
> const char *pname;
> @@ -3301,7 +3317,7 @@ static int ebt_add_policy_rule(struct nftnl_chain *c, void *data)
>
> command_jump(&cs, pname);
>
> - r = nft_rule_new(h, nftnl_chain_get_str(c, NFTNL_CHAIN_NAME),
> + r = nft_rule_new(h, &ctx, nftnl_chain_get_str(c, NFTNL_CHAIN_NAME),
> nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE), &cs);
> ebt_cs_clean(&cs);
>
> diff --git a/iptables/nft.h b/iptables/nft.h
> index 1d18982dc8cf..5acbbf82e2c2 100644
> --- a/iptables/nft.h
> +++ b/iptables/nft.h
> @@ -168,9 +168,11 @@ struct nftnl_set *nft_set_batch_lookup_byid(struct nft_handle *h,
> /*
> * Operations with rule-set.
> */
> -struct nftnl_rule;
> +struct nft_rule_ctx {
> + int command;
> +};
>
> -struct nftnl_rule *nft_rule_new(struct nft_handle *h, const char *chain, const char *table, struct iptables_command_state *cs);
> +struct nftnl_rule *nft_rule_new(struct nft_handle *h, struct nft_rule_ctx *rule, const char *chain, const char *table, struct iptables_command_state *cs);
> int nft_rule_append(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, struct nftnl_rule *ref, bool verbose);
> int nft_rule_insert(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, int rulenum, bool verbose);
> int nft_rule_check(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, bool verbose);
> @@ -188,7 +190,8 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain, const char *
> */
> int add_counters(struct nftnl_rule *r, uint64_t packets, uint64_t bytes);
> int add_verdict(struct nftnl_rule *r, int verdict);
> -int add_match(struct nft_handle *h, struct nftnl_rule *r, struct xt_entry_match *m);
> +int add_match(struct nft_handle *h, struct nft_rule_ctx *ctx,
> + struct nftnl_rule *r, struct xt_entry_match *m);
> int add_target(struct nftnl_rule *r, struct xt_entry_target *t);
> int add_jumpto(struct nftnl_rule *r, const char *name, int verdict);
> int add_action(struct nftnl_rule *r, struct iptables_command_state *cs, bool goto_set);
> --
> 2.30.2
>
--
Igor Raits | Sr. Principal SW Engineer
igor@gooddata.com
+420 775 117 817
Moravske namesti 1007/14
602 00 Brno-Veveri, Czech Republic
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support
2023-07-12 11:05 ` Igor Raits
@ 2023-07-12 14:01 ` Pablo Neira Ayuso
0 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2023-07-12 14:01 UTC (permalink / raw)
To: Igor Raits; +Cc: netfilter-devel, fw, phil
On Wed, Jul 12, 2023 at 01:05:10PM +0200, Igor Raits wrote:
> Hi Pablo,
>
> Thanks for the patch!
>
> On Wed, Jul 12, 2023 at 11:59 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > Add context structure to improve bridge among support which creates an
> > anonymous set. This context structure specifies the command and it
> > allows to optionally store a anonymous set.
> >
> > Use this context to generate native bytecode only if this is an
> > add/insert/replace command.
> >
> > This fixes a dangling anonymous set that is created on rule removal.
> >
> > Fixes: 26753888720d ("nft: bridge: Rudimental among extension support")
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Reported-and-tested-by: Igor Raits <igor@gooddata.com>
I have just pushed it out, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support
2023-07-12 9:59 [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support Pablo Neira Ayuso
2023-07-12 11:05 ` Igor Raits
@ 2023-07-12 17:13 ` Pablo Neira Ayuso
2023-07-13 11:07 ` Phil Sutter
2023-07-13 11:50 ` Phil Sutter
2 siblings, 1 reply; 7+ messages in thread
From: Pablo Neira Ayuso @ 2023-07-12 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: fw, igor, phil
On Wed, Jul 12, 2023 at 11:59:12AM +0200, Pablo Neira Ayuso wrote:
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 1cb104e75ccc..59e3fa7079c4 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
[...]
> @@ -2878,6 +2888,9 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
> {
> struct iptables_command_state cs = {};
> struct nftnl_rule *r, *new_rule;
> + struct nft_rule_ctx ctx = {
> + .command = NFT_COMPAT_RULE_ZERO,
BTW. I changed this to:
.command = NFT_COMPAT_RULE_APPEND,
before pushing it out, for the record.
> + };
> struct nft_chain *c;
> int ret = 0;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support
2023-07-12 17:13 ` Pablo Neira Ayuso
@ 2023-07-13 11:07 ` Phil Sutter
2023-07-13 11:34 ` Pablo Neira Ayuso
0 siblings, 1 reply; 7+ messages in thread
From: Phil Sutter @ 2023-07-13 11:07 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, fw, igor
On Wed, Jul 12, 2023 at 07:13:50PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jul 12, 2023 at 11:59:12AM +0200, Pablo Neira Ayuso wrote:
> > diff --git a/iptables/nft.c b/iptables/nft.c
> > index 1cb104e75ccc..59e3fa7079c4 100644
> > --- a/iptables/nft.c
> > +++ b/iptables/nft.c
> [...]
> > @@ -2878,6 +2888,9 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
> > {
> > struct iptables_command_state cs = {};
> > struct nftnl_rule *r, *new_rule;
> > + struct nft_rule_ctx ctx = {
> > + .command = NFT_COMPAT_RULE_ZERO,
>
> BTW. I changed this to:
>
> .command = NFT_COMPAT_RULE_APPEND,
>
> before pushing it out, for the record.
Hmm. :)
I'm curious how to trigger the problem. Could you please provide a
test-case?
Thanks, Phil
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support
2023-07-13 11:07 ` Phil Sutter
@ 2023-07-13 11:34 ` Pablo Neira Ayuso
0 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2023-07-13 11:34 UTC (permalink / raw)
To: Phil Sutter, netfilter-devel, fw, igor
On Thu, Jul 13, 2023 at 01:07:36PM +0200, Phil Sutter wrote:
> On Wed, Jul 12, 2023 at 07:13:50PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Jul 12, 2023 at 11:59:12AM +0200, Pablo Neira Ayuso wrote:
> > > diff --git a/iptables/nft.c b/iptables/nft.c
> > > index 1cb104e75ccc..59e3fa7079c4 100644
> > > --- a/iptables/nft.c
> > > +++ b/iptables/nft.c
> > [...]
> > > @@ -2878,6 +2888,9 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
> > > {
> > > struct iptables_command_state cs = {};
> > > struct nftnl_rule *r, *new_rule;
> > > + struct nft_rule_ctx ctx = {
> > > + .command = NFT_COMPAT_RULE_ZERO,
> >
> > BTW. I changed this to:
> >
> > .command = NFT_COMPAT_RULE_APPEND,
> >
> > before pushing it out, for the record.
>
> Hmm. :)
>
> I'm curious how to trigger the problem. Could you please provide a
> test-case?
I suspect a problem might occur if the rule that is zeroed generates a
native expression.
nft_rule_zero_counters() calls _append() in practise, to readd the
rule after deleting it to zero the counters.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support
2023-07-12 9:59 [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support Pablo Neira Ayuso
2023-07-12 11:05 ` Igor Raits
2023-07-12 17:13 ` Pablo Neira Ayuso
@ 2023-07-13 11:50 ` Phil Sutter
2 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2023-07-13 11:50 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, fw, igor
On Wed, Jul 12, 2023 at 11:59:12AM +0200, Pablo Neira Ayuso wrote:
[...]
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 1cb104e75ccc..59e3fa7079c4 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -1154,7 +1154,8 @@ gen_lookup(uint32_t sreg, const char *set_name, uint32_t set_id, uint32_t flags)
> #define NFT_DATATYPE_ETHERADDR 9
>
> static int __add_nft_among(struct nft_handle *h, const char *table,
> - struct nftnl_rule *r, struct nft_among_pair *pairs,
> + struct nft_rule_ctx *ctx, struct nftnl_rule *r,
> + struct nft_among_pair *pairs,
> int cnt, bool dst, bool inv, bool ip)
> {
> uint32_t set_id, type = NFT_DATATYPE_ETHERADDR, len = ETH_ALEN;
Is there something missing here? Neither add_nft_among() nor
__add_nft_among() use the new parameter. Or is this left over from a
previous version of the fix?
Cheers, Phil
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-13 11:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-12 9:59 [PATCH iptables] nft-bridge: pass context structure to ops->add() to improve anonymous set support Pablo Neira Ayuso
2023-07-12 11:05 ` Igor Raits
2023-07-12 14:01 ` Pablo Neira Ayuso
2023-07-12 17:13 ` Pablo Neira Ayuso
2023-07-13 11:07 ` Phil Sutter
2023-07-13 11:34 ` Pablo Neira Ayuso
2023-07-13 11:50 ` Phil Sutter
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).