[ovs-dev] [DPDK Upcalls v2 2/3] ofproto: Reorganize in preparation for direct dpdk upcalls.

Ben Pfaff blp at nicira.com
Tue Aug 12 17:39:23 UTC 2014

On Wed, Aug 06, 2014 at 07:40:24PM -0700, Ethan Jackson wrote:
> This patch reorganizes ofproto-dpif in preparation for future patches
> which allow direct upcall processing from dpif-netdev.  The main goals
> are to share as much code as possible between the dpif-linux and
> dpif-netdev upcall paths.  Additionally, to avoid confusing the
> dpif-netdev fast path, the packet processing path should treat packets
> and struct flow's as const.
> Signed-off-by: Ethan Jackson <ethan at nicira.com>

I don't see actual bugs.  I have a number of comments:

In ofproto-dpif-upcall.c, struct upcall seriously needs comments on
the members.  Some of them are baffling at first glance (put_actions,
userdata, vsp_adjusted).

I'm not sure of the value of the new 'put_actions' member.  It only
gets used in one place.  That place can't compose_slow_path() itself?
It did before this commit.

There is a new distinction between MISS_UPCALL and SLOW_UPCALL, but
they are handled identically.  Why distinguish?

recv_upcalls() and exec_upcalls() have a lot of code in common.  Can
recv_upcalls() use exec_upcalls() as a helper?

struct upcall is quite large (about 600 bytes) because of struct
xlate_out (about 500 bytes).  That makes the memset() at the beginning
of upcall_receive() quite expensive.  I think that it is unnecessary,
because about half of the space in the xlate_out is an ofpbuf stub,
and in any case xlate_actions() will initialize everything necessary
in the xlate_out.

s/datpath/datapath/ in the comment on xlate_receive().

More information about the dev mailing list