No, thank you for remembering to prompt for the documentation, I would not have noticed the bug otherwise!
v3 pushed to master, Jarno > On Sep 15, 2016, at 7:26 PM, Ben Pfaff <b...@ovn.org> wrote: > > OK. Thanks for being so careful! > > I'll look at v3. > > On Thu, Sep 15, 2016 at 06:43:39PM -0700, Jarno Rajahalme wrote: >> Thanks for the fix! >> >> While I was working with tightening the parsing, I found that I had earlier >> introduced a bug that crashes ovs-ofctl when a parsing error is found after >> parsing ‘fields’, essentially a double free. I sent a v3 fixing this, >> documenting the parsing tightening and then rebasing this patch with this >> fix, additional documentation and with the NEWS piece moved to the post-2.6b >> section. >> >> Jarno >> >>> On Sep 14, 2016, at 8:53 PM, Ben Pfaff <b...@ovn.org> wrote: >>> >>> On Wed, Sep 14, 2016 at 08:51:42PM -0700, Ben Pfaff wrote: >>>> On Mon, Sep 12, 2016 at 04:16:08PM -0700, Jarno Rajahalme wrote: >>>>> Add a new select group selection method "dp_hash", which uses minimal >>>>> number of bits from the datapath calculated packet hash to inform the >>>>> select group bucket selection. This makes the datapath flows more >>>>> generic resulting in less upcalls to userspace, but adds recirculation >>>>> prior to group selection. >>>>> >>>>> Signed-off-by: Jarno Rajahalme <ja...@ovn.org> >>>>> --- >>>>> v2: Rebase and documentation. >>>> >>>> Thanks for adding the documentation! It describes the syntax, which is >>>> useful. To make it even more helpful, I would suggest adding some >>>> advice to the user to explain when one might best choose one or the >>>> other. >>>> >>>> I think that the dp_hash method ignores fields specified by the user if >>>> any are given, so the documentation might mention that for dp_hash the >>>> "fields" option should be omitted. >>>> >>>> Thanks! >>>> >>>> Acked-by: Ben Pfaff <b...@ovn.org> >>> >>> Oh, I forgot to say that I get a compiler warning: >>> >>> ../ofproto/ofproto-dpif-xlate.c: In function 'xlate_dp_hash_select_group': >>> ../ofproto/ofproto-dpif-xlate.c:3410:32: error: variable 'buckets' set >>> but not used [-Werror=unused-but-set-variable] >>> const struct ovs_list *buckets; >>> >>> Fixed by: >>> >>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c >>> index c83132c..a74daa7 100644 >>> --- a/ofproto/ofproto-dpif-xlate.c >>> +++ b/ofproto/ofproto-dpif-xlate.c >>> @@ -3407,10 +3407,8 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, >>> struct group_dpif *group) >>> >>> ctx_trigger_recirculate_with_hash(ctx, param >> 32, (uint32_t)param); >>> } else { >>> - const struct ovs_list *buckets; >>> uint32_t n_buckets; >>> - >>> - buckets = group_dpif_get_buckets(group, &n_buckets); >>> + group_dpif_get_buckets(group, &n_buckets); >>> if (n_buckets) { >>> /* Minimal mask to cover the number of buckets. */ >>> uint32_t mask = (1 << log_2_ceil(n_buckets)) - 1; >> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev