diff mbox series

[net-next,4/7] net: dev: use kfree_skb_reason() for enqueue_to_backlog()

Message ID 20220303174707.40431-5-imagedong@tencent.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dev: add skb drop reasons to net/core/dev.c | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5978 this patch: 5978
netdev/cc_maintainers warning 5 maintainers not CCed: andrii@kernel.org kpsingh@kernel.org kafai@fb.com songliubraving@fb.com yhs@fb.com
netdev/build_clang success Errors and warnings before: 878 this patch: 878
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 6129 this patch: 6129
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 47 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Menglong Dong March 3, 2022, 5:47 p.m. UTC 
From: Menglong Dong <imagedong@tencent.com>

Replace kfree_skb() used in enqueue_to_backlog() with
kfree_skb_reason(). The skb rop reason SKB_DROP_REASON_CPU_BACKLOG is
introduced for the case of failing to enqueue the skb to the per CPU
backlog queue. The further reason can be backlog queue full or RPS
flow limition, and I think we meedn't to make further distinctions.

Signed-off-by: Menglong Dong <imagedong@tencent.com>
---
 include/linux/skbuff.h     | 6 ++++++
 include/trace/events/skb.h | 1 +
 net/core/dev.c             | 6 +++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

Comments

Eric Dumazet March 3, 2022, 5:59 p.m. UTC | #1 
On Thu, Mar 3, 2022 at 9:48 AM <menglong8.dong@gmail.com> wrote:
>
> From: Menglong Dong <imagedong@tencent.com>
>
> Replace kfree_skb() used in enqueue_to_backlog() with
> kfree_skb_reason(). The skb rop reason SKB_DROP_REASON_CPU_BACKLOG is
> introduced for the case of failing to enqueue the skb to the per CPU
> backlog queue. The further reason can be backlog queue full or RPS
> flow limition, and I think we meedn't to make further distinctions.
>
> Signed-off-by: Menglong Dong <imagedong@tencent.com>
> ---
>  include/linux/skbuff.h     | 6 ++++++
>  include/trace/events/skb.h | 1 +
>  net/core/dev.c             | 6 +++++-
>  3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 62f9d15ec6ec..d2cf87ff84c2 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -402,6 +402,12 @@ enum skb_drop_reason {
>                                          * outputting (failed to enqueue to
>                                          * current qdisc)
>                                          */
> +       SKB_DROP_REASON_CPU_BACKLOG,    /* failed to enqueue the skb to
> +                                        * the per CPU backlog queue. This
> +                                        * can be caused by backlog queue
> +                                        * full (see netdev_max_backlog in
> +                                        * net.rst) or RPS flow limit
> +                                        */
>         SKB_DROP_REASON_MAX,
>  };
>
> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
> index 80fe15d175e3..29c360b5e114 100644
> --- a/include/trace/events/skb.h
> +++ b/include/trace/events/skb.h
> @@ -47,6 +47,7 @@
>         EM(SKB_DROP_REASON_NEIGH_DEAD, NEIGH_DEAD)              \
>         EM(SKB_DROP_REASON_QDISC_EGRESS, QDISC_EGRESS)          \
>         EM(SKB_DROP_REASON_QDISC_DROP, QDISC_DROP)              \
> +       EM(SKB_DROP_REASON_CPU_BACKLOG, CPU_BACKLOG)            \
>         EMe(SKB_DROP_REASON_MAX, MAX)
>
>  #undef EM
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 3280ba2502cd..373fa7a33ffa 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4541,10 +4541,12 @@ static bool skb_flow_limit(struct sk_buff *skb, unsigned int qlen)
>  static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
>                               unsigned int *qtail)
>  {
> +       enum skb_drop_reason reason;
>         struct softnet_data *sd;
>         unsigned long flags;
>         unsigned int qlen;
>
> +       reason = SKB_DROP_REASON_NOT_SPECIFIED;
>         sd = &per_cpu(softnet_data, cpu);
>
>         rps_lock_irqsave(sd, &flags);
> @@ -4566,6 +4568,8 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
>                 if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd->backlog.state))
>                         napi_schedule_rps(sd);
>                 goto enqueue;
> +       } else {

No need for an else {} after a goto  xxx;


> +               reason = SKB_DROP_REASON_CPU_BACKLOG;
>         }
>
>  drop:
> @@ -4573,7 +4577,7 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
>         rps_unlock_irq_restore(sd, &flags);
>
>         atomic_long_inc(&skb->dev->rx_dropped);
> -       kfree_skb(skb);
> +       kfree_skb_reason(skb, reason);
>         return NET_RX_DROP;
>  }
>
> --
> 2.35.1
>
Menglong Dong March 4, 2022, 4:40 a.m. UTC | #2 
On Fri, Mar 4, 2022 at 1:59 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Mar 3, 2022 at 9:48 AM <menglong8.dong@gmail.com> wrote:
> >
> > From: Menglong Dong <imagedong@tencent.com>
> >
> > Replace kfree_skb() used in enqueue_to_backlog() with
> > kfree_skb_reason(). The skb rop reason SKB_DROP_REASON_CPU_BACKLOG is
> > introduced for the case of failing to enqueue the skb to the per CPU
> > backlog queue. The further reason can be backlog queue full or RPS
> > flow limition, and I think we meedn't to make further distinctions.
> >
> > Signed-off-by: Menglong Dong <imagedong@tencent.com>
> > ---
> >  include/linux/skbuff.h     | 6 ++++++
> >  include/trace/events/skb.h | 1 +
> >  net/core/dev.c             | 6 +++++-
> >  3 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 62f9d15ec6ec..d2cf87ff84c2 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -402,6 +402,12 @@ enum skb_drop_reason {
> >                                          * outputting (failed to enqueue to
> >                                          * current qdisc)
> >                                          */
> > +       SKB_DROP_REASON_CPU_BACKLOG,    /* failed to enqueue the skb to
> > +                                        * the per CPU backlog queue. This
> > +                                        * can be caused by backlog queue
> > +                                        * full (see netdev_max_backlog in
> > +                                        * net.rst) or RPS flow limit
> > +                                        */
> >         SKB_DROP_REASON_MAX,
> >  };
> >
> > diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
> > index 80fe15d175e3..29c360b5e114 100644
> > --- a/include/trace/events/skb.h
> > +++ b/include/trace/events/skb.h
> > @@ -47,6 +47,7 @@
> >         EM(SKB_DROP_REASON_NEIGH_DEAD, NEIGH_DEAD)              \
> >         EM(SKB_DROP_REASON_QDISC_EGRESS, QDISC_EGRESS)          \
> >         EM(SKB_DROP_REASON_QDISC_DROP, QDISC_DROP)              \
> > +       EM(SKB_DROP_REASON_CPU_BACKLOG, CPU_BACKLOG)            \
> >         EMe(SKB_DROP_REASON_MAX, MAX)
> >
> >  #undef EM
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 3280ba2502cd..373fa7a33ffa 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4541,10 +4541,12 @@ static bool skb_flow_limit(struct sk_buff *skb, unsigned int qlen)
> >  static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
> >                               unsigned int *qtail)
> >  {
> > +       enum skb_drop_reason reason;
> >         struct softnet_data *sd;
> >         unsigned long flags;
> >         unsigned int qlen;
> >
> > +       reason = SKB_DROP_REASON_NOT_SPECIFIED;
> >         sd = &per_cpu(softnet_data, cpu);
> >
> >         rps_lock_irqsave(sd, &flags);
> > @@ -4566,6 +4568,8 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
> >                 if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd->backlog.state))
> >                         napi_schedule_rps(sd);
> >                 goto enqueue;
> > +       } else {
>
> No need for an else {} after a goto  xxx;
>

Yeah, this 'else' can be omitted :) Thanks!

