* [libnftnl 2/3] set: fix set nlmsg desc parsing
2014-09-18 18:18 [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options Arturo Borrero Gonzalez
@ 2014-09-18 18:18 ` Arturo Borrero Gonzalez
2014-09-18 18:43 ` Pablo Neira Ayuso
2014-09-18 18:18 ` [nft 3/3] src: add set optimization options Arturo Borrero Gonzalez
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-09-18 18:18 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo, kaber, Arturo Borrero Gonzalez
In commit [ff62959](set: add support for set mechanism selection) the
support for parsing the nested attribute (NFTA_SET_DESC) was invalid.
This patch address the issue.
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
src/set.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/src/set.c b/src/set.c
index 3c38334..223ddec 100644
--- a/src/set.c
+++ b/src/set.c
@@ -340,7 +340,7 @@ static int nft_set_desc_parse_attr_cb(const struct nlattr *attr, void *data)
static int nft_set_desc_parse(struct nft_set *s,
const struct nlattr *attr)
{
- struct nlattr *tb[NFTA_SET_MAX+1] = {};
+ struct nlattr *tb[NFTA_SET_DESC_MAX + 1] = {};
if (mnl_attr_parse_nested(attr, nft_set_desc_parse_attr_cb, tb) < 0)
return -1;
@@ -353,24 +353,6 @@ static int nft_set_desc_parse(struct nft_set *s,
return 0;
}
-static int nft_set_nlmsg_desc_parse(const struct nlattr *nest,
- struct nft_set *s)
-{
- struct nlattr *attr;
- int ret = 0;
-
- mnl_attr_for_each_nested(attr, nest) {
- if (mnl_attr_get_type(attr) != NFTA_SET_DESC)
- return -1;
-
- ret = nft_set_desc_parse(s, attr);
- if (ret != 0)
- break;
- }
-
- return ret;
-}
-
int nft_set_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set *s)
{
struct nlattr *tb[NFTA_SET_MAX+1] = {};
@@ -417,7 +399,7 @@ int nft_set_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set *s)
s->flags |= (1 << NFT_SET_ATTR_POLICY);
}
if (tb[NFTA_SET_DESC])
- ret = nft_set_nlmsg_desc_parse(tb[NFTA_SET_DESC], s);
+ ret = nft_set_desc_parse(s, tb[NFTA_SET_DESC]);
s->family = nfg->nfgen_family;
s->flags |= (1 << NFT_SET_ATTR_FAMILY);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [nft 3/3] src: add set optimization options
2014-09-18 18:18 [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options Arturo Borrero Gonzalez
2014-09-18 18:18 ` [libnftnl 2/3] set: fix set nlmsg desc parsing Arturo Borrero Gonzalez
@ 2014-09-18 18:18 ` Arturo Borrero Gonzalez
2014-09-18 18:39 ` Patrick McHardy
2014-09-20 11:45 ` Patrick McHardy
2014-09-18 18:34 ` [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options Patrick McHardy
2014-09-20 11:39 ` Patrick McHardy
3 siblings, 2 replies; 13+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-09-18 18:18 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo, kaber, Arturo Borrero Gonzalez
This patch adds options to choose set optimization mechanisms.
Two new statements are added to the set syntax, and they can be mixed:
nft add set filter set1 { type ipv4_addr ; size 1024 ; }
nft add set filter set1 { type ipv4_addr ; policy memory ; }
nft add set filter set1 { type ipv4_addr ; policy performance ; }
nft add set filter set1 { type ipv4_addr ; policy memory ; size 1024 ; }
nft add set filter set1 { type ipv4_addr ; size 1024 ; policy memory ; }
nft add set filter set1 { type ipv4_addr ; policy performance ; size 1024 ; }
nft add set filter set1 { type ipv4_addr ; size 1024 ; policy performance ; }
Also valid for maps:
nft add map filter map1 { type ipv4_addr : verdict ; policy performace ; }
[...]
This is the output format, which can be imported later with `nft -f':
table filter {
set set1 {
type ipv4_addr
policy memory
size 1024
}
}
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
include/rule.h | 8 ++++++++
src/netlink.c | 19 +++++++++++++++++++
src/parser.y | 31 +++++++++++++++++++++++++++++++
src/rule.c | 27 +++++++++++++++++++++++++++
src/scanner.l | 5 +++++
5 files changed, 90 insertions(+)
diff --git a/include/rule.h b/include/rule.h
index 88aefc6..49a39ea 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -180,6 +180,9 @@ enum set_flags {
* @datatype: mapping data type
* @datalen: mapping data len
* @init: initializer
+ * @attr_flags: set attr flags
+ * @policy: set mechanism policy
+ * @desc: set mechanism desc
*/
struct set {
struct list_head list;
@@ -192,6 +195,11 @@ struct set {
const struct datatype *datatype;
unsigned int datalen;
struct expr *init;
+ uint32_t attr_flags;
+ uint32_t policy;
+ struct {
+ uint32_t size;
+ } desc;
};
extern struct set *set_alloc(const struct location *loc);
diff --git a/src/netlink.c b/src/netlink.c
index 84d5db3..eca4858 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1045,6 +1045,17 @@ static struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
set->datalen = data_len * BITS_PER_BYTE;
}
+ if (nft_set_attr_is_set(nls, NFT_SET_ATTR_POLICY)) {
+ set->policy = nft_set_attr_get_u32(nls, NFT_SET_ATTR_POLICY);
+ set->attr_flags |= (1 << NFT_SET_ATTR_POLICY);
+ }
+
+ if (nft_set_attr_is_set(nls, NFT_SET_ATTR_DESC_SIZE)) {
+ set->desc.size = nft_set_attr_get_u32(nls,
+ NFT_SET_ATTR_DESC_SIZE);
+ set->attr_flags |= (1 << NFT_SET_ATTR_DESC_SIZE);
+ }
+
return set;
}
@@ -1103,6 +1114,14 @@ static int netlink_add_set_batch(struct netlink_ctx *ctx,
}
set->handle.set_id = ++set_id;
nft_set_attr_set_u32(nls, NFT_SET_ATTR_ID, set->handle.set_id);
+
+ if (set->attr_flags & (1 << NFT_SET_ATTR_POLICY))
+ nft_set_attr_set_u32(nls, NFT_SET_ATTR_POLICY, set->policy);
+
+ if (set->attr_flags & (1 << NFT_SET_ATTR_DESC_SIZE))
+ nft_set_attr_set_u32(nls, NFT_SET_ATTR_DESC_SIZE,
+ set->desc.size);
+
netlink_dump_set(nls);
err = mnl_nft_set_batch_add(nf_sock, nls, NLM_F_EXCL, ctx->seqnum);
diff --git a/src/parser.y b/src/parser.y
index c9b22f0..8c8f94a 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -20,6 +20,7 @@
#include <linux/netfilter/nf_tables.h>
#include <linux/netfilter/nf_conntrack_tuple_common.h>
#include <libnftnl/common.h>
+#include <libnftnl/set.h>
#include <rule.h>
#include <statement.h>
@@ -201,6 +202,11 @@ static void location_update(struct location *loc, struct location *rhs, int n)
%token INTERVAL "interval"
%token ELEMENTS "elements"
+%token POLICY "policy"
+%token MEMORY "memory"
+%token PERFORMANCE "performance"
+%token SIZE "size"
+
%token <val> NUM "number"
%token <string> STRING "string"
%token <string> QUOTED_STRING
@@ -401,6 +407,9 @@ static void location_update(struct location *loc, struct location *rhs, int n)
%type <val> set_flag_list set_flag
+%type <val> set_policy_spec
+%type <val> set_size_spec
+
%type <set> set_block_alloc set_block
%destructor { set_free($$); } set_block_alloc
@@ -953,6 +962,7 @@ set_block : /* empty */ { $$ = $<set>-1; }
state->msgs);
YYERROR;
}
+
$$ = $1;
}
| set_block FLAGS set_flag_list stmt_seperator
@@ -965,6 +975,7 @@ set_block : /* empty */ { $$ = $<set>-1; }
$1->init = $4;
$$ = $1;
}
+ | set_block set_mechanism stmt_seperator
;
set_flag_list : set_flag_list COMMA set_flag
@@ -1018,6 +1029,26 @@ map_block : /* empty */ { $$ = $<set>-1; }
$1->init = $4;
$$ = $1;
}
+ | map_block set_mechanism stmt_seperator
+ ;
+
+set_mechanism : set_policy_spec
+ {
+ $<set>0->attr_flags |= (1 << NFT_SET_ATTR_POLICY);
+ $<set>0->policy = $1;
+ }
+ | set_size_spec
+ {
+ $<set>0->attr_flags |= (1 << NFT_SET_ATTR_DESC_SIZE);
+ $<set>0->desc.size = $1;
+ }
+ ;
+
+set_policy_spec : POLICY PERFORMANCE { $$ = NFT_SET_POL_PERFORMANCE; }
+ | POLICY MEMORY { $$ = NFT_SET_POL_MEMORY; }
+ ;
+
+set_size_spec : SIZE NUM { $$ = $2; }
;
hook_spec : TYPE STRING HOOK STRING PRIORITY NUM
diff --git a/src/rule.c b/src/rule.c
index 80deb1b..e2290d7 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -75,6 +75,7 @@ void set_free(struct set *set)
if (--set->refcnt > 0)
return;
handle_free(&set->handle);
+
xfree(set);
}
@@ -90,6 +91,9 @@ struct set *set_clone(const struct set *set)
newset->datatype = set->datatype;
newset->datalen = set->datalen;
newset->init = expr_clone(set->init);
+ newset->attr_flags = set->attr_flags;
+ newset->policy = set->policy;
+ newset->desc.size = set->desc.size;
return newset;
}
@@ -134,6 +138,18 @@ struct print_fmt_options {
const char *stmt_separator;
};
+static const char *set_policy2str(uint32_t policy)
+{
+ switch (policy) {
+ case NFT_SET_POL_PERFORMANCE:
+ return "performance";
+ case NFT_SET_POL_MEMORY:
+ return "memory";
+ default:
+ return "unknown";
+ }
+}
+
static void do_set_print(const struct set *set, struct print_fmt_options *opts)
{
const char *delim = "";
@@ -153,8 +169,19 @@ static void do_set_print(const struct set *set, struct print_fmt_options *opts)
printf("%s%stype %s", opts->tab, opts->tab, set->keytype->name);
if (set->flags & SET_F_MAP)
printf(" : %s", set->datatype->name);
+
printf("%s", opts->stmt_separator);
+ if (set->attr_flags & (1 << NFT_SET_ATTR_POLICY)) {
+ printf("%s%spolicy %s%s", opts->tab, opts->tab,
+ set_policy2str(set->policy), opts->stmt_separator);
+ }
+
+ if (set->attr_flags & (1 << NFT_SET_ATTR_DESC_SIZE)) {
+ printf("%s%ssize %u%s", opts->tab, opts->tab,
+ set->desc.size, opts->stmt_separator);
+ }
+
if (set->flags & (SET_F_CONSTANT | SET_F_INTERVAL)) {
printf("%s%sflags ", opts->tab, opts->tab);
if (set->flags & SET_F_CONSTANT) {
diff --git a/src/scanner.l b/src/scanner.l
index 8aab38f..6458d09 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -271,6 +271,11 @@ addrstring ({macaddr}|{ip4addr}|{ip6addr})
"interval" { return INTERVAL; }
"elements" { return ELEMENTS; }
+"policy" { return POLICY; }
+"size" { return SIZE; }
+"performance" { return PERFORMANCE; }
+"memory" { return MEMORY; }
+
"counter" { return COUNTER; }
"packets" { return PACKETS; }
"bytes" { return BYTES; }
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [nft 3/3] src: add set optimization options
2014-09-18 18:18 ` [nft 3/3] src: add set optimization options Arturo Borrero Gonzalez
@ 2014-09-18 18:39 ` Patrick McHardy
2014-09-19 7:04 ` Arturo Borrero Gonzalez
2014-09-20 11:45 ` Patrick McHardy
1 sibling, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2014-09-18 18:39 UTC (permalink / raw)
To: Arturo Borrero Gonzalez, netfilter-devel; +Cc: pablo
On 18. September 2014 20:18:20 MESZ, Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote:
>This patch adds options to choose set optimization mechanisms.
>
>Two new statements are added to the set syntax, and they can be mixed:
>
> nft add set filter set1 { type ipv4_addr ; size 1024 ; }
> nft add set filter set1 { type ipv4_addr ; policy memory ; }
> nft add set filter set1 { type ipv4_addr ; policy performance ; }
>nft add set filter set1 { type ipv4_addr ; policy memory ; size 1024 ;
>}
>nft add set filter set1 { type ipv4_addr ; size 1024 ; policy memory ;
>}
>nft add set filter set1 { type ipv4_addr ; policy performance ; size
>1024 ; }
>nft add set filter set1 { type ipv4_addr ; size 1024 ; policy
>performance ; }
>
>Also valid for maps:
>
>nft add map filter map1 { type ipv4_addr : verdict ; policy performace
>; }
> [...]
>
>
>This is the output format, which can be imported later with `nft -f':
>
>table filter {
> set set1 {
> type ipv4_addr
> policy memory
> size 1024
> }
>}
Conceptually this looks good, I'll have a look at the implementation after dinner.
What my patch did was only handle the case where limits can be determined automatically, IOW literal sets. Both is needed.
>
>Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
>---
> include/rule.h | 8 ++++++++
> src/netlink.c | 19 +++++++++++++++++++
> src/parser.y | 31 +++++++++++++++++++++++++++++++
> src/rule.c | 27 +++++++++++++++++++++++++++
> src/scanner.l | 5 +++++
> 5 files changed, 90 insertions(+)
>
>diff --git a/include/rule.h b/include/rule.h
>index 88aefc6..49a39ea 100644
>--- a/include/rule.h
>+++ b/include/rule.h
>@@ -180,6 +180,9 @@ enum set_flags {
> * @datatype: mapping data type
> * @datalen: mapping data len
> * @init: initializer
>+ * @attr_flags: set attr flags
>+ * @policy: set mechanism policy
>+ * @desc: set mechanism desc
> */
> struct set {
> struct list_head list;
>@@ -192,6 +195,11 @@ struct set {
> const struct datatype *datatype;
> unsigned int datalen;
> struct expr *init;
>+ uint32_t attr_flags;
>+ uint32_t policy;
>+ struct {
>+ uint32_t size;
>+ } desc;
> };
>
> extern struct set *set_alloc(const struct location *loc);
>diff --git a/src/netlink.c b/src/netlink.c
>index 84d5db3..eca4858 100644
>--- a/src/netlink.c
>+++ b/src/netlink.c
>@@ -1045,6 +1045,17 @@ static struct set
>*netlink_delinearize_set(struct netlink_ctx *ctx,
> set->datalen = data_len * BITS_PER_BYTE;
> }
>
>+ if (nft_set_attr_is_set(nls, NFT_SET_ATTR_POLICY)) {
>+ set->policy = nft_set_attr_get_u32(nls, NFT_SET_ATTR_POLICY);
>+ set->attr_flags |= (1 << NFT_SET_ATTR_POLICY);
>+ }
>+
>+ if (nft_set_attr_is_set(nls, NFT_SET_ATTR_DESC_SIZE)) {
>+ set->desc.size = nft_set_attr_get_u32(nls,
>+ NFT_SET_ATTR_DESC_SIZE);
>+ set->attr_flags |= (1 << NFT_SET_ATTR_DESC_SIZE);
>+ }
>+
> return set;
> }
>
>@@ -1103,6 +1114,14 @@ static int netlink_add_set_batch(struct
>netlink_ctx *ctx,
> }
> set->handle.set_id = ++set_id;
> nft_set_attr_set_u32(nls, NFT_SET_ATTR_ID, set->handle.set_id);
>+
>+ if (set->attr_flags & (1 << NFT_SET_ATTR_POLICY))
>+ nft_set_attr_set_u32(nls, NFT_SET_ATTR_POLICY, set->policy);
>+
>+ if (set->attr_flags & (1 << NFT_SET_ATTR_DESC_SIZE))
>+ nft_set_attr_set_u32(nls, NFT_SET_ATTR_DESC_SIZE,
>+ set->desc.size);
>+
> netlink_dump_set(nls);
>
> err = mnl_nft_set_batch_add(nf_sock, nls, NLM_F_EXCL, ctx->seqnum);
>diff --git a/src/parser.y b/src/parser.y
>index c9b22f0..8c8f94a 100644
>--- a/src/parser.y
>+++ b/src/parser.y
>@@ -20,6 +20,7 @@
> #include <linux/netfilter/nf_tables.h>
> #include <linux/netfilter/nf_conntrack_tuple_common.h>
> #include <libnftnl/common.h>
>+#include <libnftnl/set.h>
>
> #include <rule.h>
> #include <statement.h>
>@@ -201,6 +202,11 @@ static void location_update(struct location *loc,
>struct location *rhs, int n)
> %token INTERVAL "interval"
> %token ELEMENTS "elements"
>
>+%token POLICY "policy"
>+%token MEMORY "memory"
>+%token PERFORMANCE "performance"
>+%token SIZE "size"
>+
> %token <val> NUM "number"
> %token <string> STRING "string"
> %token <string> QUOTED_STRING
>@@ -401,6 +407,9 @@ static void location_update(struct location *loc,
>struct location *rhs, int n)
>
> %type <val> set_flag_list set_flag
>
>+%type <val> set_policy_spec
>+%type <val> set_size_spec
>+
> %type <set> set_block_alloc set_block
> %destructor { set_free($$); } set_block_alloc
>
>@@ -953,6 +962,7 @@ set_block : /* empty */ { $$ = $<set>-1; }
> state->msgs);
> YYERROR;
> }
>+
> $$ = $1;
> }
> | set_block FLAGS set_flag_list stmt_seperator
>@@ -965,6 +975,7 @@ set_block : /* empty */ { $$ = $<set>-1; }
> $1->init = $4;
> $$ = $1;
> }
>+ | set_block set_mechanism stmt_seperator
> ;
>
> set_flag_list : set_flag_list COMMA set_flag
>@@ -1018,6 +1029,26 @@ map_block : /* empty */ { $$ = $<set>-1; }
> $1->init = $4;
> $$ = $1;
> }
>+ | map_block set_mechanism stmt_seperator
>+ ;
>+
>+set_mechanism : set_policy_spec
>+ {
>+ $<set>0->attr_flags |= (1 << NFT_SET_ATTR_POLICY);
>+ $<set>0->policy = $1;
>+ }
>+ | set_size_spec
>+ {
>+ $<set>0->attr_flags |= (1 << NFT_SET_ATTR_DESC_SIZE);
>+ $<set>0->desc.size = $1;
>+ }
>+ ;
>+
>+set_policy_spec : POLICY PERFORMANCE { $$ = NFT_SET_POL_PERFORMANCE;
>}
>+ | POLICY MEMORY { $$ = NFT_SET_POL_MEMORY; }
>+ ;
>+
>+set_size_spec : SIZE NUM { $$ = $2; }
> ;
>
> hook_spec : TYPE STRING HOOK STRING PRIORITY NUM
>diff --git a/src/rule.c b/src/rule.c
>index 80deb1b..e2290d7 100644
>--- a/src/rule.c
>+++ b/src/rule.c
>@@ -75,6 +75,7 @@ void set_free(struct set *set)
> if (--set->refcnt > 0)
> return;
> handle_free(&set->handle);
>+
> xfree(set);
> }
>
>@@ -90,6 +91,9 @@ struct set *set_clone(const struct set *set)
> newset->datatype = set->datatype;
> newset->datalen = set->datalen;
> newset->init = expr_clone(set->init);
>+ newset->attr_flags = set->attr_flags;
>+ newset->policy = set->policy;
>+ newset->desc.size = set->desc.size;
>
> return newset;
> }
>@@ -134,6 +138,18 @@ struct print_fmt_options {
> const char *stmt_separator;
> };
>
>+static const char *set_policy2str(uint32_t policy)
>+{
>+ switch (policy) {
>+ case NFT_SET_POL_PERFORMANCE:
>+ return "performance";
>+ case NFT_SET_POL_MEMORY:
>+ return "memory";
>+ default:
>+ return "unknown";
>+ }
>+}
>+
>static void do_set_print(const struct set *set, struct
>print_fmt_options *opts)
> {
> const char *delim = "";
>@@ -153,8 +169,19 @@ static void do_set_print(const struct set *set,
>struct print_fmt_options *opts)
> printf("%s%stype %s", opts->tab, opts->tab, set->keytype->name);
> if (set->flags & SET_F_MAP)
> printf(" : %s", set->datatype->name);
>+
> printf("%s", opts->stmt_separator);
>
>+ if (set->attr_flags & (1 << NFT_SET_ATTR_POLICY)) {
>+ printf("%s%spolicy %s%s", opts->tab, opts->tab,
>+ set_policy2str(set->policy), opts->stmt_separator);
>+ }
>+
>+ if (set->attr_flags & (1 << NFT_SET_ATTR_DESC_SIZE)) {
>+ printf("%s%ssize %u%s", opts->tab, opts->tab,
>+ set->desc.size, opts->stmt_separator);
>+ }
>+
> if (set->flags & (SET_F_CONSTANT | SET_F_INTERVAL)) {
> printf("%s%sflags ", opts->tab, opts->tab);
> if (set->flags & SET_F_CONSTANT) {
>diff --git a/src/scanner.l b/src/scanner.l
>index 8aab38f..6458d09 100644
>--- a/src/scanner.l
>+++ b/src/scanner.l
>@@ -271,6 +271,11 @@ addrstring ({macaddr}|{ip4addr}|{ip6addr})
> "interval" { return INTERVAL; }
> "elements" { return ELEMENTS; }
>
>+"policy" { return POLICY; }
>+"size" { return SIZE; }
>+"performance" { return PERFORMANCE; }
>+"memory" { return MEMORY; }
>+
> "counter" { return COUNTER; }
> "packets" { return PACKETS; }
> "bytes" { return BYTES; }
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [nft 3/3] src: add set optimization options
2014-09-18 18:39 ` Patrick McHardy
@ 2014-09-19 7:04 ` Arturo Borrero Gonzalez
2014-09-19 17:24 ` Patrick McHardy
0 siblings, 1 reply; 13+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-09-19 7:04 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Development Mailing list, Pablo Neira Ayuso
On 18 September 2014 20:39, Patrick McHardy <kaber@trash.net> wrote:
> On 18. September 2014 20:18:20 MESZ, Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote:
>>This patch adds options to choose set optimization mechanisms.
>>
>>Two new statements are added to the set syntax, and they can be mixed:
>>
>> nft add set filter set1 { type ipv4_addr ; size 1024 ; }
>> nft add set filter set1 { type ipv4_addr ; policy memory ; }
>> nft add set filter set1 { type ipv4_addr ; policy performance ; }
>>nft add set filter set1 { type ipv4_addr ; policy memory ; size 1024 ;
>>}
>>nft add set filter set1 { type ipv4_addr ; size 1024 ; policy memory ;
>>}
>>nft add set filter set1 { type ipv4_addr ; policy performance ; size
>>1024 ; }
>>nft add set filter set1 { type ipv4_addr ; size 1024 ; policy
>>performance ; }
>>
>>Also valid for maps:
>>
>>nft add map filter map1 { type ipv4_addr : verdict ; policy performace
>>; }
>> [...]
>>
>>
>>This is the output format, which can be imported later with `nft -f':
>>
>>table filter {
>> set set1 {
>> type ipv4_addr
>> policy memory
>> size 1024
>> }
>>}
>
> Conceptually this looks good, I'll have a look at the implementation after dinner.
>
> What my patch did was only handle the case where limits can be determined automatically, IOW literal sets. Both is needed.
>
Do you mean to give the size parameter a value when we know the set
has a concrete number of elements?
For example:
add rule tcp dport {1 , 2 , 3} counter --> then add a set with fixed size 3.
I realize now the patch includes some newlines included by mistake. So
a v2 is likely to be needed.
--
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [nft 3/3] src: add set optimization options
2014-09-19 7:04 ` Arturo Borrero Gonzalez
@ 2014-09-19 17:24 ` Patrick McHardy
0 siblings, 0 replies; 13+ messages in thread
From: Patrick McHardy @ 2014-09-19 17:24 UTC (permalink / raw)
To: Arturo Borrero Gonzalez
Cc: Netfilter Development Mailing list, Pablo Neira Ayuso
On 19. September 2014 09:04:17 MESZ, Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote:
>On 18 September 2014 20:39, Patrick McHardy <kaber@trash.net> wrote:
>> On 18. September 2014 20:18:20 MESZ, Arturo Borrero Gonzalez
><arturo.borrero.glez@gmail.com> wrote:
>>>This patch adds options to choose set optimization mechanisms.
>>>
>>>Two new statements are added to the set syntax, and they can be
>mixed:
>>>
>>> nft add set filter set1 { type ipv4_addr ; size 1024 ; }
>>> nft add set filter set1 { type ipv4_addr ; policy memory ; }
>>> nft add set filter set1 { type ipv4_addr ; policy performance ; }
>>>nft add set filter set1 { type ipv4_addr ; policy memory ; size 1024
>;
>>>}
>>>nft add set filter set1 { type ipv4_addr ; size 1024 ; policy memory
>;
>>>}
>>>nft add set filter set1 { type ipv4_addr ; policy performance ; size
>>>1024 ; }
>>>nft add set filter set1 { type ipv4_addr ; size 1024 ; policy
>>>performance ; }
>>>
>>>Also valid for maps:
>>>
>>>nft add map filter map1 { type ipv4_addr : verdict ; policy
>performace
>>>; }
>>> [...]
>>>
>>>
>>>This is the output format, which can be imported later with `nft -f':
>>>
>>>table filter {
>>> set set1 {
>>> type ipv4_addr
>>> policy memory
>>> size 1024
>>> }
>>>}
>>
>> Conceptually this looks good, I'll have a look at the implementation
>after dinner.
Sorry, late nicht after dinner, but I'll get to it tommorrow :)
>> What my patch did was only handle the case where limits can be
>determined automatically, IOW literal sets. Both is needed.
>>
>
>Do you mean to give the size parameter a value when we know the set
>has a concrete number of elements?
>For example:
>add rule tcp dport {1 , 2 , 3} counter --> then add a set with fixed
>size 3.
Yes, this is what my patch is doing, and some further analysis of common prefixes etc for an unfinished feature.
>I realize now the patch includes some newlines included by mistake. So
>a v2 is likely to be needed.
Yeah. I've only read it on my mobile so far, but I also noticed some changes which aren't needed, so please hold off with a V2 until tommorrow.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [nft 3/3] src: add set optimization options
2014-09-18 18:18 ` [nft 3/3] src: add set optimization options Arturo Borrero Gonzalez
2014-09-18 18:39 ` Patrick McHardy
@ 2014-09-20 11:45 ` Patrick McHardy
1 sibling, 0 replies; 13+ messages in thread
From: Patrick McHardy @ 2014-09-20 11:45 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: netfilter-devel, pablo
On Thu, Sep 18, 2014 at 08:18:20PM +0200, Arturo Borrero Gonzalez wrote:
> This patch adds options to choose set optimization mechanisms.
>
> Two new statements are added to the set syntax, and they can be mixed:
>
> nft add set filter set1 { type ipv4_addr ; size 1024 ; }
> nft add set filter set1 { type ipv4_addr ; policy memory ; }
> nft add set filter set1 { type ipv4_addr ; policy performance ; }
> nft add set filter set1 { type ipv4_addr ; policy memory ; size 1024 ; }
> nft add set filter set1 { type ipv4_addr ; size 1024 ; policy memory ; }
> nft add set filter set1 { type ipv4_addr ; policy performance ; size 1024 ; }
> nft add set filter set1 { type ipv4_addr ; size 1024 ; policy performance ; }
>
> Also valid for maps:
>
> nft add map filter map1 { type ipv4_addr : verdict ; policy performace ; }
> [...]
I'd suggest to also add a global option which is the default unless
overridden by a specific set instantiation. Doesn't have to be done
in this patch however.
> @@ -1103,6 +1114,14 @@ static int netlink_add_set_batch(struct netlink_ctx *ctx,
> }
> set->handle.set_id = ++set_id;
> nft_set_attr_set_u32(nls, NFT_SET_ATTR_ID, set->handle.set_id);
> +
> + if (set->attr_flags & (1 << NFT_SET_ATTR_POLICY))
> + nft_set_attr_set_u32(nls, NFT_SET_ATTR_POLICY, set->policy);
> +
> + if (set->attr_flags & (1 << NFT_SET_ATTR_DESC_SIZE))
> + nft_set_attr_set_u32(nls, NFT_SET_ATTR_DESC_SIZE,
> + set->desc.size);
> +
As in the kernel, the size definitely doesn't need an attribute, the policy
can be argued about.
> @@ -965,6 +975,7 @@ set_block : /* empty */ { $$ = $<set>-1; }
> $1->init = $4;
> $$ = $1;
> }
> + | set_block set_mechanism stmt_seperator
> ;
>
> set_flag_list : set_flag_list COMMA set_flag
> @@ -1018,6 +1029,26 @@ map_block : /* empty */ { $$ = $<set>-1; }
> $1->init = $4;
> $$ = $1;
> }
> + | map_block set_mechanism stmt_seperator
> + ;
> +
> +set_mechanism : set_policy_spec
> + {
> + $<set>0->attr_flags |= (1 << NFT_SET_ATTR_POLICY);
> + $<set>0->policy = $1;
> + }
> + | set_size_spec
> + {
> + $<set>0->attr_flags |= (1 << NFT_SET_ATTR_DESC_SIZE);
> + $<set>0->desc.size = $1;
> + }
> + ;
> +
> +set_policy_spec : POLICY PERFORMANCE { $$ = NFT_SET_POL_PERFORMANCE; }
> + | POLICY MEMORY { $$ = NFT_SET_POL_MEMORY; }
> + ;
> +
> +set_size_spec : SIZE NUM { $$ = $2; }
We usually have rules like this:
set_policy: PERFORMANCE ...
MEMORY
and the keyword POLICY one level above.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options
2014-09-18 18:18 [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options Arturo Borrero Gonzalez
2014-09-18 18:18 ` [libnftnl 2/3] set: fix set nlmsg desc parsing Arturo Borrero Gonzalez
2014-09-18 18:18 ` [nft 3/3] src: add set optimization options Arturo Borrero Gonzalez
@ 2014-09-18 18:34 ` Patrick McHardy
2014-09-19 6:51 ` Arturo Borrero Gonzalez
2014-09-20 11:39 ` Patrick McHardy
3 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2014-09-18 18:34 UTC (permalink / raw)
To: Arturo Borrero Gonzalez, netfilter-devel; +Cc: pablo
On 18. September 2014 20:18:18 MESZ, Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote:
>The sets mechanism options was not being stored anywhere.
>
>We want to know in which cases the user explicitly set the mechanism
>options. In that case, we also want to dump back the info.
>
I don't think this is needed. Basically we always want to dump options for non-constant sets, since they couldn't have been chosen automatically, and don't need to dump them for constant sets, since they can always be determined automatically based on the contents.
>Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
>---
> include/net/netfilter/nf_tables.h | 12 +++++++++++
>net/netfilter/nf_tables_api.c | 42
>+++++++++++++++++++++++++++----------
> 2 files changed, 43 insertions(+), 11 deletions(-)
>
>diff --git a/include/net/netfilter/nf_tables.h
>b/include/net/netfilter/nf_tables.h
>index c4d8619..a9c6387 100644
>--- a/include/net/netfilter/nf_tables.h
>+++ b/include/net/netfilter/nf_tables.h
>@@ -231,6 +231,14 @@ struct nft_set_ops {
> int nft_register_set(struct nft_set_ops *ops);
> void nft_unregister_set(struct nft_set_ops *ops);
>
>+/* internal flags to know which attributes were originally set
>+ * from userspace.
>+ */
>+enum nft_set_attr {
>+ NFT_SET_ATTR_POLICY = 0x1,
>+ NFT_SET_ATTR_DESC_SIZE = 0x2,
>+};
>+
> /**
> * struct nft_set - nf_tables set instance
> *
>@@ -241,6 +249,8 @@ void nft_unregister_set(struct nft_set_ops *ops);
> * @dtype: data type (verdict or numeric type defined by userspace)
> * @size: maximum set size
> * @nelems: number of elements
>+ * @attr_flags: (enum nft_set_flags)
>+ * @policy: (enum nft_set_policies)
> * @ops: set ops
> * @flags: set flags
> * @klen: key length
>@@ -255,6 +265,8 @@ struct nft_set {
> u32 dtype;
> u32 size;
> u32 nelems;
>+ u16 attr_flags;
>+ u32 policy;
> /* runtime data below here */
> const struct nft_set_ops *ops ____cacheline_aligned;
> u16 flags;
>diff --git a/net/netfilter/nf_tables_api.c
>b/net/netfilter/nf_tables_api.c
>index 8237460..d1c3f3e 100644
>--- a/net/netfilter/nf_tables_api.c
>+++ b/net/netfilter/nf_tables_api.c
>@@ -2342,13 +2342,24 @@ static int nf_tables_fill_set(struct sk_buff
>*skb, const struct nft_ctx *ctx,
> goto nla_put_failure;
> }
>
>- desc = nla_nest_start(skb, NFTA_SET_DESC);
>- if (desc == NULL)
>- goto nla_put_failure;
>- if (set->size &&
>- nla_put_be32(skb, NFTA_SET_DESC_SIZE, htonl(set->size)))
>- goto nla_put_failure;
>- nla_nest_end(skb, desc);
>+ /* dump policy and desc info only if they were explicitly set */
>+ if (set->attr_flags & (1 << NFT_SET_ATTR_POLICY)) {
>+ if (nla_put_be32(skb, NFTA_SET_POLICY, htonl(set->policy)))
>+ goto nla_put_failure;
>+ }
>+
>+ if (set->attr_flags & (1 << NFT_SET_ATTR_DESC_SIZE)) {
>+ desc = nla_nest_start(skb, NFTA_SET_DESC);
>+ if (desc == NULL)
>+ goto nla_put_failure;
>+
>+ if (set->size &&
>+ nla_put_be32(skb, NFTA_SET_DESC_SIZE, htonl(set->size))) {
>+ goto nla_put_failure;
>+ }
>+
>+ nla_nest_end(skb, desc);
>+ }
>
> return nlmsg_end(skb, nlh);
>
>@@ -2519,7 +2530,8 @@ err:
>
> static int nf_tables_set_desc_parse(const struct nft_ctx *ctx,
> struct nft_set_desc *desc,
>- const struct nlattr *nla)
>+ const struct nlattr *nla,
>+ u16 *attr_flags)
> {
> struct nlattr *da[NFTA_SET_DESC_MAX + 1];
> int err;
>@@ -2528,8 +2540,10 @@ static int nf_tables_set_desc_parse(const struct
>nft_ctx *ctx,
> if (err < 0)
> return err;
>
>- if (da[NFTA_SET_DESC_SIZE] != NULL)
>+ if (da[NFTA_SET_DESC_SIZE] != NULL) {
> desc->size = ntohl(nla_get_be32(da[NFTA_SET_DESC_SIZE]));
>+ *attr_flags |= (1 << NFT_SET_ATTR_DESC_SIZE);
>+ }
>
> return 0;
> }
>@@ -2549,6 +2563,7 @@ static int nf_tables_newset(struct sock *nlsk,
>struct sk_buff *skb,
> unsigned int size;
> bool create;
> u32 ktype, dtype, flags, policy;
>+ u16 attr_flags = 0;
> struct nft_set_desc desc;
> int err;
>
>@@ -2602,11 +2617,14 @@ static int nf_tables_newset(struct sock *nlsk,
>struct sk_buff *skb,
> return -EINVAL;
>
> policy = NFT_SET_POL_PERFORMANCE;
>- if (nla[NFTA_SET_POLICY] != NULL)
>+ if (nla[NFTA_SET_POLICY] != NULL) {
> policy = ntohl(nla_get_be32(nla[NFTA_SET_POLICY]));
>+ attr_flags |= (1 << NFT_SET_ATTR_POLICY);
>+ }
>
> if (nla[NFTA_SET_DESC] != NULL) {
>- err = nf_tables_set_desc_parse(&ctx, &desc, nla[NFTA_SET_DESC]);
>+ err = nf_tables_set_desc_parse(&ctx, &desc, nla[NFTA_SET_DESC],
>+ &attr_flags);
> if (err < 0)
> return err;
> }
>@@ -2667,6 +2685,8 @@ static int nf_tables_newset(struct sock *nlsk,
>struct sk_buff *skb,
> set->dlen = desc.dlen;
> set->flags = flags;
> set->size = desc.size;
>+ set->attr_flags = attr_flags;
>+ set->policy = policy;
>
> err = ops->init(set, &desc, nla);
> if (err < 0)
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options
2014-09-18 18:34 ` [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options Patrick McHardy
@ 2014-09-19 6:51 ` Arturo Borrero Gonzalez
2014-09-19 17:20 ` Patrick McHardy
0 siblings, 1 reply; 13+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-09-19 6:51 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Development Mailing list, Pablo Neira Ayuso
On 18 September 2014 20:34, Patrick McHardy <kaber@trash.net> wrote:
> On 18. September 2014 20:18:18 MESZ, Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote:
>>The sets mechanism options was not being stored anywhere.
>>
>>We want to know in which cases the user explicitly set the mechanism
>>options. In that case, we also want to dump back the info.
>>
>
> I don't think this is needed. Basically we always want to dump options for non-constant sets, since they couldn't have been chosen automatically, and don't need to dump them for constant sets, since they can always be determined automatically based on the contents.
>
Could you please elaborate?
If the set is non-constant, currently the options are by default
NFT_SET_POL_PERFORMANCE and size 0.
Do you want these options to be printed to the end user even when the
user did not originally use the 'policy xx' and 'size xx' statements?
Or in the other hand, to never print the options even when the user
gave an explicit config?
In the case of constant sets, what If I want to test the performance
of different policies? I need to know which policy is applied to a
given set, (in order to change it).
--
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options
2014-09-19 6:51 ` Arturo Borrero Gonzalez
@ 2014-09-19 17:20 ` Patrick McHardy
0 siblings, 0 replies; 13+ messages in thread
From: Patrick McHardy @ 2014-09-19 17:20 UTC (permalink / raw)
To: Arturo Borrero Gonzalez
Cc: Netfilter Development Mailing list, Pablo Neira Ayuso
On 19. September 2014 08:51:48 MESZ, Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote:
>On 18 September 2014 20:34, Patrick McHardy <kaber@trash.net> wrote:
>> On 18. September 2014 20:18:18 MESZ, Arturo Borrero Gonzalez
><arturo.borrero.glez@gmail.com> wrote:
>>>The sets mechanism options was not being stored anywhere.
>>>
>>>We want to know in which cases the user explicitly set the mechanism
>>>options. In that case, we also want to dump back the info.
>>>
>>
>> I don't think this is needed. Basically we always want to dump
>options for non-constant sets, since they couldn't have been chosen
>automatically, and don't need to dump them for constant sets, since
>they can always be determined automatically based on the contents.
>>
>
>Could you please elaborate?
>
>If the set is non-constant, currently the options are by default
>NFT_SET_POL_PERFORMANCE and size 0.
>Do you want these options to be printed to the end user even when the
>user did not originally use the 'policy xx' and 'size xx' statements?
>Or in the other hand, to never print the options even when the user
>gave an explicit config?
No. When we have literal/constant sets we can automatically determine the options, and we don't need to return them from the kernel. We could, but its useless.
For named sets any options that are not the default must have been specified by the user. So we must always dump them unless they are the default.
>In the case of constant sets, what If I want to test the performance
>of different policies? I need to know which policy is applied to a
>given set, (in order to change it).
Well, the mechanism is supposed to pick the best implementation. For testing, you can unload modules, the kernel won't autoload if at least one set implementation is available.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options
2014-09-18 18:18 [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options Arturo Borrero Gonzalez
` (2 preceding siblings ...)
2014-09-18 18:34 ` [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options Patrick McHardy
@ 2014-09-20 11:39 ` Patrick McHardy
2014-09-22 10:01 ` Arturo Borrero Gonzalez
3 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2014-09-20 11:39 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: netfilter-devel, pablo
On Thu, Sep 18, 2014 at 08:18:18PM +0200, Arturo Borrero Gonzalez wrote:
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -231,6 +231,14 @@ struct nft_set_ops {
> int nft_register_set(struct nft_set_ops *ops);
> void nft_unregister_set(struct nft_set_ops *ops);
>
> +/* internal flags to know which attributes were originally set
> + * from userspace.
> + */
> +enum nft_set_attr {
> + NFT_SET_ATTR_POLICY = 0x1,
> + NFT_SET_ATTR_DESC_SIZE = 0x2,
> +};
We don't need the size, any value != 0 is set by userspace. I'm unsure
about the policy, we don't really *need* to dump if it is the default,
and otherwise its obvious that it originated from userspace.
> @@ -241,6 +249,8 @@ void nft_unregister_set(struct nft_set_ops *ops);
> * @dtype: data type (verdict or numeric type defined by userspace)
> * @size: maximum set size
> * @nelems: number of elements
> + * @attr_flags: (enum nft_set_flags)
> + * @policy: (enum nft_set_policies)
> * @ops: set ops
> * @flags: set flags
> * @klen: key length
> @@ -255,6 +265,8 @@ struct nft_set {
> u32 dtype;
> u32 size;
> u32 nelems;
> + u16 attr_flags;
> + u32 policy;
These are way to big and introduce holes in the structure.
> /* runtime data below here */
> const struct nft_set_ops *ops ____cacheline_aligned;
> u16 flags;
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 8237460..d1c3f3e 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -2342,13 +2342,24 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
> goto nla_put_failure;
> }
>
> - desc = nla_nest_start(skb, NFTA_SET_DESC);
> - if (desc == NULL)
> - goto nla_put_failure;
> - if (set->size &&
> - nla_put_be32(skb, NFTA_SET_DESC_SIZE, htonl(set->size)))
> - goto nla_put_failure;
> - nla_nest_end(skb, desc);
> + /* dump policy and desc info only if they were explicitly set */
> + if (set->attr_flags & (1 << NFT_SET_ATTR_POLICY)) {
> + if (nla_put_be32(skb, NFTA_SET_POLICY, htonl(set->policy)))
> + goto nla_put_failure;
> + }
> +
> + if (set->attr_flags & (1 << NFT_SET_ATTR_DESC_SIZE)) {
> + desc = nla_nest_start(skb, NFTA_SET_DESC);
> + if (desc == NULL)
> + goto nla_put_failure;
> +
> + if (set->size &&
> + nla_put_be32(skb, NFTA_SET_DESC_SIZE, htonl(set->size))) {
> + goto nla_put_failure;
> + }
> +
> + nla_nest_end(skb, desc);
> + }
As mentioned earlier, dumping the parameters is not necessary for sets
with NFT_SET_CONSTANT as they have been determined automatically.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [nf_tables 1/3] netfilter: nf_tables: store and dump sets mechanism options
2014-09-20 11:39 ` Patrick McHardy
@ 2014-09-22 10:01 ` Arturo Borrero Gonzalez
0 siblings, 0 replies; 13+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-09-22 10:01 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Development Mailing list, Pablo Neira Ayuso
On 20 September 2014 13:39, Patrick McHardy <kaber@trash.net> wrote:
> On Thu, Sep 18, 2014 at 08:18:18PM +0200, Arturo Borrero Gonzalez wrote:
>> --- a/include/net/netfilter/nf_tables.h
>> +++ b/include/net/netfilter/nf_tables.h
>> @@ -231,6 +231,14 @@ struct nft_set_ops {
>> int nft_register_set(struct nft_set_ops *ops);
>> void nft_unregister_set(struct nft_set_ops *ops);
>>
>> +/* internal flags to know which attributes were originally set
>> + * from userspace.
>> + */
>> +enum nft_set_attr {
>> + NFT_SET_ATTR_POLICY = 0x1,
>> + NFT_SET_ATTR_DESC_SIZE = 0x2,
>> +};
>
> We don't need the size, any value != 0 is set by userspace. I'm unsure
> about the policy, we don't really *need* to dump if it is the default,
> and otherwise its obvious that it originated from userspace.
>
>> @@ -241,6 +249,8 @@ void nft_unregister_set(struct nft_set_ops *ops);
>> * @dtype: data type (verdict or numeric type defined by userspace)
>> * @size: maximum set size
>> * @nelems: number of elements
>> + * @attr_flags: (enum nft_set_flags)
>> + * @policy: (enum nft_set_policies)
>> * @ops: set ops
>> * @flags: set flags
>> * @klen: key length
>> @@ -255,6 +265,8 @@ struct nft_set {
>> u32 dtype;
>> u32 size;
>> u32 nelems;
>> + u16 attr_flags;
>> + u32 policy;
>
> These are way to big and introduce holes in the structure.
>
>> /* runtime data below here */
>> const struct nft_set_ops *ops ____cacheline_aligned;
>> u16 flags;
>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> index 8237460..d1c3f3e 100644
>> --- a/net/netfilter/nf_tables_api.c
>> +++ b/net/netfilter/nf_tables_api.c
>> @@ -2342,13 +2342,24 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
>> goto nla_put_failure;
>> }
>>
>> - desc = nla_nest_start(skb, NFTA_SET_DESC);
>> - if (desc == NULL)
>> - goto nla_put_failure;
>> - if (set->size &&
>> - nla_put_be32(skb, NFTA_SET_DESC_SIZE, htonl(set->size)))
>> - goto nla_put_failure;
>> - nla_nest_end(skb, desc);
>> + /* dump policy and desc info only if they were explicitly set */
>> + if (set->attr_flags & (1 << NFT_SET_ATTR_POLICY)) {
>> + if (nla_put_be32(skb, NFTA_SET_POLICY, htonl(set->policy)))
>> + goto nla_put_failure;
>> + }
>> +
>> + if (set->attr_flags & (1 << NFT_SET_ATTR_DESC_SIZE)) {
>> + desc = nla_nest_start(skb, NFTA_SET_DESC);
>> + if (desc == NULL)
>> + goto nla_put_failure;
>> +
>> + if (set->size &&
>> + nla_put_be32(skb, NFTA_SET_DESC_SIZE, htonl(set->size))) {
>> + goto nla_put_failure;
>> + }
>> +
>> + nla_nest_end(skb, desc);
>> + }
>
> As mentioned earlier, dumping the parameters is not necessary for sets
> with NFT_SET_CONSTANT as they have been determined automatically.
Ok Patrick, so I understand we can drop this patch.
I can patch the fill_set() function to simply dump the policy if
policy != PERFORMANCE (the default).
How many bits should I use to store the policy?
regards.
--
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread