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
The 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]>
---
include/linux/skbuff.h | 9 +++++++++
include/trace/events/skb.h | 2 ++
net/core/neighbour.c | 4 ++--
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c310a4a8fc86..206b66f5ce6b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -393,6 +393,15 @@ enum skb_drop_reason {
* see the doc for disable_ipv6
* in ip-sysctl.rst for detail
*/
+ SKB_DROP_REASON_NEIGH_FAILED, /* dropped as the state of
+ * neighbour is NUD_FAILED
+ */
+ SKB_DROP_REASON_NEIGH_QUEUEFULL, /* the skbs that waiting
+ * for sending on the queue
+ * of neigh->arp_queue is
+ * full, and the skbs on the
+ * tail will be dropped
+ */
SKB_DROP_REASON_MAX,
};
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 47dedef7b6b8..dd06366ded4a 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -41,6 +41,8 @@
EM(SKB_DROP_REASON_BPF_CGROUP_EGRESS, \
BPF_CGROUP_EGRESS) \
EM(SKB_DROP_REASON_IPV6DSIABLED, IPV6DSIABLED) \
+ EM(SKB_DROP_REASON_NEIGH_FAILED, NEIGH_FAILED) \
+ EM(SKB_DROP_REASON_NEIGH_QUEUEFULL, NEIGH_QUEUEFULL) \
EMe(SKB_DROP_REASON_MAX, MAX)
#undef EM
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index ec0bf737b076..c353834e8fa9 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);
--
2.35.1
On 2/20/22 8:57 AM, [email protected] wrote:
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index c310a4a8fc86..206b66f5ce6b 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -393,6 +393,15 @@ enum skb_drop_reason {
> * see the doc for disable_ipv6
> * in ip-sysctl.rst for detail
> */
> + SKB_DROP_REASON_NEIGH_FAILED, /* dropped as the state of
> + * neighbour is NUD_FAILED
> + */
/* neigh entry in failed state */
> + SKB_DROP_REASON_NEIGH_QUEUEFULL, /* the skbs that waiting
> + * for sending on the queue
> + * of neigh->arp_queue is
> + * full, and the skbs on the
> + * tail will be dropped
> + */
/* arp_queue for neigh entry is full */
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index ec0bf737b076..c353834e8fa9 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);
what about out_dead: path? the tracepoint there shows that path is of
interest.
On Tue, Feb 22, 2022 at 11:17 AM David Ahern <[email protected]> wrote:
>
> On 2/20/22 8:57 AM, [email protected] wrote:
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index c310a4a8fc86..206b66f5ce6b 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -393,6 +393,15 @@ enum skb_drop_reason {
> > * see the doc for disable_ipv6
> > * in ip-sysctl.rst for detail
> > */
> > + SKB_DROP_REASON_NEIGH_FAILED, /* dropped as the state of
> > + * neighbour is NUD_FAILED
> > + */
>
> /* neigh entry in failed state */
>
> > + SKB_DROP_REASON_NEIGH_QUEUEFULL, /* the skbs that waiting
> > + * for sending on the queue
> > + * of neigh->arp_queue is
> > + * full, and the skbs on the
> > + * tail will be dropped
> > + */
>
> /* arp_queue for neigh entry is full */
>
>
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index ec0bf737b076..c353834e8fa9 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);
>
> what about out_dead: path? the tracepoint there shows that path is of
> interest.
You are right, that path should be considered too.