[ovs-dev] [xc_v2 4/4] ofproto-dpif: Track relevant fields for wildcarding and add xout cache.

Justin Pettit jpettit at nicira.com
Tue Jun 11 08:09:31 UTC 2013

On Jun 7, 2013, at 1:04 PM, Ben Pfaff <blp at nicira.com> wrote:

> I think that nxm_execute_reg_move() and nxm_execute_stack_push() could
> just mask out the subfield that is read, not the whole field that
> contains the subfield.

Fair enough.  Should be addressed in next patch.

> The 'wc' member of xlate_out is weird. 
As we discussed, the whole xout caching mechanism was getting overly brittle and error-prone.  I'll send out a patch soon with Ethan's suggestion for storing wildcards in facets.  As such, I'll skip all the comments related to that functionality.

> I just noticed this code in add_mirror_actions().  It is really
> expensive and does not make sense to me.  bitmap_scan() will return
> 4096 only if no bits are set in m->vlans, but that is not a useful
> mirroring configuration (such a mirror will never mirror anything,
> because no VLANs are selected):
>        if (m->vlans && bitmap_scan(m->vlans, 0, 4096) < 4096) {
>            ctx->xout->wc.masks.vlan_tci |= htons(VLAN_CFI | VLAN_VID_MASK);
>        }
> I think that, instead, I would just write "if (m->vlans) {", because
> that would ordinarily mean that some VLANs are selected and others are
> not.

Great point.  Fixed.


More information about the dev mailing list