2022-02-25 08:47:00

by Menglong Dong

[permalink] [raw]
Subject: [PATCH net-next v2 0/3] net: use kfree_skb_reason() for ip/neighbour

From: Menglong Dong <[email protected]>

In the series "net: use kfree_skb_reason() for ip/udp packet receive",
reasons for skb drops are added to the packet receive process of IP
layer. Link:

https://lore.kernel.org/netdev/[email protected]/

And in the first patch of this series, skb drop reasons are added to
the packet egress path of IP layer. As kfree_skb() is not used frequent,
I commit these changes at once and didn't create a patch for every
functions that involed. Following functions are handled:

__ip_queue_xmit()
ip_finish_output()
ip_mc_finish_output()
ip6_output()
ip6_finish_output()
ip6_finish_output2()

Following new drop reasons are introduced (what they mean can be seen
in the document of them):

SKB_DROP_REASON_IP_OUTNOROUTES
SKB_DROP_REASON_BPF_CGROUP_EGRESS
SKB_DROP_REASON_IPV6DSIABLED
SKB_DROP_REASON_NEIGH_CREATEFAIL

In the 2th and 3th patches, kfree_skb_reason() is used in neighbour
subsystem instead of kfree_skb(). __neigh_event_send() and
arp_error_report() are involed, and following new drop reasons are
introduced:

SKB_DROP_REASON_NEIGH_FAILED
SKB_DROP_REASON_NEIGH_QUEUEFULL
SKB_DROP_REASON_NEIGH_DEAD

Changes since v1:
- introduce SKB_DROP_REASON_NEIGH_CREATEFAIL for some path in the 1th
patch
- introduce SKB_DROP_REASON_NEIGH_DEAD in the 2th patch
- simplify the document for the new drop reasons, as David Ahern
suggested

Menglong Dong (3):
net: ip: add skb drop reasons for ip egress path
net: neigh: use kfree_skb_reason() for __neigh_event_send()
net: neigh: add skb drop reasons to arp_error_report()

include/linux/skbuff.h | 14 ++++++++++++++
include/trace/events/skb.h | 7 +++++++
net/core/neighbour.c | 6 +++---
net/ipv4/arp.c | 2 +-
net/ipv4/ip_output.c | 8 ++++----
net/ipv6/ip6_output.c | 6 +++---
6 files changed, 32 insertions(+), 11 deletions(-)

--
2.35.1


2022-02-25 08:51:57

by Menglong Dong

[permalink] [raw]
Subject: [PATCH net-next v2 1/3] net: ip: add skb drop reasons for ip egress path

From: Menglong Dong <[email protected]>

Replace kfree_skb() which is used in the packet egress path of IP layer
with kfree_skb_reason(). Functions that are involved include:

__ip_queue_xmit()
ip_finish_output()
ip_mc_finish_output()
ip6_output()
ip6_finish_output()
ip6_finish_output2()

Following new drop reasons are introduced:

SKB_DROP_REASON_IP_OUTNOROUTES
SKB_DROP_REASON_BPF_CGROUP_EGRESS
SKB_DROP_REASON_IPV6DSIABLED
SKB_DROP_REASON_NEIGH_CREATEFAIL

Reviewed-by: Mengen Sun <[email protected]>
Reviewed-by: Hao Peng <[email protected]>
Signed-off-by: Menglong Dong <[email protected]>
---
v2:
- introduce SKB_DROP_REASON_NEIGH_CREATEFAIL for neigh entry create
failure
- replace SKB_DROP_REASON_IP_OUTNOROUTES with
SKB_DROP_REASON_NEIGH_CREATEFAIL in ip6_finish_output2(), as I think
it's more suitable
---
include/linux/skbuff.h | 9 +++++++++
include/trace/events/skb.h | 5 +++++
net/ipv4/ip_output.c | 8 ++++----
net/ipv6/ip6_output.c | 6 +++---
4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a3e90efe6586..c99b944dc712 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -380,6 +380,15 @@ enum skb_drop_reason {
* the ofo queue, corresponding to
* LINUX_MIB_TCPOFOMERGE
*/
+ SKB_DROP_REASON_IP_OUTNOROUTES, /* route lookup failed */
+ SKB_DROP_REASON_BPF_CGROUP_EGRESS, /* dropped by
+ * BPF_PROG_TYPE_CGROUP_SKB
+ * eBPF program
+ */
+ SKB_DROP_REASON_IPV6DSIABLED, /* IPv6 is disabled on the device */
+ SKB_DROP_REASON_NEIGH_CREATEFAIL, /* failed to create neigh
+ * entry
+ */
SKB_DROP_REASON_MAX,
};

diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 2ab7193313aa..38a6e4e3ff9a 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -37,6 +37,11 @@
EM(SKB_DROP_REASON_TCP_OLD_DATA, TCP_OLD_DATA) \
EM(SKB_DROP_REASON_TCP_OVERWINDOW, TCP_OVERWINDOW) \
EM(SKB_DROP_REASON_TCP_OFOMERGE, TCP_OFOMERGE) \
+ EM(SKB_DROP_REASON_IP_OUTNOROUTES, IP_OUTNOROUTES) \
+ EM(SKB_DROP_REASON_BPF_CGROUP_EGRESS, \
+ BPF_CGROUP_EGRESS) \
+ EM(SKB_DROP_REASON_IPV6DSIABLED, IPV6DSIABLED) \
+ EM(SKB_DROP_REASON_NEIGH_CREATEFAIL, NEIGH_CREATEFAIL) \
EMe(SKB_DROP_REASON_MAX, MAX)

#undef EM
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 0c0574eb5f5b..7f618f72fb42 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -233,7 +233,7 @@ static int ip_finish_output2(struct net *net, struct sock *sk, struct sk_buff *s

net_dbg_ratelimited("%s: No header cache and no neighbour!\n",
__func__);
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_NEIGH_CREATEFAIL);
return -EINVAL;
}

@@ -317,7 +317,7 @@ static int ip_finish_output(struct net *net, struct sock *sk, struct sk_buff *sk
case NET_XMIT_CN:
return __ip_finish_output(net, sk, skb) ? : ret;
default:
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_BPF_CGROUP_EGRESS);
return ret;
}
}
@@ -337,7 +337,7 @@ static int ip_mc_finish_output(struct net *net, struct sock *sk,
case NET_XMIT_SUCCESS:
break;
default:
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_BPF_CGROUP_EGRESS);
return ret;
}

@@ -536,7 +536,7 @@ int __ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
no_route:
rcu_read_unlock();
IP_INC_STATS(net, IPSTATS_MIB_OUTNOROUTES);
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_IP_OUTNOROUTES);
return -EHOSTUNREACH;
}
EXPORT_SYMBOL(__ip_queue_xmit);
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 3286b64ec03d..ac40ce464c64 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -130,7 +130,7 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
rcu_read_unlock_bh();

IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTNOROUTES);
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_NEIGH_CREATEFAIL);
return -EINVAL;
}

@@ -202,7 +202,7 @@ static int ip6_finish_output(struct net *net, struct sock *sk, struct sk_buff *s
case NET_XMIT_CN:
return __ip6_finish_output(net, sk, skb) ? : ret;
default:
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_BPF_CGROUP_EGRESS);
return ret;
}
}
@@ -217,7 +217,7 @@ int ip6_output(struct net *net, struct sock *sk, struct sk_buff *skb)

if (unlikely(idev->cnf.disable_ipv6)) {
IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_IPV6DSIABLED);
return 0;
}

--
2.35.1

2022-02-25 09:41:34

by Menglong Dong

[permalink] [raw]
Subject: [PATCH net-next v2 3/3] net: neigh: add skb drop reasons to arp_error_report()

From: Menglong Dong <[email protected]>

When neighbour become invalid or destroyed, neigh_invalidate() will be
called. neigh->ops->error_report() will be called if the neighbour's
state is NUD_FAILED, and seems here is the only use of error_report().
So we can tell that the reason of skb drops in arp_error_report() is
SKB_DROP_REASON_NEIGH_FAILED.

Replace kfree_skb() used in arp_error_report() with kfree_skb_reason().

Reviewed-by: Mengen Sun <[email protected]>
Reviewed-by: Hao Peng <[email protected]>
Signed-off-by: Menglong Dong <[email protected]>
Reviewed-by: David Ahern <[email protected]>
---
net/ipv4/arp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 4db0325f6e1a..8e4ca4738c43 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -293,7 +293,7 @@ static int arp_constructor(struct neighbour *neigh)
static void arp_error_report(struct neighbour *neigh, struct sk_buff *skb)
{
dst_link_failure(skb);
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_NEIGH_FAILED);
}

/* Create and send an arp packet. */
--
2.35.1

2022-02-25 12:42:02

by Menglong Dong

[permalink] [raw]
Subject: [PATCH net-next v2 2/3] net: neigh: use kfree_skb_reason() for __neigh_event_send()

From: Menglong Dong <[email protected]>

Replace kfree_skb() used in __neigh_event_send() with
kfree_skb_reason(). Following drop reasons are added:

SKB_DROP_REASON_NEIGH_FAILED
SKB_DROP_REASON_NEIGH_QUEUEFULL
SKB_DROP_REASON_NEIGH_DEAD

The first two reasons above should be the hot path that skb drops
in neighbour subsystem.

