[ovs-dev] [PATCH] nx-match: Correct writing of value and length in set_field_to_ofast()

Simon Horman horms at verge.net.au
Fri Mar 1 01:16:17 UTC 2013


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 <horms at 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 <horms at 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 :)


More information about the dev mailing list