| Message ID | 20241009022830.83949-9-dongml2@chinatelecom.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: vxlan: add skb drop reasons support | expand |
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.
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?
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.
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!
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.