* [PATCH nf-next 0/3] NF NAT deduplication refactoring
@ 2023-03-13 13:46 Jeremy Sowden
2023-03-13 13:46 ` [PATCH nf-next 1/3] netfilter: nf_nat_redirect: use `struct nf_nat_range2` in ipv4 API Jeremy Sowden
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Jeremy Sowden @ 2023-03-13 13:46 UTC (permalink / raw)
To: Netfilter Devel
These three patches perform refactoring in NF NAT modules to remove
duplicate code.
Jeremy Sowden (3):
netfilter: nf_nat_redirect: use `struct nf_nat_range2` in ipv4 API
netfilter: nft_masq: deduplicate eval call-backs
netfilter: nft_redir: deduplicate eval call-backs
include/net/netfilter/nf_nat_redirect.h | 3 +-
net/netfilter/nf_nat_redirect.c | 58 ++++++++---------
net/netfilter/nft_masq.c | 75 +++++++++-------------
net/netfilter/nft_redir.c | 84 +++++++++----------------
net/netfilter/xt_REDIRECT.c | 10 ++-
5 files changed, 96 insertions(+), 134 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH nf-next 1/3] netfilter: nf_nat_redirect: use `struct nf_nat_range2` in ipv4 API 2023-03-13 13:46 [PATCH nf-next 0/3] NF NAT deduplication refactoring Jeremy Sowden @ 2023-03-13 13:46 ` Jeremy Sowden 2023-03-13 13:46 ` [PATCH nf-next 2/3] netfilter: nft_masq: deduplicate eval call-backs Jeremy Sowden ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Jeremy Sowden @ 2023-03-13 13:46 UTC (permalink / raw) To: Netfilter Devel; +Cc: Florian Westphal `nf_nat_redirect_ipv4` takes a `struct nf_nat_ipv4_multi_range_compat`, but converts it internally to a `struct nf_nat_range2`. Change the function to take the latter, factor out the code now shared with `nf_nat_redirect_ipv6`, move the conversion to the xt_REDIRECT module, and update the ipv4 range initialization in the nft_redir module. Replace a bare hex constant for 127.0.0.1 with a macro. Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Reviewed-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_nat_redirect.h | 3 +- net/netfilter/nf_nat_redirect.c | 58 ++++++++++++------------- net/netfilter/nft_redir.c | 15 ++++--- net/netfilter/xt_REDIRECT.c | 10 ++++- 4 files changed, 44 insertions(+), 42 deletions(-) diff --git a/include/net/netfilter/nf_nat_redirect.h b/include/net/netfilter/nf_nat_redirect.h index 2418653a66db..279380de904c 100644 --- a/include/net/netfilter/nf_nat_redirect.h +++ b/include/net/netfilter/nf_nat_redirect.h @@ -6,8 +6,7 @@ #include <uapi/linux/netfilter/nf_nat.h> unsigned int -nf_nat_redirect_ipv4(struct sk_buff *skb, - const struct nf_nat_ipv4_multi_range_compat *mr, +nf_nat_redirect_ipv4(struct sk_buff *skb, const struct nf_nat_range2 *range, unsigned int hooknum); unsigned int nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, diff --git a/net/netfilter/nf_nat_redirect.c b/net/netfilter/nf_nat_redirect.c index f91579c821e9..54ce8e6113ed 100644 --- a/net/netfilter/nf_nat_redirect.c +++ b/net/netfilter/nf_nat_redirect.c @@ -10,6 +10,7 @@ #include <linux/if.h> #include <linux/inetdevice.h> +#include <linux/in.h> #include <linux/ip.h> #include <linux/kernel.h> #include <linux/netdevice.h> @@ -24,25 +25,38 @@ #include <net/netfilter/nf_nat.h> #include <net/netfilter/nf_nat_redirect.h> +static unsigned int +nf_nat_redirect(struct sk_buff *skb, const struct nf_nat_range2 *range, + const union nf_inet_addr *newdst) +{ + struct nf_nat_range2 newrange; + enum ip_conntrack_info ctinfo; + struct nf_conn *ct; + + ct = nf_ct_get(skb, &ctinfo); + WARN_ON(!(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED))); + + newrange.flags = range->flags | NF_NAT_RANGE_MAP_IPS; + newrange.min_addr = *newdst; + newrange.max_addr = *newdst; + newrange.min_proto = range->min_proto; + newrange.max_proto = range->max_proto; + + return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); +} + unsigned int -nf_nat_redirect_ipv4(struct sk_buff *skb, - const struct nf_nat_ipv4_multi_range_compat *mr, +nf_nat_redirect_ipv4(struct sk_buff *skb, const struct nf_nat_range2 *range, unsigned int hooknum) { - struct nf_conn *ct; - enum ip_conntrack_info ctinfo; __be32 newdst; - struct nf_nat_range2 newrange; WARN_ON(hooknum != NF_INET_PRE_ROUTING && hooknum != NF_INET_LOCAL_OUT); - ct = nf_ct_get(skb, &ctinfo); - WARN_ON(!(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED))); - /* Local packets: make them go to loopback */ if (hooknum == NF_INET_LOCAL_OUT) { - newdst = htonl(0x7F000001); + newdst = htonl(INADDR_LOOPBACK); } else { const struct in_device *indev; @@ -61,17 +75,8 @@ nf_nat_redirect_ipv4(struct sk_buff *skb, return NF_DROP; } - /* Transfer from original range. */ - memset(&newrange.min_addr, 0, sizeof(newrange.min_addr)); - memset(&newrange.max_addr, 0, sizeof(newrange.max_addr)); - newrange.flags = mr->range[0].flags | NF_NAT_RANGE_MAP_IPS; - newrange.min_addr.ip = newdst; - newrange.max_addr.ip = newdst; - newrange.min_proto = mr->range[0].min; - newrange.max_proto = mr->range[0].max; - - /* Hand modified range to generic setup. */ - return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); + return nf_nat_redirect(skb, range, + &(union nf_inet_addr) { .ip = newdst }); } EXPORT_SYMBOL_GPL(nf_nat_redirect_ipv4); @@ -81,12 +86,8 @@ unsigned int nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, unsigned int hooknum) { - struct nf_nat_range2 newrange; struct in6_addr newdst; - enum ip_conntrack_info ctinfo; - struct nf_conn *ct; - ct = nf_ct_get(skb, &ctinfo); if (hooknum == NF_INET_LOCAL_OUT) { newdst = loopback_addr; } else { @@ -109,12 +110,7 @@ nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, return NF_DROP; } - newrange.flags = range->flags | NF_NAT_RANGE_MAP_IPS; - newrange.min_addr.in6 = newdst; - newrange.max_addr.in6 = newdst; - newrange.min_proto = range->min_proto; - newrange.max_proto = range->max_proto; - - return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); + return nf_nat_redirect(skb, range, + &(union nf_inet_addr) { .in6 = newdst }); } EXPORT_SYMBOL_GPL(nf_nat_redirect_ipv6); diff --git a/net/netfilter/nft_redir.c b/net/netfilter/nft_redir.c index 5f7739987559..77a459470cb7 100644 --- a/net/netfilter/nft_redir.c +++ b/net/netfilter/nft_redir.c @@ -104,20 +104,21 @@ static void nft_redir_ipv4_eval(const struct nft_expr *expr, const struct nft_pktinfo *pkt) { struct nft_redir *priv = nft_expr_priv(expr); - struct nf_nat_ipv4_multi_range_compat mr; + struct nf_nat_range2 range; - memset(&mr, 0, sizeof(mr)); + memset(&range, 0, sizeof(range)); if (priv->sreg_proto_min) { - mr.range[0].min.all = (__force __be16)nft_reg_load16( + range.min_proto.all = (__force __be16)nft_reg_load16( ®s->data[priv->sreg_proto_min]); - mr.range[0].max.all = (__force __be16)nft_reg_load16( + range.max_proto.all = (__force __be16)nft_reg_load16( ®s->data[priv->sreg_proto_max]); - mr.range[0].flags |= NF_NAT_RANGE_PROTO_SPECIFIED; + range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED; } - mr.range[0].flags |= priv->flags; + range.flags |= priv->flags; - regs->verdict.code = nf_nat_redirect_ipv4(pkt->skb, &mr, nft_hook(pkt)); + regs->verdict.code = + nf_nat_redirect_ipv4(pkt->skb, &range, nft_hook(pkt)); } static void diff --git a/net/netfilter/xt_REDIRECT.c b/net/netfilter/xt_REDIRECT.c index 353ca7801251..ff66b56a3f97 100644 --- a/net/netfilter/xt_REDIRECT.c +++ b/net/netfilter/xt_REDIRECT.c @@ -46,7 +46,6 @@ static void redirect_tg_destroy(const struct xt_tgdtor_param *par) nf_ct_netns_put(par->net, par->family); } -/* FIXME: Take multiple ranges --RR */ static int redirect_tg4_check(const struct xt_tgchk_param *par) { const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo; @@ -65,7 +64,14 @@ static int redirect_tg4_check(const struct xt_tgchk_param *par) static unsigned int redirect_tg4(struct sk_buff *skb, const struct xt_action_param *par) { - return nf_nat_redirect_ipv4(skb, par->targinfo, xt_hooknum(par)); + const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo; + struct nf_nat_range2 range = { + .flags = mr->range[0].flags, + .min_proto = mr->range[0].min, + .max_proto = mr->range[0].max, + }; + + return nf_nat_redirect_ipv4(skb, &range, xt_hooknum(par)); } static struct xt_target redirect_tg_reg[] __read_mostly = { -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nf-next 2/3] netfilter: nft_masq: deduplicate eval call-backs 2023-03-13 13:46 [PATCH nf-next 0/3] NF NAT deduplication refactoring Jeremy Sowden 2023-03-13 13:46 ` [PATCH nf-next 1/3] netfilter: nf_nat_redirect: use `struct nf_nat_range2` in ipv4 API Jeremy Sowden @ 2023-03-13 13:46 ` Jeremy Sowden 2023-03-13 13:46 ` [PATCH nf-next 3/3] netfilter: nft_redir: " Jeremy Sowden 2023-03-14 10:52 ` [PATCH nf-next 0/3] NF NAT deduplication refactoring Pablo Neira Ayuso 3 siblings, 0 replies; 8+ messages in thread From: Jeremy Sowden @ 2023-03-13 13:46 UTC (permalink / raw) To: Netfilter Devel nft_masq has separate ipv4 and ipv6 call-backs which share much of their code, and an inet one switch containing a switch that calls one of the others based on the family of the packet. Merge the ipv4 and ipv6 ones into the inet one in order to get rid of the duplicate code. Const-qualify the `priv` pointer since we don't need to write through it. Signed-off-by: Jeremy Sowden <jeremy@azazel.net> --- net/netfilter/nft_masq.c | 75 ++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 46 deletions(-) diff --git a/net/netfilter/nft_masq.c b/net/netfilter/nft_masq.c index e55e455275c4..4bd79306eebd 100644 --- a/net/netfilter/nft_masq.c +++ b/net/netfilter/nft_masq.c @@ -96,23 +96,39 @@ static int nft_masq_dump(struct sk_buff *skb, return -1; } -static void nft_masq_ipv4_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) +static void nft_masq_eval(const struct nft_expr *expr, + struct nft_regs *regs, + const struct nft_pktinfo *pkt) { - struct nft_masq *priv = nft_expr_priv(expr); + const struct nft_masq *priv = nft_expr_priv(expr); struct nf_nat_range2 range; memset(&range, 0, sizeof(range)); range.flags = priv->flags; if (priv->sreg_proto_min) { - range.min_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_min]); - range.max_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_max]); + range.min_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_min]); + range.max_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_max]); + } + + switch (nft_pf(pkt)) { + case NFPROTO_IPV4: + regs->verdict.code = nf_nat_masquerade_ipv4(pkt->skb, + nft_hook(pkt), + &range, + nft_out(pkt)); + break; +#ifdef CONFIG_NF_TABLES_IPV6 + case NFPROTO_IPV6: + regs->verdict.code = nf_nat_masquerade_ipv6(pkt->skb, &range, + nft_out(pkt)); + break; +#endif + default: + WARN_ON_ONCE(1); + break; } - regs->verdict.code = nf_nat_masquerade_ipv4(pkt->skb, nft_hook(pkt), - &range, nft_out(pkt)); } static void @@ -125,7 +141,7 @@ static struct nft_expr_type nft_masq_ipv4_type; static const struct nft_expr_ops nft_masq_ipv4_ops = { .type = &nft_masq_ipv4_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_masq)), - .eval = nft_masq_ipv4_eval, + .eval = nft_masq_eval, .init = nft_masq_init, .destroy = nft_masq_ipv4_destroy, .dump = nft_masq_dump, @@ -143,25 +159,6 @@ static struct nft_expr_type nft_masq_ipv4_type __read_mostly = { }; #ifdef CONFIG_NF_TABLES_IPV6 -static void nft_masq_ipv6_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) -{ - struct nft_masq *priv = nft_expr_priv(expr); - struct nf_nat_range2 range; - - memset(&range, 0, sizeof(range)); - range.flags = priv->flags; - if (priv->sreg_proto_min) { - range.min_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_min]); - range.max_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_max]); - } - regs->verdict.code = nf_nat_masquerade_ipv6(pkt->skb, &range, - nft_out(pkt)); -} - static void nft_masq_ipv6_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { @@ -172,7 +169,7 @@ static struct nft_expr_type nft_masq_ipv6_type; static const struct nft_expr_ops nft_masq_ipv6_ops = { .type = &nft_masq_ipv6_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_masq)), - .eval = nft_masq_ipv6_eval, + .eval = nft_masq_eval, .init = nft_masq_init, .destroy = nft_masq_ipv6_destroy, .dump = nft_masq_dump, @@ -204,20 +201,6 @@ static inline void nft_masq_module_exit_ipv6(void) {} #endif #ifdef CONFIG_NF_TABLES_INET -static void nft_masq_inet_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) -{ - switch (nft_pf(pkt)) { - case NFPROTO_IPV4: - return nft_masq_ipv4_eval(expr, regs, pkt); - case NFPROTO_IPV6: - return nft_masq_ipv6_eval(expr, regs, pkt); - } - - WARN_ON_ONCE(1); -} - static void nft_masq_inet_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { @@ -228,7 +211,7 @@ static struct nft_expr_type nft_masq_inet_type; static const struct nft_expr_ops nft_masq_inet_ops = { .type = &nft_masq_inet_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_masq)), - .eval = nft_masq_inet_eval, + .eval = nft_masq_eval, .init = nft_masq_init, .destroy = nft_masq_inet_destroy, .dump = nft_masq_dump, -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nf-next 3/3] netfilter: nft_redir: deduplicate eval call-backs 2023-03-13 13:46 [PATCH nf-next 0/3] NF NAT deduplication refactoring Jeremy Sowden 2023-03-13 13:46 ` [PATCH nf-next 1/3] netfilter: nf_nat_redirect: use `struct nf_nat_range2` in ipv4 API Jeremy Sowden 2023-03-13 13:46 ` [PATCH nf-next 2/3] netfilter: nft_masq: deduplicate eval call-backs Jeremy Sowden @ 2023-03-13 13:46 ` Jeremy Sowden 2023-03-14 10:52 ` [PATCH nf-next 0/3] NF NAT deduplication refactoring Pablo Neira Ayuso 3 siblings, 0 replies; 8+ messages in thread From: Jeremy Sowden @ 2023-03-13 13:46 UTC (permalink / raw) To: Netfilter Devel; +Cc: Florian Westphal nft_redir has separate ipv4 and ipv6 call-backs which share much of their code, and an inet one switch containing a switch that calls one of the others based on the family of the packet. Merge the ipv4 and ipv6 ones into the inet one in order to get rid of the duplicate code. Const-qualify the `priv` pointer since we don't need to write through it. Assign `priv->flags` to the range instead of OR-ing it in. Set the `NF_NAT_RANGE_PROTO_SPECIFIED` flag once during init, rather than on every eval. Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Reviewed-by: Florian Westphal <fw@strlen.de> --- net/netfilter/nft_redir.c | 81 ++++++++++++++------------------------- 1 file changed, 29 insertions(+), 52 deletions(-) diff --git a/net/netfilter/nft_redir.c b/net/netfilter/nft_redir.c index 77a459470cb7..1d52a05a8b03 100644 --- a/net/netfilter/nft_redir.c +++ b/net/netfilter/nft_redir.c @@ -64,6 +64,8 @@ static int nft_redir_init(const struct nft_ctx *ctx, } else { priv->sreg_proto_max = priv->sreg_proto_min; } + + priv->flags |= NF_NAT_RANGE_PROTO_SPECIFIED; } if (tb[NFTA_REDIR_FLAGS]) { @@ -99,26 +101,37 @@ static int nft_redir_dump(struct sk_buff *skb, return -1; } -static void nft_redir_ipv4_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) +static void nft_redir_eval(const struct nft_expr *expr, + struct nft_regs *regs, + const struct nft_pktinfo *pkt) { - struct nft_redir *priv = nft_expr_priv(expr); + const struct nft_redir *priv = nft_expr_priv(expr); struct nf_nat_range2 range; memset(&range, 0, sizeof(range)); + range.flags = priv->flags; if (priv->sreg_proto_min) { - range.min_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_min]); - range.max_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_max]); - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED; + range.min_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_min]); + range.max_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_max]); } - range.flags |= priv->flags; - - regs->verdict.code = - nf_nat_redirect_ipv4(pkt->skb, &range, nft_hook(pkt)); + switch (nft_pf(pkt)) { + case NFPROTO_IPV4: + regs->verdict.code = nf_nat_redirect_ipv4(pkt->skb, &range, + nft_hook(pkt)); + break; +#ifdef CONFIG_NF_TABLES_IPV6 + case NFPROTO_IPV6: + regs->verdict.code = nf_nat_redirect_ipv6(pkt->skb, &range, + nft_hook(pkt)); + break; +#endif + default: + WARN_ON_ONCE(1); + break; + } } static void @@ -131,7 +144,7 @@ static struct nft_expr_type nft_redir_ipv4_type; static const struct nft_expr_ops nft_redir_ipv4_ops = { .type = &nft_redir_ipv4_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_ipv4_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_ipv4_destroy, .dump = nft_redir_dump, @@ -149,28 +162,6 @@ static struct nft_expr_type nft_redir_ipv4_type __read_mostly = { }; #ifdef CONFIG_NF_TABLES_IPV6 -static void nft_redir_ipv6_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) -{ - struct nft_redir *priv = nft_expr_priv(expr); - struct nf_nat_range2 range; - - memset(&range, 0, sizeof(range)); - if (priv->sreg_proto_min) { - range.min_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_min]); - range.max_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_max]); - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED; - } - - range.flags |= priv->flags; - - regs->verdict.code = - nf_nat_redirect_ipv6(pkt->skb, &range, nft_hook(pkt)); -} - static void nft_redir_ipv6_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { @@ -181,7 +172,7 @@ static struct nft_expr_type nft_redir_ipv6_type; static const struct nft_expr_ops nft_redir_ipv6_ops = { .type = &nft_redir_ipv6_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_ipv6_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_ipv6_destroy, .dump = nft_redir_dump, @@ -200,20 +191,6 @@ static struct nft_expr_type nft_redir_ipv6_type __read_mostly = { #endif #ifdef CONFIG_NF_TABLES_INET -static void nft_redir_inet_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) -{ - switch (nft_pf(pkt)) { - case NFPROTO_IPV4: - return nft_redir_ipv4_eval(expr, regs, pkt); - case NFPROTO_IPV6: - return nft_redir_ipv6_eval(expr, regs, pkt); - } - - WARN_ON_ONCE(1); -} - static void nft_redir_inet_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { @@ -224,7 +201,7 @@ static struct nft_expr_type nft_redir_inet_type; static const struct nft_expr_ops nft_redir_inet_ops = { .type = &nft_redir_inet_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_inet_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_inet_destroy, .dump = nft_redir_dump, -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH nf-next 0/3] NF NAT deduplication refactoring 2023-03-13 13:46 [PATCH nf-next 0/3] NF NAT deduplication refactoring Jeremy Sowden ` (2 preceding siblings ...) 2023-03-13 13:46 ` [PATCH nf-next 3/3] netfilter: nft_redir: " Jeremy Sowden @ 2023-03-14 10:52 ` Pablo Neira Ayuso 2023-03-15 10:25 ` Pablo Neira Ayuso 3 siblings, 1 reply; 8+ messages in thread From: Pablo Neira Ayuso @ 2023-03-14 10:52 UTC (permalink / raw) To: Jeremy Sowden; +Cc: Netfilter Devel Hi, On Mon, Mar 13, 2023 at 01:46:46PM +0000, Jeremy Sowden wrote: > These three patches perform refactoring in NF NAT modules to remove > duplicate code. Thanks for consolidating this. I collapsed patch 1 and 3, in this particular case I think it is easy to review to do these two things at once, I can see this: +static unsigned int +nf_nat_redirect(struct sk_buff *skb, const struct nf_nat_range2 *range, + const union nf_inet_addr *newdst) +{ + struct nf_nat_range2 newrange; + enum ip_conntrack_info ctinfo; + struct nf_conn *ct; + + ct = nf_ct_get(skb, &ctinfo); + WARN_ON(!(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED))); nf_nat_setup_info() calls _is_confirmed() which expect ct to be non-null, so crash will happen right after this WARN_ON. and _is_confirmed() implicitly means that this is either IP_CT_NEW or IP_CT_RELATED. This comes from 44d6e2f27328b25411 which replaced an assertion, quite straight forward. I'd suggest to remove this. + newrange.flags = range->flags | NF_NAT_RANGE_MAP_IPS; + newrange.min_addr = *newdst; + newrange.max_addr = *newdst; + newrange.min_proto = range->min_proto; + newrange.max_proto = range->max_proto; This newrange is missing memset(), other existing nat code calls memset(), I think it might be related to iptables which is exposing this structure to userspace through blob. I would need to check the NAT core to check if this memset is really required. + return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); +} May you submit v2 with these two changes? Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nf-next 0/3] NF NAT deduplication refactoring 2023-03-14 10:52 ` [PATCH nf-next 0/3] NF NAT deduplication refactoring Pablo Neira Ayuso @ 2023-03-15 10:25 ` Pablo Neira Ayuso 2023-03-15 10:59 ` Jeremy Sowden 0 siblings, 1 reply; 8+ messages in thread From: Pablo Neira Ayuso @ 2023-03-15 10:25 UTC (permalink / raw) To: Jeremy Sowden; +Cc: Netfilter Devel [-- Attachment #1: Type: text/plain, Size: 472 bytes --] On Tue, Mar 14, 2023 at 11:52:49AM +0100, Pablo Neira Ayuso wrote: > May you submit v2 with these two changes? Something like this attached. It is doing all at once, but the patch looks relatively easier to follow. I can also document the removal of WARN_ON() and the use of union nf_inet_addr newdst = {}; I am adding the memset() on range, but that is defensive. Probably all memset() can just go away from the nftables nat code, but I need to double check. Thanks. [-- Attachment #2: 0001-netfilter-nft_redir-deduplicate-eval-call-backs.patch --] [-- Type: text/x-diff, Size: 11606 bytes --] From b4e6d901cdf7f6adf43a66ec35829f6d90196326 Mon Sep 17 00:00:00 2001 From: Jeremy Sowden <jeremy@azazel.net> Date: Mon, 13 Mar 2023 13:46:47 +0000 Subject: [PATCH nf-next 1/2] netfilter: nft_redir: deduplicate eval call-backs `nf_nat_redirect_ipv4` takes a `struct nf_nat_ipv4_multi_range_compat`, but converts it internally to a `struct nf_nat_range2`. Change the function to take the latter, factor out the code now shared with `nf_nat_redirect_ipv6`, move the conversion to the xt_REDIRECT module, and update the ipv4 range initialization in the nft_redir module. Replace a bare hex constant for 127.0.0.1 with a macro. nft_redir has separate ipv4 and ipv6 call-backs which share much of their code, and an inet one switch containing a switch that calls one of the others based on the family of the packet. Merge the ipv4 and ipv6 ones into the inet one in order to get rid of the duplicate code. Const-qualify the `priv` pointer since we don't need to write through it. Assign `priv->flags` to the range instead of OR-ing it in. Set the `NF_NAT_RANGE_PROTO_SPECIFIED` flag once during init, rather than on every eval. Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Reviewed-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_nat_redirect.h | 3 +- net/netfilter/nf_nat_redirect.c | 70 +++++++++------------ net/netfilter/nft_redir.c | 84 +++++++++---------------- net/netfilter/xt_REDIRECT.c | 10 ++- 4 files changed, 71 insertions(+), 96 deletions(-) diff --git a/include/net/netfilter/nf_nat_redirect.h b/include/net/netfilter/nf_nat_redirect.h index 2418653a66db..279380de904c 100644 --- a/include/net/netfilter/nf_nat_redirect.h +++ b/include/net/netfilter/nf_nat_redirect.h @@ -6,8 +6,7 @@ #include <uapi/linux/netfilter/nf_nat.h> unsigned int -nf_nat_redirect_ipv4(struct sk_buff *skb, - const struct nf_nat_ipv4_multi_range_compat *mr, +nf_nat_redirect_ipv4(struct sk_buff *skb, const struct nf_nat_range2 *range, unsigned int hooknum); unsigned int nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, diff --git a/net/netfilter/nf_nat_redirect.c b/net/netfilter/nf_nat_redirect.c index f91579c821e9..083e534bded0 100644 --- a/net/netfilter/nf_nat_redirect.c +++ b/net/netfilter/nf_nat_redirect.c @@ -10,6 +10,7 @@ #include <linux/if.h> #include <linux/inetdevice.h> +#include <linux/in.h> #include <linux/ip.h> #include <linux/kernel.h> #include <linux/netdevice.h> @@ -24,54 +25,55 @@ #include <net/netfilter/nf_nat.h> #include <net/netfilter/nf_nat_redirect.h> +static unsigned int +nf_nat_redirect(struct sk_buff *skb, const struct nf_nat_range2 *range, + const union nf_inet_addr *newdst) +{ + struct nf_nat_range2 newrange; + enum ip_conntrack_info ctinfo; + struct nf_conn *ct; + + ct = nf_ct_get(skb, &ctinfo); + + memset(&newrange, 0, sizeof(newrange)); + newrange.flags = range->flags | NF_NAT_RANGE_MAP_IPS; + newrange.min_addr = *newdst; + newrange.max_addr = *newdst; + newrange.min_proto = range->min_proto; + newrange.max_proto = range->max_proto; + + return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); +} + unsigned int -nf_nat_redirect_ipv4(struct sk_buff *skb, - const struct nf_nat_ipv4_multi_range_compat *mr, +nf_nat_redirect_ipv4(struct sk_buff *skb, const struct nf_nat_range2 *range, unsigned int hooknum) { - struct nf_conn *ct; - enum ip_conntrack_info ctinfo; - __be32 newdst; - struct nf_nat_range2 newrange; + union nf_inet_addr newdst = {}; WARN_ON(hooknum != NF_INET_PRE_ROUTING && hooknum != NF_INET_LOCAL_OUT); - ct = nf_ct_get(skb, &ctinfo); - WARN_ON(!(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED))); - /* Local packets: make them go to loopback */ if (hooknum == NF_INET_LOCAL_OUT) { - newdst = htonl(0x7F000001); + newdst.ip = htonl(INADDR_LOOPBACK); } else { const struct in_device *indev; - newdst = 0; - indev = __in_dev_get_rcu(skb->dev); if (indev) { const struct in_ifaddr *ifa; ifa = rcu_dereference(indev->ifa_list); if (ifa) - newdst = ifa->ifa_local; + newdst.ip = ifa->ifa_local; } - if (!newdst) + if (!newdst.ip) return NF_DROP; } - /* Transfer from original range. */ - memset(&newrange.min_addr, 0, sizeof(newrange.min_addr)); - memset(&newrange.max_addr, 0, sizeof(newrange.max_addr)); - newrange.flags = mr->range[0].flags | NF_NAT_RANGE_MAP_IPS; - newrange.min_addr.ip = newdst; - newrange.max_addr.ip = newdst; - newrange.min_proto = mr->range[0].min; - newrange.max_proto = mr->range[0].max; - - /* Hand modified range to generic setup. */ - return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); + return nf_nat_redirect(skb, range, &newdst); } EXPORT_SYMBOL_GPL(nf_nat_redirect_ipv4); @@ -81,14 +83,10 @@ unsigned int nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, unsigned int hooknum) { - struct nf_nat_range2 newrange; - struct in6_addr newdst; - enum ip_conntrack_info ctinfo; - struct nf_conn *ct; + union nf_inet_addr newdst = {}; - ct = nf_ct_get(skb, &ctinfo); if (hooknum == NF_INET_LOCAL_OUT) { - newdst = loopback_addr; + newdst.in6 = loopback_addr; } else { struct inet6_dev *idev; struct inet6_ifaddr *ifa; @@ -98,7 +96,7 @@ nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, if (idev != NULL) { read_lock_bh(&idev->lock); list_for_each_entry(ifa, &idev->addr_list, if_list) { - newdst = ifa->addr; + newdst.in6 = ifa->addr; addr = true; break; } @@ -109,12 +107,6 @@ nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, return NF_DROP; } - newrange.flags = range->flags | NF_NAT_RANGE_MAP_IPS; - newrange.min_addr.in6 = newdst; - newrange.max_addr.in6 = newdst; - newrange.min_proto = range->min_proto; - newrange.max_proto = range->max_proto; - - return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); + return nf_nat_redirect(skb, range, &newdst); } EXPORT_SYMBOL_GPL(nf_nat_redirect_ipv6); diff --git a/net/netfilter/nft_redir.c b/net/netfilter/nft_redir.c index 5f7739987559..1d52a05a8b03 100644 --- a/net/netfilter/nft_redir.c +++ b/net/netfilter/nft_redir.c @@ -64,6 +64,8 @@ static int nft_redir_init(const struct nft_ctx *ctx, } else { priv->sreg_proto_max = priv->sreg_proto_min; } + + priv->flags |= NF_NAT_RANGE_PROTO_SPECIFIED; } if (tb[NFTA_REDIR_FLAGS]) { @@ -99,25 +101,37 @@ static int nft_redir_dump(struct sk_buff *skb, return -1; } -static void nft_redir_ipv4_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) +static void nft_redir_eval(const struct nft_expr *expr, + struct nft_regs *regs, + const struct nft_pktinfo *pkt) { - struct nft_redir *priv = nft_expr_priv(expr); - struct nf_nat_ipv4_multi_range_compat mr; + const struct nft_redir *priv = nft_expr_priv(expr); + struct nf_nat_range2 range; - memset(&mr, 0, sizeof(mr)); + memset(&range, 0, sizeof(range)); + range.flags = priv->flags; if (priv->sreg_proto_min) { - mr.range[0].min.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_min]); - mr.range[0].max.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_max]); - mr.range[0].flags |= NF_NAT_RANGE_PROTO_SPECIFIED; + range.min_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_min]); + range.max_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_max]); } - mr.range[0].flags |= priv->flags; - - regs->verdict.code = nf_nat_redirect_ipv4(pkt->skb, &mr, nft_hook(pkt)); + switch (nft_pf(pkt)) { + case NFPROTO_IPV4: + regs->verdict.code = nf_nat_redirect_ipv4(pkt->skb, &range, + nft_hook(pkt)); + break; +#ifdef CONFIG_NF_TABLES_IPV6 + case NFPROTO_IPV6: + regs->verdict.code = nf_nat_redirect_ipv6(pkt->skb, &range, + nft_hook(pkt)); + break; +#endif + default: + WARN_ON_ONCE(1); + break; + } } static void @@ -130,7 +144,7 @@ static struct nft_expr_type nft_redir_ipv4_type; static const struct nft_expr_ops nft_redir_ipv4_ops = { .type = &nft_redir_ipv4_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_ipv4_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_ipv4_destroy, .dump = nft_redir_dump, @@ -148,28 +162,6 @@ static struct nft_expr_type nft_redir_ipv4_type __read_mostly = { }; #ifdef CONFIG_NF_TABLES_IPV6 -static void nft_redir_ipv6_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) -{ - struct nft_redir *priv = nft_expr_priv(expr); - struct nf_nat_range2 range; - - memset(&range, 0, sizeof(range)); - if (priv->sreg_proto_min) { - range.min_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_min]); - range.max_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_max]); - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED; - } - - range.flags |= priv->flags; - - regs->verdict.code = - nf_nat_redirect_ipv6(pkt->skb, &range, nft_hook(pkt)); -} - static void nft_redir_ipv6_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { @@ -180,7 +172,7 @@ static struct nft_expr_type nft_redir_ipv6_type; static const struct nft_expr_ops nft_redir_ipv6_ops = { .type = &nft_redir_ipv6_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_ipv6_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_ipv6_destroy, .dump = nft_redir_dump, @@ -199,20 +191,6 @@ static struct nft_expr_type nft_redir_ipv6_type __read_mostly = { #endif #ifdef CONFIG_NF_TABLES_INET -static void nft_redir_inet_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) -{ - switch (nft_pf(pkt)) { - case NFPROTO_IPV4: - return nft_redir_ipv4_eval(expr, regs, pkt); - case NFPROTO_IPV6: - return nft_redir_ipv6_eval(expr, regs, pkt); - } - - WARN_ON_ONCE(1); -} - static void nft_redir_inet_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { @@ -223,7 +201,7 @@ static struct nft_expr_type nft_redir_inet_type; static const struct nft_expr_ops nft_redir_inet_ops = { .type = &nft_redir_inet_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_inet_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_inet_destroy, .dump = nft_redir_dump, diff --git a/net/netfilter/xt_REDIRECT.c b/net/netfilter/xt_REDIRECT.c index 353ca7801251..ff66b56a3f97 100644 --- a/net/netfilter/xt_REDIRECT.c +++ b/net/netfilter/xt_REDIRECT.c @@ -46,7 +46,6 @@ static void redirect_tg_destroy(const struct xt_tgdtor_param *par) nf_ct_netns_put(par->net, par->family); } -/* FIXME: Take multiple ranges --RR */ static int redirect_tg4_check(const struct xt_tgchk_param *par) { const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo; @@ -65,7 +64,14 @@ static int redirect_tg4_check(const struct xt_tgchk_param *par) static unsigned int redirect_tg4(struct sk_buff *skb, const struct xt_action_param *par) { - return nf_nat_redirect_ipv4(skb, par->targinfo, xt_hooknum(par)); + const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo; + struct nf_nat_range2 range = { + .flags = mr->range[0].flags, + .min_proto = mr->range[0].min, + .max_proto = mr->range[0].max, + }; + + return nf_nat_redirect_ipv4(skb, &range, xt_hooknum(par)); } static struct xt_target redirect_tg_reg[] __read_mostly = { -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH nf-next 0/3] NF NAT deduplication refactoring 2023-03-15 10:25 ` Pablo Neira Ayuso @ 2023-03-15 10:59 ` Jeremy Sowden 0 siblings, 0 replies; 8+ messages in thread From: Jeremy Sowden @ 2023-03-15 10:59 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Netfilter Devel [-- Attachment #1: Type: text/plain, Size: 13577 bytes --] On 2023-03-15, at 11:25:02 +0100, Pablo Neira Ayuso wrote: > On Tue, Mar 14, 2023 at 11:52:49AM +0100, Pablo Neira Ayuso wrote: > > May you submit v2 with these two changes? > > Something like this attached. It is doing all at once, but the patch > looks relatively easier to follow. Thanks for this. I did make a start on v2, but yesterday was not very productive. I'll incorporate your suggestions and send it out later to-day. > I can also document the removal of WARN_ON() and the use of > union nf_inet_addr newdst = {}; > > I am adding the memset() on range, but that is defensive. Probably all > memset() can just go away from the nftables nat code, but I need to > double check. Yeah, I didn't see anywhere that appeared to require the memset, which is why I took it out, but I didn't trace the whole lifecycle of the variable, so I may have missed something. J. > From b4e6d901cdf7f6adf43a66ec35829f6d90196326 Mon Sep 17 00:00:00 2001 > From: Jeremy Sowden <jeremy@azazel.net> > Date: Mon, 13 Mar 2023 13:46:47 +0000 > Subject: [PATCH nf-next 1/2] netfilter: nft_redir: deduplicate eval call-backs > > `nf_nat_redirect_ipv4` takes a `struct nf_nat_ipv4_multi_range_compat`, > but converts it internally to a `struct nf_nat_range2`. Change the > function to take the latter, factor out the code now shared with > `nf_nat_redirect_ipv6`, move the conversion to the xt_REDIRECT module, > and update the ipv4 range initialization in the nft_redir module. > > Replace a bare hex constant for 127.0.0.1 with a macro. > > nft_redir has separate ipv4 and ipv6 call-backs which share much of > their code, and an inet one switch containing a switch that calls one of > the others based on the family of the packet. Merge the ipv4 and ipv6 > ones into the inet one in order to get rid of the duplicate code. > > Const-qualify the `priv` pointer since we don't need to write through > it. > > Assign `priv->flags` to the range instead of OR-ing it in. > > Set the `NF_NAT_RANGE_PROTO_SPECIFIED` flag once during init, rather > than on every eval. > > Signed-off-by: Jeremy Sowden <jeremy@azazel.net> > Reviewed-by: Florian Westphal <fw@strlen.de> > --- > include/net/netfilter/nf_nat_redirect.h | 3 +- > net/netfilter/nf_nat_redirect.c | 70 +++++++++------------ > net/netfilter/nft_redir.c | 84 +++++++++---------------- > net/netfilter/xt_REDIRECT.c | 10 ++- > 4 files changed, 71 insertions(+), 96 deletions(-) > > diff --git a/include/net/netfilter/nf_nat_redirect.h b/include/net/netfilter/nf_nat_redirect.h > index 2418653a66db..279380de904c 100644 > --- a/include/net/netfilter/nf_nat_redirect.h > +++ b/include/net/netfilter/nf_nat_redirect.h > @@ -6,8 +6,7 @@ > #include <uapi/linux/netfilter/nf_nat.h> > > unsigned int > -nf_nat_redirect_ipv4(struct sk_buff *skb, > - const struct nf_nat_ipv4_multi_range_compat *mr, > +nf_nat_redirect_ipv4(struct sk_buff *skb, const struct nf_nat_range2 *range, > unsigned int hooknum); > unsigned int > nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, > diff --git a/net/netfilter/nf_nat_redirect.c b/net/netfilter/nf_nat_redirect.c > index f91579c821e9..083e534bded0 100644 > --- a/net/netfilter/nf_nat_redirect.c > +++ b/net/netfilter/nf_nat_redirect.c > @@ -10,6 +10,7 @@ > > #include <linux/if.h> > #include <linux/inetdevice.h> > +#include <linux/in.h> > #include <linux/ip.h> > #include <linux/kernel.h> > #include <linux/netdevice.h> > @@ -24,54 +25,55 @@ > #include <net/netfilter/nf_nat.h> > #include <net/netfilter/nf_nat_redirect.h> > > +static unsigned int > +nf_nat_redirect(struct sk_buff *skb, const struct nf_nat_range2 *range, > + const union nf_inet_addr *newdst) > +{ > + struct nf_nat_range2 newrange; > + enum ip_conntrack_info ctinfo; > + struct nf_conn *ct; > + > + ct = nf_ct_get(skb, &ctinfo); > + > + memset(&newrange, 0, sizeof(newrange)); > + newrange.flags = range->flags | NF_NAT_RANGE_MAP_IPS; > + newrange.min_addr = *newdst; > + newrange.max_addr = *newdst; > + newrange.min_proto = range->min_proto; > + newrange.max_proto = range->max_proto; > + > + return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); > +} > + > unsigned int > -nf_nat_redirect_ipv4(struct sk_buff *skb, > - const struct nf_nat_ipv4_multi_range_compat *mr, > +nf_nat_redirect_ipv4(struct sk_buff *skb, const struct nf_nat_range2 *range, > unsigned int hooknum) > { > - struct nf_conn *ct; > - enum ip_conntrack_info ctinfo; > - __be32 newdst; > - struct nf_nat_range2 newrange; > + union nf_inet_addr newdst = {}; > > WARN_ON(hooknum != NF_INET_PRE_ROUTING && > hooknum != NF_INET_LOCAL_OUT); > > - ct = nf_ct_get(skb, &ctinfo); > - WARN_ON(!(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED))); > - > /* Local packets: make them go to loopback */ > if (hooknum == NF_INET_LOCAL_OUT) { > - newdst = htonl(0x7F000001); > + newdst.ip = htonl(INADDR_LOOPBACK); > } else { > const struct in_device *indev; > > - newdst = 0; > - > indev = __in_dev_get_rcu(skb->dev); > if (indev) { > const struct in_ifaddr *ifa; > > ifa = rcu_dereference(indev->ifa_list); > if (ifa) > - newdst = ifa->ifa_local; > + newdst.ip = ifa->ifa_local; > } > > - if (!newdst) > + if (!newdst.ip) > return NF_DROP; > } > > - /* Transfer from original range. */ > - memset(&newrange.min_addr, 0, sizeof(newrange.min_addr)); > - memset(&newrange.max_addr, 0, sizeof(newrange.max_addr)); > - newrange.flags = mr->range[0].flags | NF_NAT_RANGE_MAP_IPS; > - newrange.min_addr.ip = newdst; > - newrange.max_addr.ip = newdst; > - newrange.min_proto = mr->range[0].min; > - newrange.max_proto = mr->range[0].max; > - > - /* Hand modified range to generic setup. */ > - return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); > + return nf_nat_redirect(skb, range, &newdst); > } > EXPORT_SYMBOL_GPL(nf_nat_redirect_ipv4); > > @@ -81,14 +83,10 @@ unsigned int > nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, > unsigned int hooknum) > { > - struct nf_nat_range2 newrange; > - struct in6_addr newdst; > - enum ip_conntrack_info ctinfo; > - struct nf_conn *ct; > + union nf_inet_addr newdst = {}; > > - ct = nf_ct_get(skb, &ctinfo); > if (hooknum == NF_INET_LOCAL_OUT) { > - newdst = loopback_addr; > + newdst.in6 = loopback_addr; > } else { > struct inet6_dev *idev; > struct inet6_ifaddr *ifa; > @@ -98,7 +96,7 @@ nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, > if (idev != NULL) { > read_lock_bh(&idev->lock); > list_for_each_entry(ifa, &idev->addr_list, if_list) { > - newdst = ifa->addr; > + newdst.in6 = ifa->addr; > addr = true; > break; > } > @@ -109,12 +107,6 @@ nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range, > return NF_DROP; > } > > - newrange.flags = range->flags | NF_NAT_RANGE_MAP_IPS; > - newrange.min_addr.in6 = newdst; > - newrange.max_addr.in6 = newdst; > - newrange.min_proto = range->min_proto; > - newrange.max_proto = range->max_proto; > - > - return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_DST); > + return nf_nat_redirect(skb, range, &newdst); > } > EXPORT_SYMBOL_GPL(nf_nat_redirect_ipv6); > diff --git a/net/netfilter/nft_redir.c b/net/netfilter/nft_redir.c > index 5f7739987559..1d52a05a8b03 100644 > --- a/net/netfilter/nft_redir.c > +++ b/net/netfilter/nft_redir.c > @@ -64,6 +64,8 @@ static int nft_redir_init(const struct nft_ctx *ctx, > } else { > priv->sreg_proto_max = priv->sreg_proto_min; > } > + > + priv->flags |= NF_NAT_RANGE_PROTO_SPECIFIED; > } > > if (tb[NFTA_REDIR_FLAGS]) { > @@ -99,25 +101,37 @@ static int nft_redir_dump(struct sk_buff *skb, > return -1; > } > > -static void nft_redir_ipv4_eval(const struct nft_expr *expr, > - struct nft_regs *regs, > - const struct nft_pktinfo *pkt) > +static void nft_redir_eval(const struct nft_expr *expr, > + struct nft_regs *regs, > + const struct nft_pktinfo *pkt) > { > - struct nft_redir *priv = nft_expr_priv(expr); > - struct nf_nat_ipv4_multi_range_compat mr; > + const struct nft_redir *priv = nft_expr_priv(expr); > + struct nf_nat_range2 range; > > - memset(&mr, 0, sizeof(mr)); > + memset(&range, 0, sizeof(range)); > + range.flags = priv->flags; > if (priv->sreg_proto_min) { > - mr.range[0].min.all = (__force __be16)nft_reg_load16( > - ®s->data[priv->sreg_proto_min]); > - mr.range[0].max.all = (__force __be16)nft_reg_load16( > - ®s->data[priv->sreg_proto_max]); > - mr.range[0].flags |= NF_NAT_RANGE_PROTO_SPECIFIED; > + range.min_proto.all = (__force __be16) > + nft_reg_load16(®s->data[priv->sreg_proto_min]); > + range.max_proto.all = (__force __be16) > + nft_reg_load16(®s->data[priv->sreg_proto_max]); > } > > - mr.range[0].flags |= priv->flags; > - > - regs->verdict.code = nf_nat_redirect_ipv4(pkt->skb, &mr, nft_hook(pkt)); > + switch (nft_pf(pkt)) { > + case NFPROTO_IPV4: > + regs->verdict.code = nf_nat_redirect_ipv4(pkt->skb, &range, > + nft_hook(pkt)); > + break; > +#ifdef CONFIG_NF_TABLES_IPV6 > + case NFPROTO_IPV6: > + regs->verdict.code = nf_nat_redirect_ipv6(pkt->skb, &range, > + nft_hook(pkt)); > + break; > +#endif > + default: > + WARN_ON_ONCE(1); > + break; > + } > } > > static void > @@ -130,7 +144,7 @@ static struct nft_expr_type nft_redir_ipv4_type; > static const struct nft_expr_ops nft_redir_ipv4_ops = { > .type = &nft_redir_ipv4_type, > .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), > - .eval = nft_redir_ipv4_eval, > + .eval = nft_redir_eval, > .init = nft_redir_init, > .destroy = nft_redir_ipv4_destroy, > .dump = nft_redir_dump, > @@ -148,28 +162,6 @@ static struct nft_expr_type nft_redir_ipv4_type __read_mostly = { > }; > > #ifdef CONFIG_NF_TABLES_IPV6 > -static void nft_redir_ipv6_eval(const struct nft_expr *expr, > - struct nft_regs *regs, > - const struct nft_pktinfo *pkt) > -{ > - struct nft_redir *priv = nft_expr_priv(expr); > - struct nf_nat_range2 range; > - > - memset(&range, 0, sizeof(range)); > - if (priv->sreg_proto_min) { > - range.min_proto.all = (__force __be16)nft_reg_load16( > - ®s->data[priv->sreg_proto_min]); > - range.max_proto.all = (__force __be16)nft_reg_load16( > - ®s->data[priv->sreg_proto_max]); > - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED; > - } > - > - range.flags |= priv->flags; > - > - regs->verdict.code = > - nf_nat_redirect_ipv6(pkt->skb, &range, nft_hook(pkt)); > -} > - > static void > nft_redir_ipv6_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) > { > @@ -180,7 +172,7 @@ static struct nft_expr_type nft_redir_ipv6_type; > static const struct nft_expr_ops nft_redir_ipv6_ops = { > .type = &nft_redir_ipv6_type, > .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), > - .eval = nft_redir_ipv6_eval, > + .eval = nft_redir_eval, > .init = nft_redir_init, > .destroy = nft_redir_ipv6_destroy, > .dump = nft_redir_dump, > @@ -199,20 +191,6 @@ static struct nft_expr_type nft_redir_ipv6_type __read_mostly = { > #endif > > #ifdef CONFIG_NF_TABLES_INET > -static void nft_redir_inet_eval(const struct nft_expr *expr, > - struct nft_regs *regs, > - const struct nft_pktinfo *pkt) > -{ > - switch (nft_pf(pkt)) { > - case NFPROTO_IPV4: > - return nft_redir_ipv4_eval(expr, regs, pkt); > - case NFPROTO_IPV6: > - return nft_redir_ipv6_eval(expr, regs, pkt); > - } > - > - WARN_ON_ONCE(1); > -} > - > static void > nft_redir_inet_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) > { > @@ -223,7 +201,7 @@ static struct nft_expr_type nft_redir_inet_type; > static const struct nft_expr_ops nft_redir_inet_ops = { > .type = &nft_redir_inet_type, > .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), > - .eval = nft_redir_inet_eval, > + .eval = nft_redir_eval, > .init = nft_redir_init, > .destroy = nft_redir_inet_destroy, > .dump = nft_redir_dump, > diff --git a/net/netfilter/xt_REDIRECT.c b/net/netfilter/xt_REDIRECT.c > index 353ca7801251..ff66b56a3f97 100644 > --- a/net/netfilter/xt_REDIRECT.c > +++ b/net/netfilter/xt_REDIRECT.c > @@ -46,7 +46,6 @@ static void redirect_tg_destroy(const struct xt_tgdtor_param *par) > nf_ct_netns_put(par->net, par->family); > } > > -/* FIXME: Take multiple ranges --RR */ > static int redirect_tg4_check(const struct xt_tgchk_param *par) > { > const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo; > @@ -65,7 +64,14 @@ static int redirect_tg4_check(const struct xt_tgchk_param *par) > static unsigned int > redirect_tg4(struct sk_buff *skb, const struct xt_action_param *par) > { > - return nf_nat_redirect_ipv4(skb, par->targinfo, xt_hooknum(par)); > + const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo; > + struct nf_nat_range2 range = { > + .flags = mr->range[0].flags, > + .min_proto = mr->range[0].min, > + .max_proto = mr->range[0].max, > + }; > + > + return nf_nat_redirect_ipv4(skb, &range, xt_hooknum(par)); > } > > static struct xt_target redirect_tg_reg[] __read_mostly = { > -- > 2.30.2 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH nf-next 0/3] NF NAT deduplication refactoring @ 2023-03-15 21:47 Jeremy Sowden 2023-03-15 21:47 ` [PATCH nf-next 3/3] netfilter: nft_redir: deduplicate eval call-backs Jeremy Sowden 0 siblings, 1 reply; 8+ messages in thread From: Jeremy Sowden @ 2023-03-15 21:47 UTC (permalink / raw) To: Netfilter Devel These three patches perform refactoring in NF NAT modules to remove duplicate code. Jeremy Sowden (3): netfilter: nf_nat_redirect: use `struct nf_nat_range2` in ipv4 API netfilter: nft_masq: deduplicate eval call-backs netfilter: nft_redir: deduplicate eval call-backs include/net/netfilter/nf_nat_redirect.h | 3 +- net/netfilter/nf_nat_redirect.c | 58 ++++++++--------- net/netfilter/nft_masq.c | 75 +++++++++------------- net/netfilter/nft_redir.c | 84 +++++++++---------------- net/netfilter/xt_REDIRECT.c | 10 ++- 5 files changed, 96 insertions(+), 134 deletions(-) -- 2.39.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH nf-next 3/3] netfilter: nft_redir: deduplicate eval call-backs 2023-03-15 21:47 Jeremy Sowden @ 2023-03-15 21:47 ` Jeremy Sowden 0 siblings, 0 replies; 8+ messages in thread From: Jeremy Sowden @ 2023-03-15 21:47 UTC (permalink / raw) To: Netfilter Devel; +Cc: Florian Westphal nft_redir has separate ipv4 and ipv6 call-backs which share much of their code, and an inet one switch containing a switch that calls one of the others based on the family of the packet. Merge the ipv4 and ipv6 ones into the inet one in order to get rid of the duplicate code. Const-qualify the `priv` pointer since we don't need to write through it. Assign `priv->flags` to the range instead of OR-ing it in. Set the `NF_NAT_RANGE_PROTO_SPECIFIED` flag once during init, rather than on every eval. Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Reviewed-by: Florian Westphal <fw@strlen.de> --- net/netfilter/nft_redir.c | 81 ++++++++++++++------------------------- 1 file changed, 29 insertions(+), 52 deletions(-) diff --git a/net/netfilter/nft_redir.c b/net/netfilter/nft_redir.c index 77a459470cb7..1d52a05a8b03 100644 --- a/net/netfilter/nft_redir.c +++ b/net/netfilter/nft_redir.c @@ -64,6 +64,8 @@ static int nft_redir_init(const struct nft_ctx *ctx, } else { priv->sreg_proto_max = priv->sreg_proto_min; } + + priv->flags |= NF_NAT_RANGE_PROTO_SPECIFIED; } if (tb[NFTA_REDIR_FLAGS]) { @@ -99,26 +101,37 @@ static int nft_redir_dump(struct sk_buff *skb, return -1; } -static void nft_redir_ipv4_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) +static void nft_redir_eval(const struct nft_expr *expr, + struct nft_regs *regs, + const struct nft_pktinfo *pkt) { - struct nft_redir *priv = nft_expr_priv(expr); + const struct nft_redir *priv = nft_expr_priv(expr); struct nf_nat_range2 range; memset(&range, 0, sizeof(range)); + range.flags = priv->flags; if (priv->sreg_proto_min) { - range.min_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_min]); - range.max_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_max]); - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED; + range.min_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_min]); + range.max_proto.all = (__force __be16) + nft_reg_load16(®s->data[priv->sreg_proto_max]); } - range.flags |= priv->flags; - - regs->verdict.code = - nf_nat_redirect_ipv4(pkt->skb, &range, nft_hook(pkt)); + switch (nft_pf(pkt)) { + case NFPROTO_IPV4: + regs->verdict.code = nf_nat_redirect_ipv4(pkt->skb, &range, + nft_hook(pkt)); + break; +#ifdef CONFIG_NF_TABLES_IPV6 + case NFPROTO_IPV6: + regs->verdict.code = nf_nat_redirect_ipv6(pkt->skb, &range, + nft_hook(pkt)); + break; +#endif + default: + WARN_ON_ONCE(1); + break; + } } static void @@ -131,7 +144,7 @@ static struct nft_expr_type nft_redir_ipv4_type; static const struct nft_expr_ops nft_redir_ipv4_ops = { .type = &nft_redir_ipv4_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_ipv4_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_ipv4_destroy, .dump = nft_redir_dump, @@ -149,28 +162,6 @@ static struct nft_expr_type nft_redir_ipv4_type __read_mostly = { }; #ifdef CONFIG_NF_TABLES_IPV6 -static void nft_redir_ipv6_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) -{ - struct nft_redir *priv = nft_expr_priv(expr); - struct nf_nat_range2 range; - - memset(&range, 0, sizeof(range)); - if (priv->sreg_proto_min) { - range.min_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_min]); - range.max_proto.all = (__force __be16)nft_reg_load16( - ®s->data[priv->sreg_proto_max]); - range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED; - } - - range.flags |= priv->flags; - - regs->verdict.code = - nf_nat_redirect_ipv6(pkt->skb, &range, nft_hook(pkt)); -} - static void nft_redir_ipv6_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { @@ -181,7 +172,7 @@ static struct nft_expr_type nft_redir_ipv6_type; static const struct nft_expr_ops nft_redir_ipv6_ops = { .type = &nft_redir_ipv6_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_ipv6_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_ipv6_destroy, .dump = nft_redir_dump, @@ -200,20 +191,6 @@ static struct nft_expr_type nft_redir_ipv6_type __read_mostly = { #endif #ifdef CONFIG_NF_TABLES_INET -static void nft_redir_inet_eval(const struct nft_expr *expr, - struct nft_regs *regs, - const struct nft_pktinfo *pkt) -{ - switch (nft_pf(pkt)) { - case NFPROTO_IPV4: - return nft_redir_ipv4_eval(expr, regs, pkt); - case NFPROTO_IPV6: - return nft_redir_ipv6_eval(expr, regs, pkt); - } - - WARN_ON_ONCE(1); -} - static void nft_redir_inet_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { @@ -224,7 +201,7 @@ static struct nft_expr_type nft_redir_inet_type; static const struct nft_expr_ops nft_redir_inet_ops = { .type = &nft_redir_inet_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_redir)), - .eval = nft_redir_inet_eval, + .eval = nft_redir_eval, .init = nft_redir_init, .destroy = nft_redir_inet_destroy, .dump = nft_redir_dump, -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-15 21:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-13 13:46 [PATCH nf-next 0/3] NF NAT deduplication refactoring Jeremy Sowden 2023-03-13 13:46 ` [PATCH nf-next 1/3] netfilter: nf_nat_redirect: use `struct nf_nat_range2` in ipv4 API Jeremy Sowden 2023-03-13 13:46 ` [PATCH nf-next 2/3] netfilter: nft_masq: deduplicate eval call-backs Jeremy Sowden 2023-03-13 13:46 ` [PATCH nf-next 3/3] netfilter: nft_redir: " Jeremy Sowden 2023-03-14 10:52 ` [PATCH nf-next 0/3] NF NAT deduplication refactoring Pablo Neira Ayuso 2023-03-15 10:25 ` Pablo Neira Ayuso 2023-03-15 10:59 ` Jeremy Sowden 2023-03-15 21:47 Jeremy Sowden 2023-03-15 21:47 ` [PATCH nf-next 3/3] netfilter: nft_redir: deduplicate eval call-backs Jeremy Sowden
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).