net: pass receive socket to drop tracepoint

RFC Superseded Yan Zhai 3 participants 12 messages Jun 1, 2024 – Jun 4, 2024

Subsystems (6)

Networking Network drivers Tracing TCP +2 more subsystems

lore

Share & tools

lore

Yan Zhai

author

2024-06-01 01:42 UTC #0 lore raw

v2 cover net: pass receive socket to drop tracepoint

We set up our production packet drop monitoring around the kfree_skb tracepoint. While this tracepoint is extremely valuable for diagnosing critical problems, it also has some limitation with drops on the local receive path: this tracepoint can only inspect the dropped skb itself, but such skb might not carry enough information to:

1. determine in which netns/container this skb gets dropped
2. determine by which socket/service this skb oughts to be received

The 1st issue is because skb->dev is the only member field with valid netns reference. But skb->dev can get cleared or reused. For example, tcp_v4_rcv will clear skb->dev and in later processing it might be reused for OFO tree.

The 2nd issue is because there is no reference on an skb that reliably points to a receiving socket. skb->sk usually points to the local sending socket, and it only points to a receive socket briefly after early demux stage, yet the socket can get stolen later. For certain drop reason like TCP OFO_MERGE, Zerowindow, UDP at PROTO_MEM error, etc, it is hard to infer which receiving socket is impacted. This cannot be overcome by simply looking at the packet header, because of complications like sk lookup programs. In the past, single purpose tracepoints like trace_udp_fail_queue_rcv_skb, trace_sock_rcvqueue_full, etc are added as needed to provide more visibility. This could be handled in a more generic way.

In this change set we propose a new 'sk_skb_reason_drop' call as a drop-in replacement for kfree_skb_reason at various local input path. It accepts an extra receiving socket argument. Both issues above can be resolved via this new argument.

V1->V2: instead of using skb->cb, directly add the needed argument to trace_kfree_skb tracepoint. Also renamed functions as Eric Dumazet suggested.

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

Yan Zhai (7):
net: add rx_sk to trace_kfree_skb
net: introduce sk_skb_reason_drop function
ping: use sk_skb_reason_drop to free rx packets
net: raw: use sk_skb_reason_drop to free rx packets
tcp: use sk_skb_reason_drop to free rx packets
udp: use sk_skb_reason_drop to free rx packets
af_packet: use sk_skb_reason_drop to free rx packets

 include/linux/skbuff.h     | 10 ++++++++--
 include/trace/events/skb.h | 11 +++++++----
 net/core/dev.c             |  2 +-
 net/core/skbuff.c          | 22 ++++++++++++----------
 net/ipv4/ping.c            |  2 +-
 net/ipv4/raw.c             |  4 ++--
 net/ipv4/syncookies.c      |  2 +-
 net/ipv4/tcp_input.c       |  2 +-
 net/ipv4/tcp_ipv4.c        |  6 +++---
 net/ipv4/udp.c             | 10 +++++-----
 net/ipv6/raw.c             |  8 ++++----
 net/ipv6/syncookies.c      |  2 +-
 net/ipv6/tcp_ipv6.c        |  6 +++---
 net/ipv6/udp.c             | 10 +++++-----
 net/packet/af_packet.c     |  6 +++---
 15 files changed, 57 insertions(+), 46 deletions(-)

Signature · 1 line

2.30.2

Yan Zhai

author

2024-06-01 01:42 UTC #1 lore raw

RFC v2 1/7 net: add rx_sk to trace_kfree_skb

skb does not include enough information to find out receiving sockets/services and netns/containers on packet drops. In theory skb->dev tells about netns, but it can get cleared/reused, e.g. by TCP stack for OOO packet lookup. Similarly, skb->sk often identifies a local sender, and tells nothing about a receiver.

Allow passing an extra receiving socket to the tracepoint to improve the visibility on receiving drops.

Signed-off-by Yan Zhai <[email protected]>

---
 include/trace/events/skb.h | 11 +++++++----
 net/core/dev.c             |  2 +-
 net/core/skbuff.c          |  2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

include/trace/events/skb.h +7 -4

index 07e0715628ec..aa6b46b6172c 100644
@@ -24,15 +24,16 @@ DEFINE_DROP_REASON(FN, FN)
 TRACE_EVENT(kfree_skb,
 
 	TP_PROTO(struct sk_buff *skb, void *location,
-		 enum skb_drop_reason reason),
+		 enum skb_drop_reason reason, struct sock *rx_sk),
 
-	TP_ARGS(skb, location, reason),
+	TP_ARGS(skb, location, reason, rx_sk),
 
 	TP_STRUCT__entry(
 		__field(void *,		skbaddr)
 		__field(void *,		location)
 		__field(unsigned short,	protocol)
 		__field(enum skb_drop_reason,	reason)
+		__field(void *,		rx_skaddr)
 	),
 
 	TP_fast_assign(

@@ -40,12 +41,14 @@ TRACE_EVENT(kfree_skb,
 		__entry->location = location;
 		__entry->protocol = ntohs(skb->protocol);
 		__entry->reason = reason;
+		__entry->rx_skaddr = rx_sk;
 	),
 
-	TP_printk("skbaddr=%p protocol=%u location=%pS reason: %s",
+	TP_printk("skbaddr=%p protocol=%u location=%pS reason: %s rx_skaddr=%p",
 		  __entry->skbaddr, __entry->protocol, __entry->location,
 		  __print_symbolic(__entry->reason,
-				   DEFINE_DROP_REASON(FN, FNe)))
+				   DEFINE_DROP_REASON(FN, FNe)),
+		  __entry->rx_skaddr)
 );
 
 #undef FN

net/core/dev.c +1 -1

