[ovs-dev] [PATCH v2 06/19] bundles: Validate bundled messages.

[Ben Pfaff]

On Mon, Jun 01, 2015 at 06:06:14PM -0700, Jarno Rajahalme wrote:
> 
> > On Jun 1, 2015, at 2:48 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 
> > On Mon, Jun 01, 2015 at 01:58:41PM -0700, Jarno Rajahalme wrote:
> >> 
> >>> On Jun 1, 2015, at 1:47 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
> >>> 
> >>> 
> >>>> On May 29, 2015, at 5:38 PM, Ben Pfaff <blp at nicira.com> wrote:
> >>>> 
> >>>> On Mon, May 18, 2015 at 04:10:15PM -0700, Jarno Rajahalme wrote:
> >>>>> OpenFlow bundle messages should be decoded and validated at the time
> >>>>> they are added to the bundle.  This commit does this for flow mod and
> >>>>> port mod messages.
> >>>>> 
> >>>>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> >>>> 
> >>>> It's pretty expensive to give every ofp_bundle_entry a 1-kB stub for
> >>>> actions, when there might be many thousands of them allocated at a time
> >>>> into a bundle.  It might be better to accumulate the actions into a
> >>>> function-local stub then use ofpbuf_clone() or similar to allocate a
> >>>> correctly sized buffer for ofp_bundle_entry.
> >>>> 
> >>>> I guess that it would be even cheaper, memory-wise (struct match by
> >>>> itself is almost 1/2 kB!), to just decode the raw message a second time
> >>>> when we apply the bundle, but maybe that is not worth the extra trouble.
> >>> 
> >>> I???ll look if I can get the actual replacement rule created at the validation time for the versioned case (after patch 17). For this patch, I???ll just make the stub smaller (64 bytes?).
> >>> 
> >> 
> >> Are you OK with deferring further optimization to the end of the series? I.e., can I push this with a smaller stub?
> > 
> > Why bother with a stub at all in the struct?  I suggest using a
> > function-local stub on the stack when decoding the flow_mod, then
> > ofpbuf_steal_data() to put that data into the ofp_bundle.
> 
> 
> OK, I got it now :-) Incremental:

Thanks, that's what I meant,
Acked-by: Ben Pfaff <blp at nicira.com>