[ovs-dev] [PATCH] dpif-netdev: Do not mix recirculation depth into RSS hash itself.

2019-10-24 Thread Ilya Maximets
Mixing of RSS hash with recirculation depth is useful for flow lookup
because same packet after recirculation should match with different
datapath rule.  Setting of the mixed value back to the packet is
completely unnecessary because recirculation depth is different on
each recirculation, i.e. we will have different packet hash for
flow lookup anyway.

This should fix the issue that packets from the same flow could be
directed to different buckets based on a dp_hash or different ports of
a balanced bonding in case they were recirculated different number of
times (e.g. due to conntrack rules).
With this change, the original RSS hash will remain the same making
it possible to calculate equal dp_hash values for such packets.

Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/363127.html
Fixes: 048963aa8507 ("dpif-netdev: Reset RSS hash when recirculating.")
Signed-off-by: Ilya Maximets 
---
 lib/dpif-netdev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4546b55e8..c09b8fd95 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6288,7 +6288,6 @@ dpif_netdev_packet_get_rss_hash(struct dp_packet *packet,
 recirc_depth = *recirc_depth_get_unsafe();
 if (OVS_UNLIKELY(recirc_depth)) {
 hash = hash_finish(hash, recirc_depth);
-dp_packet_set_rss_hash(packet, hash);
 }
 return hash;
 }
-- 
2.17.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Do not mix recirculation depth into RSS hash itself.

2019-10-24 Thread Jan Scheurich via dev
Even simpler solution to the problem.
Acked-by: Jan Scheurich 

BR, Jan

> -Original Message-
> From: Ilya Maximets 
> Sent: Thursday, 24 October, 2019 14:32
> To: ovs-dev@openvswitch.org
> Cc: Ian Stokes ; Kevin Traynor ;
> Jan Scheurich ; ychen103...@163.com; Ilya
> Maximets 
> Subject: [PATCH] dpif-netdev: Do not mix recirculation depth into RSS hash
> itself.
> 
> Mixing of RSS hash with recirculation depth is useful for flow lookup because
> same packet after recirculation should match with different datapath rule.
> Setting of the mixed value back to the packet is completely unnecessary
> because recirculation depth is different on each recirculation, i.e. we will 
> have
> different packet hash for flow lookup anyway.
> 
> This should fix the issue that packets from the same flow could be directed to
> different buckets based on a dp_hash or different ports of a balanced bonding
> in case they were recirculated different number of times (e.g. due to 
> conntrack
> rules).
> With this change, the original RSS hash will remain the same making it 
> possible
> to calculate equal dp_hash values for such packets.
> 
> Reported-at: https://protect2.fireeye.com/v1/url?k=0a51a6c3-56db840c-
> 0a51e658-0cc47ad93ea4-b7f1e9be8f7bbef8&q=1&e=c9f55798-de3c-45f4-afeb-
> 9a87d3d594ca&u=https%3A%2F%2Fmail.openvswitch.org%2Fpipermail%2Fovs-
> dev%2F2019-September%2F363127.html
> Fixes: 048963aa8507 ("dpif-netdev: Reset RSS hash when recirculating.")
> Signed-off-by: Ilya Maximets 
> ---
>  lib/dpif-netdev.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 4546b55e8..c09b8fd95
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -6288,7 +6288,6 @@ dpif_netdev_packet_get_rss_hash(struct dp_packet
> *packet,
>  recirc_depth = *recirc_depth_get_unsafe();
>  if (OVS_UNLIKELY(recirc_depth)) {
>  hash = hash_finish(hash, recirc_depth);
> -dp_packet_set_rss_hash(packet, hash);
>  }
>  return hash;
>  }
> --
> 2.17.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Do not mix recirculation depth into RSS hash itself.

2019-10-28 Thread Ilya Maximets

On 24.10.2019 15:36, Jan Scheurich wrote:

Even simpler solution to the problem.
Acked-by: Jan Scheurich 

