[ovs-dev] [PATCH 3/3] mirror: do not mirror packet on recirculation

Huanle Han hanxueluo at gmail.com
Tue Jan 30 15:12:34 UTC 2018


Thanks for your review.

Here is a test case:
1. add a trunk, balance-tcp bond to a bridge
2. add a access port tag=xxx to same bridge
3. add a mirror, which mirrors all ports in vlan=xxx to another out port
4. send packet from access port to bond (simply use arp). as a result,
*mirror send 2 duplicated packets to outport *

Commands for example:
br=br1
src_port=vnet1
mirror_port=vnet12

ovs-vsctl --if-exists del-port $mirror_port -- add-port $br $mirror_port
ovs-vsctl --if-exists del-port $src_port -- add-port $br $src_port tag=199

ovs-vsctl -- set Bridge $br mirrors=@m --  \
    --id=@p0 get Port $mirror_port -- \
    --id=@m create Mirror select_all=true name=mm select_vlan=199
output_port=@p0

Result of datapath flows: 11,which is the mirror outport, is outputed twice.
recirc_id(0x2),dp_hash(0xb8/0xff),in_port(10),eth_type(0x8100),vlan(vid=199),encap(eth_type(0x0806),
packets:9, bytes:414, used:0.605s, actions:9,11
recirc_id(0),in_port(10),eth(src=fa:da:41:1d:30:0e,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=7.7.7.18,tip=7.7.7.192,op=1/0xff),
packets:9, bytes:378, used:0.604s,
actions:push_vlan(vid=199,pcp=0),11,pop_vlan,7,push_vlan(vid=199,pcp=0),12,hash(hash_l4(0),recirc(0x2)

About patch:
I didn't notice 'frozen_state' thing util you pointed it out. And
after review the code, I think ovs doesn't save the mirror information
in frozen_state for bond recirc. My old patch is naive. I would
appreciate it if you would fix it.


On Thu, Jan 25, 2018 at 3:30 AM, Ben Pfaff <blp at ovn.org> wrote:
> On Wed, Jan 24, 2018 at 09:41:12AM -0800, Ben Pfaff wrote:
>> From: Huanle Han <hanxueluo at gmail.com>
>>
>> Signed-off-by: Huanle Han <hanxueluo at gmail.com>
>> ---
>>  ofproto/ofproto-dpif-xlate.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index 9d6ca94afc82..23938c8c8cf3 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -1931,6 +1931,10 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle,
>>          return;
>>      }
>>
>> +    if (ctx->xin->flow.recirc_id != 0) {
>> +        return;
>> +    }
>> +
>
> Can you help me understand what cases this addresses?  The frozen_state
> that comes along with a recirculation should keep track of what mirrors
> have already been output, which should prevent duplicate mirroring on
> recirculation.  If it doesn't work in every case, then probably we
> should address that instead of just disabling mirroring on
> recirculation.
>
> Thanks,
>
> Ben.


More information about the dev mailing list