On Mon, Mar 25, 2024 at 10:04 PM Zhangweiwei <[email protected]> wrote: > > The ethernet addresses of two ICMP request packets are indeed different . One > is original packet and the other is modified. It is an expected behavior > according to the code. > Actually, when a packet sent by port A is changed by flow table and then is > sent to itself, we expect to capture this packet. However, when this packet > is changed and is sent to another port, should we still capture the packet on > port A?
Currently in ovs-tcpdump we capture the packet on ingress and then on egress if it is modified. You could make the mirror without ovs-tcpdump, and only set the select_dst_port option but not the select_src_port one. select_src_port is checked during ingress and select_dst_port is set during egress. Hope this helps, M > > [root@localhost infiniband]# ovs-tcpdump -i tapVm71 -nnvve > 11.11.70.1 > 1.1.70.2: ICMP echo request, id 15498, seq 17, length 64 > 09:36:52.822232 68:05:ca:21:d6:e5 > 52:54:00:67:d5:61, ethertype IPv4 > (0x0800), length 98: (tos 0x0, ttl 63, id 22101, offset 0, flags [none], > proto ICMP (1), length 84) > 1.1.70.2 > 11.11.70.1: ICMP echo reply, id 15498, seq 17, length 64 > 09:36:53.862137 52:54:00:67:d5:61 > 68:05:ca:21:d6:e5, ethertype IPv4 > (0x0800), length 98: (tos 0x0, ttl 64, id 26518, offset 0, flags [DF], proto > ICMP (1), length 84) > 11.11.70.1 > 1.1.70.2: ICMP echo request, id 15498, seq 18, length 64 > 09:36:53.862139 68:05:ca:21:d6:e5 > 52:54:00:9a:bf:ed, ethertype IPv4 > (0x0800), length 98: (tos 0x0, ttl 63, id 26518, offset 0, flags [DF], proto > ICMP (1), length 84) > 11.11.70.1 > 1.1.70.2: ICMP echo request, id 15498, seq 18, length 64 > 09:36:53.862230 68:05:ca:21:d6:e5 > 52:54:00:67:d5:61, ethertype IPv4 > (0x0800), length 98: (tos 0x0, ttl 63, id 22176, offset 0, flags [none], > proto ICMP (1), length 84) > 1.1.70.2 > 11.11.70.1: ICMP echo reply, id 15498, seq 18, length 64 > > -----邮件原件----- > 发件人: Mike Pattrick [mailto:[email protected]] > 发送时间: 2024年3月25日 22:26 > 收件人: zhangweiwei (RD) <[email protected]> > 抄送: [email protected] > 主题: Re: [PATCH] ofproto-dpif-upcall: Don't mirror packets that aren't > modified. > > On Mon, Mar 25, 2024 at 3:48 AM Zhangweiwei <[email protected]> wrote: > > > > 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_typ > > e(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(i > > pv4(ttl=63)),ct(zone=6),recirc(0x3e8) > > ct_state(+est-rel-rpl),recirc_id(0x3e8),in_port(11),packet_type(ns=0,i > > d=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 > > > Hello, thanks for the report. Is it possible to run the command "ovs-tcpdump > -i tapVm71 -ennvv" ? I ask because I see your actions reset the ethernet > address. If the ethernet address is different then this would be the expected > behavior, the collection of the packet as it enters, and then as it exists > modified. > > > Thank you, > > Mike > > > > > > -----邮件原件----- > > 发件人: 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
