Thank you for your review. I will fix it.
Best regards, Jun.
On 1/14/26 17:53, Mike Pattrick wrote:
On Sun, Jan 4, 2026 at 9:46 PM Jun Gu <[email protected]> wrote:
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
Hi Jun, the adding of p6 and mymirror could probably be done in the
OVS_VSWITCHD_START block above. If not, adding the mirror should be wrapped
in AT_CHECK.
Other than that change, this change looks correct to me.
-M
+
+# 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
