[PATCH 1/4] netfilter: flowtable: fix excessive hw offload attempts after failure

[PATCH 1/4] netfilter: flowtable: fix excessive hw offload attempts after failure

[Felix Fietkau]

If a flow cannot be offloaded, the code currently repeatedly tries again as
quickly as possible, which can significantly increase system load.
Fix this by limiting flow timeout update and hardware offload retry to once
per second.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/netfilter/nf_flow_table_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 3db256da919b..20b4a14e5d4e 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -335,8 +335,10 @@ void flow_offload_refresh(struct nf_flowtable *flow_table,
 	u32 timeout;
 
 	timeout = nf_flowtable_time_stamp + flow_offload_get_timeout(flow);
-	if (READ_ONCE(flow->timeout) != timeout)
+	if (timeout - READ_ONCE(flow->timeout) > HZ)
 		WRITE_ONCE(flow->timeout, timeout);
+	else
+		return;
 
 	if (likely(!nf_flowtable_hw_offload(flow_table)))
 		return;
-- 
2.35.1

[PATCH 2/4] netfilter: nft_flow_offload: skip dst neigh lookup for ppp devices

[Felix Fietkau]

The dst entry does not contain a valid hardware address, so skip the lookup
in order to avoid running into errors here.
The proper hardware address is filled in from nft_dev_path_info

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/netfilter/nft_flow_offload.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 900d48c810a1..d88de26aad75 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -36,6 +36,15 @@ static void nft_default_forward_path(struct nf_flow_route *route,
 	route->tuple[dir].xmit_type	= nft_xmit_type(dst_cache);
 }
 
+static bool nft_is_valid_ether_device(const struct net_device *dev)
+{
+	if (!dev || (dev->flags & IFF_LOOPBACK) || dev->type != ARPHRD_ETHER ||
+	    dev->addr_len != ETH_ALEN || !is_valid_ether_addr(dev->dev_addr))
+		return false;
+
+	return true;
+}
+
 static int nft_dev_fill_forward_path(const struct nf_flow_route *route,
 				     const struct dst_entry *dst_cache,
 				     const struct nf_conn *ct,
@@ -47,6 +56,9 @@ static int nft_dev_fill_forward_path(const struct nf_flow_route *route,
 	struct neighbour *n;
 	u8 nud_state;
 
+	if (!nft_is_valid_ether_device(dev))
+		goto out;
+
 	n = dst_neigh_lookup(dst_cache, daddr);
 	if (!n)
 		return -1;
@@ -60,6 +72,7 @@ static int nft_dev_fill_forward_path(const struct nf_flow_route *route,
 	if (!(nud_state & NUD_VALID))
 		return -1;
 
+out:
 	return dev_fill_forward_path(dev, ha, stack);
 }
 
@@ -78,15 +91,6 @@ struct nft_forward_info {
 	enum flow_offload_xmit_type xmit_type;
 };
 
-static bool nft_is_valid_ether_device(const struct net_device *dev)
-{
-	if (!dev || (dev->flags & IFF_LOOPBACK) || dev->type != ARPHRD_ETHER ||
-	    dev->addr_len != ETH_ALEN || !is_valid_ether_addr(dev->dev_addr))
-		return false;
-
-	return true;
-}
-
 static void nft_dev_path_info(const struct net_device_path_stack *stack,
 			      struct nft_forward_info *info,
 			      unsigned char *ha, struct nf_flowtable *flowtable)
-- 
2.35.1

[PATCH 3/4] net: fix dev_fill_forward_path with pppoe + bridge

[Felix Fietkau]

When calling dev_fill_forward_path on a pppoe device, the provided destination
address is invalid. In order for the bridge fdb lookup to succeed, the pppoe
code needs to update ctx->daddr to the correct value.
Fix this by storing the address inside struct net_device_path_ctx

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 drivers/net/ethernet/mediatek/mtk_ppe_offload.c | 2 +-
 drivers/net/ppp/pppoe.c                         | 1 +
 include/linux/netdevice.h                       | 2 +-
 net/core/dev.c                                  | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
index 1fe31058b0f2..d4a0126082f2 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
@@ -90,7 +90,6 @@ mtk_flow_get_wdma_info(struct net_device *dev, const u8 *addr, struct mtk_wdma_i
 {
 	struct net_device_path_ctx ctx = {
 		.dev = dev,
-		.daddr = addr,
 	};
 	struct net_device_path path = {};
 
@@ -100,6 +99,7 @@ mtk_flow_get_wdma_info(struct net_device *dev, const u8 *addr, struct mtk_wdma_i
 	if (!dev->netdev_ops->ndo_fill_forward_path)
 		return -1;
 
+	memcpy(ctx.daddr, addr, sizeof(ctx.daddr));
 	if (dev->netdev_ops->ndo_fill_forward_path(&ctx, &path))
 		return -1;
 
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 1b41cd9732d7..ce2cbb5903d7 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -988,6 +988,7 @@ static int pppoe_fill_forward_path(struct net_device_path_ctx *ctx,
 	path->encap.proto = htons(ETH_P_PPP_SES);
 	path->encap.id = be16_to_cpu(po->num);
 	memcpy(path->encap.h_dest, po->pppoe_pa.remote, ETH_ALEN);
+	memcpy(ctx->daddr, po->pppoe_pa.remote, ETH_ALEN);
 	path->dev = ctx->dev;
 	ctx->dev = dev;
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eaf66e57d891..a87dbbd22cbb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -909,7 +909,7 @@ struct net_device_path_stack {
 
 struct net_device_path_ctx {
 	const struct net_device *dev;
-	const u8		*daddr;
+	u8			daddr[ETH_ALEN];
 
 	int			num_vlans;
 	struct {
diff --git a/net/core/dev.c b/net/core/dev.c
index c2d73595a7c3..0c5c020304a0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -682,11 +682,11 @@ int dev_fill_forward_path(const struct net_device *dev, const u8 *daddr,
 	const struct net_device *last_dev;
 	struct net_device_path_ctx ctx = {
 		.dev	= dev,
-		.daddr	= daddr,
 	};
 	struct net_device_path *path;
 	int ret = 0;
 
+	memcpy(ctx.daddr, daddr, sizeof(ctx.daddr));
 	stack->num_paths = 0;
 	while (ctx.dev && ctx.dev->netdev_ops->ndo_fill_forward_path) {
 		last_dev = ctx.dev;
-- 
2.35.1

[PATCH 4/4] netfilter: nft_flow_offload: fix offload with pppoe + vlan

[Felix Fietkau]

When running a combination of PPPoE on top of a VLAN, we need to set
info->outdev to the PPPoE device, otherwise PPPoE encap is skipped
during software offload.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/netfilter/nft_flow_offload.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index d88de26aad75..187b8cb9a510 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -123,7 +123,8 @@ static void nft_dev_path_info(const struct net_device_path_stack *stack,
 				info->indev = NULL;
 				break;
 			}
-			info->outdev = path->dev;
+			if (!info->outdev)
+				info->outdev = path->dev;
 			info->encap[info->num_encaps].id = path->encap.id;
 			info->encap[info->num_encaps].proto = path->encap.proto;
 			info->num_encaps++;
-- 
2.35.1

Re: [PATCH 2/4] netfilter: nft_flow_offload: skip dst neigh lookup for ppp devices

[Pablo Neira Ayuso]

Series LGTM.

Would you repost adding Fixes: tag and target nf tree?

Thanks.

On Fri, May 06, 2022 at 03:18:39PM +0200, Felix Fietkau wrote:
> The dst entry does not contain a valid hardware address, so skip the lookup
> in order to avoid running into errors here.
> The proper hardware address is filled in from nft_dev_path_info
> 
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>  net/netfilter/nft_flow_offload.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> index 900d48c810a1..d88de26aad75 100644
> --- a/net/netfilter/nft_flow_offload.c
> +++ b/net/netfilter/nft_flow_offload.c
> @@ -36,6 +36,15 @@ static void nft_default_forward_path(struct nf_flow_route *route,
>  	route->tuple[dir].xmit_type	= nft_xmit_type(dst_cache);
>  }
>  
> +static bool nft_is_valid_ether_device(const struct net_device *dev)
> +{
> +	if (!dev || (dev->flags & IFF_LOOPBACK) || dev->type != ARPHRD_ETHER ||
> +	    dev->addr_len != ETH_ALEN || !is_valid_ether_addr(dev->dev_addr))
> +		return false;
> +
> +	return true;
> +}
> +
>  static int nft_dev_fill_forward_path(const struct nf_flow_route *route,
>  				     const struct dst_entry *dst_cache,
>  				     const struct nf_conn *ct,
> @@ -47,6 +56,9 @@ static int nft_dev_fill_forward_path(const struct nf_flow_route *route,
>  	struct neighbour *n;
>  	u8 nud_state;
>  
> +	if (!nft_is_valid_ether_device(dev))
> +		goto out;
> +
>  	n = dst_neigh_lookup(dst_cache, daddr);
>  	if (!n)
>  		return -1;
> @@ -60,6 +72,7 @@ static int nft_dev_fill_forward_path(const struct nf_flow_route *route,
>  	if (!(nud_state & NUD_VALID))
>  		return -1;
>  
> +out:
>  	return dev_fill_forward_path(dev, ha, stack);
>  }
>  
> @@ -78,15 +91,6 @@ struct nft_forward_info {
>  	enum flow_offload_xmit_type xmit_type;
>  };
>  
> -static bool nft_is_valid_ether_device(const struct net_device *dev)
> -{
> -	if (!dev || (dev->flags & IFF_LOOPBACK) || dev->type != ARPHRD_ETHER ||
> -	    dev->addr_len != ETH_ALEN || !is_valid_ether_addr(dev->dev_addr))
> -		return false;
> -
> -	return true;
> -}
> -
>  static void nft_dev_path_info(const struct net_device_path_stack *stack,
>  			      struct nft_forward_info *info,
>  			      unsigned char *ha, struct nf_flowtable *flowtable)
> -- 
> 2.35.1
> 

Re: [PATCH 2/4] netfilter: nft_flow_offload: skip dst neigh lookup for ppp devices

[Felix Fietkau]

On 09.05.22 08:26, Pablo Neira Ayuso wrote:
> Series LGTM.
> 
> Would you repost adding Fixes: tag and target nf tree?
> 
> Thanks.

Sent. Please note that this will require a fixup when it gets merged 
into -next, since the mtk_ppe_offload code is affected by the 
ndo_fill_forward_path related api change.

- Felix