> 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).
Done. > 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. Done > s/datpath/datapath/ in the comment on xlate_receive(). Done _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
