[ovs-dev] [PATCH] mirroring: Fix check to avoid outputing to input port.

Jesse Gross jesse at nicira.com
Thu Oct 8 19:58:37 UTC 2009



Ben Pfaff wrote:
> Jesse Gross <jesse at nicira.com> writes:
>
>   
>> When RSPANing with a bond, ensure that we don't receive a packet on
>> one interface and then output it on the other interface of the bond.
>> Also avoid sending the packet out the same interface it was received
>> on when an interfaced is configured with the VLAN that we are using
>> for RSPAN.
>>     
>
> I had to stare at this for a very long time before I saw how it
> would affect bond behavior.  Would you mind breaking it into two
> patches, one for the bond fix and one for the vlan fix, then, to
> make it clear which is which?
>
>   

Yeah, I knew that it wasn't all that clear but I didn't have a good way 
to explain the change.  Hopefully the new patch set is more descriptive.

> I don't understand the vlan fix.  It appears to be making sure
> that 0 and OFP_VLAN_NONE are treated the same way.  But I think
> that we have this property already, because of these invariants
> on the values in the existing test "dst->vlan == vlan":
>
>         * dst->vlan is in 0...4095 (although perhaps we should
>           only allow mirroring to nonzero VLANs).
>
>         * vlan is in 0...4095 (see the assignment and test under
>           the comment "Figure out what VLAN this packet belongs
>           to" in process_flow()).
>   

The key thing is that it is using the vlan tag from the original flow, 
instead of the one stored in the vlan parameter.  The vlan parameter 
contains the vlan after any implicit tagging takes place.  For packets 
that are sent to output ports with implicit vlans, dst->vlan already has 
the tag stripped off.  In this case the comparison will fail and packets 
will be sent out the input port on the same vlan.  Also, I don't believe 
that your first invariant is always correct: dst->vlan can also be 
OFP_VLAN_NONE (-1), while vlan would be 0 in this situation.  This fix 
also handles that (probably rare) case.




More information about the dev mailing list