[ovs-dev] [PATCH] ofp: ensure that l2 is set in ofpmp_reserve()

Simon Horman horms at verge.net.au
Tue Feb 12 23:09:25 UTC 2013


On Tue, Feb 12, 2013 at 09:15:24AM -0800, Ben Pfaff wrote:
> On Tue, Feb 12, 2013 at 05:19:38PM +0900, Simon Horman wrote:
> > Ensure that the buffer returned by ofpmp_reserve() has buf->l2 set
> > as this may be required by nxm_reg_load_to_nxast() when generating
> > the reply to an stats request
> > 
> > This problem was observed when dumping a large number of flows
> > with set_field actions using ovs-ofctl dump-flows.
> > 
> > Signed-off-by: Simon Horman <horms at verge.net.au>
> 
> Good catch.
> 
> I don't think ofpbuf_clone_with_headroom() is a good choice, because
> headroom comes before the cloned data, whereas we want space to expand
> after the cloned data.

Oops.

> I think that the following patch also fixes the problem.  Does it look
> good to you?

I was thinking that it would be good to have a helper that
updated all the relevant pointers. But I believe the much
simpler fix you propose below looks correct to me and resolves
the problem that I observed.

> diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c
> index efc4198..af4178e 100644
> --- a/lib/ofp-msgs.c
> +++ b/lib/ofp-msgs.c
> @@ -851,6 +851,8 @@ ofpmp_reserve(struct list *replies, size_t len)
>  
>          next = ofpbuf_new(MAX(1024, hdrs_len + len));
>          ofpbuf_put(next, msg->data, hdrs_len);
> +        next->l2 = next->data;
> +        next->l3 = ofpbuf_tail(next);
>          list_push_back(replies, &next->list_node);
>  
>          *ofpmp_flags__(msg->data) |= htons(OFPSF_REPLY_MORE);
> 
> Thanks,
> 
> Ben.
> 



More information about the dev mailing list