On Fri, Oct 7, 2011 at 7:54 PM, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Oct 07, 2011 at 06:25:57PM -0700, Jesse Gross wrote: >> 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. > > OK, here's v2. Unlike the previous version, this one is compile-tested > only because I'm away from my desk.
Looks good, thanks. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
