On Fri, Oct 7, 2011 at 5:57 PM, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Oct 07, 2011 at 05:05:15PM -0700, Jesse Gross wrote: >> On Fri, Oct 7, 2011 at 4:42 PM, Ben Pfaff <b...@nicira.com> wrote: >> > Commit b063d9f0 "datapath: Use unicast Netlink sockets for upcalls" that >> > introduced an 'upcall_pid' member into struct dpif_linux_vport, struct >> > dpif_linux_dp, and struct dpif_linux_flow neglected to do so only if the >> > member was nonzero. ??This caused every datapath, vport, and flow operation >> > to supply an upcall_pid. ??In particular, the netdev_set_config() called at >> > startup when a vport already existed caused the upcall_pid for that vport >> > to be reset to 0, which in turn caused all packets received on the vport to >> > be dropped instead of forwarded to ovs-vswitchd. >> > >> > Reported-by: Shih-Hao Li <s...@nicira.com> >> >> I think we actually want to distinguish between unset and zero. When >> the listen_mask indicates that a packet type shouldn't be received >> then we intentionally generate an upcall_pid of 0 to shut off those >> types of upcalls. Most of dpif-linux.c deals with this by simply >> always including the appropriate upcall_pid but that was missed for >> the calls in netdev-vport. At this point, nothing ever turns off >> parts of listen_mask, so it doesn't really matter but that was the >> intention. > > I actually understood these two cases as I wrote up the commit, but I > didn't see anything that currently needed to take advantage of it so I > ignored it. > > I can fix it up to separate "no change" and "set to zero", though, if > you prefer.
I guess it seems better to separate them out, otherwise the code is confusing because it's doing something in one place but ignoring it in another. Another possibility is to actually get the correct PID using your patch to assign PIDs to userspace actions (since it exposes the get_pid function). _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