index 85fe8138f3e4..7844227ecbfd 100644
@@ -5233,7 +5233,7 @@ static __latent_entropy void net_tx_action(struct softirq_action *h)
 				trace_consume_skb(skb, net_tx_action);
 			else
 				trace_kfree_skb(skb, net_tx_action,
-						get_kfree_skb_cb(skb)->reason);
+						get_kfree_skb_cb(skb)->reason, NULL);
 
 			if (skb->fclone != SKB_FCLONE_UNAVAILABLE)
 				__kfree_skb(skb);

net/core/skbuff.c +1 -1

index 466999a7515e..2854afdd713f 100644
@@ -1203,7 +1203,7 @@ bool __kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason)
 	if (reason == SKB_CONSUMED)
 		trace_consume_skb(skb, __builtin_return_address(0));
 	else
-		trace_kfree_skb(skb, __builtin_return_address(0), reason);
+		trace_kfree_skb(skb, __builtin_return_address(0), reason, NULL);
 	return true;
 }

Signature · 1 line

2.30.2

kernel test robot

build bot

2024-06-01 08:56 UTC #2 lore raw

Re: [RFC v2 net-next 1/7] net: add rx_sk to trace_kfree_skb

kernel test robot noticed the following build errors:

Hi Yan,

[This is a private test report for your RFC patch.] kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Yan-Zhai/net-add-rx_sk-to-trace_kfree_skb/20240601-094726
base:   net-next/main
patch link:    https://lore.kernel.org/r/451ae2a5c2ddb3c127cfddaf4a6579d6e85791f3.1717206060.git.yan%40cloudflare.com
patch subject: [RFC v2 net-next 1/7] net: add rx_sk to trace_kfree_skb
config: um-allyesconfig (https://download.01.org/0day-ci/archive/20240601/[email protected]/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240601/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

net/core/drop_monitor.c: In function 'net_dm_trace_on_set':

>> net/core/drop_monitor.c:1162:42: error: passing argument 1 of 'register_trace_kfree_skb' from incompatible pointer type [-Werror=incompatible-pointer-types]
    1162 |         rc = register_trace_kfree_skb(ops->kfree_skb_probe, NULL);
         |                                       ~~~^~~~~~~~~~~~~~~~~
         |                                          |
         |                                          void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason)
   In file included from include/trace/events/skb.h:10,
                    from net/core/drop_monitor.c:36:
   include/linux/tracepoint.h:270:38: note: expected 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason,  struct sock *)' but argument is of type 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason)'
     270 |         register_trace_##name(void (*probe)(data_proto), void *data)    \
         |                               ~~~~~~~^~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:434:9: note: in expansion of macro '__DECLARE_TRACE'
     434 |         __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),              \
         |         ^~~~~~~~~~~~~~~
   include/linux/tracepoint.h:568:9: note: in expansion of macro 'DECLARE_TRACE'
     568 |         DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
         |         ^~~~~~~~~~~~~
   include/trace/events/skb.h:24:1: note: in expansion of macro 'TRACE_EVENT'
      24 | TRACE_EVENT(kfree_skb,
         | ^~~~~~~~~~~
>> net/core/drop_monitor.c:1177:39: error: passing argument 1 of 'unregister_trace_kfree_skb' from incompatible pointer type [-Werror=incompatible-pointer-types]
    1177 |         unregister_trace_kfree_skb(ops->kfree_skb_probe, NULL);
         |                                    ~~~^~~~~~~~~~~~~~~~~
         |                                       |
         |                                       void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason)
   include/linux/tracepoint.h:283:40: note: expected 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason,  struct sock *)' but argument is of type 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason)'
     283 |         unregister_trace_##name(void (*probe)(data_proto), void *data)  \
         |                                 ~~~~~~~^~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:434:9: note: in expansion of macro '__DECLARE_TRACE'
     434 |         __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),              \
         |         ^~~~~~~~~~~~~~~
   include/linux/tracepoint.h:568:9: note: in expansion of macro 'DECLARE_TRACE'
     568 |         DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
         |         ^~~~~~~~~~~~~
   include/trace/events/skb.h:24:1: note: in expansion of macro 'TRACE_EVENT'
      24 | TRACE_EVENT(kfree_skb,
         | ^~~~~~~~~~~
   net/core/drop_monitor.c: In function 'net_dm_trace_off_set':
   net/core/drop_monitor.c:1200:39: error: passing argument 1 of 'unregister_trace_kfree_skb' from incompatible pointer type [-Werror=incompatible-pointer-types]
    1200 |         unregister_trace_kfree_skb(ops->kfree_skb_probe, NULL);
         |                                    ~~~^~~~~~~~~~~~~~~~~
         |                                       |
         |                                       void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason)
   include/linux/tracepoint.h:283:40: note: expected 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason,  struct sock *)' but argument is of type 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason)'
     283 |         unregister_trace_##name(void (*probe)(data_proto), void *data)  \
         |                                 ~~~~~~~^~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:434:9: note: in expansion of macro '__DECLARE_TRACE'
     434 |         __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),              \
         |         ^~~~~~~~~~~~~~~
   include/linux/tracepoint.h:568:9: note: in expansion of macro 'DECLARE_TRACE'
     568 |         DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
         |         ^~~~~~~~~~~~~
   include/trace/events/skb.h:24:1: note: in expansion of macro 'TRACE_EVENT'
      24 | TRACE_EVENT(kfree_skb,
         | ^~~~~~~~~~~

cc1: some warnings being treated as errors

vim +/register_trace_kfree_skb +1162 net/core/drop_monitor.c

8e94c3bc922e702 Ido Schimmel 2019-08-17  1135  
7c747838a55818f Ido Schimmel 2019-08-11  1136  static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
9a8afc8d3962f3e Neil Horman  2009-03-11  1137  {
28315f7999870bb Ido Schimmel 2019-08-11  1138  	const struct net_dm_alert_ops *ops;
70c69274f354ecb Ido Schimmel 2019-08-11  1139  	int cpu, rc;
4b706372f18de53 Neil Horman  2010-07-20  1140  
28315f7999870bb Ido Schimmel 2019-08-11  1141  	ops = net_dm_alert_ops_arr[net_dm_alert_mode];
28315f7999870bb Ido Schimmel 2019-08-11  1142  
cad456d5abbb630 Neil Horman  2012-05-17  1143  	if (!try_module_get(THIS_MODULE)) {
965100966efe85e Ido Schimmel 2019-08-06  1144  		NL_SET_ERR_MSG_MOD(extack, "Failed to take reference on module");
7c747838a55818f Ido Schimmel 2019-08-11  1145  		return -ENODEV;
cad456d5abbb630 Neil Horman  2012-05-17  1146  	}
cad456d5abbb630 Neil Horman  2012-05-17  1147  
70c69274f354ecb Ido Schimmel 2019-08-11  1148  	for_each_possible_cpu(cpu) {
70c69274f354ecb Ido Schimmel 2019-08-11  1149  		struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
44075f563793880 Ido Schimmel 2019-08-11  1150  		struct sk_buff *skb;
70c69274f354ecb Ido Schimmel 2019-08-11  1151  
28315f7999870bb Ido Schimmel 2019-08-11  1152  		INIT_WORK(&data->dm_alert_work, ops->work_item_func);
70c69274f354ecb Ido Schimmel 2019-08-11  1153  		timer_setup(&data->send_timer, sched_send_work, 0);
44075f563793880 Ido Schimmel 2019-08-11  1154  		/* Allocate a new per-CPU skb for the summary alert message and
44075f563793880 Ido Schimmel 2019-08-11  1155  		 * free the old one which might contain stale data from
44075f563793880 Ido Schimmel 2019-08-11  1156  		 * previous tracing.
44075f563793880 Ido Schimmel 2019-08-11  1157  		 */
44075f563793880 Ido Schimmel 2019-08-11  1158  		skb = reset_per_cpu_data(data);
44075f563793880 Ido Schimmel 2019-08-11  1159  		consume_skb(skb);
70c69274f354ecb Ido Schimmel 2019-08-11  1160  	}
70c69274f354ecb Ido Schimmel 2019-08-11  1161  
28315f7999870bb Ido Schimmel 2019-08-11 @1162  	rc = register_trace_kfree_skb(ops->kfree_skb_probe, NULL);
7c747838a55818f Ido Schimmel 2019-08-11  1163  	if (rc) {
7c747838a55818f Ido Schimmel 2019-08-11  1164  		NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to kfree_skb() tracepoint");
7c747838a55818f Ido Schimmel 2019-08-11  1165  		goto err_module_put;
7c747838a55818f Ido Schimmel 2019-08-11  1166  	}
cad456d5abbb630 Neil Horman  2012-05-17  1167  
28315f7999870bb Ido Schimmel 2019-08-11  1168  	rc = register_trace_napi_poll(ops->napi_poll_probe, NULL);
7c747838a55818f Ido Schimmel 2019-08-11  1169  	if (rc) {
7c747838a55818f Ido Schimmel 2019-08-11  1170  		NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to napi_poll() tracepoint");
7c747838a55818f Ido Schimmel 2019-08-11  1171  		goto err_unregister_trace;
7c747838a55818f Ido Schimmel 2019-08-11  1172  	}
7c747838a55818f Ido Schimmel 2019-08-11  1173  
7c747838a55818f Ido Schimmel 2019-08-11  1174  	return 0;
7c747838a55818f Ido Schimmel 2019-08-11  1175  
7c747838a55818f Ido Schimmel 2019-08-11  1176  err_unregister_trace:
28315f7999870bb Ido Schimmel 2019-08-11 @1177  	unregister_trace_kfree_skb(ops->kfree_skb_probe, NULL);
7c747838a55818f Ido Schimmel 2019-08-11  1178  err_module_put:
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1179  	for_each_possible_cpu(cpu) {
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1180  		struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1181  		struct sk_buff *skb;
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1182  
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1183  		del_timer_sync(&data->send_timer);
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1184  		cancel_work_sync(&data->dm_alert_work);
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1185  		while ((skb = __skb_dequeue(&data->drop_queue)))
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1186  			consume_skb(skb);
9398e9c0b1d44ee Ido Schimmel 2021-03-10  1187  	}
7c747838a55818f Ido Schimmel 2019-08-11  1188  	module_put(THIS_MODULE);
7c747838a55818f Ido Schimmel 2019-08-11  1189  	return rc;
7c747838a55818f Ido Schimmel 2019-08-11  1190  }
7c747838a55818f Ido Schimmel 2019-08-11  1191

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

kernel test robot

build bot

2024-06-01 12:57 UTC #3 lore raw

Re: [RFC v2 net-next 1/7] net: add rx_sk to trace_kfree_skb

In reply to Yan Zhai

kernel test robot noticed the following build errors:

Hi Yan,

[This is a private test report for your RFC patch.] kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Yan-Zhai/net-add-rx_sk-to-trace_kfree_skb/20240601-094726
base:   net-next/main
patch link:    https://lore.kernel.org/r/451ae2a5c2ddb3c127cfddaf4a6579d6e85791f3.1717206060.git.yan%40cloudflare.com
patch subject: [RFC v2 net-next 1/7] net: add rx_sk to trace_kfree_skb
config: arm-randconfig-002-20240601 (https://download.01.org/0day-ci/archive/20240601/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project bafda89a0944d947fc4b3b5663185e07a397ac30)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240601/[email protected]/reproduce)

All errors (new ones prefixed by >>):

In file included from net/core/drop_monitor.c:10:
In file included from include/linux/netdevice.h:38:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:8:
In file included from include/linux/cacheflush.h:5:
In file included from arch/arm/include/asm/cacheflush.h:10:
In file included from include/linux/mm.h:2253:
include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]

     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> net/core/drop_monitor.c:1162:32: error: incompatible function pointer types passing 'void (*const)(void *, struct sk_buff *, void *, enum skb_drop_reason)' to parameter of type 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason, struct sock *)' [-Wincompatible-function-pointer-types]
    1162 |         rc = register_trace_kfree_skb(ops->kfree_skb_probe, NULL);
         |                                       ^~~~~~~~~~~~~~~~~~~~
   include/trace/events/skb.h:24:1: note: passing argument to parameter 'probe' here
      24 | TRACE_EVENT(kfree_skb,
         | ^
   include/linux/tracepoint.h:568:2: note: expanded from macro 'TRACE_EVENT'
     568 |         DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
         |         ^
   include/linux/tracepoint.h:434:2: note: expanded from macro 'DECLARE_TRACE'
     434 |         __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),              \
         |         ^
   include/linux/tracepoint.h:270:31: note: expanded from macro '__DECLARE_TRACE'
     270 |         register_trace_##name(void (*probe)(data_proto), void *data)    \
         |                                      ^
   net/core/drop_monitor.c:1177:29: error: incompatible function pointer types passing 'void (*const)(void *, struct sk_buff *, void *, enum skb_drop_reason)' to parameter of type 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason, struct sock *)' [-Wincompatible-function-pointer-types]
    1177 |         unregister_trace_kfree_skb(ops->kfree_skb_probe, NULL);
         |                                    ^~~~~~~~~~~~~~~~~~~~
   include/trace/events/skb.h:24:1: note: passing argument to parameter 'probe' here
      24 | TRACE_EVENT(kfree_skb,
         | ^
   include/linux/tracepoint.h:568:2: note: expanded from macro 'TRACE_EVENT'
     568 |         DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
         |         ^
   include/linux/tracepoint.h:434:2: note: expanded from macro 'DECLARE_TRACE'
     434 |         __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),              \
         |         ^
   include/linux/tracepoint.h:283:33: note: expanded from macro '__DECLARE_TRACE'
     283 |         unregister_trace_##name(void (*probe)(data_proto), void *data)  \
         |                                        ^
   net/core/drop_monitor.c:1200:29: error: incompatible function pointer types passing 'void (*const)(void *, struct sk_buff *, void *, enum skb_drop_reason)' to parameter of type 'void (*)(void *, struct sk_buff *, void *, enum skb_drop_reason, struct sock *)' [-Wincompatible-function-pointer-types]
    1200 |         unregister_trace_kfree_skb(ops->kfree_skb_probe, NULL);
         |                                    ^~~~~~~~~~~~~~~~~~~~
   include/trace/events/skb.h:24:1: note: passing argument to parameter 'probe' here
      24 | TRACE_EVENT(kfree_skb,
         | ^
   include/linux/tracepoint.h:568:2: note: expanded from macro 'TRACE_EVENT'
     568 |         DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
         |         ^
   include/linux/tracepoint.h:434:2: note: expanded from macro 'DECLARE_TRACE'
     434 |         __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),              \
         |         ^
   include/linux/tracepoint.h:283:33: note: expanded from macro '__DECLARE_TRACE'
     283 |         unregister_trace_##name(void (*probe)(data_proto), void *data)  \
         |                                        ^

1 warning and 3 errors generated.

vim +1162 net/core/drop_monitor.c

8e94c3bc922e70 Ido Schimmel 2019-08-17  1135  
7c747838a55818 Ido Schimmel 2019-08-11  1136  static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
9a8afc8d3962f3 Neil Horman  2009-03-11  1137  {
28315f7999870b Ido Schimmel 2019-08-11  1138  	const struct net_dm_alert_ops *ops;
70c69274f354ec Ido Schimmel 2019-08-11  1139  	int cpu, rc;
4b706372f18de5 Neil Horman  2010-07-20  1140  
28315f7999870b Ido Schimmel 2019-08-11  1141  	ops = net_dm_alert_ops_arr[net_dm_alert_mode];
28315f7999870b Ido Schimmel 2019-08-11  1142  
cad456d5abbb63 Neil Horman  2012-05-17  1143  	if (!try_module_get(THIS_MODULE)) {
965100966efe85 Ido Schimmel 2019-08-06  1144  		NL_SET_ERR_MSG_MOD(extack, "Failed to take reference on module");
7c747838a55818 Ido Schimmel 2019-08-11  1145  		return -ENODEV;
cad456d5abbb63 Neil Horman  2012-05-17  1146  	}
cad456d5abbb63 Neil Horman  2012-05-17  1147  
70c69274f354ec Ido Schimmel 2019-08-11  1148  	for_each_possible_cpu(cpu) {
70c69274f354ec Ido Schimmel 2019-08-11  1149  		struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
44075f56379388 Ido Schimmel 2019-08-11  1150  		struct sk_buff *skb;
70c69274f354ec Ido Schimmel 2019-08-11  1151  
28315f7999870b Ido Schimmel 2019-08-11  1152  		INIT_WORK(&data->dm_alert_work, ops->work_item_func);
70c69274f354ec Ido Schimmel 2019-08-11  1153  		timer_setup(&data->send_timer, sched_send_work, 0);
44075f56379388 Ido Schimmel 2019-08-11  1154  		/* Allocate a new per-CPU skb for the summary alert message and
44075f56379388 Ido Schimmel 2019-08-11  1155  		 * free the old one which might contain stale data from
44075f56379388 Ido Schimmel 2019-08-11  1156  		 * previous tracing.
44075f56379388 Ido Schimmel 2019-08-11  1157  		 */
44075f56379388 Ido Schimmel 2019-08-11  1158  		skb = reset_per_cpu_data(data);
44075f56379388 Ido Schimmel 2019-08-11  1159  		consume_skb(skb);
70c69274f354ec Ido Schimmel 2019-08-11  1160  	}
70c69274f354ec Ido Schimmel 2019-08-11  1161  
28315f7999870b Ido Schimmel 2019-08-11 @1162  	rc = register_trace_kfree_skb(ops->kfree_skb_probe, NULL);
7c747838a55818 Ido Schimmel 2019-08-11  1163  	if (rc) {
7c747838a55818 Ido Schimmel 2019-08-11  1164  		NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to kfree_skb() tracepoint");
7c747838a55818 Ido Schimmel 2019-08-11  1165  		goto err_module_put;
7c747838a55818 Ido Schimmel 2019-08-11  1166  	}
cad456d5abbb63 Neil Horman  2012-05-17  1167  
28315f7999870b Ido Schimmel 2019-08-11  1168  	rc = register_trace_napi_poll(ops->napi_poll_probe, NULL);
7c747838a55818 Ido Schimmel 2019-08-11  1169  	if (rc) {
7c747838a55818 Ido Schimmel 2019-08-11  1170  		NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to napi_poll() tracepoint");
7c747838a55818 Ido Schimmel 2019-08-11  1171  		goto err_unregister_trace;
7c747838a55818 Ido Schimmel 2019-08-11  1172  	}
7c747838a55818 Ido Schimmel 2019-08-11  1173  
7c747838a55818 Ido Schimmel 2019-08-11  1174  	return 0;
7c747838a55818 Ido Schimmel 2019-08-11  1175  
7c747838a55818 Ido Schimmel 2019-08-11  1176  err_unregister_trace:
28315f7999870b Ido Schimmel 2019-08-11  1177  	unregister_trace_kfree_skb(ops->kfree_skb_probe, NULL);
7c747838a55818 Ido Schimmel 2019-08-11  1178  err_module_put:
9398e9c0b1d44e Ido Schimmel 2021-03-10  1179  	for_each_possible_cpu(cpu) {
9398e9c0b1d44e Ido Schimmel 2021-03-10  1180  		struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
9398e9c0b1d44e Ido Schimmel 2021-03-10  1181  		struct sk_buff *skb;
9398e9c0b1d44e Ido Schimmel 2021-03-10  1182  
9398e9c0b1d44e Ido Schimmel 2021-03-10  1183  		del_timer_sync(&data->send_timer);
9398e9c0b1d44e Ido Schimmel 2021-03-10  1184  		cancel_work_sync(&data->dm_alert_work);
9398e9c0b1d44e Ido Schimmel 2021-03-10  1185  		while ((skb = __skb_dequeue(&data->drop_queue)))
9398e9c0b1d44e Ido Schimmel 2021-03-10  1186  			consume_skb(skb);
9398e9c0b1d44e Ido Schimmel 2021-03-10  1187  	}
7c747838a55818 Ido Schimmel 2019-08-11  1188  	module_put(THIS_MODULE);
7c747838a55818 Ido Schimmel 2019-08-11  1189  	return rc;
7c747838a55818 Ido Schimmel 2019-08-11  1190  }
7c747838a55818 Ido Schimmel 2019-08-11  1191

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Yan Zhai

