On Jun 7, 2013, at 1:04 PM, Ben Pfaff <b...@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.
--Justin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