Reviewed-by: Mengen Sun <[email protected]>
Reviewed-by: Hao Peng <[email protected]>
Signed-off-by: Menglong Dong <[email protected]>
---
v2:
- introduce the new drop reason 'SKB_DROP_REASON_NEIGH_DEAD'
- simplify the document for the new drop reasons
---
include/linux/skbuff.h | 5 +++++
include/trace/events/skb.h | 3 +++
net/core/neighbour.c | 6 +++---
3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c99b944dc712..5559ddeda728 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -389,6 +389,11 @@ enum skb_drop_reason {
SKB_DROP_REASON_NEIGH_CREATEFAIL, /* failed to create neigh
* entry
*/
+ SKB_DROP_REASON_NEIGH_FAILED, /* neigh entry in failed state */
+ SKB_DROP_REASON_NEIGH_QUEUEFULL, /* arp_queue for neigh
+ * entry is full
+ */
+ SKB_DROP_REASON_NEIGH_DEAD, /* neigh entry is dead */
SKB_DROP_REASON_MAX,
};

diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 38a6e4e3ff9a..d647f519f900 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -42,6 +42,9 @@
BPF_CGROUP_EGRESS) \
EM(SKB_DROP_REASON_IPV6DSIABLED, IPV6DSIABLED) \
EM(SKB_DROP_REASON_NEIGH_CREATEFAIL, NEIGH_CREATEFAIL) \
+ EM(SKB_DROP_REASON_NEIGH_FAILED, NEIGH_FAILED) \
+ EM(SKB_DROP_REASON_NEIGH_QUEUEFULL, NEIGH_QUEUEFULL) \
+ EM(SKB_DROP_REASON_NEIGH_DEAD, NEIGH_DEAD) \
EMe(SKB_DROP_REASON_MAX, MAX)

#undef EM
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index ec0bf737b076..f64ebd050f6c 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1171,7 +1171,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb,
neigh->updated = jiffies;
write_unlock_bh(&neigh->lock);

- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_NEIGH_FAILED);
return 1;
}
} else if (neigh->nud_state & NUD_STALE) {
@@ -1193,7 +1193,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb,
if (!buff)
break;
neigh->arp_queue_len_bytes -= buff->truesize;
- kfree_skb(buff);
+ kfree_skb_reason(buff, SKB_DROP_REASON_NEIGH_QUEUEFULL);
NEIGH_CACHE_STAT_INC(neigh->tbl, unres_discards);
}
skb_dst_force(skb);
@@ -1215,7 +1215,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb,
if (neigh->nud_state & NUD_STALE)
goto out_unlock_bh;
write_unlock_bh(&neigh->lock);
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_NEIGH_DEAD);
trace_neigh_event_send_dead(neigh, 1);
return 1;
}
--
2.35.1

2022-02-25 21:30:12

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH net-next v2 1/3] net: ip: add skb drop reasons for ip egress path

On 2/25/22 12:17 AM, [email protected] wrote:
> From: Menglong Dong <[email protected]>
>
> Replace kfree_skb() which is used in the packet egress path of IP layer
> with kfree_skb_reason(). Functions that are involved include:
>
> __ip_queue_xmit()
> ip_finish_output()
> ip_mc_finish_output()
> ip6_output()
> ip6_finish_output()
> ip6_finish_output2()
>
> Following new drop reasons are introduced:
>
> SKB_DROP_REASON_IP_OUTNOROUTES
> SKB_DROP_REASON_BPF_CGROUP_EGRESS
> SKB_DROP_REASON_IPV6DSIABLED

A new version is needed to fix the typo Roman noticed; logic wise:

Reviewed-by: David Ahern <[email protected]>

2022-02-26 01:27:59

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH net-next v2 2/3] net: neigh: use kfree_skb_reason() for __neigh_event_send()

On 2/25/22 12:17 AM, [email protected] wrote:
> From: Menglong Dong <[email protected]>
>
> Replace kfree_skb() used in __neigh_event_send() with
> kfree_skb_reason(). Following drop reasons are added:
>
> SKB_DROP_REASON_NEIGH_FAILED
> SKB_DROP_REASON_NEIGH_QUEUEFULL
> SKB_DROP_REASON_NEIGH_DEAD
>
> The first two reasons above should be the hot path that skb drops
> in neighbour subsystem.
>
> Reviewed-by: Mengen Sun <[email protected]>
> Reviewed-by: Hao Peng <[email protected]>
> Signed-off-by: Menglong Dong <[email protected]>
> ---
> v2:
> - introduce the new drop reason 'SKB_DROP_REASON_NEIGH_DEAD'
> - simplify the document for the new drop reasons
> ---
> include/linux/skbuff.h | 5 +++++
> include/trace/events/skb.h | 3 +++
> net/core/neighbour.c | 6 +++---
> 3 files changed, 11 insertions(+), 3 deletions(-)
>

Reviewed-by: David Ahern <[email protected]>