On Fri, Dec 06, 2013 at 11:48:58AM -0800, Jarno Rajahalme wrote: > > On Dec 6, 2013, at 9:40 AM, Ben Pfaff <b...@nicira.com> wrote: > > > On Thu, Dec 05, 2013 at 04:27:24PM -0800, Jarno Rajahalme wrote: > >> Ofproto always provides a valid pointer to a mask, but the mask length > >> is zero when megaflows are disabled. Use match_wc_init() to get an > >> appropriate exact match mask when there is no mask. > > > > I'm having a little trouble understanding the commit message. > > Everything in it makes sense, but it doesn't explain the problem to me. > > It's like there should be another clause in the first sentence: "...are > > disabled, and this causes a problem because?? > > Would this be better: > > "Normally OVS userspace supplies a mask along with a flow key for > each new data path flow that should be created. OVS also provides an > option to disable the kernel wildcarding, in which case the flows > are created without a mask. When kernel wildcarding is disabled, the > datapath should use exact match, i.e. not wildcard any bits in the > flow key. Currently, what happens with the userspace datapath > instead is that a datapath flow with mostly empty mask is created > (i.e., most fields are wildcarded), as the current code does not > examine the given mask key length to find out that the mask key is > actually empty. This results in the same datapath flow matching on > packets of multiple different flows, wrong actions being processed, > and stats being incorrect. > > This patch refactors userspace datapath code to use match_wc_init() > to get an appropriate exact match mask when a flow put without a > mask is executed."
That's great, thank you!. I know what to look for now so I'll finish the review. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
