| Message ID | 0fd9ff8df81ea1300ab3247b7c8f64a4e069d97d.camel@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] ofproto-dpif: Increase dp_hash default max buckets. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Thu, Oct 21, 2021, at 00:32, Mike Pattrick wrote: > Currently when a user creates an openflow group with with multiple > buckets without specifying a selection type, the efficient dp_hash is > only selected if the user is creating fewer than 64 buckets. But when > dp_hash is explicitly selected, up to 256 buckets are supported. > > While up to 64 buckets seems like a lot, certain OVN/Open Stack > workloads could result in the user creating more than 64 buckets. For > example, when using OVN to load balance. This patch increases the > default maximum from 64 to 256. > > This change to the default limit doesn't affect how many buckets are > actually created, that is specified by the user when the group is > created, just how traffic is distributed across buckets. > > Signed-off-by: Mike Pattrick <mkp@redhat.com> Going back to the patch from Jan Scheurich introducing the 64 buckets limit, I do not find a justification for this value. I have not found a good explanation for the 256 limit either. Reading group_setup_dp_hash_table() it is not clear why it's in place. So with the caveat that the big picture is eluding me, this change seems ok. It seems reasonable to improve the group selection method to have a better chance to use the more efficient method, while respecting the limits imposed by the implementation. It would be better to get to the bottom of it but it's not justification enough to block this change. Tests are passing, in particular the ones regarding group selection. Acked-by: Gaetan Rivet <grive@u256.net>
On 10/21/21 00:32, Mike Pattrick wrote: > Currently when a user creates an openflow group with with multiple > buckets without specifying a selection type, the efficient dp_hash is > only selected if the user is creating fewer than 64 buckets. But when > dp_hash is explicitly selected, up to 256 buckets are supported. > > While up to 64 buckets seems like a lot, certain OVN/Open Stack > workloads could result in the user creating more than 64 buckets. For > example, when using OVN to load balance. This patch increases the > default maximum from 64 to 256. > > This change to the default limit doesn't affect how many buckets are > actually created, that is specified by the user when the group is > created, just how traffic is distributed across buckets. > > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- Hey, Mike. Thanks for the patch! This looks like a user-visible change, so could you, please, add a couple of lines of a high-level description to the NEWS file for it? Bets regards, Ilya Maximets.
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index cba49a99e..bc3df8ea1 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, 256)) { /* 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..94a55dbc5 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1083,8 +1083,9 @@ This field is optional for \fBadd\-group\fR, \fBadd\-groups\fR and 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. +L3/L4 hash algorithm, as long as the weighted group buckets can be mapped to +dp_hash values with sufficient accuracy. In 2.10 this was restricted to a +maximum of 64 buckets, and in 2.17 the limit was raised to 256 buckets. 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
Currently when a user creates an openflow group with with multiple buckets without specifying a selection type, the efficient dp_hash is only selected if the user is creating fewer than 64 buckets. But when dp_hash is explicitly selected, up to 256 buckets are supported. While up to 64 buckets seems like a lot, certain OVN/Open Stack workloads could result in the user creating more than 64 buckets. For example, when using OVN to load balance. This patch increases the default maximum from 64 to 256. This change to the default limit doesn't affect how many buckets are actually created, that is specified by the user when the group is created, just how traffic is distributed across buckets. Signed-off-by: Mike Pattrick <mkp@redhat.com> --- ofproto/ofproto-dpif.c | 4 ++-- utilities/ovs-ofctl.8.in | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-)