[PATCH net-next 08/10] net: vxlan: add drop reasons support to vxlan_xmit_one()

Menglong Dong posted 10 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH net-next 08/10] net: vxlan: add drop reasons support to vxlan_xmit_one()
Posted by Menglong Dong 1 year, 5 months ago
Replace kfree_skb/dev_kfree_skb with kfree_skb_reason in vxlan_xmit_one.
The new skb drop reason "VXLAN_DROP_REMOTE_IP" is introduced, which is
for a invalid remote ip.

The only concern of mine is replacing dev_kfree_skb with
kfree_skb_reason. The dev_kfree_skb is equal to consume_skb, and I'm not
sure if we can change it to kfree_skb here. In my option, the skb is
"dropped" here, isn't it?

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 drivers/net/vxlan/drop.h       |  1 +
 drivers/net/vxlan/vxlan_core.c | 18 ++++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/vxlan/drop.h b/drivers/net/vxlan/drop.h
index da30cb4a9ed9..542f391b1273 100644
--- a/drivers/net/vxlan/drop.h
+++ b/drivers/net/vxlan/drop.h
@@ -14,6 +14,7 @@
 	R(VXLAN_DROP_MAC)			\
 	R(VXLAN_DROP_TXINFO)			\
 	R(VXLAN_DROP_REMOTE)			\
+	R(VXLAN_DROP_REMOTE_IP)			\
 	/* deliberate comment for trailing \ */
 
 enum vxlan_drop_reason {
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 22e2bf532ac3..c1bae120727f 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -2375,6 +2375,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	bool xnet = !net_eq(vxlan->net, dev_net(vxlan->dev));
 	bool no_eth_encap;
 	__be32 vni = 0;
+	SKB_DR(reason);
 
 	no_eth_encap = flags & VXLAN_F_GPE && skb->protocol != htons(ETH_P_TEB);
 	if (!skb_vlan_inet_prepare(skb, no_eth_encap))
@@ -2396,6 +2397,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 						   default_vni, true);
 				return;
 			}
+			reason = (u32)VXLAN_DROP_REMOTE_IP;
 			goto drop;
 		}
 
@@ -2483,6 +2485,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 					   tos, use_cache ? dst_cache : NULL);
 		if (IS_ERR(rt)) {
 			err = PTR_ERR(rt);
+			reason = SKB_DROP_REASON_IP_OUTNOROUTES;
 			goto tx_error;
 		}
 
@@ -2534,8 +2537,10 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		ttl = ttl ? : ip4_dst_hoplimit(&rt->dst);
 		err = vxlan_build_skb(skb, ndst, sizeof(struct iphdr),
 				      vni, md, flags, udp_sum);
-		if (err < 0)
+		if (err < 0) {
+			reason = SKB_DROP_REASON_NOMEM;
 			goto tx_error;
+		}
 
 		udp_tunnel_xmit_skb(rt, sock4->sock->sk, skb, saddr,
 				    pkey->u.ipv4.dst, tos, ttl, df,
@@ -2555,6 +2560,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		if (IS_ERR(ndst)) {
 			err = PTR_ERR(ndst);
 			ndst = NULL;
+			reason = SKB_DROP_REASON_IP_OUTNOROUTES;
 			goto tx_error;
 		}
 
@@ -2595,8 +2601,10 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		skb_scrub_packet(skb, xnet);
 		err = vxlan_build_skb(skb, ndst, sizeof(struct ipv6hdr),
 				      vni, md, flags, udp_sum);
-		if (err < 0)
+		if (err < 0) {
+			reason = SKB_DROP_REASON_NOMEM;
 			goto tx_error;
+		}
 
 		udp_tunnel6_xmit_skb(ndst, sock6->sock->sk, skb, dev,
 				     &saddr, &pkey->u.ipv6.dst, tos, ttl,
@@ -2611,7 +2619,8 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 drop:
 	dev_core_stats_tx_dropped_inc(dev);
 	vxlan_vnifilter_count(vxlan, vni, NULL, VXLAN_VNI_STATS_TX_DROPS, 0);
-	dev_kfree_skb(skb);
+	SKB_DR_RESET(reason);
+	kfree_skb_reason(skb, reason);
 	return;
 
 tx_error:
@@ -2623,7 +2632,8 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	dst_release(ndst);
 	DEV_STATS_INC(dev, tx_errors);
 	vxlan_vnifilter_count(vxlan, vni, NULL, VXLAN_VNI_STATS_TX_ERRORS, 0);
-	kfree_skb(skb);
+	SKB_DR_RESET(reason);
+	kfree_skb_reason(skb, reason);
 }
 
 static void vxlan_xmit_nh(struct sk_buff *skb, struct net_device *dev,
-- 
2.39.2
Re: [PATCH net-next 08/10] net: vxlan: add drop reasons support to vxlan_xmit_one()
Posted by Ido Schimmel 1 year, 5 months ago
On Thu, Aug 15, 2024 at 08:43:00PM +0800, Menglong Dong wrote:
> diff --git a/drivers/net/vxlan/drop.h b/drivers/net/vxlan/drop.h
> index da30cb4a9ed9..542f391b1273 100644
> --- a/drivers/net/vxlan/drop.h
> +++ b/drivers/net/vxlan/drop.h
> @@ -14,6 +14,7 @@
>  	R(VXLAN_DROP_MAC)			\
>  	R(VXLAN_DROP_TXINFO)			\
>  	R(VXLAN_DROP_REMOTE)			\
> +	R(VXLAN_DROP_REMOTE_IP)			\
>  	/* deliberate comment for trailing \ */
>  
>  enum vxlan_drop_reason {
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 22e2bf532ac3..c1bae120727f 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -2375,6 +2375,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
>  	bool xnet = !net_eq(vxlan->net, dev_net(vxlan->dev));
>  	bool no_eth_encap;
>  	__be32 vni = 0;
> +	SKB_DR(reason);
>  
>  	no_eth_encap = flags & VXLAN_F_GPE && skb->protocol != htons(ETH_P_TEB);
>  	if (!skb_vlan_inet_prepare(skb, no_eth_encap))
> @@ -2396,6 +2397,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
>  						   default_vni, true);
>  				return;
>  			}
> +			reason = (u32)VXLAN_DROP_REMOTE_IP;

This looks quite obscure to me. I didn't know you can add 0.0.0.0 as
remote and I'm not sure what is the use case. Personally I wouldn't
bother with this reason.

>  			goto drop;
>  		}
Re: [PATCH net-next 08/10] net: vxlan: add drop reasons support to vxlan_xmit_one()
Posted by Menglong Dong 1 year, 5 months ago
On Tue, Aug 20, 2024 at 8:33 PM Ido Schimmel <idosch@nvidia.com> wrote:
>
> On Thu, Aug 15, 2024 at 08:43:00PM +0800, Menglong Dong wrote:
> > diff --git a/drivers/net/vxlan/drop.h b/drivers/net/vxlan/drop.h
> > index da30cb4a9ed9..542f391b1273 100644
> > --- a/drivers/net/vxlan/drop.h
> > +++ b/drivers/net/vxlan/drop.h
> > @@ -14,6 +14,7 @@
> >       R(VXLAN_DROP_MAC)                       \
> >       R(VXLAN_DROP_TXINFO)                    \
> >       R(VXLAN_DROP_REMOTE)                    \
> > +     R(VXLAN_DROP_REMOTE_IP)                 \
> >       /* deliberate comment for trailing \ */
> >
> >  enum vxlan_drop_reason {
> > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> > index 22e2bf532ac3..c1bae120727f 100644
> > --- a/drivers/net/vxlan/vxlan_core.c
> > +++ b/drivers/net/vxlan/vxlan_core.c
> > @@ -2375,6 +2375,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
> >       bool xnet = !net_eq(vxlan->net, dev_net(vxlan->dev));
> >       bool no_eth_encap;
> >       __be32 vni = 0;
> > +     SKB_DR(reason);
> >
> >       no_eth_encap = flags & VXLAN_F_GPE && skb->protocol != htons(ETH_P_TEB);
> >       if (!skb_vlan_inet_prepare(skb, no_eth_encap))
> > @@ -2396,6 +2397,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
> >                                                  default_vni, true);
> >                               return;
> >                       }
> > +                     reason = (u32)VXLAN_DROP_REMOTE_IP;
>
> This looks quite obscure to me. I didn't know you can add 0.0.0.0 as
> remote and I'm not sure what is the use case. Personally I wouldn't
> bother with this reason.
>

I know. I'm hesitant about this commit, as I don't find a case
for this dropping, and the remote seems can't be 0.0.0.0.
I add this reason as the code drops the packet here.

Enn...I will abandon this commit.

Thanks!
Menglong Dong

> >                       goto drop;
> >               }