Re: [PATCH net-next 06/10] net: vxlan: add skb drop reasons to vxlan_rcv()
From: Ido Schimmel <idosch@nvidia.com>
Date: 2024-08-20 12:27:11
Also in:
lkml
On Thu, Aug 15, 2024 at 08:42:58PM +0800, Menglong Dong wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c index e971c4785962..9a61f04bb95d 100644 --- a/drivers/net/vxlan/vxlan_core.c +++ b/drivers/net/vxlan/vxlan_core.c@@ -1668,6 +1668,7 @@ static bool vxlan_ecn_decapsulate(struct vxlan_sock *vs, void *oiph, /* Callback from net/ipv4/udp.c to receive packets */ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) { + enum skb_drop_reason reason = pskb_may_pull_reason(skb, VXLAN_HLEN); struct vxlan_vni_node *vninode = NULL; struct vxlan_dev *vxlan; struct vxlan_sock *vs;@@ -1681,7 +1682,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) int nh; /* Need UDP and VXLAN header to be present */ - if (!pskb_may_pull(skb, VXLAN_HLEN)) + if (reason != SKB_NOT_DROPPED_YET) goto drop; unparsed = *vxlan_hdr(skb);@@ -1690,6 +1691,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) netdev_dbg(skb->dev, "invalid vxlan flags=%#x vni=%#x\n", ntohl(vxlan_hdr(skb)->vx_flags), ntohl(vxlan_hdr(skb)->vx_vni)); + reason = (u32)VXLAN_DROP_FLAGS;
I don't find "FLAGS" very descriptive. AFAICT the reason is used in these two cases: 1. "I flag" is not set 2. The reserved fields are not zero Maybe call it VXLAN_DROP_INVALID_HDR ? And I agree with the comment about documenting these drop reasons like in include/net/dropreason-core.h
quoted hunk ↗ jump to hunk
/* Return non vxlan pkt */ goto drop; }@@ -1703,8 +1705,10 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) vni = vxlan_vni(vxlan_hdr(skb)->vx_vni); vxlan = vxlan_vs_find_vni(vs, skb->dev->ifindex, vni, &vninode); - if (!vxlan) + if (!vxlan) { + reason = (u32)VXLAN_DROP_VNI;
Same comment here. Maybe VXLAN_DROP_VNI_NOT_FOUND ?
quoted hunk ↗ jump to hunk
goto drop; + } /* For backwards compatibility, only allow reserved fields to be * used by VXLAN extensions if explicitly requested.@@ -1717,12 +1721,16 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) } if (__iptunnel_pull_header(skb, VXLAN_HLEN, protocol, raw_proto, - !net_eq(vxlan->net, dev_net(vxlan->dev)))) + !net_eq(vxlan->net, dev_net(vxlan->dev)))) { + reason = SKB_DROP_REASON_NOMEM; goto drop; + } - if (vs->flags & VXLAN_F_REMCSUM_RX) - if (unlikely(!vxlan_remcsum(&unparsed, skb, vs->flags))) + if (vs->flags & VXLAN_F_REMCSUM_RX) { + reason = vxlan_remcsum(&unparsed, skb, vs->flags); + if (unlikely(reason != SKB_NOT_DROPPED_YET)) goto drop; + } if (vxlan_collect_metadata(vs)) { IP_TUNNEL_DECLARE_FLAGS(flags) = { };@@ -1732,8 +1740,10 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) tun_dst = udp_tun_rx_dst(skb, vxlan_get_sk_family(vs), flags, key32_to_tunnel_id(vni), sizeof(*md)); - if (!tun_dst) + if (!tun_dst) { + reason = SKB_DROP_REASON_NOMEM; goto drop; + } md = ip_tunnel_info_opts(&tun_dst->u.tun_info);@@ -1757,12 +1767,15 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) * is more robust and provides a little more security in * adding extensions to VXLAN. */ + reason = (u32)VXLAN_DROP_FLAGS; goto drop; } if (!raw_proto) { - if (!vxlan_set_mac(vxlan, vs, skb, vni)) + if (!vxlan_set_mac(vxlan, vs, skb, vni)) { + reason = (u32)VXLAN_DROP_MAC;
The function drops the packet for various reasons: 1. Source MAC is equal to the VXLAN device's MAC 2. Source MAC is invalid (all zeroes or multicast) 3. Trying to migrate a static entry or one pointing to a nexthop They are all quite obscure so it might be fine to fold them under the same reason, but I can't find a descriptive name. If you split 1-2 to one reason and 3 to another, then they can become VXLAN_DROP_INVALID_SMAC and VXLAN_DROP_ENTRY_EXISTS
goto drop; + }