[PATCH net-next v7 08/12] net: vxlan: use kfree_skb_reason() in vxlan_xmit()

Menglong Dong posted 12 patches 1 month, 2 weeks ago
[PATCH net-next v7 08/12] net: vxlan: use kfree_skb_reason() in vxlan_xmit()
Posted by Menglong Dong 1 month, 2 weeks ago
Replace kfree_skb() with kfree_skb_reason() in vxlan_xmit(). Following
new skb drop reasons are introduced for vxlan:

/* no remote found for xmit */
SKB_DROP_REASON_VXLAN_NO_REMOTE
/* packet without necessary metadata reached a device which is
 * in "external" mode
 */
SKB_DROP_REASON_TUNNEL_TXINFO

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
Reviewed-by: Simon Horman <horms@kernel.org>
---
v7:
- fix some typos in the document for SKB_DROP_REASON_TUNNEL_TXINFO
v6:
- fix some typos in the document for SKB_DROP_REASON_TUNNEL_TXINFO
v5:
- modify the document for SKB_DROP_REASON_TUNNEL_TXINFO
v2:
- move the drop reason "TXINFO" from vxlan to core
- rename VXLAN_DROP_REMOTE to VXLAN_DROP_NO_REMOTE
---
 drivers/net/vxlan/vxlan_core.c | 6 +++---
 include/net/dropreason-core.h  | 9 +++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 41191a28252a..b677ec901807 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -2730,7 +2730,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 			if (info && info->mode & IP_TUNNEL_INFO_TX)
 				vxlan_xmit_one(skb, dev, vni, NULL, false);
 			else
-				kfree_skb(skb);
+				kfree_skb_reason(skb, SKB_DROP_REASON_TUNNEL_TXINFO);
 			return NETDEV_TX_OK;
 		}
 	}
@@ -2793,7 +2793,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 			dev_core_stats_tx_dropped_inc(dev);
 			vxlan_vnifilter_count(vxlan, vni, NULL,
 					      VXLAN_VNI_STATS_TX_DROPS, 0);
-			kfree_skb(skb);
+			kfree_skb_reason(skb, SKB_DROP_REASON_VXLAN_NO_REMOTE);
 			return NETDEV_TX_OK;
 		}
 	}
@@ -2816,7 +2816,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 		if (fdst)
 			vxlan_xmit_one(skb, dev, vni, fdst, did_rsc);
 		else
-			kfree_skb(skb);
+			kfree_skb_reason(skb, SKB_DROP_REASON_VXLAN_NO_REMOTE);
 	}
 
 	return NETDEV_TX_OK;
diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index fbf92d442c1b..d59bb96c5a02 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -96,7 +96,9 @@
 	FN(VXLAN_VNI_NOT_FOUND)		\
 	FN(MAC_INVALID_SOURCE)		\
 	FN(VXLAN_ENTRY_EXISTS)		\
+	FN(VXLAN_NO_REMOTE)		\
 	FN(IP_TUNNEL_ECN)		\
+	FN(TUNNEL_TXINFO)		\
 	FN(LOCAL_MAC)			\
 	FNe(MAX)
 
