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
