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

[Ben Pfaff]

On Sat, Aug 03, 2013 at 12:22:09AM -0700, Justin Pettit wrote:
> On Aug 2, 2013, at 11:07 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Fri, Aug 02, 2013 at 10:53:40PM -0700, Justin Pettit wrote:
> >>> In the case where we do emit a "set" action, we un-wildcard not just the
> >>> single field but all the fields that have to be set in the datapath in
> >>> one go.  Is that necessary here?  My initial thought is "no" but I want
> >>> to make sure that you have also considered that.
> >> 
> >> I think we need to do that, since we're overwriting all those fields.
> >> Isn't it essentially the same issue that unless we un-wildcard it,
> >> we'll overwrite other header fields regardless of their value based on
> >> too loose a megaflow?
> > 
> > Here is my logic.
> > 
> > For OFPACT_SET_VLAN_VID, we need to un-wildcard the VID and CFI bits
> > because we want the datapath actions to be correct regardless of the
> > VLAN in the microflow we're actually looking at.  Another way to look at
> > it: if we do not un-wildcard it, then the datapath actions we emit could
> > be different depending on whether the VLAN being set is the VLAN in the
> > microflow for which we're generating actions.
> > 
> > On the other hand, again for OFPACT_SET_VLAN_VID, I do not think the
> > same argument applies to the PCP bits.  The PCP bits in the microflow
> > for which we're generating action do not affect whether we generate a
> > datapath action to set the VLAN tag.  If we don't emit such an action,
> > no problem.  If we do emit such an action, then commit_vlan_action()
> > will ensure that we un-wildcard the whole VLAN tag.
> > 
> > Does that make sense?
> 
> As we discussed off-line, I misunderstood what you were suggesting,
> and my v2 patch had already addressed your concerns.

Yes.  ("Off-line"?  It was IM.  Off-list, anyhow.)

Thanks,

Ben.