author

2024-06-01 01:42 UTC #4 lore raw

RFC v2 2/7 net: introduce sk_skb_reason_drop function

In reply to Yan Zhai

Long used destructors kfree_skb and kfree_skb_reason do not pass receiving socket to packet drop tracepoints trace_kfree_skb. This makes it hard to track packet drops of a certain netns (container) or a socket (user application).

The naming of these destructors are also not consistent with most sk/skb operating functions, i.e. functions named "sk_xxx" or "skb_xxx". Introduce a new functions sk_skb_reason_drop as drop-in replacement for kfree_skb_reason on local receiving path. Callers can now pass receiving sockets to the tracepoints.

kfree_skb and kfree_skb_reason are still usable but they are now just inline helpers that call sk_skb_reason_drop.

Note it is not feasible to do the same to consume_skb. Packets not dropped can flow through multiple receive handlers, and have multiple receiving sockets. Leave it untouched for now.

Signed-off-by Yan Zhai <[email protected]>

---
v1->v2: changes function names to be more consistent with common sk/skb operations

---
 include/linux/skbuff.h | 10 ++++++++--
 net/core/skbuff.c      | 22 ++++++++++++----------
 2 files changed, 20 insertions(+), 12 deletions(-)

include/linux/skbuff.h +8 -2

