Re: [ovs-dev] [PATCH] ofproto-dpif: Increase dp_hash default max buckets

2021-10-15 Thread Eelco Chaudron
Hi Mike,

See some comments below…

Cheers,

Eelco

On 15 Oct 2021, at 4:05, Mike Pattrick wrote:

> Currently when a user creates an open flow group with with multiple
> buckets without specifying a selection type, the efficient dp_hash is
> only selected if there are fewer than 64 buckets. But when dp_hash is
> explicitly selected, up to 256 buckets are supported.
>
> While 64 buckets seems like a lot, certain OVN/Open Stack workloads
> could result in more than 64 buckets. For example, when using OVN to
> load balance. This patch increases the maximum default from 64 to 256.
>
> Signed-off-by: Mike Pattrick 
> ---
>  ofproto/ofproto-dpif.c   | 4 ++--
>  utilities/ovs-ofctl.8.in | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index cba49a99e..6e8ec50ae 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5152,9 +5152,9 @@ group_set_selection_method(struct group_dpif *group)
>  if (selection_method[0] == '\0') {
>  VLOG_DBG("No selection method specified. Trying dp_hash.");
>  /* If the controller has not specified a selection method, check if
> - * the dp_hash selection method with max 64 hash values is 
> appropriate
> + * the dp_hash selection method with max 256 hash values is 
> appropriate
>   * for the given bucket configuration. */
> -if (group_setup_dp_hash_table(group, 64)) {
> +if (group_setup_dp_hash_table(group, 0)) {

I would just put in the number 256 here, rather than 0, because if someone 
changes MAX_SELECT_GROUP_HASH_VALUES it could behave differently.

Also, are you sure just changing this value will create more buckets? It seems 
the parameter is only used to check the maximum potential buckets, not the 
number actually created? Or are you implying that with 64 it’s falling back to 
SEL_METHOD_DEFAULT?

>  /* Use dp_hash selection method with symmetric L4 hash. */
>  group->selection_method = SEL_METHOD_DP_HASH;
>  group->hash_alg = OVS_HASH_ALG_SYM_L4;
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index 2017c6eba..f5e3963cf 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -1084,7 +1084,7 @@ otherwise.  If no selection method is specified, Open 
> vSwitch up to
>  release 2.9 applies the \fBhash\fR method with default fields. From
>  2.10 onwards Open vSwitch defaults to the \fBdp_hash\fR method with symmetric
>  L3/L4 hash algorithm, unless the weighted group buckets cannot be mapped to
> -a maximum of 64 dp_hash values with sufficient accuracy.
> +a maximum of 256 dp_hash values with sufficient accuracy.

Changing the text this way implies that from 2.10 onwards, we have 256 dp_hash 
values. Maybe you can reword it, so 256 is from 2.17 onwards.

>  In those rare cases Open vSwitch 2.10 and later fall back to the \fBhash\fR
>  method with the default set of hash fields.
>  .RS
> -- 
> 2.30.2
>
>
>
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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


[ovs-dev] [PATCH] ofproto-dpif: Increase dp_hash default max buckets

2021-10-14 Thread Mike Pattrick
Currently when a user creates an open flow group with with multiple
buckets without specifying a selection type, the efficient dp_hash is
only selected if there are fewer than 64 buckets. But when dp_hash is
explicitly selected, up to 256 buckets are supported.

While 64 buckets seems like a lot, certain OVN/Open Stack workloads
could result in more than 64 buckets. For example, when using OVN to
load balance. This patch increases the maximum default from 64 to 256.

Signed-off-by: Mike Pattrick 
---
 ofproto/ofproto-dpif.c   | 4 ++--
 utilities/ovs-ofctl.8.in | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index cba49a99e..6e8ec50ae 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5152,9 +5152,9 @@ group_set_selection_method(struct group_dpif *group)
 if (selection_method[0] == '\0') {
 VLOG_DBG("No selection method specified. Trying dp_hash.");
 /* If the controller has not specified a selection method, check if
- * the dp_hash selection method with max 64 hash values is appropriate
+ * the dp_hash selection method with max 256 hash values is appropriate
  * for the given bucket configuration. */
-if (group_setup_dp_hash_table(group, 64)) {
+if (group_setup_dp_hash_table(group, 0)) {
 /* Use dp_hash selection method with symmetric L4 hash. */
 group->selection_method = SEL_METHOD_DP_HASH;
 group->hash_alg = OVS_HASH_ALG_SYM_L4;
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 2017c6eba..f5e3963cf 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1084,7 +1084,7 @@ otherwise.  If no selection method is specified, Open 
vSwitch up to
 release 2.9 applies the \fBhash\fR method with default fields. From
 2.10 onwards Open vSwitch defaults to the \fBdp_hash\fR method with symmetric
 L3/L4 hash algorithm, unless the weighted group buckets cannot be mapped to
-a maximum of 64 dp_hash values with sufficient accuracy.
+a maximum of 256 dp_hash values with sufficient accuracy.
 In those rare cases Open vSwitch 2.10 and later fall back to the \fBhash\fR
 method with the default set of hash fields.
 .RS
-- 
2.30.2




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


Re: [ovs-dev] [PATCH] ofproto-dpif: Increase dp_hash default max buckets

2021-10-14 Thread 0-day Robot
Bleep bloop.  Greetings Mike Pattrick, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Mike Pattrick  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Michael Pattrick 
Lines checked: 52, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto-dpif: Increase dp_hash default max buckets

2021-10-14 Thread Mike Pattrick
Currently when a user creates an open flow group with with multiple
buckets without specifying a selection type, the efficient dp_hash is
only selected if there are fewer than 64 buckets. But when dp_hash is
explicitly selected, up to 256 buckets are supported.

While 64 buckets seems like a lot, certain OVN/Open Stack workloads
could result in more than 64 buckets. For example, when using OVN to
load balance. This patch increases the maximum default from 64 to 256.

Signed-off-by: Michael Pattrick 
---
 ofproto/ofproto-dpif.c   | 4 ++--
 utilities/ovs-ofctl.8.in | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index cba49a99e..6e8ec50ae 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5152,9 +5152,9 @@ group_set_selection_method(struct group_dpif *group)
 if (selection_method[0] == '\0') {
 VLOG_DBG("No selection method specified. Trying dp_hash.");
 /* If the controller has not specified a selection method, check if
- * the dp_hash selection method with max 64 hash values is appropriate
+ * the dp_hash selection method with max 256 hash values is appropriate
  * for the given bucket configuration. */
-if (group_setup_dp_hash_table(group, 64)) {
+if (group_setup_dp_hash_table(group, 0)) {
 /* Use dp_hash selection method with symmetric L4 hash. */
 group->selection_method = SEL_METHOD_DP_HASH;
 group->hash_alg = OVS_HASH_ALG_SYM_L4;
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 2017c6eba..f5e3963cf 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1084,7 +1084,7 @@ otherwise.  If no selection method is specified, Open 
vSwitch up to
 release 2.9 applies the \fBhash\fR method with default fields. From
 2.10 onwards Open vSwitch defaults to the \fBdp_hash\fR method with symmetric
 L3/L4 hash algorithm, unless the weighted group buckets cannot be mapped to
-a maximum of 64 dp_hash values with sufficient accuracy.
+a maximum of 256 dp_hash values with sufficient accuracy.
 In those rare cases Open vSwitch 2.10 and later fall back to the \fBhash\fR
 method with the default set of hash fields.
 .RS
-- 
2.30.2



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