| Message ID | 20240815124302.982711-9-dongml2@chinatelecom.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: vxlan: add skb drop reasons support | expand |
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; > }
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; > > }
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,
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(-)