A bond, BM_TCP with dp_hash, is configured in the select_dst_port column of the mirror table. When a packet is forwarded for output through such a bond, there will be two recircs generated. Both recircs currently forward the packet to the mirror's output_port.
However, the second recirc only performs a rule lookup in table 254 matching on dp_hash to determine the output and does not need to mirror the packet. This patch avoids forwarding packet to the mirror’s output_port during the second recirc to prevent duplicate mirroring. Signed-off-by: Jun Gu <[email protected]> --- ofproto/ofproto-dpif-rid.c | 2 ++ tests/ofproto-dpif.at | 53 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index f01468025..73be23037 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -292,6 +292,8 @@ recirc_alloc_id(struct ofproto_dpif *ofproto) struct frozen_state state = { .table_id = TBL_INTERNAL, .ofproto_uuid = ofproto->uuid, + /* Avoid duplicate mirrors on bond using BM_TCP with dp_hash. */ + .mirrors = UINT32_MAX, .metadata = { .tunnel = { .ip_dst = htonl(0), diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 7ebbee56d..bb5188d1b 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -6441,6 +6441,59 @@ AT_CHECK([tail -1 stdout | grep -E "trunc\(200\),2,trunc\(300\),3,100|trunc\(300 OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - mirroring, avoid duplicate mirrors on balance-tcp bonding with dp_hash]) +AT_KEYWORDS([mirror mirrors mirroring]) +OVS_VSWITCHD_START( + [add-bond br0 bond0 p1 p2 bond_mode=balance-tcp lacp=active \ + other-config:lacp-time=fast other-config:bond-rebalance-interval=0 -- \ + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ + add-br br1 -- \ + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ + fail-mode=standalone -- \ + add-bond br1 bond1 p3 p4 bond_mode=balance-tcp lacp=active \ + other-config:lacp-time=fast other-config:bond-rebalance-interval=0 -- \ + set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ + set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ + add-port br1 br1- -- set interface br1- type=patch options:peer=br1+ ofport_request=100 -- \ + add-br br-int -- \ + set bridge br-int other-config:hwaddr=aa:77:aa:77:00:00 -- \ + set bridge br-int datapath-type=dummy other-config:datapath-id=1235 \ + fail-mode=secure -- \ + add-port br-int br1+ -- set interface br1+ type=patch options:peer=br1- ofport_request=101 -- \ + add-port br-int p5 -- set interface p5 ofport_request=5 type=dummy +]) +AT_CHECK([ovs-appctl netdev-dummy/set-admin-state up], 0, [OK +]) + +# Waits for all ifaces enabled. +OVS_WAIT_UNTIL([test `ovs-appctl bond/show | grep -- "may_enable: true" | wc -l` -ge 4]) + +# The dl_vlan flow should not be ever matched, +# since recirculation should not change the flow handling. +AT_DATA([flows.txt], [dnl +table=0 priority=1 in_port=5 actions=mod_vlan_vid:1,output(101) +table=0 priority=2 in_port=5 dl_vlan=1 actions=drop +]) +AT_CHECK([ovs-ofctl add-flows br-int flows.txt]) + +add_of_ports br1 6 +ovs-vsctl \ + set Bridge br1 mirrors=@m --\ + --id=@bond1 get Port bond1 -- --id=@p6 get Port p6 --\ + --id=@m create Mirror name=mymirror select_dst_port=@bond1 select_src_port=@bond1 output_port=@p6 + +# Sends a packet to trigger recirculation. +AT_CHECK([ovs-appctl netdev-dummy/receive p5 "in_port(5),eth(src=50:54:00:00:00:05,dst=50:54:00:00:01:00),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1)"]) + +AT_CHECK([ovs-appctl dpctl/dump-flows --names | grep 'in_port(p5)' | dnl + sed 's/.*actions://' | grep -o 'p6' | wc -l | tr -d '[[:space:]]'], [0], [1]) + +OVS_VSWITCHD_STOP() +AT_CLEANUP + + # This test verifies that the table ID is preserved across recirculation # when a resubmit action requires it (because the action is relative to # the current table rather than specifying a table). -- 2.34.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
