[ovs-dev] [PATCHv2] ofproto-dpif: Always un-wildcard fields that are being set.

[Ben Pfaff]

On Fri, Aug 02, 2013 at 11:24:58PM -0700, Jesse Gross wrote:
> On Fri, Aug 2, 2013 at 11:13 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Fri, Aug 02, 2013 at 10:57:53PM -0700, Justin Pettit wrote:
> >> The ODP library has an optimization to not set a header if the field was
> >> not changed, regardless of whether an action to set the field was
> >> present.  That library is also responsible for un-wildcarding fields
> >> that are bieng modified.  This leads to a problem where a packet matches
> >> a flow that updates a field, but that particular packet's field already
> >> has that value.  As such, an overly loose megaflow will be generated
> >> that doesn't match on that field and the actions won't update it.  A
> >> second packet that should have the field set will match that flow and
> >> will not be modified.
> >>
> >> This commit changes the behavior to always un-wildcard fields that are
> >> being modified.  Since the ODP library updates the entire header if a
> >> field in it is modified, and all those fields will be un-wildcarded, the
> >> generated flows may be different.  However, they should be correct.
> >>
> >> Bug #18946.
> >>
> >> Reported-by: Jesse Gross <jesse at nicira.com>
> >> Signed-off-by: Justin Pettit <jpettit at nicira.com>
> >
> > I would think that in the OFPACT_STRIP_VLAN and OFPACT_PUSH_VLAN cases
> > we would need to set all the bits in wc->masks.vlan_tci to 1-bits, since
> > we are potentially modifying all of them.
> 
> I'm not sure that it matters that we are modifying them, the wildcards
> really just relate to what we are reading to make the decision. For
> example, in the case of STRIP_VLAN we will emit the same action
> regardless of the tag value as long as there is a tag.

True.