Hi,
I have tried this patch, however, there are still some issues when the packets
contents are changed across recirculation. On the follow example, packets are
modified in recirc_id(0) after mirror, the mirror context reset. Therefore,
there are two ICMP request packets are mirrored on port mitapVm71.
In the following example, ICMP packets ared sent from port(11) to port(14),
[root@localhost ~]# ovs-appctl dpif/dump-flows vds1-br
ct_state(-new-est-rel-rpl-inv-trk),recirc_id(0),in_port(11),packet_type(ns=0,id=0),eth(src=52:54:00:67:d5:61,dst=68:05:ca:21:d6:e5),eth_type(0x0800),ipv4(src=11.11.70.1,dst=1.1.70.2,proto=1,ttl=64,frag=no),
packets:431, bytes:42238, used:0.574s,
actions:10,set(eth(src=68:05:ca:21:d6:e5,dst=52:54:00:9a:bf:ed)),set(ipv4(ttl=63)),ct(zone=6),recirc(0x3e8)
ct_state(+est-rel-rpl),recirc_id(0x3e8),in_port(11),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:430, bytes:42140, used:0.574s, actions:10,14
ct_state(-new+est-rel+rpl-inv+trk),recirc_id(0x3e9),in_port(14),packet_type(ns=0,id=0),eth(src=52:54:00:9a:bf:ed,dst=68:05:ca:21:d6:e5),eth_type(0x0800),ipv4(dst=11.11.70.1,proto=1,ttl=64,frag=no),
packets:431, bytes:42238, used:0.574s,
actions:set(eth(src=68:05:ca:21:d6:e5,dst=52:54:00:67:d5:61)),set(ipv4(ttl=63)),11,10
ct_state(-trk),recirc_id(0),in_port(14),packet_type(ns=0,id=0),eth(src=52:54:00:9a:bf:ed),eth_type(0x0800),ipv4(src=1.1.70.2,proto=1,frag=no),
packets:431, bytes:42238, used:0.574s, actions:ct(zone=6),recirc(0x3e9)
[root@localhost ~]# ovs-appctl dpif/show
netdev@ovs-netdev: hit:2552 missed:3019
vds1-br:
mitapVm71 14/10: (system)
tapVm71 5/11: (dpdkvhostuserclient: configured_rx_queues=1,
configured_tx_queues=1, mtu=1500, requested_rx_queues=1, requested_tx_queues=1)
tapVm72 6/14: (dpdkvhostuserclient: configured_rx_queues=1,
configured_tx_queues=1, mtu=1500, requested_rx_queues=1, requested_tx_queues=1)
[root@localhost ~]# ovs-tcpdump -i tapVm71
14:38:53.702142 IP 11.11.70.1 > 1.1.70.2: ICMP echo request, id 13483, seq
2014, length 64
14:38:53.702143 IP 11.11.70.1 > 1.1.70.2: ICMP echo request, id 13483, seq
2014, length 64
14:38:53.702185 IP 1.1.70.2 > 11.11.70.1: ICMP echo reply, id 13483, seq 2014,
length 64
14:38:54.742141 IP 11.11.70.1 > 1.1.70.2: ICMP echo request, id 13483, seq
2015, length 64
14:38:54.742143 IP 11.11.70.1 > 1.1.70.2: ICMP echo request, id 13483, seq
2015, length 64
14:38:54.742183 IP 1.1.70.2 > 11.11.70.1: ICMP echo reply, id 13483, seq 2015,
length 64
14:38:55.782142 IP 11.11.70.1 > 1.1.70.2: ICMP echo request, id 13483, seq
2016, length 64
14:38:55.782144 IP 11.11.70.1 > 1.1.70.2: ICMP echo request, id 13483, seq
2016, length 64
14:38:55.782186 IP 1.1.70.2 > 11.11.70.1: ICMP echo reply, id 13483, seq 2016,
length 64
-----邮件原件-----
发件人: Mike Pattrick [mailto:[email protected]]
发送时间: 2024年3月13日 1:37
收件人: [email protected]
抄送: Mike Pattrick <[email protected]>; zhangweiwei (RD) <[email protected]>
主题: [PATCH] ofproto-dpif-upcall: Don't mirror packets that aren't modified.
Previously OVS reset the mirror contents when a packet is modified in such a
way that the packets contents changes. However, this change incorrectly reset
that mirror context when only metadata changes as well.
Now we check for all metadata fields, instead of just tunnel metadata, before
resetting the mirror context.
Fixes: feed7f677505 ("ofproto-dpif-upcall: Mirror packets that are modified.")
Reported-by: Zhangweiwei <[email protected]>
Signed-off-by: Mike Pattrick <[email protected]>
---
include/openvswitch/meta-flow.h | 1 +
lib/meta-flow.c | 109 ++++++++++++++++++++++++++++++++
ofproto/ofproto-dpif-xlate.c | 2 +-
tests/ofproto-dpif.at | 5 +-
4 files changed, 114 insertions(+), 3 deletions(-)
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 3b0220aaa..96aad3933 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -2305,6 +2305,7 @@ void mf_set_flow_value_masked(const struct mf_field *,
const union mf_value *mask,
struct flow *); bool mf_is_tun_metadata(const
struct mf_field *);
+bool mf_is_metadata(const struct mf_field *);
bool mf_is_frozen_metadata(const struct mf_field *); bool
mf_is_pipeline_field(const struct mf_field *); bool mf_is_set(const struct
mf_field *, const struct flow *); diff --git a/lib/meta-flow.c
b/lib/meta-flow.c index aa7cf1fcb..7ecec334e 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -1788,6 +1788,115 @@ mf_is_tun_metadata(const struct mf_field *mf)
mf->id < MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS; }
+bool
+mf_is_metadata(const struct mf_field *mf) {
+ switch (mf->id) {
+ CASE_MFF_TUN_METADATA:
+ case MFF_METADATA:
+ case MFF_IN_PORT:
+ case MFF_IN_PORT_OXM:
+ CASE_MFF_REGS:
+ CASE_MFF_XREGS:
+ CASE_MFF_XXREGS:
+ case MFF_PACKET_TYPE:
+ case MFF_DP_HASH:
+ case MFF_RECIRC_ID:
+ case MFF_CONJ_ID:
+ case MFF_ACTSET_OUTPUT:
+ case MFF_SKB_PRIORITY:
+ case MFF_PKT_MARK:
+ case MFF_CT_STATE:
+ case MFF_CT_ZONE:
+ case MFF_CT_MARK:
+ case MFF_CT_LABEL:
+ case MFF_CT_NW_PROTO:
+ case MFF_CT_NW_SRC:
+ case MFF_CT_NW_DST:
+ case MFF_CT_IPV6_SRC:
+ case MFF_CT_IPV6_DST:
+ case MFF_CT_TP_SRC:
+ case MFF_CT_TP_DST:
+ case MFF_N_IDS:
+ return true;
+
+ case MFF_TUN_ID:
+ case MFF_TUN_SRC:
+ case MFF_TUN_DST:
+ case MFF_TUN_IPV6_SRC:
+ case MFF_TUN_IPV6_DST:
+ case MFF_TUN_FLAGS:
+ case MFF_TUN_GBP_ID:
+ case MFF_TUN_GBP_FLAGS:
+ case MFF_TUN_ERSPAN_VER:
+ case MFF_TUN_ERSPAN_IDX:
+ case MFF_TUN_ERSPAN_DIR:
+ case MFF_TUN_ERSPAN_HWID:
+ case MFF_TUN_GTPU_FLAGS:
+ case MFF_TUN_GTPU_MSGTYPE:
+ case MFF_TUN_TTL:
+ case MFF_TUN_TOS:
+ case MFF_ETH_SRC:
+ case MFF_ETH_DST:
+ case MFF_ETH_TYPE:
+ case MFF_VLAN_TCI:
+ case MFF_DL_VLAN:
+ case MFF_VLAN_VID:
+ case MFF_DL_VLAN_PCP:
+ case MFF_VLAN_PCP:
+ case MFF_MPLS_LABEL:
+ case MFF_MPLS_TC:
+ case MFF_MPLS_BOS:
+ case MFF_MPLS_TTL:
+ case MFF_IPV4_SRC:
+ case MFF_IPV4_DST:
+ case MFF_IPV6_SRC:
+ case MFF_IPV6_DST:
+ case MFF_IPV6_LABEL:
+ case MFF_IP_PROTO:
+ case MFF_IP_DSCP:
+ case MFF_IP_DSCP_SHIFTED:
+ case MFF_IP_ECN:
+ case MFF_IP_TTL:
+ case MFF_IP_FRAG:
+ case MFF_ARP_OP:
+ case MFF_ARP_SPA:
+ case MFF_ARP_TPA:
+ case MFF_ARP_SHA:
+ case MFF_ARP_THA:
+ case MFF_TCP_SRC:
+ case MFF_TCP_DST:
+ case MFF_TCP_FLAGS:
+ case MFF_UDP_SRC:
+ case MFF_UDP_DST:
+ case MFF_SCTP_SRC:
+ case MFF_SCTP_DST:
+ case MFF_ICMPV4_TYPE:
+ case MFF_ICMPV4_CODE:
+ case MFF_ICMPV6_TYPE:
+ case MFF_ICMPV6_CODE:
+ case MFF_ND_TARGET:
+ case MFF_ND_SLL:
+ case MFF_ND_TLL:
+ case MFF_ND_RESERVED:
+ case MFF_ND_OPTIONS_TYPE:
+ case MFF_NSH_FLAGS:
+ case MFF_NSH_TTL:
+ case MFF_NSH_MDTYPE:
+ case MFF_NSH_NP:
+ case MFF_NSH_SPI:
+ case MFF_NSH_SI:
+ case MFF_NSH_C1:
+ case MFF_NSH_C2:
+ case MFF_NSH_C3:
+ case MFF_NSH_C4:
+ return false;
+
+ default:
+ OVS_NOT_REACHED();
+ }
+}
+
bool
mf_is_frozen_metadata(const struct mf_field *mf) { diff --git
a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index
89f183182..faa364ec8 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7141,7 +7141,7 @@ reset_mirror_ctx(struct xlate_ctx *ctx, const struct flow
*flow,
set_field = ofpact_get_SET_FIELD(a);
mf = set_field->field;
- if (mf_are_prereqs_ok(mf, flow, NULL) && !mf_is_tun_metadata(mf)) {
+ if (mf_are_prereqs_ok(mf, flow, NULL) && !mf_is_metadata(mf)) {
ctx->mirrors = 0;
}
return;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index
a1393f7f8..245e209c3 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5443,7 +5443,8 @@ AT_CLEANUP
# This test verifies that mirror state is preserved across recirculation.
#
# Otherwise, post-recirculation the ingress and the output to port 4 -# would
cause the packet to be mirrored to port 3 a second time.
+# would cause the packet to be mirrored to port 3 a second time. A
+register # is also modified to verify that this doesn't reset the mirror
context.
AT_SETUP([ofproto-dpif - mirroring with recirculation]) AT_KEYWORDS([mirror
mirrors mirroring]) OVS_VSWITCHD_START @@ -5454,7 +5455,7 @@ ovs-vsctl \
--id=@m create Mirror name=mymirror select_all=true output_port=@p3
AT_DATA([flows.txt], [dnl
-in_port=1 actions=2,debug_recirc,4
+in_port=1 actions=2,debug_recirc,set_field:1->reg4,4
])
AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
--
2.39.3
-------------------------------------------------------------------------------------------------------------------------------------
本邮件及其附件含有新华三集团的保密信息，仅限于发送给上面地址中列出
的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、
或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本
邮件！
This e-mail and its attachments contain confidential information from New H3C,
which is
intended only for the person or entity whose address is listed above. Any use
of the
information contained herein in any way (including, but not limited to, total
or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify
the sender
by phone or email immediately and delete it!
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
