* [PATCH 1/3] netfilter: nft_ct: add missing ifdef for NFT_MARK setting
2014-03-29 10:43 [PATCH 0/3] netfilter: nf_tables: cleanup and a bugfix Patrick McHardy
@ 2014-03-29 10:43 ` Patrick McHardy
2014-03-29 10:43 ` [PATCH 2/3] netfilter: nft_meta: split nft_meta_init() into two functions for get/set Patrick McHardy
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Patrick McHardy @ 2014-03-29 10:43 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
The set operation for ct mark is only valid if CONFIG_NF_CONNTRACK_MARK is
enabled.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/netfilter/nft_ct.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index bd0d41e..a2c45bd 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -268,8 +268,10 @@ static int nft_ct_init_validate_get(const struct nft_expr *expr,
static int nft_ct_init_validate_set(uint32_t key)
{
switch (key) {
+#ifdef CONFIG_NF_CONNTRACK_MARK
case NFT_CT_MARK:
break;
+#endif
default:
return -EOPNOTSUPP;
}
--
1.9.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] netfilter: nft_meta: split nft_meta_init() into two functions for get/set
2014-03-29 10:43 [PATCH 0/3] netfilter: nf_tables: cleanup and a bugfix Patrick McHardy
2014-03-29 10:43 ` [PATCH 1/3] netfilter: nft_ct: add missing ifdef for NFT_MARK setting Patrick McHardy
@ 2014-03-29 10:43 ` Patrick McHardy
2014-03-31 12:37 ` Tomasz Bursztyka
2014-03-29 10:43 ` [PATCH 3/3] netfilter: nft_ct: split nft_ct_init() " Patrick McHardy
2014-03-30 11:35 ` [PATCH 0/3] netfilter: nf_tables: cleanup and a bugfix Pablo Neira Ayuso
3 siblings, 1 reply; 7+ messages in thread
From: Patrick McHardy @ 2014-03-29 10:43 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
For value spanning multiple registers, we need to validate the length
of data loads. In order to add this to nft_meta, we need the length from
key validation. Split the nft_meta_init() function into two functions
for the get and set operations as preparation for that.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/netfilter/nft_meta.c | 65 ++++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 35 deletions(-)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 425cf39..6d0b8cc2 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -170,21 +170,15 @@ static const struct nla_policy nft_meta_policy[NFTA_META_MAX + 1] = {
[NFTA_META_SREG] = { .type = NLA_U32 },
};
-static int nft_meta_init_validate_set(uint32_t key)
+static int nft_meta_get_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
{
- switch (key) {
- case NFT_META_MARK:
- case NFT_META_PRIORITY:
- case NFT_META_NFTRACE:
- return 0;
- default:
- return -EOPNOTSUPP;
- }
-}
+ struct nft_meta *priv = nft_expr_priv(expr);
+ int err;
-static int nft_meta_init_validate_get(uint32_t key)
-{
- switch (key) {
+ priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
+ switch (priv->key) {
case NFT_META_LEN:
case NFT_META_PROTOCOL:
case NFT_META_NFPROTO:
@@ -205,39 +199,40 @@ static int nft_meta_init_validate_get(uint32_t key)
#ifdef CONFIG_NETWORK_SECMARK
case NFT_META_SECMARK:
#endif
- return 0;
+ break;
default:
return -EOPNOTSUPP;
}
+ priv->dreg = ntohl(nla_get_be32(tb[NFTA_META_DREG]));
+ err = nft_validate_output_register(priv->dreg);
+ if (err < 0)
+ return err;
+
+ err = nft_validate_data_load(ctx, priv->dreg, NULL, NFT_DATA_VALUE);
+ if (err < 0)
+ return err;
+
+ return 0;
}
-static int nft_meta_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
- const struct nlattr * const tb[])
+static int nft_meta_set_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
{
struct nft_meta *priv = nft_expr_priv(expr);
int err;
priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
-
- if (tb[NFTA_META_DREG]) {
- err = nft_meta_init_validate_get(priv->key);
- if (err < 0)
- return err;
-
- priv->dreg = ntohl(nla_get_be32(tb[NFTA_META_DREG]));
- err = nft_validate_output_register(priv->dreg);
- if (err < 0)
- return err;
-
- return nft_validate_data_load(ctx, priv->dreg, NULL,
- NFT_DATA_VALUE);
+ switch (priv->key) {
+ case NFT_META_MARK:
+ case NFT_META_PRIORITY:
+ case NFT_META_NFTRACE:
+ break;
+ default:
+ return -EOPNOTSUPP;
}
- err = nft_meta_init_validate_set(priv->key);
- if (err < 0)
- return err;
-
priv->sreg = ntohl(nla_get_be32(tb[NFTA_META_SREG]));
err = nft_validate_input_register(priv->sreg);
if (err < 0)
@@ -282,7 +277,7 @@ static const struct nft_expr_ops nft_meta_get_ops = {
.type = &nft_meta_type,
.size = NFT_EXPR_SIZE(sizeof(struct nft_meta)),
.eval = nft_meta_get_eval,
- .init = nft_meta_init,
+ .init = nft_meta_get_init,
.dump = nft_meta_get_dump,
};
@@ -290,7 +285,7 @@ static const struct nft_expr_ops nft_meta_set_ops = {
.type = &nft_meta_type,
.size = NFT_EXPR_SIZE(sizeof(struct nft_meta)),
.eval = nft_meta_set_eval,
- .init = nft_meta_init,
+ .init = nft_meta_set_init,
.dump = nft_meta_set_dump,
};
--
1.9.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] netfilter: nft_meta: split nft_meta_init() into two functions for get/set
2014-03-29 10:43 ` [PATCH 2/3] netfilter: nft_meta: split nft_meta_init() into two functions for get/set Patrick McHardy
@ 2014-03-31 12:37 ` Tomasz Bursztyka
2014-04-02 11:43 ` Patrick McHardy
0 siblings, 1 reply; 7+ messages in thread
From: Tomasz Bursztyka @ 2014-03-31 12:37 UTC (permalink / raw)
To: Patrick McHardy, pablo; +Cc: netfilter-devel
Hi Patrick,
> diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
> index 425cf39..6d0b8cc2 100644
> --- a/net/netfilter/nft_meta.c
> +++ b/net/netfilter/nft_meta.c
> @@ -170,21 +170,15 @@ static const struct nla_policy nft_meta_policy[NFTA_META_MAX + 1] = {
> [NFTA_META_SREG] = { .type = NLA_U32 },
> };
>
> -static int nft_meta_init_validate_set(uint32_t key)
> +static int nft_meta_get_init(const struct nft_ctx *ctx,
> + const struct nft_expr *expr,
> + const struct nlattr * const tb[])
> {
> - switch (key) {
> - case NFT_META_MARK:
> - case NFT_META_PRIORITY:
> - case NFT_META_NFTRACE:
> - return 0;
> - default:
> - return -EOPNOTSUPP;
> - }
> -}
> + struct nft_meta *priv = nft_expr_priv(expr);
> + int err;
>
> -static int nft_meta_init_validate_get(uint32_t key)
> -{
> - switch (key) {
With the disappearance of nft_meta_init_validate_get(), I will need to add
an #ifdef/#endif clause for bridge key support. Or I would copy and adapt
this new nft_meta_get_init() in nft_meta_bridge.c.
Don't you think nft_meta_validate_get() could be kept?
> + priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
> + switch (priv->key) {
> case NFT_META_LEN:
> case NFT_META_PROTOCOL:
> case NFT_META_NFPROTO:
> @@ -205,39 +199,40 @@ static int nft_meta_init_validate_get(uint32_t key)
> #ifdef CONFIG_NETWORK_SECMARK
> case NFT_META_SECMARK:
> #endif
> - return 0;
> + break;
> default:
> return -EOPNOTSUPP;
> }
>
> + priv->dreg = ntohl(nla_get_be32(tb[NFTA_META_DREG]));
> + err = nft_validate_output_register(priv->dreg);
> + if (err < 0)
> + return err;
> +
> + err = nft_validate_data_load(ctx, priv->dreg, NULL, NFT_DATA_VALUE);
> + if (err < 0)
> + return err;
> +
> + return 0;
> }
>
> -static int nft_meta_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
> - const struct nlattr * const tb[])
> +static int nft_meta_set_init(const struct nft_ctx *ctx,
> + const struct nft_expr *expr,
> + const struct nlattr * const tb[])
> {
> struct nft_meta *priv = nft_expr_priv(expr);
> int err;
>
> priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
> -
> - if (tb[NFTA_META_DREG]) {
> - err = nft_meta_init_validate_get(priv->key);
> - if (err < 0)
> - return err;
> -
> - priv->dreg = ntohl(nla_get_be32(tb[NFTA_META_DREG]));
> - err = nft_validate_output_register(priv->dreg);
> - if (err < 0)
> - return err;
> -
> - return nft_validate_data_load(ctx, priv->dreg, NULL,
> - NFT_DATA_VALUE);
> + switch (priv->key) {
> + case NFT_META_MARK:
> + case NFT_META_PRIORITY:
> + case NFT_META_NFTRACE:
> + break;
> + default:
> + return -EOPNOTSUPP;
> }
>
> - err = nft_meta_init_validate_set(priv->key);
> - if (err < 0)
> - return err;
> -
> priv->sreg = ntohl(nla_get_be32(tb[NFTA_META_SREG]));
> err = nft_validate_input_register(priv->sreg);
> if (err < 0)
> @@ -282,7 +277,7 @@ static const struct nft_expr_ops nft_meta_get_ops = {
> .type = &nft_meta_type,
> .size = NFT_EXPR_SIZE(sizeof(struct nft_meta)),
> .eval = nft_meta_get_eval,
> - .init = nft_meta_init,
> + .init = nft_meta_get_init,
> .dump = nft_meta_get_dump,
> };
>
> @@ -290,7 +285,7 @@ static const struct nft_expr_ops nft_meta_set_ops = {
> .type = &nft_meta_type,
> .size = NFT_EXPR_SIZE(sizeof(struct nft_meta)),
> .eval = nft_meta_set_eval,
> - .init = nft_meta_init,
> + .init = nft_meta_set_init,
> .dump = nft_meta_set_dump,
> };
>
Rest is fine and indeed it simplifies things on my side (no need to c/p
the old nft_meta_init,
here I can reuse existing nft_meta_set_init)
Tomasz
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] netfilter: nft_meta: split nft_meta_init() into two functions for get/set
2014-03-31 12:37 ` Tomasz Bursztyka
@ 2014-04-02 11:43 ` Patrick McHardy
0 siblings, 0 replies; 7+ messages in thread
From: Patrick McHardy @ 2014-04-02 11:43 UTC (permalink / raw)
To: Tomasz Bursztyka; +Cc: pablo, netfilter-devel
On Mon, Mar 31, 2014 at 03:37:43PM +0300, Tomasz Bursztyka wrote:
> Hi Patrick,
>
> >diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
> >index 425cf39..6d0b8cc2 100644
> >--- a/net/netfilter/nft_meta.c
> >+++ b/net/netfilter/nft_meta.c
> >@@ -170,21 +170,15 @@ static const struct nla_policy nft_meta_policy[NFTA_META_MAX + 1] = {
> > [NFTA_META_SREG] = { .type = NLA_U32 },
> > };
> >-static int nft_meta_init_validate_set(uint32_t key)
> >+static int nft_meta_get_init(const struct nft_ctx *ctx,
> >+ const struct nft_expr *expr,
> >+ const struct nlattr * const tb[])
> > {
> >- switch (key) {
> >- case NFT_META_MARK:
> >- case NFT_META_PRIORITY:
> >- case NFT_META_NFTRACE:
> >- return 0;
> >- default:
> >- return -EOPNOTSUPP;
> >- }
> >-}
> >+ struct nft_meta *priv = nft_expr_priv(expr);
> >+ int err;
> >-static int nft_meta_init_validate_get(uint32_t key)
> >-{
> >- switch (key) {
>
> With the disappearance of nft_meta_init_validate_get(), I will need to add
> an #ifdef/#endif clause for bridge key support. Or I would copy and adapt
> this new nft_meta_get_init() in nft_meta_bridge.c.
>
> Don't you think nft_meta_validate_get() could be kept?
Why don't you add a bridge specific init function and invoke
nft_meta_get_init() for unknown keys?
It could be kept by passing a unsigned int *size for returning the size
of the key, but I would prefer to solve this differently.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] netfilter: nft_ct: split nft_ct_init() into two functions for get/set
2014-03-29 10:43 [PATCH 0/3] netfilter: nf_tables: cleanup and a bugfix Patrick McHardy
2014-03-29 10:43 ` [PATCH 1/3] netfilter: nft_ct: add missing ifdef for NFT_MARK setting Patrick McHardy
2014-03-29 10:43 ` [PATCH 2/3] netfilter: nft_meta: split nft_meta_init() into two functions for get/set Patrick McHardy
@ 2014-03-29 10:43 ` Patrick McHardy
2014-03-30 11:35 ` [PATCH 0/3] netfilter: nf_tables: cleanup and a bugfix Pablo Neira Ayuso
3 siblings, 0 replies; 7+ messages in thread
From: Patrick McHardy @ 2014-03-29 10:43 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
For value spanning multiple registers, we need to validate the length
of data loads. In order to add this to nft_ct, we need the length from
key validation. Split the nft_ct_init() function into two functions
for the get and set operations as preparation for that.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/netfilter/nft_ct.c | 96 ++++++++++++++++++++++----------------------------
1 file changed, 43 insertions(+), 53 deletions(-)
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index a2c45bd..cc56030 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -215,22 +215,14 @@ static void nft_ct_l3proto_module_put(uint8_t family)
nf_ct_l3proto_module_put(family);
}
-static int nft_ct_init_validate_get(const struct nft_expr *expr,
- const struct nlattr * const tb[])
+static int nft_ct_get_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
{
struct nft_ct *priv = nft_expr_priv(expr);
+ int err;
- if (tb[NFTA_CT_DIRECTION] != NULL) {
- priv->dir = nla_get_u8(tb[NFTA_CT_DIRECTION]);
- switch (priv->dir) {
- case IP_CT_DIR_ORIGINAL:
- case IP_CT_DIR_REPLY:
- break;
- default:
- return -EINVAL;
- }
- }
-
+ priv->key = ntohl(nla_get_be32(tb[NFTA_CT_KEY]));
switch (priv->key) {
case NFT_CT_STATE:
case NFT_CT_DIRECTION:
@@ -262,12 +254,42 @@ static int nft_ct_init_validate_get(const struct nft_expr *expr,
return -EOPNOTSUPP;
}
+ if (tb[NFTA_CT_DIRECTION] != NULL) {
+ priv->dir = nla_get_u8(tb[NFTA_CT_DIRECTION]);
+ switch (priv->dir) {
+ case IP_CT_DIR_ORIGINAL:
+ case IP_CT_DIR_REPLY:
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ priv->dreg = ntohl(nla_get_be32(tb[NFTA_CT_DREG]));
+ err = nft_validate_output_register(priv->dreg);
+ if (err < 0)
+ return err;
+
+ err = nft_validate_data_load(ctx, priv->dreg, NULL, NFT_DATA_VALUE);
+ if (err < 0)
+ return err;
+
+ err = nft_ct_l3proto_try_module_get(ctx->afi->family);
+ if (err < 0)
+ return err;
+
return 0;
}
-static int nft_ct_init_validate_set(uint32_t key)
+static int nft_ct_set_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
{
- switch (key) {
+ struct nft_ct *priv = nft_expr_priv(expr);
+ int err;
+
+ priv->key = ntohl(nla_get_be32(tb[NFTA_CT_KEY]));
+ switch (priv->key) {
#ifdef CONFIG_NF_CONNTRACK_MARK
case NFT_CT_MARK:
break;
@@ -276,42 +298,10 @@ static int nft_ct_init_validate_set(uint32_t key)
return -EOPNOTSUPP;
}
- return 0;
-}
-
-static int nft_ct_init(const struct nft_ctx *ctx,
- const struct nft_expr *expr,
- const struct nlattr * const tb[])
-{
- struct nft_ct *priv = nft_expr_priv(expr);
- int err;
-
- priv->key = ntohl(nla_get_be32(tb[NFTA_CT_KEY]));
-
- if (tb[NFTA_CT_DREG]) {
- err = nft_ct_init_validate_get(expr, tb);
- if (err < 0)
- return err;
-
- priv->dreg = ntohl(nla_get_be32(tb[NFTA_CT_DREG]));
- err = nft_validate_output_register(priv->dreg);
- if (err < 0)
- return err;
-
- err = nft_validate_data_load(ctx, priv->dreg, NULL,
- NFT_DATA_VALUE);
- if (err < 0)
- return err;
- } else {
- err = nft_ct_init_validate_set(priv->key);
- if (err < 0)
- return err;
-
- priv->sreg = ntohl(nla_get_be32(tb[NFTA_CT_SREG]));
- err = nft_validate_input_register(priv->sreg);
- if (err < 0)
- return err;
- }
+ priv->sreg = ntohl(nla_get_be32(tb[NFTA_CT_SREG]));
+ err = nft_validate_input_register(priv->sreg);
+ if (err < 0)
+ return err;
err = nft_ct_l3proto_try_module_get(ctx->afi->family);
if (err < 0)
@@ -372,7 +362,7 @@ static const struct nft_expr_ops nft_ct_get_ops = {
.type = &nft_ct_type,
.size = NFT_EXPR_SIZE(sizeof(struct nft_ct)),
.eval = nft_ct_get_eval,
- .init = nft_ct_init,
+ .init = nft_ct_get_init,
.destroy = nft_ct_destroy,
.dump = nft_ct_get_dump,
};
@@ -381,7 +371,7 @@ static const struct nft_expr_ops nft_ct_set_ops = {
.type = &nft_ct_type,
.size = NFT_EXPR_SIZE(sizeof(struct nft_ct)),
.eval = nft_ct_set_eval,
- .init = nft_ct_init,
+ .init = nft_ct_set_init,
.destroy = nft_ct_destroy,
.dump = nft_ct_set_dump,
};
--
1.9.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/3] netfilter: nf_tables: cleanup and a bugfix
2014-03-29 10:43 [PATCH 0/3] netfilter: nf_tables: cleanup and a bugfix Patrick McHardy
` (2 preceding siblings ...)
2014-03-29 10:43 ` [PATCH 3/3] netfilter: nft_ct: split nft_ct_init() " Patrick McHardy
@ 2014-03-30 11:35 ` Pablo Neira Ayuso
3 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2014-03-30 11:35 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Sat, Mar 29, 2014 at 10:43:00AM +0000, Patrick McHardy wrote:
> These patches contain some cleanup and preparation for variable sized data.
> To support variable sized data, we need the length of each key for validation
> of the register load. Move the key validation back into the init functions
> of meta/ct to do this. This also qualifies as a cleanup in my opinion, the
> split is quite unnatural and the code is easier to read by handling get and
> set operations seperately.
>
> Additionally a patch to add a missing ifdef for NFT_CT_MARK is included.
Applied, thanks Patrick.
^ permalink raw reply [flat|nested] 7+ messages in thread