| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4E783855.4020907@dev.mellanox.co.il>
Date: Tue, 20 Sep 2011 09:53:09 +0300
From: Amir Vadai <amirv@....mellanox.co.il>
To: Ben Hutchings <bhutchings@...arflare.com>
CC: Tom Herbert <therbert@...gle.com>, oren@...lanox.co.il,
liranl@...lanox.co.il, netdev@...r.kernel.org,
amirv@...lanox.co.il, Diego Crupnicoff <Diego@...lanox.com>
Subject: Re: RFS issue: no HW filter for paused stream
This will unset the current CPU of the rflow that belongs to the desired
CPU.
The problem is when the stream resumes and it goes to the wrong RXQ - in
our HW, it will be according to RSS, as long as there is no specific
flow steering rule for the stream.
We need to unset the current CPU of the rflow of the actual RXQ that the
packet arrived at:
diff --git a/net/core/dev.c b/net/core/dev.c
index 4b9981c..a6b3bc8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2685,6 +2685,12 @@ set_rps_cpu(struct net_device *dev, struct
sk_buff *skb,
rflow = &flow_table->flows[flow_id];
rflow->cpu = next_cpu;
rflow->filter = rc;
+ /* If this flow (or a flow with the same hash value) becomes
+ * active on the CPU as before, we want to restore the
+ * hardware filter. Unset the current CPU to ensure that
+ * set_rps_cpu() will be called then.
+ */
+ old_rflow->cpu = RPS_NO_CPU;
if (old_rflow->filter == rflow->filter)
old_rflow->filter = RPS_NO_FILTER;
out:
Or even better, not set it in the first place - but I'm not sure I
undersdtand the implications on RPS:
diff --git a/net/core/dev.c b/net/core/dev.c
index 4b9981c..748acdb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2654,7 +2654,7 @@ set_rps_cpu(struct net_device *dev, struct sk_buff
*skb,
{
u16 tcpu;
- tcpu = rflow->cpu = next_cpu;
+ tcpu = next_cpu;
if (tcpu != RPS_NO_CPU) {
#ifdef CONFIG_RFS_ACCEL
struct netdev_rx_queue *rxqueue;
- Amir
On 09/19/2011 06:52 PM, Ben Hutchings wrote:
> On Mon, 2011-09-19 at 08:13 -0700, Tom Herbert wrote:
>> Ben: Once a accel RFS flow expires (because flow is idle?), how
>> should it get re-instantiated if thread's CPU doesn't change?
>
> Good question.
>
>> Tom
>>
>> On Sun, Sep 18, 2011 at 11:05 PM, Amir Vadai<amirv@....mellanox.co.il> wrote:
>>> (Resending in plain text)
>>>
>>> Tom Hi,
>>> When a stream is paused, and its rule is expired while it is paused,
>>> no new rule will be configured to the HW when traffic resume.
>>>
>>> Scenario:
>>> 1. Start iperf.
>>> 2. Pause it using Ctrl-Z
>>> 3. Start another iperf (to make sure first stream rule is expired)
>>> 4. Stop the second stream.
>>> 5. Resume first stream. Traffic is not steered to the right rx-queue.
>>>
>>> From looking at the code:
>>> - When first stream started, RSS steered traffic to rx-queue 'x'.
>>> Because iperf server was running on a different CPU, a new rule was
>>> added and current-cpu was set to desired-cpu.
>>> - After paused, rule was expired and removed from HW by net driver.
>>> But current-cpu wasn't cleared and still is equal to desired-cpu.
>>> - When stream was resumed, traffic was steered again by RSS, and
>>> because current-cpu was equal to desired-cpu, ndo_rx_flow_steer
>>> wasn't called and no rule was configured to the HW.
>>>
>>> Why isn't current-cpu cleared when expiring a rule?
>
> Because I wrongly assumed that rules could be independently expired by
> the driver and the RPS/RFS core code.
>
> Try this (I haven't tested it myself yet):
>
> From: Ben Hutchings<bhutchings@...arflare.com>
> Date: Mon, 19 Sep 2011 16:44:13 +0100
> Subject: [PATCH net-next] RPS: When a hardware filter is expired, ensure it
> can be re-added later
>
> Amir Vadai wrote:
>> When a stream is paused, and its rule is expired while it is paused,
>> no new rule will be configured to the HW when traffic resume.
> [...]
>> - When stream was resumed, traffic was steered again by RSS, and
>> because current-cpu was equal to desired-cpu, ndo_rx_flow_steer
>> wasn't called and no rule was configured to the HW.
>>
>> Why isn't current-cpu cleared when expiring a rule?
>
> When rps_may_expire_flow() matches a filter to a flow that is found to
> be idle, unset the current CPU for that flow.
>
> Reported-by: Amir Vadai<amirv@....mellanox.co.il>
> ---
> net/core/dev.c | 21 ++++++++++++++++-----
> 1 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b2e262e..3caf65a 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2817,11 +2817,22 @@ bool rps_may_expire_flow(struct net_device *dev, u16 rxq_index,
> if (flow_table&& flow_id<= flow_table->mask) {
> rflow =&flow_table->flows[flow_id];
> cpu = ACCESS_ONCE(rflow->cpu);
> - if (rflow->filter == filter_id&& cpu != RPS_NO_CPU&&
> - ((int)(per_cpu(softnet_data, cpu).input_queue_head -
> - rflow->last_qtail)<
> - (int)(10 * flow_table->mask)))
> - expire = false;
> + if (rflow->filter == filter_id&& cpu != RPS_NO_CPU) {
> + if ((int)(per_cpu(softnet_data, cpu).input_queue_head -
> + rflow->last_qtail)<
> + (int)(10 * flow_table->mask)) {
> + expire = false;
> + } else {
> + /* If this flow (or a flow with the
> + * same hash value) becomes active
> + * on the CPU as before, we want to
> + * restore the hardware filter. Unset
> + * the current CPU to ensure that
> + * set_rps_cpu() will be called then.
> + */
> + rflow->cpu = RPS_NO_CPU;
> + }
> + }
> }
> rcu_read_unlock();
> return expire;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists