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 | 97 ++++++++++++++++++++++++++++++++++++ tests/ofproto-dpif.at | 6 +-- 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 4928ea99c..5563ac292 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -7046,6 +7046,101 @@ xlate_ofpact_unroll_xlate(struct xlate_ctx *ctx, "cookie=%#"PRIx64, a->rule_table_id, a->rule_cookie); } +/* Reset the mirror context if we modify the packet and would like to mirror + * the new copy. */ +static void +reset_mirror_ctx(struct xlate_ctx *ctx, const struct flow *flow, + const struct ofpact *a) +{ + switch (a->type) { + case OFPACT_STRIP_VLAN: + case OFPACT_PUSH_VLAN: + case OFPACT_SET_ETH_SRC: + case OFPACT_SET_ETH_DST: + case OFPACT_PUSH_MPLS: + case OFPACT_POP_MPLS: + case OFPACT_SET_MPLS_LABEL: + case OFPACT_SET_MPLS_TC: + case OFPACT_SET_MPLS_TTL: + case OFPACT_DEC_MPLS_TTL: + case OFPACT_DEC_NSH_TTL: + case OFPACT_DEC_TTL: + case OFPACT_SET_VLAN_VID: + case OFPACT_SET_VLAN_PCP: + case OFPACT_ENCAP: + case OFPACT_DECAP: + case OFPACT_NAT: + ctx->mirrors = 0; + return; + case OFPACT_SET_FIELD: { + const struct ofpact_set_field *set_field; + const struct mf_field *mf; + + set_field = ofpact_get_SET_FIELD(a); + mf = set_field->field; + if (mf_are_prereqs_ok(mf, flow, ctx->wc)) { + ctx->mirrors = 0; + } + return; + } + case OFPACT_SET_IPV4_SRC: + case OFPACT_SET_IPV4_DST: + if (flow->dl_type == htons(ETH_TYPE_IP)) { + ctx->mirrors = 0; + } + return; + case OFPACT_SET_IP_DSCP: + case OFPACT_SET_IP_ECN: + case OFPACT_SET_IP_TTL: + if (is_ip_any(flow)) { + ctx->mirrors = 0; + } + return; + case OFPACT_SET_L4_SRC_PORT: + case OFPACT_SET_L4_DST_PORT: + if (is_ip_any(flow) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { + ctx->mirrors = 0; + } + return; + case OFPACT_OUTPUT_REG: + case OFPACT_OUTPUT_TRUNC: + case OFPACT_GROUP: + case OFPACT_OUTPUT: + case OFPACT_CONTROLLER: + case OFPACT_RESUBMIT: + case OFPACT_GOTO_TABLE: + case OFPACT_WRITE_METADATA: + case OFPACT_SET_TUNNEL: + case OFPACT_REG_MOVE: + case OFPACT_STACK_PUSH: + case OFPACT_STACK_POP: + case OFPACT_LEARN: + case OFPACT_ENQUEUE: + case OFPACT_SET_QUEUE: + case OFPACT_POP_QUEUE: + case OFPACT_MULTIPATH: + case OFPACT_BUNDLE: + case OFPACT_EXIT: + case OFPACT_UNROLL_XLATE: + case OFPACT_FIN_TIMEOUT: + case OFPACT_CLEAR_ACTIONS: + case OFPACT_WRITE_ACTIONS: + case OFPACT_METER: + case OFPACT_SAMPLE: + case OFPACT_CLONE: + case OFPACT_DEBUG_RECIRC: + case OFPACT_DEBUG_SLOW: + case OFPACT_CT: + case OFPACT_CT_CLEAR: + case OFPACT_CHECK_PKT_LARGER: + case OFPACT_DELETE_FIELD: + case OFPACT_NOTE: + case OFPACT_CONJUNCTION: + return; + } + OVS_NOT_REACHED(); +} + static void xlate_trace(struct xlate_ctx *ctx, const struct ofpact *a) { @@ -7112,6 +7207,8 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; } + reset_mirror_ctx(ctx, flow, a); + if (OVS_UNLIKELY(ctx->xin->trace)) { xlate_trace(ctx, a); } diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 6824ce0bb..f242f77f3 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.39.3 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