>
> > +               reason = SKB_DROP_REASON_CPU_BACKLOG;
> >         }
> >
> >  drop:
> > @@ -4573,7 +4577,7 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
> >         rps_unlock_irq_restore(sd, &flags);
> >
> >         atomic_long_inc(&skb->dev->rx_dropped);
> > -       kfree_skb(skb);
> > +       kfree_skb_reason(skb, reason);
> >         return NET_RX_DROP;
> >  }
> >
> > --
> > 2.35.1
> >
diff mbox series

Patch

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 62f9d15ec6ec..d2cf87ff84c2 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -402,6 +402,12 @@  enum skb_drop_reason {
 					 * outputting (failed to enqueue to
 					 * current qdisc)
 					 */
+	SKB_DROP_REASON_CPU_BACKLOG,	/* failed to enqueue the skb to
+					 * the per CPU backlog queue. This
+					 * can be caused by backlog queue
+					 * full (see netdev_max_backlog in
+					 * net.rst) or RPS flow limit
+					 */
 	SKB_DROP_REASON_MAX,
 };
 
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 80fe15d175e3..29c360b5e114 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -47,6 +47,7 @@ 
 	EM(SKB_DROP_REASON_NEIGH_DEAD, NEIGH_DEAD)		\
 	EM(SKB_DROP_REASON_QDISC_EGRESS, QDISC_EGRESS)		\
 	EM(SKB_DROP_REASON_QDISC_DROP, QDISC_DROP)		\
+	EM(SKB_DROP_REASON_CPU_BACKLOG, CPU_BACKLOG)		\
 	EMe(SKB_DROP_REASON_MAX, MAX)
 
 #undef EM
diff --git a/net/core/dev.c b/net/core/dev.c
index 3280ba2502cd..373fa7a33ffa 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4541,10 +4541,12 @@  static bool skb_flow_limit(struct sk_buff *skb, unsigned int qlen)
 static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
 			      unsigned int *qtail)
 {
+	enum skb_drop_reason reason;
 	struct softnet_data *sd;
 	unsigned long flags;
 	unsigned int qlen;
 
+	reason = SKB_DROP_REASON_NOT_SPECIFIED;
 	sd = &per_cpu(softnet_data, cpu);
 
 	rps_lock_irqsave(sd, &flags);
@@ -4566,6 +4568,8 @@  static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
 		if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd->backlog.state))
 			napi_schedule_rps(sd);
 		goto enqueue;
+	} else {
+		reason = SKB_DROP_REASON_CPU_BACKLOG;
 	}
 
 drop:
@@ -4573,7 +4577,7 @@  static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
 	rps_unlock_irq_restore(sd, &flags);
 
 	atomic_long_inc(&skb->dev->rx_dropped);
-	kfree_skb(skb);
+	kfree_skb_reason(skb, reason);
 	return NET_RX_DROP;
 }