[ovs-dev] [PATCH v5 2/4] dpif-netdev: Do not check mask if it doesn't exist.

Ben Pfaff blp at nicira.com
Fri Dec 6 20:53:39 UTC 2013


On Fri, Dec 06, 2013 at 11:48:58AM -0800, Jarno Rajahalme wrote:
> 
> On Dec 6, 2013, at 9:40 AM, Ben Pfaff <blp at 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.



More information about the dev mailing list