diff mbox series

[net-next,2/3] net: neigh: use kfree_skb_reason() for __neigh_event_send()

Message ID 20220220155705.194266-3-imagedong@tencent.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: use kfree_skb_reason() for ip/neighbour | 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: 5979 this patch: 5979
netdev/cc_maintainers success CCed 12 of 12 maintainers
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: 6130 this patch: 6130
netdev/checkpatch warning WARNING: line length of 88 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Menglong Dong Feb. 20, 2022, 3:57 p.m. UTC 
From: Menglong Dong <imagedong@tencent.com>

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 <mengensun@tencent.com>
Reviewed-by: Hao Peng <flyingpeng@tencent.com>
Signed-off-by: Menglong Dong <imagedong@tencent.com>
---
 include/linux/skbuff.h     | 9 +++++++++
 include/trace/events/skb.h | 2 ++
 net/core/neighbour.c       | 4 ++--
 3 files changed, 13 insertions(+), 2 deletions(-)

Comments

David Ahern Feb. 22, 2022, 3:17 a.m. UTC | #1 
On 2/20/22 8:57 AM, menglong8.dong@gmail.com 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.
Menglong Dong Feb. 22, 2022, 3:47 a.m. UTC | #2 
On Tue, Feb 22, 2022 at 11:17 AM David Ahern <dsahern@kernel.org> wrote:
>
> On 2/20/22 8:57 AM, menglong8.dong@gmail.com 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.
diff mbox series

Patch

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);