| Message ID | 20180331171741.11642-1-fw@strlen.de |
|---|---|
| State | RFC |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [RFC,xt-compat] ebtables: full nft translations | expand |
Cc'ing Arturo, he added the ebtables-compat layer so he probably remember more details on this. On Sat, Mar 31, 2018 at 07:17:41PM +0200, Florian Westphal wrote: > This (haycky) patch translates 'ebtables --mark' to a native 'meta mark' > and dissects meta mark back to the ebt_mark_m binary representation when > parsing back nftables rules. > > Plan is to do this for all the ebt matches/watchers/targets so that > 1. 'nft list ruleset' shows correct/expected output > 2. we can (eventually) remove ebtables support from kernel > 3. make iptables install 'ebtables' by default as drop-in replacement. > > ebtables is much more feature limited than iptables, > nft should already provide all/complete feature set. > > Downside: Because we can't (should not) add nftables specific code > to libebt_XXX.c, this 'leaks' ebt_mark_m_info struct definition to > nft-bridge.c. I think its okay, we could also eventually remove all > the libebt_XXX files and provide everything from nft-bridge.c as builtin > functionality. > > Does that appear sane to you? So far the compat infrastructure uses nft_compat.c, which uses xtables matches/targets from the kernel. I mean, probably get this working through nft_compat instead? We could do similar in many other spots in the iptables-compat, although not sure it's worth the effort. So probably nft-bridge and ebtables-compat is missing match/target support and we get in sync with what we do in iptables-compat? -- 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 Sat, Mar 31, 2018 at 08:57:21PM +0200, Pablo Neira Ayuso wrote: > Cc'ing Arturo, he added the ebtables-compat layer so he probably > remember more details on this. > > On Sat, Mar 31, 2018 at 07:17:41PM +0200, Florian Westphal wrote: > > This (haycky) patch translates 'ebtables --mark' to a native 'meta mark' > > and dissects meta mark back to the ebt_mark_m binary representation when > > parsing back nftables rules. > > > > Plan is to do this for all the ebt matches/watchers/targets so that > > 1. 'nft list ruleset' shows correct/expected output > > 2. we can (eventually) remove ebtables support from kernel > > 3. make iptables install 'ebtables' by default as drop-in replacement. > > > > ebtables is much more feature limited than iptables, > > nft should already provide all/complete feature set. > > > > Downside: Because we can't (should not) add nftables specific code > > to libebt_XXX.c, this 'leaks' ebt_mark_m_info struct definition to > > nft-bridge.c. I think its okay, we could also eventually remove all > > the libebt_XXX files and provide everything from nft-bridge.c as builtin > > functionality. > > > > Does that appear sane to you? > > So far the compat infrastructure uses nft_compat.c, which uses xtables > matches/targets from the kernel. > > I mean, probably get this working through nft_compat instead? > > We could do similar in many other spots in the iptables-compat, > although not sure it's worth the effort. > > So probably nft-bridge and ebtables-compat is missing match/target > support and we get in sync with what we do in iptables-compat? I mean, this patch is an alternative to be more native, I'm not against it, actually this is work done already so I don't want you to drop it necessarily, my intention was just to point that we have followed a different path with iptables-compat. -- 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
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > Cc'ing Arturo, he added the ebtables-compat layer so he probably > remember more details on this. > > On Sat, Mar 31, 2018 at 07:17:41PM +0200, Florian Westphal wrote: > > This (haycky) patch translates 'ebtables --mark' to a native 'meta mark' > > and dissects meta mark back to the ebt_mark_m binary representation when > > parsing back nftables rules. > > > > Plan is to do this for all the ebt matches/watchers/targets so that > > 1. 'nft list ruleset' shows correct/expected output > > 2. we can (eventually) remove ebtables support from kernel > > 3. make iptables install 'ebtables' by default as drop-in replacement. > > > > ebtables is much more feature limited than iptables, > > nft should already provide all/complete feature set. > > > > Downside: Because we can't (should not) add nftables specific code > > to libebt_XXX.c, this 'leaks' ebt_mark_m_info struct definition to > > nft-bridge.c. I think its okay, we could also eventually remove all > > the libebt_XXX files and provide everything from nft-bridge.c as builtin > > functionality. > > > > Does that appear sane to you? > > So far the compat infrastructure uses nft_compat.c, which uses xtables > matches/targets from the kernel. > > I mean, probably get this working through nft_compat instead? It already works via nft_compat. Only issue is that nft list ruleset won't display this correctly because ebt lacks xlate() functions. However, I thought ebtables might be a low-hanging fruit to ditch nft_compat and make it work with nftables directly. > So probably nft-bridge and ebtables-compat is missing match/target > support and we get in sync with what we do in iptables-compat? -- 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 Sat, Mar 31, 2018 at 09:04:55PM +0200, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Cc'ing Arturo, he added the ebtables-compat layer so he probably > > remember more details on this. > > > > On Sat, Mar 31, 2018 at 07:17:41PM +0200, Florian Westphal wrote: > > > This (haycky) patch translates 'ebtables --mark' to a native 'meta mark' > > > and dissects meta mark back to the ebt_mark_m binary representation when > > > parsing back nftables rules. > > > > > > Plan is to do this for all the ebt matches/watchers/targets so that > > > 1. 'nft list ruleset' shows correct/expected output > > > 2. we can (eventually) remove ebtables support from kernel > > > 3. make iptables install 'ebtables' by default as drop-in replacement. > > > > > > ebtables is much more feature limited than iptables, > > > nft should already provide all/complete feature set. > > > > > > Downside: Because we can't (should not) add nftables specific code > > > to libebt_XXX.c, this 'leaks' ebt_mark_m_info struct definition to > > > nft-bridge.c. I think its okay, we could also eventually remove all > > > the libebt_XXX files and provide everything from nft-bridge.c as builtin > > > functionality. > > > > > > Does that appear sane to you? > > > > So far the compat infrastructure uses nft_compat.c, which uses xtables > > matches/targets from the kernel. > > > > I mean, probably get this working through nft_compat instead? > > It already works via nft_compat. > > Only issue is that nft list ruleset won't display this correctly > because ebt lacks xlate() functions. > > However, I thought ebtables might be a low-hanging fruit to > ditch nft_compat and make it work with nftables directly. That's also fine yes. We can do different for this case. -- 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/iptables/nft-bridge.c b/iptables/nft-bridge.c index 22940cf79beb..c7dc0b5e4e01 100644 --- a/iptables/nft-bridge.c +++ b/iptables/nft-bridge.c @@ -151,6 +151,35 @@ static int _add_action(struct nftnl_rule *r, struct ebtables_command_state *cs) return ret; } +struct ebt_mark_m_info { + unsigned long mark, mask; + __u8 invert; + __u8 bitmask; +}; + +static int nft_bridge_add_native_mark_m(struct nftnl_rule *r, struct xt_entry_match *m) +{ + const struct ebt_mark_m_info *mm = (const void *)m->data; + uint32_t mask = mm->mask; + uint32_t op; + + if (mm->bitmask & 2) /* or match */ + op = mm->invert ? NFT_CMP_EQ : NFT_CMP_NEQ; + else + op = mm->invert ? NFT_CMP_NEQ : NFT_CMP_EQ; + + add_mark(r, mm->mark, mask, op); + return 0; +} + +static int nft_bridge_add_native(struct nftnl_rule *r, struct xt_entry_match *m) +{ + if (strcmp(m->u.user.name, "mark_m") == 0) + return nft_bridge_add_native_mark_m(r, m); + + return -1; +} + static int nft_bridge_add(struct nftnl_rule *r, void *data) { struct ebtables_command_state *cs = data; @@ -205,6 +234,9 @@ static int nft_bridge_add(struct nftnl_rule *r, void *data) add_compat(r, fw->ethproto, fw->invflags); for (iter = cs->match_list; iter; iter = iter->next) { + if (nft_bridge_add_native(r, iter->u.match->m) == 0) + continue; + if (iter->ismatch) { if (add_match(r, iter->u.match->m)) break; @@ -220,6 +252,42 @@ static int nft_bridge_add(struct nftnl_rule *r, void *data) return _add_action(r, cs); } +static void nft_bridge_parse_match(struct xtables_match *m, void *data); + +static void nft_bridge_parse_mark(struct nft_xt_ctx *ctx, + struct nftnl_expr *e, void *data) +{ + struct ebt_mark_m_info *mark_m; + uint32_t mark, mask = ~0u; + struct xtables_match *match; + uint8_t mode = 1, inv = 0; + + mark = nftnl_expr_get_u32(e, NFTNL_EXPR_CMP_DATA); + if (ctx->flags & NFT_XT_CTX_BITWISE) { + mask = ctx->bitwise.mask[0]; + if (mark == 0) { + mode = 2; + if (nftnl_expr_get_u32(e, NFTNL_EXPR_CMP_OP) == NFT_CMP_EQ) + inv = 1; + } + ctx->flags &= ~NFT_XT_CTX_BITWISE; + } else if (nftnl_expr_get_u32(e, NFTNL_EXPR_CMP_OP) == NFT_CMP_NEQ) { + inv = 1; + } + + match = nft_make_match(ctx, "mark_m", sizeof(*mark_m), 0); + if (match == NULL) + return; + + mark_m = (void *)match->m->data; + mark_m->mark = mark; + mark_m->mask = mask; + mark_m->bitmask = mode; + mark_m->invert = inv; + + nft_bridge_parse_match(match, ctx->state.cs_eb); +} + static void nft_bridge_parse_meta(struct nft_xt_ctx *ctx, struct nftnl_expr *e, void *data) { @@ -266,6 +334,9 @@ static void nft_bridge_parse_meta(struct nft_xt_ctx *ctx, memset(fw->out_mask, 0xff, len + 1); } break; + case NFT_META_MARK: + nft_bridge_parse_mark(ctx, e, data); + break; default: break; } diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c index 68e5c55d2c61..b72f172d8730 100644 --- a/iptables/nft-shared.c +++ b/iptables/nft-shared.c @@ -83,6 +83,23 @@ void add_bitwise_u16(struct nftnl_rule *r, int mask, int xor) nftnl_rule_add_expr(r, expr); } +static void add_bitwise_u32(struct nftnl_rule *r, uint32_t mask, uint32_t xor) +{ + struct nftnl_expr *expr; + + expr = nftnl_expr_alloc("bitwise"); + if (expr == NULL) + return; + + nftnl_expr_set_u32(expr, NFTNL_EXPR_BITWISE_SREG, NFT_REG_1); + nftnl_expr_set_u32(expr, NFTNL_EXPR_BITWISE_DREG, NFT_REG_1); + nftnl_expr_set_u32(expr, NFTNL_EXPR_BITWISE_LEN, sizeof(uint32_t)); + nftnl_expr_set(expr, NFTNL_EXPR_BITWISE_MASK, &mask, sizeof(uint32_t)); + nftnl_expr_set(expr, NFTNL_EXPR_BITWISE_XOR, &xor, sizeof(uint32_t)); + + nftnl_rule_add_expr(r, expr); +} + static void add_bitwise(struct nftnl_rule *r, uint8_t *mask, size_t len) { struct nftnl_expr *expr; @@ -157,6 +174,14 @@ void add_outiface(struct nftnl_rule *r, char *iface, uint32_t op) add_cmp_ptr(r, op, iface, iface_len + 1); } +void add_mark(struct nftnl_rule *r, uint32_t mark, uint32_t mask, uint32_t op) +{ + add_meta(r, NFT_META_MARK); + if (mask < UINT32_MAX) + add_bitwise_u32(r, mask, 0); + add_cmp_u32(r, mark, op); +} + void add_addr(struct nftnl_rule *r, int offset, void *data, void *mask, size_t len, uint32_t op) { @@ -320,15 +345,12 @@ void nft_parse_target(struct nft_xt_ctx *ctx, struct nftnl_expr *e) ops->parse_target(target, data); } -void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e) +struct xtables_match *nft_make_match(struct nft_xt_ctx *ctx, const char *mt_name, + unsigned int mt_len, uint8_t revision) { - uint32_t mt_len; - const char *mt_name = nftnl_expr_get_str(e, NFTNL_EXPR_MT_NAME); - const void *mt_info = nftnl_expr_get(e, NFTNL_EXPR_MT_INFO, &mt_len); - struct xtables_match *match; struct xtables_rule_match **matches; + struct xtables_match *match; struct xt_entry_match *m; - struct nft_family_ops *ops; switch (ctx->family) { case NFPROTO_IPV4: @@ -339,14 +361,20 @@ void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e) matches = &ctx->state.cs_eb->matches; break; default: - fprintf(stderr, "BUG: nft_parse_match() unknown family %d\n", - ctx->family); + fprintf(stderr, "BUG: %s() unknown family %d\n", + __func__, ctx->family); exit(EXIT_FAILURE); } match = xtables_find_match(mt_name, XTF_TRY_LOAD, matches); if (match == NULL) - return; + return NULL; + + if (match->size != mt_len) { + fprintf(stderr, "BUG: %s() wrong size (%u vs %u) for %s\n", + __func__, (unsigned int)match->size, mt_len, mt_name); + exit(EXIT_FAILURE); + } m = calloc(1, sizeof(struct xt_entry_match) + mt_len); if (m == NULL) { @@ -354,13 +382,31 @@ void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e) exit(EXIT_FAILURE); } - memcpy(&m->data, mt_info, mt_len); m->u.match_size = mt_len + XT_ALIGN(sizeof(struct xt_entry_match)); - m->u.user.revision = nftnl_expr_get_u32(e, NFTNL_EXPR_TG_REV); + m->u.user.revision = revision; strcpy(m->u.user.name, match->name); match->m = m; + return match; +} + +void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e) +{ + uint32_t mt_len; + const char *mt_name = nftnl_expr_get_str(e, NFTNL_EXPR_MT_NAME); + const void *mt_info = nftnl_expr_get(e, NFTNL_EXPR_MT_INFO, &mt_len); + struct xtables_match *match; + struct nft_family_ops *ops; + + match = nft_make_match(ctx, mt_name, mt_len, + nftnl_expr_get_u32(e, NFTNL_EXPR_TG_REV)); + + if (!match) + return; + + memcpy(&match->m->data, mt_info, mt_len); + ops = nft_family_ops_lookup(ctx->family); if (ops->parse_match != NULL) ops->parse_match(match, nft_get_data(ctx)); diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h index c0948fd4e266..9e7fd8d1a1f0 100644 --- a/iptables/nft-shared.h +++ b/iptables/nft-shared.h @@ -118,6 +118,7 @@ void add_addr(struct nftnl_rule *r, int offset, void *data, void *mask, size_t len, uint32_t op); void add_proto(struct nftnl_rule *r, int offset, size_t len, uint8_t proto, uint32_t op); +void add_mark(struct nftnl_rule *r, uint32_t mark, uint32_t mask, uint32_t op); void add_compat(struct nftnl_rule *r, uint32_t proto, bool inv); bool is_same_interfaces(const char *a_iniface, const char *a_outiface, @@ -132,6 +133,10 @@ int parse_meta(struct nftnl_expr *e, uint8_t key, char *iniface, unsigned char *outiface_mask, uint8_t *invflags); void print_proto(uint16_t proto, int invert); void get_cmp_data(struct nftnl_expr *e, void *data, size_t dlen, bool *inv); + +struct xtables_match *nft_make_match(struct nft_xt_ctx *ctx, + const char *mt_name, + unsigned int mt_len, uint8_t revision); void nft_parse_bitwise(struct nft_xt_ctx *ctx, struct nftnl_expr *e); void nft_parse_cmp(struct nft_xt_ctx *ctx, struct nftnl_expr *e); void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e);