@@ -439,11 +441,18 @@ enum skb_drop_reason {
 	 * entry or an entry pointing to a nexthop.
 	 */
 	SKB_DROP_REASON_VXLAN_ENTRY_EXISTS,
+	/** @SKB_DROP_REASON_VXLAN_NO_REMOTE: no remote found for xmit */
+	SKB_DROP_REASON_VXLAN_NO_REMOTE,
 	/**
 	 * @SKB_DROP_REASON_IP_TUNNEL_ECN: skb is dropped according to
 	 * RFC 6040 4.2, see __INET_ECN_decapsulate() for detail.
 	 */
 	SKB_DROP_REASON_IP_TUNNEL_ECN,
+	/**
+	 * @SKB_DROP_REASON_TUNNEL_TXINFO: packet without necessary metadata
+	 * reached a device which is in "external" mode.
+	 */
+	SKB_DROP_REASON_TUNNEL_TXINFO,
 	/**
 	 * @SKB_DROP_REASON_LOCAL_MAC: the source MAC address is equal to
 	 * the MAC address of the local netdev.
-- 
2.39.5
Re: [PATCH net-next v7 08/12] net: vxlan: use kfree_skb_reason() in vxlan_xmit()
Posted by Ido Schimmel 1 month, 2 weeks ago
On Wed, Oct 09, 2024 at 10:28:26AM +0800, Menglong Dong wrote:
> Replace kfree_skb() with kfree_skb_reason() in vxlan_xmit(). Following
> new skb drop reasons are introduced for vxlan:
> 
> /* no remote found for xmit */
> SKB_DROP_REASON_VXLAN_NO_REMOTE
> /* packet without necessary metadata reached a device which is
>  * in "external" mode
>  */
> SKB_DROP_REASON_TUNNEL_TXINFO
> 
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> Reviewed-by: Simon Horman <horms@kernel.org>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

The first reason might be useful for the bridge driver as well when
there are no ports to forward the packet to (because of egress filtering
for example), but we can make it more generic if / when the bridge
driver is annotated.
Re: [PATCH net-next v7 08/12] net: vxlan: use kfree_skb_reason() in vxlan_xmit()
Posted by Menglong Dong 1 month, 2 weeks ago
On Sun, Oct 13, 2024 at 8:43 PM Ido Schimmel <idosch@nvidia.com> wrote:
>
> On Wed, Oct 09, 2024 at 10:28:26AM +0800, Menglong Dong wrote:
> > Replace kfree_skb() with kfree_skb_reason() in vxlan_xmit(). Following
> > new skb drop reasons are introduced for vxlan:
> >
> > /* no remote found for xmit */
> > SKB_DROP_REASON_VXLAN_NO_REMOTE
> > /* packet without necessary metadata reached a device which is
> >  * in "external" mode
> >  */
> > SKB_DROP_REASON_TUNNEL_TXINFO
> >
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > Reviewed-by: Simon Horman <horms@kernel.org>
>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
>
> The first reason might be useful for the bridge driver as well when
> there are no ports to forward the packet to (because of egress filtering
> for example), but we can make it more generic if / when the bridge
> driver is annotated.

You are right. As we already need a new version, so we can
do something for this patch too. As you said, maybe we can rename the
reason VXLAN_NO_REMOTE to NO_REMOTE for more generic
usage?
Re: [PATCH net-next v7 08/12] net: vxlan: use kfree_skb_reason() in vxlan_xmit()
Posted by Ido Schimmel 1 month, 2 weeks ago
On Mon, Oct 14, 2024 at 08:35:57PM +0800, Menglong Dong wrote:
> On Sun, Oct 13, 2024 at 8:43 PM Ido Schimmel <idosch@nvidia.com> wrote:
> >
> > On Wed, Oct 09, 2024 at 10:28:26AM +0800, Menglong Dong wrote:
> > > Replace kfree_skb() with kfree_skb_reason() in vxlan_xmit(). Following
> > > new skb drop reasons are introduced for vxlan:
> > >
> > > /* no remote found for xmit */
> > > SKB_DROP_REASON_VXLAN_NO_REMOTE
> > > /* packet without necessary metadata reached a device which is
> > >  * in "external" mode
> > >  */
> > > SKB_DROP_REASON_TUNNEL_TXINFO
> > >
> > > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > > Reviewed-by: Simon Horman <horms@kernel.org>
> >
> > Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> >
> > The first reason might be useful for the bridge driver as well when
> > there are no ports to forward the packet to (because of egress filtering
> > for example), but we can make it more generic if / when the bridge
> > driver is annotated.
> 
> You are right. As we already need a new version, so we can
> do something for this patch too. As you said, maybe we can rename the
> reason VXLAN_NO_REMOTE to NO_REMOTE for more generic
> usage?

"NO_REMOTE" is not really applicable to the bridge driver as there are
no remotes, but bridge ports. I'm fine with keeping it as is for now and
changing it later if / when needed.
Re: [PATCH net-next v7 08/12] net: vxlan: use kfree_skb_reason() in vxlan_xmit()
Posted by Menglong Dong 1 month, 1 week ago
On Mon, Oct 14, 2024 at 11:47 PM Ido Schimmel <idosch@nvidia.com> wrote:
>
> On Mon, Oct 14, 2024 at 08:35:57PM +0800, Menglong Dong wrote:
> > On Sun, Oct 13, 2024 at 8:43 PM Ido Schimmel <idosch@nvidia.com> wrote:
> > >
> > > On Wed, Oct 09, 2024 at 10:28:26AM +0800, Menglong Dong wrote:
> > > > Replace kfree_skb() with kfree_skb_reason() in vxlan_xmit(). Following
> > > > new skb drop reasons are introduced for vxlan:
> > > >
> > > > /* no remote found for xmit */
> > > > SKB_DROP_REASON_VXLAN_NO_REMOTE
> > > > /* packet without necessary metadata reached a device which is
> > > >  * in "external" mode
> > > >  */
> > > > SKB_DROP_REASON_TUNNEL_TXINFO
> > > >
> > > > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > > > Reviewed-by: Simon Horman <horms@kernel.org>
> > >
> > > Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> > >
> > > The first reason might be useful for the bridge driver as well when
> > > there are no ports to forward the packet to (because of egress filtering
> > > for example), but we can make it more generic if / when the bridge
> > > driver is annotated.
> >
> > You are right. As we already need a new version, so we can
> > do something for this patch too. As you said, maybe we can rename the
> > reason VXLAN_NO_REMOTE to NO_REMOTE for more generic
> > usage?
>
> "NO_REMOTE" is not really applicable to the bridge driver as there are
> no remotes, but bridge ports. I'm fine with keeping it as is for now and
> changing it later if / when needed.

Okay!