index fe7d8dbef77e..c479a2515a62 100644
@@ -1251,8 +1251,14 @@ static inline bool skb_data_unref(const struct sk_buff *skb,
 	return true;
 }
 
-void __fix_address
-kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason);
+void __fix_address sk_skb_reason_drop(struct sock *sk, struct sk_buff *skb,
+				      enum skb_drop_reason reason);
+
+static inline void
+kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason)
+{
+	sk_skb_reason_drop(NULL, skb, reason);
+}
 
 /**
  *	kfree_skb - free an sk_buff with 'NOT_SPECIFIED' reason

net/core/skbuff.c +12 ~~-10~~

index 2854afdd713f..9def11fe42c4 100644
@@ -1190,7 +1190,8 @@ void __kfree_skb(struct sk_buff *skb)
 EXPORT_SYMBOL(__kfree_skb);
 
 static __always_inline
-bool __kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason)
+bool __sk_skb_reason_drop(struct sock *sk, struct sk_buff *skb,
+			  enum skb_drop_reason reason)
 {
 	if (unlikely(!skb_unref(skb)))
 		return false;

@@ -1203,26 +1204,27 @@ bool __kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason)
 	if (reason == SKB_CONSUMED)
 		trace_consume_skb(skb, __builtin_return_address(0));
 	else
-		trace_kfree_skb(skb, __builtin_return_address(0), reason, NULL);
+		trace_kfree_skb(skb, __builtin_return_address(0), reason, sk);
 	return true;
 }
 
 /**
- *	kfree_skb_reason - free an sk_buff with special reason
+ *	sk_skb_reason_drop - free an sk_buff with special reason
+ *	@sk: the socket to receive @skb, or NULL if not applicable
  *	@skb: buffer to free
  *	@reason: reason why this skb is dropped
  *
- *	Drop a reference to the buffer and free it if the usage count has
- *	hit zero. Meanwhile, pass the drop reason to 'kfree_skb'
- *	tracepoint.
+ *	Drop a reference to the buffer and free it if the usage count has hit
+ *	zero. Meanwhile, pass the receiving socket and drop reason to
+ *	'kfree_skb' tracepoint.
  */
 void __fix_address
-kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason)
+sk_skb_reason_drop(struct sock *sk, struct sk_buff *skb, enum skb_drop_reason reason)
 {
-	if (__kfree_skb_reason(skb, reason))
+	if (__sk_skb_reason_drop(sk, skb, reason))
 		__kfree_skb(skb);
 }
-EXPORT_SYMBOL(kfree_skb_reason);
+EXPORT_SYMBOL(sk_skb_reason_drop);
 
 #define KFREE_SKB_BULK_SIZE	16

@@ -1261,7 +1263,7 @@ kfree_skb_list_reason(struct sk_buff *segs, enum skb_drop_reason reason)
 	while (segs) {
 		struct sk_buff *next = segs->next;
 
-		if (__kfree_skb_reason(segs, reason)) {
+		if (__sk_skb_reason_drop(NULL, segs, reason)) {
 			skb_poison_list(segs);
 			kfree_skb_add_bulk(segs, &sa, reason);
 		}

Signature · 1 line

2.30.2

Yan Zhai

author

2024-06-01 01:42 UTC #5 lore raw

RFC v2 3/7 ping: use sk_skb_reason_drop to free rx packets

In reply to Yan Zhai

Replace kfree_skb_reason with sk_skb_reason_drop and pass the receiving socket to the tracepoint.

Signed-off-by Yan Zhai <[email protected]>

---
 net/ipv4/ping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/ipv4/ping.c +1 -1

index 823306487a82..619ddc087957 100644
@@ -946,7 +946,7 @@ static enum skb_drop_reason __ping_queue_rcv_skb(struct sock *sk,
 	pr_debug("ping_queue_rcv_skb(sk=%p,sk->num=%d,skb=%p)\n",
 		 inet_sk(sk), inet_sk(sk)->inet_num, skb);
 	if (sock_queue_rcv_skb_reason(sk, skb, &reason) < 0) {
-		kfree_skb_reason(skb, reason);
+		sk_skb_reason_drop(sk, skb, reason);
 		pr_debug("ping_queue_rcv_skb -> failed\n");
 		return reason;
 	}

Signature · 1 line

2.30.2

Yan Zhai

author

2024-06-01 01:42 UTC #6 lore raw

RFC v2 4/7 net: raw: use sk_skb_reason_drop to free rx packets

In reply to Yan Zhai

Replace kfree_skb_reason with sk_skb_reason_drop and pass the receiving socket to the tracepoint.

Signed-off-by Yan Zhai <[email protected]>

---
 net/ipv4/raw.c | 4 ++--
 net/ipv6/raw.c | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

net/ipv4/raw.c +2 -2

index 1a0953650356..474dfd263c8b 100644
@@ -301,7 +301,7 @@ static int raw_rcv_skb(struct sock *sk, struct sk_buff *skb)
 
 	ipv4_pktinfo_prepare(sk, skb, true);
 	if (sock_queue_rcv_skb_reason(sk, skb, &reason) < 0) {
-		kfree_skb_reason(skb, reason);
+		sk_skb_reason_drop(sk, skb, reason);
 		return NET_RX_DROP;
 	}

@@ -312,7 +312,7 @@ int raw_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb)) {
 		atomic_inc(&sk->sk_drops);
-		kfree_skb_reason(skb, SKB_DROP_REASON_XFRM_POLICY);
+		sk_skb_reason_drop(sk, skb, SKB_DROP_REASON_XFRM_POLICY);
 		return NET_RX_DROP;
 	}
 	nf_reset_ct(skb);

net/ipv6/raw.c +4 -4

index f838366e8256..608fa9d05b55 100644
@@ -362,14 +362,14 @@ static inline int rawv6_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	if ((raw6_sk(sk)->checksum || rcu_access_pointer(sk->sk_filter)) &&
 	    skb_checksum_complete(skb)) {
 		atomic_inc(&sk->sk_drops);
-		kfree_skb_reason(skb, SKB_DROP_REASON_SKB_CSUM);
+		sk_skb_reason_drop(sk, skb, SKB_DROP_REASON_SKB_CSUM);
 		return NET_RX_DROP;
 	}
 
 	/* Charge it to the socket. */
 	skb_dst_drop(skb);
 	if (sock_queue_rcv_skb_reason(sk, skb, &reason) < 0) {
-		kfree_skb_reason(skb, reason);
+		sk_skb_reason_drop(sk, skb, reason);
 		return NET_RX_DROP;
 	}

@@ -390,7 +390,7 @@ int rawv6_rcv(struct sock *sk, struct sk_buff *skb)
 
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb)) {
 		atomic_inc(&sk->sk_drops);
-		kfree_skb_reason(skb, SKB_DROP_REASON_XFRM_POLICY);
+		sk_skb_reason_drop(sk, skb, SKB_DROP_REASON_XFRM_POLICY);
 		return NET_RX_DROP;
 	}
 	nf_reset_ct(skb);

