[ovs-dev] [DPDK Upcalls v2 2/3] ofproto: Reorganize in preparation for direct dpdk upcalls.
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,
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