[ovs-dev] [RFC 2/3] dpif: Allow batching flow_get.

[Ben Pfaff]

On Wed, Jul 02, 2014 at 08:14:14PM +1200, Joe Stringer wrote:
> This patch adds support for batching flow_get operations to the dpif. As
> part of this, it also now allows fetching the mask (this was previously
> not possible using a 'get' operation).
> 
> Signed-off-by: Joe Stringer <joestringer at nicira.com>

This approach makes sense.

The comments on dpif_flow_get() don't match the parameters.  (Possibly
we could just remove that function.  It has no callers.)

I think it might be time to make the dpif_class 'operate' function
non-optional.  At that point, we can delete 'flow_get', 'flow_put',
'execute', and 'flow_del' from the dpif_class interface.  (This
doesn't have to be done in this patch or this series, it's just a
note.)

In dpif_linux_operate__(), I think that it is unnecessary to put
NLM_F_ECHO in the message flags.

I do not think that the code in dpif_linux_operate__() is safe.  It
looks to me like the reply is constructed in a stub on the stack (in
the local auxes[] array) and then the caller's get->buffer points into
that stub.  If so, then I think it would be better to require the
caller to not just supply a pointer to an ofpbuf but actually an
initialized ofpbuf that includes storage (probably a stub in most
cases) that dpif_operate() can use.

I don't think the comment on 'buffer' in the definition of
dpif_flow_get() describes the semantics well enough.  As is, I think
that any programmer who tries to use the interface will have to read
code a couple of layers deep, into dpif-linux or dpif-netdev, to use
it correctly.