@@ -415,7 +415,7 @@ int rawv6_rcv(struct sock *sk, struct sk_buff *skb)
 	if (inet_test_bit(HDRINCL, sk)) {
 		if (skb_checksum_complete(skb)) {
 			atomic_inc(&sk->sk_drops);
-			kfree_skb_reason(skb, SKB_DROP_REASON_SKB_CSUM);
+			sk_skb_reason_drop(sk, skb, SKB_DROP_REASON_SKB_CSUM);
 			return NET_RX_DROP;
 		}
 	}

Signature · 1 line

2.30.2

Yan Zhai

author

2024-06-01 01:42 UTC #7 lore raw

RFC v2 5/7 tcp: use sk_skb_reason_drop to free rx packets

In reply to Yan Zhai

Replace kfree_skb_reason with sk_skb_reason_drop and pass the receiving socket to the tracepoint.

Signed-off-by Yan Zhai <[email protected]>

---
 net/ipv4/syncookies.c | 2 +-
 net/ipv4/tcp_input.c  | 2 +-
 net/ipv4/tcp_ipv4.c   | 6 +++---
 net/ipv6/syncookies.c | 2 +-
 net/ipv6/tcp_ipv6.c   | 6 +++---
 5 files changed, 9 insertions(+), 9 deletions(-)

net/ipv4/syncookies.c +1 -1

index b61d36810fe3..1948d15f1f28 100644
@@ -496,6 +496,6 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 out_free:
 	reqsk_free(req);
 out_drop:
-	kfree_skb_reason(skb, reason);
+	sk_skb_reason_drop(sk, skb, reason);
 	return NULL;
 }

net/ipv4/tcp_input.c +1 -1

index 5aadf64e554d..bedb079de1f0 100644
@@ -4859,7 +4859,7 @@ static void tcp_drop_reason(struct sock *sk, struct sk_buff *skb,
 			    enum skb_drop_reason reason)
 {
 	sk_drops_add(sk, skb);
-	kfree_skb_reason(skb, reason);
+	sk_skb_reason_drop(sk, skb, reason);
 }
 
 /* This one checks to see if we can put data from the

net/ipv4/tcp_ipv4.c +3 -3

index 041c7eda9abe..f7a046bc4b27 100644
@@ -1939,7 +1939,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 reset:
 	tcp_v4_send_reset(rsk, skb, sk_rst_convert_drop_reason(reason));
 discard:
-	kfree_skb_reason(skb, reason);
+	sk_skb_reason_drop(sk, skb, reason);
 	/* Be careful here. If this function gets more complicated and
 	 * gcc suffers from register pressure on the x86, sk (in %ebx)
 	 * might be destroyed here. This current version compiles correctly,

@@ -2176,8 +2176,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
 	int dif = inet_iif(skb);
 	const struct iphdr *iph;
 	const struct tcphdr *th;
+	struct sock *sk = NULL;
 	bool refcounted;
-	struct sock *sk;
 	int ret;
 	u32 isn;

@@ -2376,7 +2376,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
 discard_it:
 	SKB_DR_OR(drop_reason, NOT_SPECIFIED);
 	/* Discard frame. */
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return 0;
 
 discard_and_relse:

net/ipv6/syncookies.c +1 -1

index bfad1e89b6a6..9d83eadd308b 100644
@@ -275,6 +275,6 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 out_free:
 	reqsk_free(req);
 out_drop:
-	kfree_skb_reason(skb, reason);
+	sk_skb_reason_drop(sk, skb, reason);
 	return NULL;
 }

net/ipv6/tcp_ipv6.c +3 -3

index 1ac7502e1bf5..93967accc35d 100644
@@ -1678,7 +1678,7 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 discard:
 	if (opt_skb)
 		__kfree_skb(opt_skb);
-	kfree_skb_reason(skb, reason);
+	sk_skb_reason_drop(sk, skb, reason);
 	return 0;
 csum_err:
 	reason = SKB_DROP_REASON_TCP_CSUM;

@@ -1751,8 +1751,8 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 	int dif = inet6_iif(skb);
 	const struct tcphdr *th;
 	const struct ipv6hdr *hdr;
+	struct sock *sk = NULL;
 	bool refcounted;
-	struct sock *sk;
 	int ret;
 	u32 isn;
 	struct net *net = dev_net(skb->dev);

@@ -1944,7 +1944,7 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 
 discard_it:
 	SKB_DR_OR(drop_reason, NOT_SPECIFIED);
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return 0;
 
 discard_and_relse:

Signature · 1 line

2.30.2

Yan Zhai

author

2024-06-01 01:43 UTC #8 lore raw

RFC v2 6/7 udp: use sk_skb_reason_drop to free rx packets

In reply to Yan Zhai

Replace kfree_skb_reason with sk_skb_reason_drop and pass the receiving socket to the tracepoint.

Signed-off-by Yan Zhai <[email protected]>

---
 net/ipv4/udp.c | 10 +++++-----
 net/ipv6/udp.c | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

net/ipv4/udp.c +5 -5

index 189c9113fe9a..ecafb1695999 100644
@@ -2074,7 +2074,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		}
 		UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
 		trace_udp_fail_queue_rcv_skb(rc, sk, skb);
-		kfree_skb_reason(skb, drop_reason);
+		sk_skb_reason_drop(sk, skb, drop_reason);
 		return -1;
 	}

@@ -2196,7 +2196,7 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
 drop:
 	__UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
 	atomic_inc(&sk->sk_drops);
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return -1;
 }

