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 <et...@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(). _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
