On Mon, Oct 10, 2011 at 09:56:18AM -0700, Jesse Gross wrote: > 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.
Thank you. I re-tested v2 and pushed it to master. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