@@ -2383,7 +2383,7 @@ static int udp_unicast_rcv_skb(struct sock *sk, struct sk_buff *skb,
 int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 		   int proto)
 {
-	struct sock *sk;
+	struct sock *sk = NULL;
 	struct udphdr *uh;
 	unsigned short ulen;
 	struct rtable *rt = skb_rtable(skb);

@@ -2460,7 +2460,7 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	 * Hmm.  We got an UDP packet to a port to which we
 	 * don't wanna listen.  Ignore it.
 	 */
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return 0;
 
 short_packet:

@@ -2485,7 +2485,7 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	__UDP_INC_STATS(net, UDP_MIB_CSUMERRORS, proto == IPPROTO_UDPLITE);
 drop:
 	__UDP_INC_STATS(net, UDP_MIB_INERRORS, proto == IPPROTO_UDPLITE);
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return 0;
 }

net/ipv6/udp.c +5 -5

index c81a07ac0463..b56f0b9f4307 100644
@@ -673,7 +673,7 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		}
 		UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
 		trace_udp_fail_queue_rcv_skb(rc, sk, skb);
-		kfree_skb_reason(skb, drop_reason);
+		sk_skb_reason_drop(sk, skb, drop_reason);
 		return -1;
 	}

@@ -776,7 +776,7 @@ static int udpv6_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
 drop:
 	__UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
 	atomic_inc(&sk->sk_drops);
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return -1;
 }

@@ -940,8 +940,8 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED;
 	const struct in6_addr *saddr, *daddr;
 	struct net *net = dev_net(skb->dev);
+	struct sock *sk = NULL;
 	struct udphdr *uh;
-	struct sock *sk;
 	bool refcounted;
 	u32 ulen = 0;

@@ -1033,7 +1033,7 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	__UDP6_INC_STATS(net, UDP_MIB_NOPORTS, proto == IPPROTO_UDPLITE);
 	icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
 
-	kfree_skb_reason(skb, reason);
+	sk_skb_reason_drop(sk, skb, reason);
 	return 0;
 
 short_packet:

@@ -1054,7 +1054,7 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	__UDP6_INC_STATS(net, UDP_MIB_CSUMERRORS, proto == IPPROTO_UDPLITE);
 discard:
 	__UDP6_INC_STATS(net, UDP_MIB_INERRORS, proto == IPPROTO_UDPLITE);
-	kfree_skb_reason(skb, reason);
+	sk_skb_reason_drop(sk, skb, reason);
 	return 0;
 }

Signature · 1 line

2.30.2

Yan Zhai

author

2024-06-01 01:43 UTC #9 lore raw

RFC v2 7/7 af_packet: use sk_skb_reason_drop to free rx packets

In reply to Yan Zhai

Replace kfree_skb_reason with sk_skb_reason_drop and pass the receiving socket to the tracepoint.

Signed-off-by Yan Zhai <[email protected]>

---
 net/packet/af_packet.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

net/packet/af_packet.c +3 -3

index fce390887591..3133d4eb4a1b 100644
@@ -2226,7 +2226,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
 		skb->len = skb_len;
 	}
 drop:
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return 0;
 }

@@ -2494,7 +2494,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 		skb->len = skb_len;
 	}
 drop:
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return 0;
 
 drop_n_account:

@@ -2503,7 +2503,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 	drop_reason = SKB_DROP_REASON_PACKET_SOCK_ERROR;
 
 	sk->sk_data_ready(sk);
-	kfree_skb_reason(copy_skb, drop_reason);
+	sk_skb_reason_drop(sk, copy_skb, drop_reason);
 	goto drop_n_restore;
 }

Signature · 1 line

2.30.2

Simon Horman

reviewer

2024-06-04 15:19 UTC #10 lore raw

Re: [RFC v2 net-next 7/7] af_packet: use sk_skb_reason_drop to free rx packets

net/packet/af_packet.c

Quoted 12 lines · Yan Zhai

Replace kfree_skb_reason with sk_skb_reason_drop and pass the receiving socket to the tracepoint.
Signed-off-by Yan Zhai <[email protected]>
---
 net/packet/af_packet.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/packet/af_packet.c +0 -0
index fce390887591..3133d4eb4a1b 100644

Hi Yan Zhai,

Near the top of packet_rcv,
immediately after local variable declarations, and before sk is initialised is the following:

	if (skb->pkt_type == PACKET_LOOPBACK)
		goto drop;

net/packet/af_packet.c static int packet_rcv(struct sk_buff *skb, struct net_device *dev, · line 2229

Quoted 6 lines

net/packet/af_packet.c +1 -1

@@ -2226,7 +2226,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
 		skb->len = skb_len;
 	}
 drop:
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);

So sk may be used uninitialised here.

Similarly in tpacket_rcv()

Flagged by clang-18 W=1 allmodconfig builds on x86_64.

Quoted 25 lines

net/packet/af_packet.c +2 -2

 	return 0;
 }

@@ -2494,7 +2494,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 		skb->len = skb_len;
 	}
 drop:
-	kfree_skb_reason(skb, drop_reason);
+	sk_skb_reason_drop(sk, skb, drop_reason);
 	return 0;
 
 drop_n_account:

@@ -2503,7 +2503,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 	drop_reason = SKB_DROP_REASON_PACKET_SOCK_ERROR;
 
 	sk->sk_data_ready(sk);
-	kfree_skb_reason(copy_skb, drop_reason);
+	sk_skb_reason_drop(sk, copy_skb, drop_reason);
 	goto drop_n_restore;
 }

Signature · 1 line

2.30.2

Yan Zhai

author

2024-06-04 20:36 UTC #11 lore raw

Re: [RFC v2 net-next 7/7] af_packet: use sk_skb_reason_drop to free rx packets

HI Simon,

Thanks for spotting the problem. I somehow missed the test bots
warning on V1 for this thread. Will add the missing tags in the next version.

Yan