| Message ID | 1491208197-5493-1-git-send-email-zlpnobody@163.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Mon, Apr 03, 2017 at 04:29:57PM +0800, Liping Zhang wrote: > From: Liping Zhang <zlpnobody@gmail.com> > > Typing the "nft add rule x y ct mark set jhash ip saddr mod 2" will > not generate a random seed, instead, the seed will always be zero. > > So if seed option is empty, we shoulde not set the NFTA_HASH_SEED > attribute, then a random seed will be generted in the kernel. > > Also: just to keep it simple, "seed 0" is equal to "seed opt is empty", > since this is not a big problem. > > Signed-off-by: Liping Zhang <zlpnobody@gmail.com> > --- > Note, another kernel patch is necessary to avoid the annoying warning > from "nft-test.py ip/hash.t": > ip/hash.t: WARNING: line: 5: 'src/nft add rule --debug=netlink ip test-ip4 > pre ct mark set jhash ip saddr . ip daddr mod 2': 'ct mark set jhash ip saddr > . ip daddr mod 2' mismatches 'ct mark set jhash ip saddr . ip daddr mod 2 > seed 0xd6ab633c' > > src/netlink_linearize.c | 3 ++- > tests/py/ip/hash.t | 1 + > tests/py/ip/hash.t.payload | 7 +++++++ > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c > index b2f27b7..0dba658 100644 > --- a/src/netlink_linearize.c > +++ b/src/netlink_linearize.c > @@ -139,7 +139,8 @@ static void netlink_gen_hash(struct netlink_linearize_ctx *ctx, > } > netlink_put_register(nle, NFTNL_EXPR_HASH_DREG, dreg); > nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_MODULUS, expr->hash.mod); > - nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); > + if (expr->hash.seed) > + nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); I prefer we have a hash.seed_set, instead of relying on 0 meaning "unset". I'm thinking of people willing to implement some sort of poor man symmetric hashing with two rules, one per each direction. The seed needs to be the same so the jhash is consistent. -- 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
On Thu, Apr 13, 2017 at 10:57:09PM +0200, Pablo Neira Ayuso wrote: > On Mon, Apr 03, 2017 at 04:29:57PM +0800, Liping Zhang wrote: > > From: Liping Zhang <zlpnobody@gmail.com> > > > > Typing the "nft add rule x y ct mark set jhash ip saddr mod 2" will > > not generate a random seed, instead, the seed will always be zero. > > > > So if seed option is empty, we shoulde not set the NFTA_HASH_SEED > > attribute, then a random seed will be generted in the kernel. > > > > Also: just to keep it simple, "seed 0" is equal to "seed opt is empty", > > since this is not a big problem. > > > > Signed-off-by: Liping Zhang <zlpnobody@gmail.com> > > --- > > Note, another kernel patch is necessary to avoid the annoying warning > > from "nft-test.py ip/hash.t": > > ip/hash.t: WARNING: line: 5: 'src/nft add rule --debug=netlink ip test-ip4 > > pre ct mark set jhash ip saddr . ip daddr mod 2': 'ct mark set jhash ip saddr > > . ip daddr mod 2' mismatches 'ct mark set jhash ip saddr . ip daddr mod 2 > > seed 0xd6ab633c' > > > > src/netlink_linearize.c | 3 ++- > > tests/py/ip/hash.t | 1 + > > tests/py/ip/hash.t.payload | 7 +++++++ > > 3 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c > > index b2f27b7..0dba658 100644 > > --- a/src/netlink_linearize.c > > +++ b/src/netlink_linearize.c > > @@ -139,7 +139,8 @@ static void netlink_gen_hash(struct netlink_linearize_ctx *ctx, > > } > > netlink_put_register(nle, NFTNL_EXPR_HASH_DREG, dreg); > > nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_MODULUS, expr->hash.mod); > > - nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); > > + if (expr->hash.seed) > > + nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); > > I prefer we have a hash.seed_set, instead of relying on 0 meaning > "unset". > > I'm thinking of people willing to implement some sort of poor man > symmetric hashing with two rules, one per each direction. The seed > needs to be the same so the jhash is consistent. I'm thinking of things like: iif eth0 jhash ip saddr . tcp dport seed 0xdeadbeef iif eth1 jhash ip daddr . tcp sport seed 0xdeadbeef I think may be useful in case of several uplinks are available, and you want something a bit more configurable that symhash, at the cost of having two rules, one per direction. -- 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
Hi Pablo, 2017-04-14 4:57 GMT+08:00 Pablo Neira Ayuso <pablo@netfilter.org>: [...] >> - nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); >> + if (expr->hash.seed) >> + nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); > > I prefer we have a hash.seed_set, instead of relying on 0 meaning > "unset". OK, I will send V2 based on your suggestions. > I'm thinking of people willing to implement some sort of poor man > symmetric hashing with two rules, one per each direction. The seed > needs to be the same so the jhash is consistent. -- 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
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c index b2f27b7..0dba658 100644 --- a/src/netlink_linearize.c +++ b/src/netlink_linearize.c @@ -139,7 +139,8 @@ static void netlink_gen_hash(struct netlink_linearize_ctx *ctx, } netlink_put_register(nle, NFTNL_EXPR_HASH_DREG, dreg); nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_MODULUS, expr->hash.mod); - nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); + if (expr->hash.seed) + nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_SEED, expr->hash.seed); nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_OFFSET, expr->hash.offset); nftnl_expr_set_u32(nle, NFTNL_EXPR_HASH_TYPE, expr->hash.type); nftnl_rule_add_expr(ctx->nlr, nle); diff --git a/tests/py/ip/hash.t b/tests/py/ip/hash.t index 2becef6..a8e975b 100644 --- a/tests/py/ip/hash.t +++ b/tests/py/ip/hash.t @@ -3,6 +3,7 @@ ct mark set jhash ip saddr . ip daddr mod 2 seed 0xdeadbeef;ok ct mark set jhash ip saddr . ip daddr mod 2;ok +ct mark set jhash ip saddr . ip daddr mod 2 seed 0x0;ok;ct mark set jhash ip saddr . ip daddr mod 2 ct mark set jhash ip saddr . ip daddr mod 2 seed 0xdeadbeef offset 100;ok ct mark set jhash ip saddr . ip daddr mod 2 offset 100;ok dnat to jhash ip saddr mod 2 seed 0xdeadbeef map { 0 : 192.168.20.100, 1 : 192.168.30.100 };ok diff --git a/tests/py/ip/hash.t.payload b/tests/py/ip/hash.t.payload index 21227e9..71ab065 100644 --- a/tests/py/ip/hash.t.payload +++ b/tests/py/ip/hash.t.payload @@ -12,6 +12,13 @@ ip test-ip4 pre [ hash reg 1 = jhash(reg 2, 8, 0x0) % mod 2 ] [ ct set mark with reg 1 ] +# ct mark set jhash ip saddr . ip daddr mod 2 seed 0x0 +ip test-ip4 pre + [ payload load 4b @ network header + 12 => reg 2 ] + [ payload load 4b @ network header + 16 => reg 13 ] + [ hash reg 1 = jhash(reg 2, 8, 0x0) % mod 2 ] + [ ct set mark with reg 1 ] + # ct mark set jhash ip saddr . ip daddr mod 2 seed 0xdeadbeef offset 100 ip test-ip4 pre [ payload load 4b @ network header + 12 => reg 2 ]