On Sun, Jan 24, 2016 at 11:59:57PM +0800, Huanle Han wrote: > I think a more line "mirrors &= ~ctx->mirrors;" is needed at the end of the > "while" loop. > Otherwise, duplicated mirror action may be applied in recursive > mirror_packet called by output_normal.
Did you arrive at that conclusion from testing or code inspection? I believe that dup_mirrors always includes the mirror we're handling, so that "mirrors &= ~dup_mirrors;" in the while loop should include that case; also note the "mirrors &= ~ctx->mirrors;" at the beginning of the function. > Here are the test cases: > > Test case for the bug I mentioned in previous mail: > 1. setup port: vnet0 tag = 201, vnet1 tag=200 > 2. setup flow: add-flow in_port=vnet0,actions:vnet1 > 3. setup mirror( mirror1 or mirror2 ). and packet from vnet0 to vnet1 > a) mirror1: select_all=false, select_src_port=[ ], > select_dst_port=[vnet1], select_vlan=[200], output=vnet2 > ==> result: this mirror works. > b) mirror2: select_all=false, select_src_port=[vnet0], > select_dst_port=[vnet1], select_vlan=[200], output=vnet2 > ==> result: this mirror doesn't work. > > Test case for the bug I mentioned in this mail: > 1. set port: vnet0, vnet1, vnet2 > 2. setup 2 mirrors: > mirror1: select_all=true,output_vlan=500 > mirror2: select_all=true,output_vlan=501 > 3. packet from vnet0, result: duplicated mirror actions are generated. Do you mean that you ran these test cases with the patch I supplied, and saw this bug? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
