@@ -2308,6 +2308,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_any_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 *);
@@ -1790,6 +1790,115 @@ mf_is_tun_metadata(const struct mf_field *mf)
mf->id < MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS;
}
+bool
+mf_is_any_metadata(const struct mf_field *mf)
+{
+ switch (mf->id) {
+ case MFF_DP_HASH:
+ case MFF_RECIRC_ID:
+ case MFF_PACKET_TYPE:
+ case MFF_CONJ_ID:
+ case MFF_TUN_ERSPAN_DIR:
+ CASE_MFF_TUN_METADATA:
+ case MFF_METADATA:
+ case MFF_IN_PORT:
+ case MFF_IN_PORT_OXM:
+ 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_REGS:
+ CASE_MFF_XREGS:
+ CASE_MFF_XXREGS:
+ 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_TTL:
+ case MFF_TUN_TOS:
+ case MFF_TUN_GBP_ID:
+ case MFF_TUN_GBP_FLAGS:
+ case MFF_TUN_ERSPAN_IDX:
+ case MFF_TUN_ERSPAN_VER:
+ case MFF_TUN_ERSPAN_HWID:
+ case MFF_TUN_GTPU_FLAGS:
+ case MFF_TUN_GTPU_MSGTYPE:
+ 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_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:
+ case MFF_NSH_TTL:
+ return false;
+
+ case MFF_N_IDS:
+ default:
+ OVS_NOT_REACHED();
+ }
+}
+
bool
mf_is_frozen_metadata(const struct mf_field *mf)
{
@@ -7278,7 +7278,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_any_metadata(mf)) {
ctx->mirrors = 0;
}
return;
@@ -5514,6 +5514,33 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0],
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - mirroring, metadata modification])
+AT_KEYWORDS([mirror mirrors mirroring])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2 3
+AT_CHECK([ovs-vsctl set Bridge br0 mirrors=@m -- \
+ --id=@p1 get Port p1 -- --id=@p3 get Port p3 -- \
+ --id=@m create Mirror name=mymirror select_all=true output_port=@p3],
+ [0], [ignore])
+
+AT_DATA([flows.txt], [dnl
+in_port=1 actions=load:0x00->NXM_OF_IN_PORT[[]],output:2
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+dnl Metadata modified, duplicate packet shouldn't be delivered to mirror.
+m4_define([ICMP_FLOW], [m4_join([,],
+ [in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800)],
+ [ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)],
+ [icmp(type=8,code=0)])])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "ICMP_FLOW"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+ [Datapath actions: 3,2
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - mirroring, OFPP_NONE ingress port])
AT_KEYWORDS([mirror mirrors mirroring])
OVS_VSWITCHD_START
Previously a commit attempted to reset the mirror context when packets were modified. However, this commit erroneously also reset the mirror context when only a packet's metadata was modified. An intermediate commit corrected this for tunnel metadata, but now that correction is extended to other forms of metadata as well. Fixes: feed7f677505 ("ofproto-dpif-upcall: Mirror packets that are modified.") Reported-at: https://issues.redhat.com/browse/FDP-699 Acked-by: Eelco Chaudron <echaudro@redhat.com> Acked-by: Simon Horman <horms@ovn.org> Signed-off-by: Mike Pattrick <mkp@redhat.com> --- v2: - Added extra whitespace - Moved N_IDS to never reached section - Added unit test v3: - Fixed broken unit test. v4: - Changed unit test to use m4_define and changed comment - Reordered switch statement --- include/openvswitch/meta-flow.h | 1 + lib/meta-flow.c | 109 ++++++++++++++++++++++++++++++++ ofproto/ofproto-dpif-xlate.c | 2 +- tests/ofproto-dpif.at | 27 ++++++++ 4 files changed, 138 insertions(+), 1 deletion(-)