Thanks a lot for the extra details. I'm having some trouble getting the setup just right so that I can reproduce your results. So far, I've managed to set it up, but I don't yet see the duplication. Can you help me by providing a full set of commands?
On Tue, Jan 30, 2018 at 11:12:34PM +0800, Huanle Han wrote: > 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 <b...@ovn.org> wrote: > > On Wed, Jan 24, 2018 at 09:41:12AM -0800, Ben Pfaff wrote: > >> From: Huanle Han <hanxue...@gmail.com> > >> > >> Signed-off-by: Huanle Han <hanxue...@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. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev