Currently OVS keeps track of which mirrors that each packet has been sent to for the purpose of deduplication. However, this doesn't consider that openflow rules can make significant changes to packets after ingress.
For example, OVN can create OpenFlow rules that turn an echo request into an echo response by flipping source/destination addresses and setting the ICMP type to Reply. When a mirror is configured, only the request gets mirrored even though a response is recieved. This can cause a false impression of the actual traffic on wire if someone inspects the mirror and doesn't see an echo reply even though one has been sent. This patch resets the mirrors every time a packet is modified, so mirrors will recieve every copy of a packet that is sent for output. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2155579 Signed-off-by: Mike Pattrick <m...@redhat.com> --- ofproto/ofproto-dpif-xlate.c | 37 +++++++++++++++++++++++++++++++----- tests/ofproto-dpif.at | 6 +++--- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index a9cf3cbee..34b4b4018 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -7078,6 +7078,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; case OFPACT_SET_VLAN_VID: + ctx->mirrors = 0; wc->masks.vlans[0].tci |= htons(VLAN_VID_MASK | VLAN_CFI); if (flow->vlans[0].tci & htons(VLAN_CFI) || ofpact_get_SET_VLAN_VID(a)->push_vlan_if_needed) { @@ -7092,6 +7093,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; case OFPACT_SET_VLAN_PCP: + ctx->mirrors = 0; wc->masks.vlans[0].tci |= htons(VLAN_PCP_MASK | VLAN_CFI); if (flow->vlans[0].tci & htons(VLAN_CFI) || ofpact_get_SET_VLAN_PCP(a)->push_vlan_if_needed) { @@ -7106,27 +7108,32 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; case OFPACT_STRIP_VLAN: + ctx->mirrors = 0; flow_pop_vlan(flow, wc); break; case OFPACT_PUSH_VLAN: + ctx->mirrors = 0; flow_push_vlan_uninit(flow, wc); flow->vlans[0].tpid = ofpact_get_PUSH_VLAN(a)->ethertype; flow->vlans[0].tci = htons(VLAN_CFI); break; case OFPACT_SET_ETH_SRC: + ctx->mirrors = 0; WC_MASK_FIELD(wc, dl_src); flow->dl_src = ofpact_get_SET_ETH_SRC(a)->mac; break; case OFPACT_SET_ETH_DST: + ctx->mirrors = 0; WC_MASK_FIELD(wc, dl_dst); flow->dl_dst = ofpact_get_SET_ETH_DST(a)->mac; break; case OFPACT_SET_IPV4_SRC: if (flow->dl_type == htons(ETH_TYPE_IP)) { + ctx->mirrors = 0; memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src); flow->nw_src = ofpact_get_SET_IPV4_SRC(a)->ipv4; } @@ -7134,6 +7141,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_SET_IPV4_DST: if (flow->dl_type == htons(ETH_TYPE_IP)) { + ctx->mirrors = 0; memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst); flow->nw_dst = ofpact_get_SET_IPV4_DST(a)->ipv4; } @@ -7141,6 +7149,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_SET_IP_DSCP: if (is_ip_any(flow)) { + ctx->mirrors = 0; wc->masks.nw_tos |= IP_DSCP_MASK; flow->nw_tos &= ~IP_DSCP_MASK; flow->nw_tos |= ofpact_get_SET_IP_DSCP(a)->dscp; @@ -7149,6 +7158,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_SET_IP_ECN: if (is_ip_any(flow)) { + ctx->mirrors = 0; wc->masks.nw_tos |= IP_ECN_MASK; flow->nw_tos &= ~IP_ECN_MASK; flow->nw_tos |= ofpact_get_SET_IP_ECN(a)->ecn; @@ -7157,6 +7167,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_SET_IP_TTL: if (is_ip_any(flow)) { + ctx->mirrors = 0; wc->masks.nw_ttl = 0xff; flow->nw_ttl = ofpact_get_SET_IP_TTL(a)->ttl; } @@ -7164,6 +7175,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_SET_L4_SRC_PORT: if (is_ip_any(flow) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { + ctx->mirrors = 0; memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src); flow->tp_src = htons(ofpact_get_SET_L4_SRC_PORT(a)->port); @@ -7172,6 +7184,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_SET_L4_DST_PORT: if (is_ip_any(flow) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { + ctx->mirrors = 0; memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst); flow->tp_dst = htons(ofpact_get_SET_L4_DST_PORT(a)->port); @@ -7224,6 +7237,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, /* Set the field only if the packet actually has it. */ if (mf_are_prereqs_ok(mf, flow, wc)) { + ctx->mirrors = 0; mf_mask_field_masked(mf, ofpact_set_field_mask(set_field), wc); mf_set_flow_value_masked(mf, set_field->value, ofpact_set_field_mask(set_field), @@ -7246,39 +7260,47 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; case OFPACT_PUSH_MPLS: + ctx->mirrors = 0; compose_mpls_push_action(ctx, ofpact_get_PUSH_MPLS(a)); break; case OFPACT_POP_MPLS: + ctx->mirrors = 0; compose_mpls_pop_action(ctx, ofpact_get_POP_MPLS(a)->ethertype); break; case OFPACT_SET_MPLS_LABEL: + ctx->mirrors = 0; compose_set_mpls_label_action( ctx, ofpact_get_SET_MPLS_LABEL(a)->label); break; case OFPACT_SET_MPLS_TC: + ctx->mirrors = 0; compose_set_mpls_tc_action(ctx, ofpact_get_SET_MPLS_TC(a)->tc); break; case OFPACT_SET_MPLS_TTL: + ctx->mirrors = 0; compose_set_mpls_ttl_action(ctx, ofpact_get_SET_MPLS_TTL(a)->ttl); break; case OFPACT_DEC_MPLS_TTL: + ctx->mirrors = 0; if (compose_dec_mpls_ttl_action(ctx)) { return; } break; case OFPACT_DEC_NSH_TTL: + ctx->mirrors = 0; if (compose_dec_nsh_ttl_action(ctx)) { return; } break; case OFPACT_DEC_TTL: + ctx->mirrors = 0; wc->masks.nw_ttl = 0xff; if (compose_dec_ttl(ctx, ofpact_get_DEC_TTL(a))) { return; @@ -7370,17 +7392,21 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; case OFPACT_ENCAP: + ctx->mirrors = 0; xlate_generic_encap_action(ctx, ofpact_get_ENCAP(a)); break; case OFPACT_DECAP: { bool recirc_needed = xlate_generic_decap_action(ctx, ofpact_get_DECAP(a)); - if (!ctx->error && recirc_needed) { - /* Recirculate for parsing of inner packet. */ - ctx_trigger_freeze(ctx); - /* Then continue with next action. */ - a = ofpact_next(a); + if (!ctx->error) { + ctx->mirrors = 0; + if (recirc_needed) { + /* Recirculate for parsing of inner packet. */ + ctx_trigger_freeze(ctx); + /* Then continue with next action. */ + a = ofpact_next(a); + } } break; } @@ -7397,6 +7423,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, case OFPACT_NAT: /* This will be processed by compose_conntrack_action(). */ + ctx->mirrors = 0; ctx->ct_nat_action = ofpact_get_NAT(a); break; diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 222415ac0..b027e8c2c 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -5349,7 +5349,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) flow="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 "$flow"], [0], [stdout]) AT_CHECK_UNQUOTED([tail -1 stdout], [0], - [Datapath actions: 3,push_vlan(vid=17,pcp=0),2 + [Datapath actions: 3,push_vlan(vid=17,pcp=0),2,3 ]) flow="in_port(2),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)" @@ -5388,7 +5388,7 @@ flow="in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x080 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow"], [0], [stdout]) actual=`tail -1 stdout | sed 's/Datapath actions: //'` -expected="push_vlan(vid=17,pcp=0),1,pop_vlan,push_vlan(vid=12,pcp=0),1,2,100" +expected="push_vlan(vid=12,pcp=0),100,2,1,pop_vlan,push_vlan(vid=17,pcp=0),1,pop_vlan,push_vlan(vid=12,pcp=0),100,2,1" AT_CHECK([ovs-dpctl normalize-actions "$flow" "$expected"], [0], [stdout]) mv stdout expout AT_CHECK([ovs-dpctl normalize-actions "$flow" "$actual"], [0], [expout]) @@ -5656,7 +5656,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) flow="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 "$flow"], [0], [stdout]) AT_CHECK_UNQUOTED([tail -1 stdout], [0], - [Datapath actions: trunc(100),3,push_vlan(vid=17,pcp=0),2 + [Datapath actions: trunc(100),3,push_vlan(vid=17,pcp=0),2,trunc(100),3 ]) flow="in_port(2),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)" -- 2.31.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
