* [PATCH net-next 0/8] net: rps: misc changes
@ 2024-03-28 17:03 Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 1/8] net: move kick_defer_list_purge() to net/core/dev.h Eric Dumazet
` (8 more replies)
0 siblings, 9 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
Make RPS/RFS a bit more efficient with better cache locality
and heuristics.
Aso shrink include/linux/netdevice.h a bit.
Eric Dumazet (8):
net: move kick_defer_list_purge() to net/core/dev.h
net: move dev_xmit_recursion() helpers to net/core/dev.h
net: enqueue_to_backlog() change vs not running device
net: make softnet_data.dropped an atomic_t
net: enqueue_to_backlog() cleanup
net: rps: change input_queue_tail_incr_save()
net: rps: add rps_input_queue_head_add() helper
net: rps: move received_rps field to a better location
include/linux/netdevice.h | 38 ++------------------
include/net/rps.h | 28 +++++++++++++++
net/core/dev.c | 73 ++++++++++++++++++++++-----------------
net/core/dev.h | 23 ++++++++++--
net/core/net-procfs.c | 3 +-
5 files changed, 95 insertions(+), 70 deletions(-)
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next 1/8] net: move kick_defer_list_purge() to net/core/dev.h
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
@ 2024-03-28 17:03 ` Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 2/8] net: move dev_xmit_recursion() helpers " Eric Dumazet
` (7 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
kick_defer_list_purge() is defined in net/core/dev.c
and used from net/core/skubff.c
Because we need softnet_data, include <linux/netdevice.h>
from net/core/dev.h
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/netdevice.h | 1 -
net/core/dev.h | 6 +++---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e41d30ebaca61e48a2ceb43edf777fa8b9859ef2..cb37817d6382c29117afd8ce54db6dba94f8c930 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3287,7 +3287,6 @@ static inline void dev_xmit_recursion_dec(void)
__this_cpu_dec(softnet_data.xmit.recursion);
}
-void kick_defer_list_purge(struct softnet_data *sd, unsigned int cpu);
void __netif_schedule(struct Qdisc *q);
void netif_schedule_queue(struct netdev_queue *txq);
diff --git a/net/core/dev.h b/net/core/dev.h
index 2bcaf8eee50c179db2ca59880521b9be6ecd45c8..9d0f8b4587f81f4c12487f1783d8ba5cc49fc1d6 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -4,11 +4,9 @@
#include <linux/types.h>
#include <linux/rwsem.h>
+#include <linux/netdevice.h>
struct net;
-struct net_device;
-struct netdev_bpf;
-struct netdev_phys_item_id;
struct netlink_ext_ack;
struct cpumask;
@@ -150,4 +148,6 @@ static inline void xdp_do_check_flushed(struct napi_struct *napi) { }
#endif
struct napi_struct *napi_by_id(unsigned int napi_id);
+void kick_defer_list_purge(struct softnet_data *sd, unsigned int cpu);
+
#endif
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next 2/8] net: move dev_xmit_recursion() helpers to net/core/dev.h
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 1/8] net: move kick_defer_list_purge() to net/core/dev.h Eric Dumazet
@ 2024-03-28 17:03 ` Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device Eric Dumazet
` (6 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
Move dev_xmit_recursion() and friends to net/core/dev.h
They are only used from net/core/dev.c and net/core/filter.c.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/netdevice.h | 17 -----------------
net/core/dev.h | 17 +++++++++++++++++
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cb37817d6382c29117afd8ce54db6dba94f8c930..70775021cc269e0983f538619115237b0067d408 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3270,23 +3270,6 @@ static inline int dev_recursion_level(void)
return this_cpu_read(softnet_data.xmit.recursion);
}
-#define XMIT_RECURSION_LIMIT 8
-static inline bool dev_xmit_recursion(void)
-{
- return unlikely(__this_cpu_read(softnet_data.xmit.recursion) >
- XMIT_RECURSION_LIMIT);
-}
-
-static inline void dev_xmit_recursion_inc(void)
-{
- __this_cpu_inc(softnet_data.xmit.recursion);
-}
-
-static inline void dev_xmit_recursion_dec(void)
-{
- __this_cpu_dec(softnet_data.xmit.recursion);
-}
-
void __netif_schedule(struct Qdisc *q);
void netif_schedule_queue(struct netdev_queue *txq);
diff --git a/net/core/dev.h b/net/core/dev.h
index 9d0f8b4587f81f4c12487f1783d8ba5cc49fc1d6..8572d2c8dc4adce75c98868c888363e6a32e0f52 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -150,4 +150,21 @@ static inline void xdp_do_check_flushed(struct napi_struct *napi) { }
struct napi_struct *napi_by_id(unsigned int napi_id);
void kick_defer_list_purge(struct softnet_data *sd, unsigned int cpu);
+#define XMIT_RECURSION_LIMIT 8
+static inline bool dev_xmit_recursion(void)
+{
+ return unlikely(__this_cpu_read(softnet_data.xmit.recursion) >
+ XMIT_RECURSION_LIMIT);
+}
+
+static inline void dev_xmit_recursion_inc(void)
+{
+ __this_cpu_inc(softnet_data.xmit.recursion);
+}
+
+static inline void dev_xmit_recursion_dec(void)
+{
+ __this_cpu_dec(softnet_data.xmit.recursion);
+}
+
#endif
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 1/8] net: move kick_defer_list_purge() to net/core/dev.h Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 2/8] net: move dev_xmit_recursion() helpers " Eric Dumazet
@ 2024-03-28 17:03 ` Eric Dumazet
2024-03-29 3:21 ` Jason Xing
2024-03-28 17:03 ` [PATCH net-next 4/8] net: make softnet_data.dropped an atomic_t Eric Dumazet
` (5 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
If the device attached to the packet given to enqueue_to_backlog()
is not running, we drop the packet.
But we accidentally increase sd->dropped, giving false signals
to admins: sd->dropped should be reserved to cpu backlog pressure,
not to temporary glitches at device dismantles.
While we are at it, perform the netif_running() test before
we get the rps lock, and use REASON_DEV_READY
drop reason instead of NOT_SPECIFIED.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/core/dev.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 5d36a634f468ffdeaca598c3dd033fe06d240bd0..af7a34b0a7d6683c6ffb21dd3388ed678473d95e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4791,12 +4791,13 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
unsigned long flags;
unsigned int qlen;
- reason = SKB_DROP_REASON_NOT_SPECIFIED;
+ reason = SKB_DROP_REASON_DEV_READY;
+ if (!netif_running(skb->dev))
+ goto bad_dev;
+
sd = &per_cpu(softnet_data, cpu);
backlog_lock_irq_save(sd, &flags);
- if (!netif_running(skb->dev))
- goto drop;
qlen = skb_queue_len(&sd->input_pkt_queue);
if (qlen <= READ_ONCE(net_hotdata.max_backlog) &&
!skb_flow_limit(skb, qlen)) {
@@ -4817,10 +4818,10 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
}
reason = SKB_DROP_REASON_CPU_BACKLOG;
-drop:
sd->dropped++;
backlog_unlock_irq_restore(sd, &flags);
+bad_dev:
dev_core_stats_rx_dropped_inc(skb->dev);
kfree_skb_reason(skb, reason);
return NET_RX_DROP;
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next 4/8] net: make softnet_data.dropped an atomic_t
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
` (2 preceding siblings ...)
2024-03-28 17:03 ` [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device Eric Dumazet
@ 2024-03-28 17:03 ` Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 5/8] net: enqueue_to_backlog() cleanup Eric Dumazet
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
If under extreme cpu backlog pressure enqueue_to_backlog() has
to drop a packet, it could do this without dirtying a cache line
and potentially slowing down the target cpu.
Move sd->dropped into a separate cache line, and make it atomic.
In non pressure mode, this field is not touched, no need to consume
valuable space in a hot cache line.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/netdevice.h | 3 ++-
net/core/dev.c | 13 +++++++++----
net/core/net-procfs.c | 3 ++-
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 70775021cc269e0983f538619115237b0067d408..1c31cd2691d32064613836141fbdeeebc831b21f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3236,10 +3236,11 @@ struct softnet_data {
unsigned int input_queue_tail;
#endif
unsigned int received_rps;
- unsigned int dropped;
struct sk_buff_head input_pkt_queue;
struct napi_struct backlog;
+ atomic_t dropped ____cacheline_aligned_in_smp;
+
/* Another possibly contended cache line */
spinlock_t defer_lock ____cacheline_aligned_in_smp;
int defer_count;
diff --git a/net/core/dev.c b/net/core/dev.c
index af7a34b0a7d6683c6ffb21dd3388ed678473d95e..be2392ba56bc57bed456e2748b332d4971c83a4e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4790,17 +4790,22 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
struct softnet_data *sd;
unsigned long flags;
unsigned int qlen;
+ int max_backlog;
reason = SKB_DROP_REASON_DEV_READY;
if (!netif_running(skb->dev))
goto bad_dev;
+ reason = SKB_DROP_REASON_CPU_BACKLOG;
sd = &per_cpu(softnet_data, cpu);
+ qlen = skb_queue_len_lockless(&sd->input_pkt_queue);
+ max_backlog = READ_ONCE(net_hotdata.max_backlog);
+ if (unlikely(qlen > max_backlog))
+ goto cpu_backlog_drop;
backlog_lock_irq_save(sd, &flags);
qlen = skb_queue_len(&sd->input_pkt_queue);
- if (qlen <= READ_ONCE(net_hotdata.max_backlog) &&
- !skb_flow_limit(skb, qlen)) {
+ if (qlen <= max_backlog && !skb_flow_limit(skb, qlen)) {
if (qlen) {
enqueue:
__skb_queue_tail(&sd->input_pkt_queue, skb);
@@ -4816,11 +4821,11 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
napi_schedule_rps(sd);
goto enqueue;
}
- reason = SKB_DROP_REASON_CPU_BACKLOG;
- sd->dropped++;
backlog_unlock_irq_restore(sd, &flags);
+cpu_backlog_drop:
+ atomic_inc(&sd->dropped);
bad_dev:
dev_core_stats_rx_dropped_inc(skb->dev);
kfree_skb_reason(skb, reason);
diff --git a/net/core/net-procfs.c b/net/core/net-procfs.c
index a97eceb84e61ec347ad132ff0f22c8ce82f12e90..fa6d3969734a6ec154c3444d1b25ee93edfc5588 100644
--- a/net/core/net-procfs.c
+++ b/net/core/net-procfs.c
@@ -144,7 +144,8 @@ static int softnet_seq_show(struct seq_file *seq, void *v)
seq_printf(seq,
"%08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x "
"%08x %08x\n",
- sd->processed, sd->dropped, sd->time_squeeze, 0,
+ sd->processed, atomic_read(&sd->dropped),
+ sd->time_squeeze, 0,
0, 0, 0, 0, /* was fastroute */
0, /* was cpu_collision */
sd->received_rps, flow_limit_count,
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next 5/8] net: enqueue_to_backlog() cleanup
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
` (3 preceding siblings ...)
2024-03-28 17:03 ` [PATCH net-next 4/8] net: make softnet_data.dropped an atomic_t Eric Dumazet
@ 2024-03-28 17:03 ` Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save() Eric Dumazet
` (3 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
We can remove a goto and a label by reversing a condition.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/core/dev.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index be2392ba56bc57bed456e2748b332d4971c83a4e..4e52745f23412bac6d3ff1b9f4d9f2ce4a2eb666 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4806,20 +4806,18 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
backlog_lock_irq_save(sd, &flags);
qlen = skb_queue_len(&sd->input_pkt_queue);
if (qlen <= max_backlog && !skb_flow_limit(skb, qlen)) {
- if (qlen) {
-enqueue:
- __skb_queue_tail(&sd->input_pkt_queue, skb);
- input_queue_tail_incr_save(sd, qtail);
- backlog_unlock_irq_restore(sd, &flags);
- return NET_RX_SUCCESS;
+ if (!qlen) {
+ /* Schedule NAPI for backlog device. We can use
+ * non atomic operation as we own the queue lock.
+ */
+ if (!__test_and_set_bit(NAPI_STATE_SCHED,
+ &sd->backlog.state))
+ napi_schedule_rps(sd);
}
-
- /* Schedule NAPI for backlog device
- * We can use non atomic operation since we own the queue lock
- */
- if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd->backlog.state))
- napi_schedule_rps(sd);
- goto enqueue;
+ __skb_queue_tail(&sd->input_pkt_queue, skb);
+ input_queue_tail_incr_save(sd, qtail);
+ backlog_unlock_irq_restore(sd, &flags);
+ return NET_RX_SUCCESS;
}
backlog_unlock_irq_restore(sd, &flags);
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save()
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
` (4 preceding siblings ...)
2024-03-28 17:03 ` [PATCH net-next 5/8] net: enqueue_to_backlog() cleanup Eric Dumazet
@ 2024-03-28 17:03 ` Eric Dumazet
2024-03-29 12:07 ` kernel test robot
2024-03-29 12:29 ` kernel test robot
2024-03-28 17:03 ` [PATCH net-next 7/8] net: rps: add rps_input_queue_head_add() helper Eric Dumazet
` (2 subsequent siblings)
8 siblings, 2 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
input_queue_tail_incr_save() is incrementing the sd queue_tail
and save it in the flow last_qtail.
Two issues here :
- no lock protects the write on last_qtail, we should use appropriate
annotations.
- We can perform this write after releasing the per-cpu backlog lock,
to decrease this lock hold duration (move away the cache line miss)
Also move input_queue_head_incr() and rps helpers to include/net/rps.h,
while adding rps_ prefix to better reflect their role.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/netdevice.h | 15 ---------------
include/net/rps.h | 23 +++++++++++++++++++++++
net/core/dev.c | 20 ++++++++++++--------
3 files changed, 35 insertions(+), 23 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1c31cd2691d32064613836141fbdeeebc831b21f..14f19cc2616452d7e6afbbaa52f8ad3e61a419e9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3249,21 +3249,6 @@ struct softnet_data {
call_single_data_t defer_csd;
};
-static inline void input_queue_head_incr(struct softnet_data *sd)
-{
-#ifdef CONFIG_RPS
- sd->input_queue_head++;
-#endif
-}
-
-static inline void input_queue_tail_incr_save(struct softnet_data *sd,
- unsigned int *qtail)
-{
-#ifdef CONFIG_RPS
- *qtail = ++sd->input_queue_tail;
-#endif
-}
-
DECLARE_PER_CPU_ALIGNED(struct softnet_data, softnet_data);
static inline int dev_recursion_level(void)
diff --git a/include/net/rps.h b/include/net/rps.h
index 7660243e905b92651a41292e04caf72c5f12f26e..c13f829b8556fda63e76544c332f2c089f0d6ea4 100644
--- a/include/net/rps.h
+++ b/include/net/rps.h
@@ -35,6 +35,29 @@ struct rps_dev_flow {
};
#define RPS_NO_FILTER 0xffff
+static inline u32 rps_input_queue_tail_incr(struct softnet_data *sd)
+{
+#ifdef CONFIG_RPS
+ return ++sd->input_queue_tail;
+#else
+ return 0;
+#endif
+}
+
+static inline void rps_input_queue_tail_save(u32 *dest, u32 tail)
+{
+#ifdef CONFIG_RPS
+ WRITE_ONCE(*dest, tail);
+#endif
+}
+
+static inline void rps_input_queue_head_incr(struct softnet_data *sd)
+{
+#ifdef CONFIG_RPS
+ sd->input_queue_head++;
+#endif
+}
+
/*
* The rps_dev_flow_table structure contains a table of flow mappings.
*/
diff --git a/net/core/dev.c b/net/core/dev.c
index 4e52745f23412bac6d3ff1b9f4d9f2ce4a2eb666..1fe7c6b10793d45a03461ee581d240d2442f9e17 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4601,7 +4601,7 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
if (unlikely(tcpu != next_cpu) &&
(tcpu >= nr_cpu_ids || !cpu_online(tcpu) ||
((int)(per_cpu(softnet_data, tcpu).input_queue_head -
- rflow->last_qtail)) >= 0)) {
+ READ_ONCE(rflow->last_qtail))) >= 0)) {
tcpu = next_cpu;
rflow = set_rps_cpu(dev, skb, rflow, next_cpu);
}
@@ -4656,7 +4656,7 @@ bool rps_may_expire_flow(struct net_device *dev, u16 rxq_index,
cpu = READ_ONCE(rflow->cpu);
if (rflow->filter == filter_id && cpu < nr_cpu_ids &&
((int)(per_cpu(softnet_data, cpu).input_queue_head -
- rflow->last_qtail) <
+ READ_ONCE(rflow->last_qtail)) <
(int)(10 * flow_table->mask)))
expire = false;
}
@@ -4791,6 +4791,7 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
unsigned long flags;
unsigned int qlen;
int max_backlog;
+ u32 tail;
reason = SKB_DROP_REASON_DEV_READY;
if (!netif_running(skb->dev))
@@ -4815,8 +4816,11 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
napi_schedule_rps(sd);
}
__skb_queue_tail(&sd->input_pkt_queue, skb);
- input_queue_tail_incr_save(sd, qtail);
+ tail = rps_input_queue_tail_incr(sd);
backlog_unlock_irq_restore(sd, &flags);
+
+ /* save the tail outside of the critical section */
+ rps_input_queue_tail_save(qtail, tail);
return NET_RX_SUCCESS;
}
@@ -5894,7 +5898,7 @@ static void flush_backlog(struct work_struct *work)
if (skb->dev->reg_state == NETREG_UNREGISTERING) {
__skb_unlink(skb, &sd->input_pkt_queue);
dev_kfree_skb_irq(skb);
- input_queue_head_incr(sd);
+ rps_input_queue_head_incr(sd);
}
}
backlog_unlock_irq_enable(sd);
@@ -5903,7 +5907,7 @@ static void flush_backlog(struct work_struct *work)
if (skb->dev->reg_state == NETREG_UNREGISTERING) {
__skb_unlink(skb, &sd->process_queue);
kfree_skb(skb);
- input_queue_head_incr(sd);
+ rps_input_queue_head_incr(sd);
}
}
local_bh_enable();
@@ -6031,7 +6035,7 @@ static int process_backlog(struct napi_struct *napi, int quota)
rcu_read_lock();
__netif_receive_skb(skb);
rcu_read_unlock();
- input_queue_head_incr(sd);
+ rps_input_queue_head_incr(sd);
if (++work >= quota)
return work;
@@ -11445,11 +11449,11 @@ static int dev_cpu_dead(unsigned int oldcpu)
/* Process offline CPU's input_pkt_queue */
while ((skb = __skb_dequeue(&oldsd->process_queue))) {
netif_rx(skb);
- input_queue_head_incr(oldsd);
+ rps_input_queue_head_incr(oldsd);
}
while ((skb = skb_dequeue(&oldsd->input_pkt_queue))) {
netif_rx(skb);
- input_queue_head_incr(oldsd);
+ rps_input_queue_head_incr(oldsd);
}
return 0;
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next 7/8] net: rps: add rps_input_queue_head_add() helper
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
` (5 preceding siblings ...)
2024-03-28 17:03 ` [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save() Eric Dumazet
@ 2024-03-28 17:03 ` Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 8/8] net: rps: move received_rps field to a better location Eric Dumazet
2024-03-29 5:45 ` [PATCH net-next 0/8] net: rps: misc changes Jakub Kicinski
8 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
process_backlog() can batch increments of sd->input_queue_head,
saving some memory bandwidth.
Also add READ_ONCE()/WRITE_ONCE() annotations around sd->input_queue_head
accesses.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/rps.h | 9 +++++++--
net/core/dev.c | 13 ++++++++-----
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/include/net/rps.h b/include/net/rps.h
index c13f829b8556fda63e76544c332f2c089f0d6ea4..135427bc6fcd29b9dad92a671c9a9f4efc975dec 100644
--- a/include/net/rps.h
+++ b/include/net/rps.h
@@ -51,13 +51,18 @@ static inline void rps_input_queue_tail_save(u32 *dest, u32 tail)
#endif
}
-static inline void rps_input_queue_head_incr(struct softnet_data *sd)
+static inline void rps_input_queue_head_add(struct softnet_data *sd, int val)
{
#ifdef CONFIG_RPS
- sd->input_queue_head++;
+ WRITE_ONCE(sd->input_queue_head, sd->input_queue_head + val);
#endif
}
+static inline void rps_input_queue_head_incr(struct softnet_data *sd)
+{
+ rps_input_queue_head_add(sd, 1);
+}
+
/*
* The rps_dev_flow_table structure contains a table of flow mappings.
*/
diff --git a/net/core/dev.c b/net/core/dev.c
index 1fe7c6b10793d45a03461ee581d240d2442f9e17..59e7fc30e8f03880340bfbeda0fa9e9ac757a168 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4518,7 +4518,7 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
out:
#endif
rflow->last_qtail =
- per_cpu(softnet_data, next_cpu).input_queue_head;
+ READ_ONCE(per_cpu(softnet_data, next_cpu).input_queue_head);
}
rflow->cpu = next_cpu;
@@ -4600,7 +4600,7 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
*/
if (unlikely(tcpu != next_cpu) &&
(tcpu >= nr_cpu_ids || !cpu_online(tcpu) ||
- ((int)(per_cpu(softnet_data, tcpu).input_queue_head -
+ ((int)(READ_ONCE(per_cpu(softnet_data, tcpu).input_queue_head) -
READ_ONCE(rflow->last_qtail))) >= 0)) {
tcpu = next_cpu;
rflow = set_rps_cpu(dev, skb, rflow, next_cpu);
@@ -4655,7 +4655,7 @@ bool rps_may_expire_flow(struct net_device *dev, u16 rxq_index,
rflow = &flow_table->flows[flow_id];
cpu = READ_ONCE(rflow->cpu);
if (rflow->filter == filter_id && cpu < nr_cpu_ids &&
- ((int)(per_cpu(softnet_data, cpu).input_queue_head -
+ ((int)(READ_ONCE(per_cpu(softnet_data, cpu).input_queue_head) -
READ_ONCE(rflow->last_qtail)) <
(int)(10 * flow_table->mask)))
expire = false;
@@ -6035,9 +6035,10 @@ static int process_backlog(struct napi_struct *napi, int quota)
rcu_read_lock();
__netif_receive_skb(skb);
rcu_read_unlock();
- rps_input_queue_head_incr(sd);
- if (++work >= quota)
+ if (++work >= quota) {
+ rps_input_queue_head_add(sd, work);
return work;
+ }
}
@@ -6060,6 +6061,8 @@ static int process_backlog(struct napi_struct *napi, int quota)
backlog_unlock_irq_enable(sd);
}
+ if (work)
+ rps_input_queue_head_add(sd, work);
return work;
}
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next 8/8] net: rps: move received_rps field to a better location
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
` (6 preceding siblings ...)
2024-03-28 17:03 ` [PATCH net-next 7/8] net: rps: add rps_input_queue_head_add() helper Eric Dumazet
@ 2024-03-28 17:03 ` Eric Dumazet
2024-03-29 5:45 ` [PATCH net-next 0/8] net: rps: misc changes Jakub Kicinski
8 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-28 17:03 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet
Commit 14d898f3c1b3 ("dev: Move received_rps counter next
to RPS members in softnet data") was unfortunate:
received_rps is dirtied by a cpu and never read by other
cpus in fast path.
Its presence in the hot RPS cache line (shared by many cpus)
is hurting RPS/RFS performance.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/netdevice.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 14f19cc2616452d7e6afbbaa52f8ad3e61a419e9..274d8db48b4858c70b43ea4628544e924ba6a263 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3203,6 +3203,7 @@ struct softnet_data {
struct softnet_data *rps_ipi_list;
#endif
+ unsigned int received_rps;
bool in_net_rx_action;
bool in_napi_threaded_poll;
@@ -3235,7 +3236,6 @@ struct softnet_data {
unsigned int cpu;
unsigned int input_queue_tail;
#endif
- unsigned int received_rps;
struct sk_buff_head input_pkt_queue;
struct napi_struct backlog;
--
2.44.0.478.gd926399ef9-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device
2024-03-28 17:03 ` [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device Eric Dumazet
@ 2024-03-29 3:21 ` Jason Xing
2024-03-29 6:31 ` Eric Dumazet
0 siblings, 1 reply; 16+ messages in thread
From: Jason Xing @ 2024-03-29 3:21 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet
On Fri, Mar 29, 2024 at 1:07 AM Eric Dumazet <edumazet@google.com> wrote:
>
> If the device attached to the packet given to enqueue_to_backlog()
> is not running, we drop the packet.
>
> But we accidentally increase sd->dropped, giving false signals
> to admins: sd->dropped should be reserved to cpu backlog pressure,
> not to temporary glitches at device dismantles.
It seems that drop action happening is intended in this case (see
commit e9e4dd3267d0c ("net: do not process device backlog during
unregistration")). We can see the strange/unexpected behaviour at
least through simply taking a look at /proc/net/softnet_stat file.
>
> While we are at it, perform the netif_running() test before
> we get the rps lock, and use REASON_DEV_READY
> drop reason instead of NOT_SPECIFIED.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> net/core/dev.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5d36a634f468ffdeaca598c3dd033fe06d240bd0..af7a34b0a7d6683c6ffb21dd3388ed678473d95e 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4791,12 +4791,13 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
> unsigned long flags;
> unsigned int qlen;
>
> - reason = SKB_DROP_REASON_NOT_SPECIFIED;
> + reason = SKB_DROP_REASON_DEV_READY;
> + if (!netif_running(skb->dev))
> + goto bad_dev;
> +
> sd = &per_cpu(softnet_data, cpu);
>
> backlog_lock_irq_save(sd, &flags);
> - if (!netif_running(skb->dev))
> - goto drop;
> qlen = skb_queue_len(&sd->input_pkt_queue);
> if (qlen <= READ_ONCE(net_hotdata.max_backlog) &&
> !skb_flow_limit(skb, qlen)) {
> @@ -4817,10 +4818,10 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
> }
> reason = SKB_DROP_REASON_CPU_BACKLOG;
>
> -drop:
> sd->dropped++;
> backlog_unlock_irq_restore(sd, &flags);
>
> +bad_dev:
> dev_core_stats_rx_dropped_inc(skb->dev);
> kfree_skb_reason(skb, reason);
> return NET_RX_DROP;
> --
> 2.44.0.478.gd926399ef9-goog
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 0/8] net: rps: misc changes
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
` (7 preceding siblings ...)
2024-03-28 17:03 ` [PATCH net-next 8/8] net: rps: move received_rps field to a better location Eric Dumazet
@ 2024-03-29 5:45 ` Jakub Kicinski
2024-03-29 6:44 ` Eric Dumazet
8 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2024-03-29 5:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, Paolo Abeni, netdev, eric.dumazet
On Thu, 28 Mar 2024 17:03:01 +0000 Eric Dumazet wrote:
> Make RPS/RFS a bit more efficient with better cache locality
> and heuristics.
>
> Aso shrink include/linux/netdevice.h a bit.
Looks like it breaks kunit build:
../net/core/dev.c: In function ‘enqueue_to_backlog’:
../net/core/dev.c:4829:24: error: implicit declaration of function ‘rps_input_queue_tail_incr’ [-Werror=implicit-function-declaration]
4829 | tail = rps_input_queue_tail_incr(sd);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
../net/core/dev.c:4833:17: error: implicit declaration of function ‘rps_input_queue_tail_save’ [-Werror=implicit-function-declaration]
4833 | rps_input_queue_tail_save(qtail, tail);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
../net/core/dev.c: In function ‘flush_backlog’:
../net/core/dev.c:5911:25: error: implicit declaration of function ‘rps_input_queue_head_incr’ [-Werror=implicit-function-declaration]
5911 | rps_input_queue_head_incr(sd);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
../net/core/dev.c: In function ‘process_backlog’:
../net/core/dev.c:6049:33: error: implicit declaration of function ‘rps_input_queue_head_add’ [-Werror=implicit-function-declaration]
6049 | rps_input_queue_head_add(sd, work);
| ^~~~~~~~~~~~~~~~~~~~~~~~
--
pw-bot: cr
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device
2024-03-29 3:21 ` Jason Xing
@ 2024-03-29 6:31 ` Eric Dumazet
2024-03-29 8:44 ` Jason Xing
0 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2024-03-29 6:31 UTC (permalink / raw)
To: Jason Xing
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet
On Fri, Mar 29, 2024 at 4:21 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> On Fri, Mar 29, 2024 at 1:07 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > If the device attached to the packet given to enqueue_to_backlog()
> > is not running, we drop the packet.
> >
> > But we accidentally increase sd->dropped, giving false signals
> > to admins: sd->dropped should be reserved to cpu backlog pressure,
> > not to temporary glitches at device dismantles.
>
> It seems that drop action happening is intended in this case (see
> commit e9e4dd3267d0c ("net: do not process device backlog during
> unregistration")). We can see the strange/unexpected behaviour at
> least through simply taking a look at /proc/net/softnet_stat file.
I disagree.
We are dismantling a device, temporary drops are expected, and this
patch adds a more precise drop_reason.
I have seen admins being worried about this counter being not zero on
carefully tuned hosts.
If you think we have to carry these drops forever in
/proc/net/softnet_stat, you will have give
a more precise reason than "This was intentionally added in 2015"
e9e4dd3267d0c5234c changelog was very long, but said nothing
about why sd->dropped _had_ to be updated.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 0/8] net: rps: misc changes
2024-03-29 5:45 ` [PATCH net-next 0/8] net: rps: misc changes Jakub Kicinski
@ 2024-03-29 6:44 ` Eric Dumazet
0 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2024-03-29 6:44 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David S . Miller, Paolo Abeni, netdev, eric.dumazet
On Fri, Mar 29, 2024 at 6:45 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 28 Mar 2024 17:03:01 +0000 Eric Dumazet wrote:
> > Make RPS/RFS a bit more efficient with better cache locality
> > and heuristics.
> >
> > Aso shrink include/linux/netdevice.h a bit.
>
> Looks like it breaks kunit build:
>
> ../net/core/dev.c: In function ‘enqueue_to_backlog’:
> ../net/core/dev.c:4829:24: error: implicit declaration of function ‘rps_input_queue_tail_incr’ [-Werror=implicit-function-declaration]
> 4829 | tail = rps_input_queue_tail_incr(sd);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> ../net/core/dev.c:4833:17: error: implicit declaration of function ‘rps_input_queue_tail_save’ [-Werror=implicit-function-declaration]
> 4833 | rps_input_queue_tail_save(qtail, tail);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> ../net/core/dev.c: In function ‘flush_backlog’:
> ../net/core/dev.c:5911:25: error: implicit declaration of function ‘rps_input_queue_head_incr’ [-Werror=implicit-function-declaration]
> 5911 | rps_input_queue_head_incr(sd);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> ../net/core/dev.c: In function ‘process_backlog’:
> ../net/core/dev.c:6049:33: error: implicit declaration of function ‘rps_input_queue_head_add’ [-Werror=implicit-function-declaration]
> 6049 | rps_input_queue_head_add(sd, work);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
Oh right, I need to define rps_input_queue_tail_incr() and friends
outside of the #ifdef CONFIG_RPS
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device
2024-03-29 6:31 ` Eric Dumazet
@ 2024-03-29 8:44 ` Jason Xing
0 siblings, 0 replies; 16+ messages in thread
From: Jason Xing @ 2024-03-29 8:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet
On Fri, Mar 29, 2024 at 2:31 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Mar 29, 2024 at 4:21 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > On Fri, Mar 29, 2024 at 1:07 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > If the device attached to the packet given to enqueue_to_backlog()
> > > is not running, we drop the packet.
> > >
> > > But we accidentally increase sd->dropped, giving false signals
> > > to admins: sd->dropped should be reserved to cpu backlog pressure,
> > > not to temporary glitches at device dismantles.
> >
> > It seems that drop action happening is intended in this case (see
> > commit e9e4dd3267d0c ("net: do not process device backlog during
> > unregistration")). We can see the strange/unexpected behaviour at
> > least through simply taking a look at /proc/net/softnet_stat file.
>
> I disagree.
>
> We are dismantling a device, temporary drops are expected, and this
> patch adds a more precise drop_reason.
>
> I have seen admins being worried about this counter being not zero on
> carefully tuned hosts.
>
> If you think we have to carry these drops forever in
> /proc/net/softnet_stat, you will have give
> a more precise reason than "This was intentionally added in 2015"
>
> e9e4dd3267d0c5234c changelog was very long, but said nothing
> about why sd->dropped _had_ to be updated.
Indeed, the author might think the skb is dropped in the RPS process
so it's necessary to record that and reflect these actions in this
proc file.
Don't get me wrong. I'm not against what you did, just proposing my
concern about changing the meaning/understanding of 'drop'.
Thanks,
Jason
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save()
2024-03-28 17:03 ` [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save() Eric Dumazet
@ 2024-03-29 12:07 ` kernel test robot
2024-03-29 12:29 ` kernel test robot
1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2024-03-29 12:07 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: oe-kbuild-all, netdev, eric.dumazet, Eric Dumazet
Hi Eric,
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/Eric-Dumazet/net-move-kick_defer_list_purge-to-net-core-dev-h/20240329-011413
base: net-next/main
patch link: https://lore.kernel.org/r/20240328170309.2172584-7-edumazet%40google.com
patch subject: [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save()
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20240329/202403291901.lqImStGD-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240329/202403291901.lqImStGD-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403291901.lqImStGD-lkp@intel.com/
All errors (new ones prefixed by >>):
net/core/dev.c: In function 'enqueue_to_backlog':
>> net/core/dev.c:4819:24: error: implicit declaration of function 'rps_input_queue_tail_incr' [-Werror=implicit-function-declaration]
4819 | tail = rps_input_queue_tail_incr(sd);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
>> net/core/dev.c:4823:17: error: implicit declaration of function 'rps_input_queue_tail_save' [-Werror=implicit-function-declaration]
4823 | rps_input_queue_tail_save(qtail, tail);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
net/core/dev.c: In function 'flush_backlog':
>> net/core/dev.c:5901:25: error: implicit declaration of function 'rps_input_queue_head_incr' [-Werror=implicit-function-declaration]
5901 | rps_input_queue_head_incr(sd);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/rps_input_queue_tail_incr +4819 net/core/dev.c
4781
4782 /*
4783 * enqueue_to_backlog is called to queue an skb to a per CPU backlog
4784 * queue (may be a remote CPU queue).
4785 */
4786 static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
4787 unsigned int *qtail)
4788 {
4789 enum skb_drop_reason reason;
4790 struct softnet_data *sd;
4791 unsigned long flags;
4792 unsigned int qlen;
4793 int max_backlog;
4794 u32 tail;
4795
4796 reason = SKB_DROP_REASON_DEV_READY;
4797 if (!netif_running(skb->dev))
4798 goto bad_dev;
4799
4800 reason = SKB_DROP_REASON_CPU_BACKLOG;
4801 sd = &per_cpu(softnet_data, cpu);
4802
4803 qlen = skb_queue_len_lockless(&sd->input_pkt_queue);
4804 max_backlog = READ_ONCE(net_hotdata.max_backlog);
4805 if (unlikely(qlen > max_backlog))
4806 goto cpu_backlog_drop;
4807 backlog_lock_irq_save(sd, &flags);
4808 qlen = skb_queue_len(&sd->input_pkt_queue);
4809 if (qlen <= max_backlog && !skb_flow_limit(skb, qlen)) {
4810 if (!qlen) {
4811 /* Schedule NAPI for backlog device. We can use
4812 * non atomic operation as we own the queue lock.
4813 */
4814 if (!__test_and_set_bit(NAPI_STATE_SCHED,
4815 &sd->backlog.state))
4816 napi_schedule_rps(sd);
4817 }
4818 __skb_queue_tail(&sd->input_pkt_queue, skb);
> 4819 tail = rps_input_queue_tail_incr(sd);
4820 backlog_unlock_irq_restore(sd, &flags);
4821
4822 /* save the tail outside of the critical section */
> 4823 rps_input_queue_tail_save(qtail, tail);
4824 return NET_RX_SUCCESS;
4825 }
4826
4827 backlog_unlock_irq_restore(sd, &flags);
4828
4829 cpu_backlog_drop:
4830 atomic_inc(&sd->dropped);
4831 bad_dev:
4832 dev_core_stats_rx_dropped_inc(skb->dev);
4833 kfree_skb_reason(skb, reason);
4834 return NET_RX_DROP;
4835 }
4836
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save()
2024-03-28 17:03 ` [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save() Eric Dumazet
2024-03-29 12:07 ` kernel test robot
@ 2024-03-29 12:29 ` kernel test robot
1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2024-03-29 12:29 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: llvm, oe-kbuild-all, netdev, eric.dumazet, Eric Dumazet
Hi Eric,
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/Eric-Dumazet/net-move-kick_defer_list_purge-to-net-core-dev-h/20240329-011413
base: net-next/main
patch link: https://lore.kernel.org/r/20240328170309.2172584-7-edumazet%40google.com
patch subject: [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save()
config: um-x86_64_defconfig (https://download.01.org/0day-ci/archive/20240329/202403292036.FloftiJL-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240329/202403292036.FloftiJL-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403292036.FloftiJL-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from net/core/dev.c:89:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from net/core/dev.c:89:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from net/core/dev.c:89:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> net/core/dev.c:4819:10: error: call to undeclared function 'rps_input_queue_tail_incr'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
tail = rps_input_queue_tail_incr(sd);
^
>> net/core/dev.c:4823:3: error: call to undeclared function 'rps_input_queue_tail_save'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
rps_input_queue_tail_save(qtail, tail);
^
>> net/core/dev.c:5901:4: error: call to undeclared function 'rps_input_queue_head_incr'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
rps_input_queue_head_incr(sd);
^
net/core/dev.c:5910:4: error: call to undeclared function 'rps_input_queue_head_incr'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
rps_input_queue_head_incr(sd);
^
net/core/dev.c:6038:4: error: call to undeclared function 'rps_input_queue_head_incr'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
rps_input_queue_head_incr(sd);
^
net/core/dev.c:11452:3: error: call to undeclared function 'rps_input_queue_head_incr'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
rps_input_queue_head_incr(oldsd);
^
net/core/dev.c:11456:3: error: call to undeclared function 'rps_input_queue_head_incr'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
rps_input_queue_head_incr(oldsd);
^
12 warnings and 7 errors generated.
vim +/rps_input_queue_tail_incr +4819 net/core/dev.c
4781
4782 /*
4783 * enqueue_to_backlog is called to queue an skb to a per CPU backlog
4784 * queue (may be a remote CPU queue).
4785 */
4786 static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
4787 unsigned int *qtail)
4788 {
4789 enum skb_drop_reason reason;
4790 struct softnet_data *sd;
4791 unsigned long flags;
4792 unsigned int qlen;
4793 int max_backlog;
4794 u32 tail;
4795
4796 reason = SKB_DROP_REASON_DEV_READY;
4797 if (!netif_running(skb->dev))
4798 goto bad_dev;
4799
4800 reason = SKB_DROP_REASON_CPU_BACKLOG;
4801 sd = &per_cpu(softnet_data, cpu);
4802
4803 qlen = skb_queue_len_lockless(&sd->input_pkt_queue);
4804 max_backlog = READ_ONCE(net_hotdata.max_backlog);
4805 if (unlikely(qlen > max_backlog))
4806 goto cpu_backlog_drop;
4807 backlog_lock_irq_save(sd, &flags);
4808 qlen = skb_queue_len(&sd->input_pkt_queue);
4809 if (qlen <= max_backlog && !skb_flow_limit(skb, qlen)) {
4810 if (!qlen) {
4811 /* Schedule NAPI for backlog device. We can use
4812 * non atomic operation as we own the queue lock.
4813 */
4814 if (!__test_and_set_bit(NAPI_STATE_SCHED,
4815 &sd->backlog.state))
4816 napi_schedule_rps(sd);
4817 }
4818 __skb_queue_tail(&sd->input_pkt_queue, skb);
> 4819 tail = rps_input_queue_tail_incr(sd);
4820 backlog_unlock_irq_restore(sd, &flags);
4821
4822 /* save the tail outside of the critical section */
> 4823 rps_input_queue_tail_save(qtail, tail);
4824 return NET_RX_SUCCESS;
4825 }
4826
4827 backlog_unlock_irq_restore(sd, &flags);
4828
4829 cpu_backlog_drop:
4830 atomic_inc(&sd->dropped);
4831 bad_dev:
4832 dev_core_stats_rx_dropped_inc(skb->dev);
4833 kfree_skb_reason(skb, reason);
4834 return NET_RX_DROP;
4835 }
4836
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-03-29 12:29 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-28 17:03 [PATCH net-next 0/8] net: rps: misc changes Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 1/8] net: move kick_defer_list_purge() to net/core/dev.h Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 2/8] net: move dev_xmit_recursion() helpers " Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 3/8] net: enqueue_to_backlog() change vs not running device Eric Dumazet
2024-03-29 3:21 ` Jason Xing
2024-03-29 6:31 ` Eric Dumazet
2024-03-29 8:44 ` Jason Xing
2024-03-28 17:03 ` [PATCH net-next 4/8] net: make softnet_data.dropped an atomic_t Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 5/8] net: enqueue_to_backlog() cleanup Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 6/8] net: rps: change input_queue_tail_incr_save() Eric Dumazet
2024-03-29 12:07 ` kernel test robot
2024-03-29 12:29 ` kernel test robot
2024-03-28 17:03 ` [PATCH net-next 7/8] net: rps: add rps_input_queue_head_add() helper Eric Dumazet
2024-03-28 17:03 ` [PATCH net-next 8/8] net: rps: move received_rps field to a better location Eric Dumazet
2024-03-29 5:45 ` [PATCH net-next 0/8] net: rps: misc changes Jakub Kicinski
2024-03-29 6:44 ` Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.