BR, Jan


-Original Message-
From: Ilya Maximets 
Sent: Thursday, 24 October, 2019 14:32
To: ovs-dev@openvswitch.org
Cc: Ian Stokes ; Kevin Traynor ;
Jan Scheurich ; ychen103...@163.com; Ilya
Maximets 
Subject: [PATCH] dpif-netdev: Do not mix recirculation depth into RSS hash
itself.

Mixing of RSS hash with recirculation depth is useful for flow lookup because
same packet after recirculation should match with different datapath rule.
Setting of the mixed value back to the packet is completely unnecessary
because recirculation depth is different on each recirculation, i.e. we will 
have
different packet hash for flow lookup anyway.

This should fix the issue that packets from the same flow could be directed to
different buckets based on a dp_hash or different ports of a balanced bonding
in case they were recirculated different number of times (e.g. due to conntrack
rules).
With this change, the original RSS hash will remain the same making it possible
to calculate equal dp_hash values for such packets.

Reported-at: https://protect2.fireeye.com/v1/url?k=0a51a6c3-56db840c-
0a51e658-0cc47ad93ea4-b7f1e9be8f7bbef8&q=1&e=c9f55798-de3c-45f4-afeb-
9a87d3d594ca&u=https%3A%2F%2Fmail.openvswitch.org%2Fpipermail%2Fovs-
dev%2F2019-September%2F363127.html
Fixes: 048963aa8507 ("dpif-netdev: Reset RSS hash when recirculating.")
Signed-off-by: Ilya Maximets 
---
  lib/dpif-netdev.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 4546b55e8..c09b8fd95
100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6288,7 +6288,6 @@ dpif_netdev_packet_get_rss_hash(struct dp_packet
*packet,
  recirc_depth = *recirc_depth_get_unsafe();
  if (OVS_UNLIKELY(recirc_depth)) {
  hash = hash_finish(hash, recirc_depth);
-dp_packet_set_rss_hash(packet, hash);
  }
  return hash;
  }
--
2.17.1




Thanks, Jan! Applied to master and backported.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Do not mix recirculation depth into RSS hash itself.

2019-10-30 Thread ychen
Thanks!
I have simply verified in our testing enviroment, and it really worked!








At 2019-10-24 20:32:11, "Ilya Maximets"  wrote:
>Mixing of RSS hash with recirculation depth is useful for flow lookup
>because same packet after recirculation should match with different
>datapath rule.  Setting of the mixed value back to the packet is
>completely unnecessary because recirculation depth is different on
>each recirculation, i.e. we will have different packet hash for
>flow lookup anyway.
>
>This should fix the issue that packets from the same flow could be
>directed to different buckets based on a dp_hash or different ports of
>a balanced bonding in case they were recirculated different number of
>times (e.g. due to conntrack rules).
>With this change, the original RSS hash will remain the same making
>it possible to calculate equal dp_hash values for such packets.
>
>Reported-at: 
>https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/363127.html
>Fixes: 048963aa8507 ("dpif-netdev: Reset RSS hash when recirculating.")
>Signed-off-by: Ilya Maximets 
>---
> lib/dpif-netdev.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index 4546b55e8..c09b8fd95 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -6288,7 +6288,6 @@ dpif_netdev_packet_get_rss_hash(struct dp_packet *packet,
> recirc_depth = *recirc_depth_get_unsafe();
> if (OVS_UNLIKELY(recirc_depth)) {
> hash = hash_finish(hash, recirc_depth);
>-dp_packet_set_rss_hash(packet, hash);
> }
> return hash;
> }
>-- 
>2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] dpif-netdev: Do not mix recirculation depth into RSS hash itself.

2019-10-30 Thread Ilya Maximets

On 30.10.2019 9:51, ychen wrote:

Thanks!
I have simply verified in our testing enviroment, and it really worked!


That's good. Thank you for testing!

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev