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

Ethan Jackson ethan at nicira.com
Wed Aug 13 21:35:19 UTC 2014

> 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.

So in the final patch, dpif-netdev will use the put_actions to figure
exactly what to install for slow path actions.  An alternative would
be for the upcall callback to somehow indicate that slow path actions
are required, and that it should do the conversion itself.  Do you
think that might be cleaner?

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

Leftover from old patch versions.  I've changed it.

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

Yep, this is just an intermediate issue, the very next patch fixes
this.  I'd prefer to leave it if that's alright as it makes the diffs
a bit cleaner.

> 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