diff mbox series

[nf-next,12/13] netfilter: nft_redir: deduplicate eval call-backs

Message ID 20230305121817.2234734-13-jeremy@azazel.net
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series Support for shifted port-ranges in NAT | expand

Commit Message

Jeremy Sowden March 5, 2023, 12:18 p.m. UTC 
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.

Set the `NF_NAT_RANGE_PROTO_SPECIFIED` flag once during init, rather
than on every eval.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 net/netfilter/nft_redir.c | 78 ++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 50 deletions(-)

Comments

Florian Westphal March 7, 2023, 12:37 p.m. UTC | #1 
Jeremy Sowden <jeremy@azazel.net> wrote:
> 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.
> 
> Set the `NF_NAT_RANGE_PROTO_SPECIFIED` flag once during init, rather
> than on every eval.

Reviewed-by: Florian Westphal <fw@strlen.de>

> -	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));
>  	if (priv->sreg_proto_min) {
> -		range.min_proto.all = (__force __be16)nft_reg_load16(
> -			&regs->data[priv->sreg_proto_min]);
> -		range.max_proto.all = (__force __be16)nft_reg_load16(
> -			&regs->data[priv->sreg_proto_max]);
> -		range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
> +		range.min_proto.all = (__force __be16)
> +			nft_reg_load16(&regs->data[priv->sreg_proto_min]);
> +		range.max_proto.all = (__force __be16)
> +			nft_reg_load16(&regs->data[priv->sreg_proto_max]);
>  	}
>  
>  	range.flags |= priv->flags;

Nit: This could be updated to 'range.flags = priv->flags'
Jeremy Sowden March 7, 2023, 6:42 p.m. UTC | #2 
On 2023-03-07, at 13:37:40 +0100, Florian Westphal wrote:
> Jeremy Sowden <jeremy@azazel.net> wrote:
> > 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.
> > 
> > Set the `NF_NAT_RANGE_PROTO_SPECIFIED` flag once during init, rather
> > than on every eval.
> 
> Reviewed-by: Florian Westphal <fw@strlen.de>
> 
> > -	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));
> >  	if (priv->sreg_proto_min) {
> > -		range.min_proto.all = (__force __be16)nft_reg_load16(
> > -			&regs->data[priv->sreg_proto_min]);
> > -		range.max_proto.all = (__force __be16)nft_reg_load16(
> > -			&regs->data[priv->sreg_proto_max]);
> > -		range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
> > +		range.min_proto.all = (__force __be16)
> > +			nft_reg_load16(&regs->data[priv->sreg_proto_min]);
> > +		range.max_proto.all = (__force __be16)
> > +			nft_reg_load16(&regs->data[priv->sreg_proto_max]);
> >  	}
> >  
> >  	range.flags |= priv->flags;
> 
> Nit: This could be updated to 'range.flags = priv->flags'

Will fix.

J.
diff mbox series

Patch

diff --git a/net/netfilter/nft_redir.c b/net/netfilter/nft_redir.c
index 32a74576fd22..24f14771f9ab 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,38 @@  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));
 	if (priv->sreg_proto_min) {
-		range.min_proto.all = (__force __be16)nft_reg_load16(
-			&regs->data[priv->sreg_proto_min]);
-		range.max_proto.all = (__force __be16)nft_reg_load16(
-			&regs->data[priv->sreg_proto_max]);
-		range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
+		range.min_proto.all = (__force __be16)
+			nft_reg_load16(&regs->data[priv->sreg_proto_min]);
+		range.max_proto.all = (__force __be16)
+			nft_reg_load16(&regs->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 +145,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 +163,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(
-			&regs->data[priv->sreg_proto_min]);
-		range.max_proto.all = (__force __be16)nft_reg_load16(
-			&regs->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 +173,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 +192,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 +202,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,