On Thu, Feb 28, 2013 at 10:33:12AM -0800, Ben Pfaff wrote:
> On Thu, Feb 28, 2013 at 02:00:22PM +0900, Simon Horman wrote:
> > On Wed, Feb 27, 2013 at 09:41:41AM -0800, Ben Pfaff wrote:
> > > On Wed, Feb 27, 2013 at 04:12:16PM +0900, Simon Horman wrote:
> > > > ofpbuf_put_* may reallocate the underlying buffer of the ofpbuf and
> > > > thus writing data after a ofpbuf_put_* call must write to memory
> > > > relative to the pointer returned by the call.
> > > >
> > > > Prior to this change the length and trailing value would not be written
> > > > to
> > > > the set_field action if ofpbuf_put_* may reallocated the underlying
> > > > buffer.
> > > >
> > > > Also make use of ofpbuf_put_zero() to avoid calling memset() directly.
> > > >
> > > > Signed-off-by: Simon Horman <ho...@verge.net.au>
> > >
> > > Good catch!
> > >
> > > I think that we don't really need to reload oasf at all, because we
> > > can just initialize oasf->len earlier and then not refer back to oasf
> > > again after the ofpbuf_put(). That seems a little nicer. So how
> > > about this:
> >
> > Indeed it does, how silly of me not to notice that.
> >
> > I have verified the patch below passes the test case I have which
> > is to add 120k flows and then run the following:
> >
> > ovs-ofctl --protocols OpenFlow12 dump-flows br0
> >
> > On the off chance that it is of value to you:
> >
> > Tested-by: Simon Horman <ho...@verge.net.au>
>
> Testing is always valuable. Thank you!
>
> I applied this to master and branch-1.{9,10}.
Thanks.
I only noticed this problem (which I believe I introduced) while
using 120k flows to exercise some other code paths. So indeed
testing is good :)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
