From 7efbc3b7c4006caed79cc9afa799cd0f9b8f5d38 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 29 Jul 2015 17:00:49 -0700 Subject: [PATCH] ofproto-dpif-xlate: Rewrite mirroring to better fit flow translation. Until now, mirroring has been implemented by accumulating, across the whole translation process, a set of mirrors that should receive a mirrored packet. After translation was complete, mirroring restored the original version of the packet and sent that version to the mirrors. That implementation was ugly for multiple reasons. First, it means that we have to keep a copy of the original packet (or its headers, actually), which is expensive. Second, it doesn't really make sense to mirror a version of a packet that is different from the one originally output. Third, it interacted with recirculation; mirroring needed to happen only after recirculation was complete, but this was never properly implemented, so that (I think) mirroring never happened for packets that were recirculated. This commit changes how mirroring works. Now, a packet is mirrored at the point in translation when it becomes eligible for it: for mirrors based on ingress port, this is at ingress; for mirrors based on egress port, this is at egress. (Duplicates are dropped.) Mirroring happens on the version of the packet as it exists when it becomes eligible. Finally, since mirroring happens immediately, it interacts better with recirculation (it still isn't perfect, since duplicate mirroring will occur if a packet is eligible for mirroring both before and after recirculation; this is not difficult to fix and an upcoming commit later in this series will do so). Finally, this commit removes more code from xlate_actions() than it adds, which in my opinion makes it easier to understand. Signed-off-by: Ben Pfaff Acked-by: Jarno Rajahalme --- ofproto/ofproto-dpif-xlate.c | 113 ++++++++++++++++------------------- tests/ofproto-dpif.at | 12 ++-- vswitchd/vswitch.xml | 26 ++++++++ 3 files changed, 82 insertions(+), 69 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 8c8da9ab2..cbcd2a37d 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1530,56 +1530,55 @@ lookup_input_bundle(const struct xbridge *xbridge, ofp_port_t in_port, } static void -add_mirror_actions(struct xlate_ctx *ctx, const struct flow *orig_flow) +mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle, + mirror_mask_t mirrors) { - const struct xbridge *xbridge = ctx->xbridge; - mirror_mask_t mirrors; - struct xbundle *in_xbundle; - uint16_t vlan; - uint16_t vid; - - mirrors = ctx->mirrors; - ctx->mirrors = 0; - - in_xbundle = lookup_input_bundle(xbridge, orig_flow->in_port.ofp_port, - ctx->xin->packet != NULL, NULL); - if (!in_xbundle) { + bool warn = ctx->xin->packet != NULL; + uint16_t vid = vlan_tci_to_vid(ctx->xin->flow.vlan_tci); + if (!input_vid_is_valid(vid, xbundle, warn)) { return; } - mirrors |= xbundle_mirror_src(xbridge, in_xbundle); + uint16_t vlan = input_vid_to_vlan(xbundle, vid); - /* Check VLAN. */ - vid = vlan_tci_to_vid(orig_flow->vlan_tci); - if (!input_vid_is_valid(vid, in_xbundle, ctx->xin->packet != NULL)) { - return; - } - vlan = input_vid_to_vlan(in_xbundle, vid); + const struct xbridge *xbridge = ctx->xbridge; + /* Don't mirror to destinations that we've already mirrored to. */ + mirrors &= ~ctx->mirrors; if (!mirrors) { return; } - /* Restore the original packet before adding the mirror actions. */ - ctx->xin->flow = *orig_flow; + /* Record these mirrors so that we don't mirror to them again. */ + ctx->mirrors |= mirrors; + + if (ctx->xin->resubmit_stats) { + mirror_update_stats(xbridge->mbridge, mirrors, + ctx->xin->resubmit_stats->n_packets, + ctx->xin->resubmit_stats->n_bytes); + } + if (ctx->xin->xcache) { + struct xc_entry *entry; + + entry = xlate_cache_add_entry(ctx->xin->xcache, XC_MIRROR); + entry->u.mirror.mbridge = mbridge_ref(xbridge->mbridge); + entry->u.mirror.mirrors = mirrors; + } while (mirrors) { + const unsigned long *vlans; mirror_mask_t dup_mirrors; struct ofbundle *out; - const unsigned long *vlans; - bool vlan_mirrored; - bool has_mirror; int out_vlan; - has_mirror = mirror_get(xbridge->mbridge, raw_ctz(mirrors), - &vlans, &dup_mirrors, &out, &out_vlan); + bool has_mirror = mirror_get(xbridge->mbridge, raw_ctz(mirrors), + &vlans, &dup_mirrors, &out, &out_vlan); ovs_assert(has_mirror); if (vlans) { ctx->wc->masks.vlan_tci |= htons(VLAN_CFI | VLAN_VID_MASK); } - vlan_mirrored = !vlans || bitmap_is_set(vlans, vlan); - if (!vlan_mirrored) { + if (vlans && !bitmap_is_set(vlans, vlan)) { mirrors = zero_rightmost_1bit(mirrors); continue; } @@ -1593,7 +1592,7 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct flow *orig_flow) output_normal(ctx, out_xbundle, vlan); } } else if (vlan != out_vlan - && !eth_addr_is_reserved(orig_flow->dl_dst)) { + && !eth_addr_is_reserved(ctx->xin->flow.dl_dst)) { struct xbundle *xbundle; LIST_FOR_EACH (xbundle, list_node, &xbridge->xbundles) { @@ -1606,6 +1605,20 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct flow *orig_flow) } } +static void +mirror_ingress_packet(struct xlate_ctx *ctx) +{ + if (mbridge_has_mirrors(ctx->xbridge->mbridge)) { + bool warn = ctx->xin->packet != NULL; + struct xbundle *xbundle = lookup_input_bundle( + ctx->xbridge, ctx->xin->flow.in_port.ofp_port, warn, NULL); + if (xbundle) { + mirror_packet(ctx, xbundle, + xbundle_mirror_src(ctx->xbridge, xbundle)); + } + } +} + /* Given 'vid', the VID obtained from the 802.1Q header that was received as * part of a packet (specify 0 if there was no 802.1Q header), and 'in_xbundle', * the bundle on which the packet was received, returns the VLAN to which the @@ -2812,11 +2825,6 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, } } - if (mbridge_has_mirrors(ctx->xbridge->mbridge) && xport->xbundle) { - ctx->mirrors |= xbundle_mirror_dst(xport->xbundle->xbridge, - xport->xbundle); - } - if (xport->peer) { const struct xport *peer = xport->peer; struct flow old_flow = ctx->xin->flow; @@ -3032,6 +3040,12 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, ctx->nf_output_iface = ofp_port; } + if (mbridge_has_mirrors(ctx->xbridge->mbridge) && xport->xbundle) { + mirror_packet(ctx, xport->xbundle, + xbundle_mirror_dst(xport->xbundle->xbridge, + xport->xbundle)); + } + out: /* Restore flow */ flow->vlan_tci = flow_vlan_tci; @@ -4878,13 +4892,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) } xout->fail_open = ctx.rule && rule_dpif_is_fail_open(ctx.rule); - struct flow orig_flow; - if (mbridge_has_mirrors(xbridge->mbridge)) { - /* Do this conditionally because the copy is expensive enough that it - * shows up in profiles. */ - orig_flow = *flow; - } - /* Get the proximate input port of the packet. (If xin->recirc, * flow->in_port is the ultimate input port of the packet.) */ struct xport *in_port = get_ofp_port(xbridge, @@ -4947,6 +4954,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) OVS_NOT_REACHED(); } + mirror_ingress_packet(&ctx); do_xlate_actions(ofpacts, ofpacts_len, &ctx); /* We've let OFPP_NORMAL and the learning action look at the @@ -4986,11 +4994,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) if (user_cookie_offset) { fix_sflow_action(&ctx, user_cookie_offset); } - /* Only mirror fully processed packets. */ - if (!exit_recirculates(&ctx) - && mbridge_has_mirrors(xbridge->mbridge)) { - add_mirror_actions(&ctx, &orig_flow); - } } if (nl_attr_oversized(ctx.odp_actions->size)) { @@ -5005,22 +5008,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) ctx.xout->slow |= SLOW_ACTION; } - /* Update mirror stats only for packets really received by the bridge. */ - if (!xin->recirc && mbridge_has_mirrors(xbridge->mbridge)) { - if (ctx.xin->resubmit_stats) { - mirror_update_stats(xbridge->mbridge, ctx.mirrors, - ctx.xin->resubmit_stats->n_packets, - ctx.xin->resubmit_stats->n_bytes); - } - if (ctx.xin->xcache) { - struct xc_entry *entry; - - entry = xlate_cache_add_entry(ctx.xin->xcache, XC_MIRROR); - entry->u.mirror.mbridge = mbridge_ref(xbridge->mbridge); - entry->u.mirror.mirrors = ctx.mirrors; - } - } - /* Do netflow only for packets really received by the bridge and not sent * to the controller. We consider packets sent to the controller to be * part of the control plane rather than the data plane. */ diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 2656ca77a..9bfb7945a 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -3954,13 +3954,13 @@ 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: 2,3 + [Datapath actions: 3,2 ]) 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)" AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow"], [0], [stdout]) AT_CHECK_UNQUOTED([tail -1 stdout], [0], - [Datapath actions: 1,3 + [Datapath actions: 3,1 ]) OVS_VSWITCHD_STOP @@ -3984,7 +3984,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: 2,3 + [Datapath actions: 3,2 ]) 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)" @@ -4074,7 +4074,7 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0], flow="in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=11,pcp=0),encap(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: 2,3 + [Datapath actions: 3,2 ]) OVS_VSWITCHD_STOP @@ -4098,13 +4098,13 @@ 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: push_vlan(vid=17,pcp=0),2,pop_vlan,3 + [Datapath actions: 3,push_vlan(vid=17,pcp=0),2 ]) 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)" AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow"], [0], [stdout]) AT_CHECK_UNQUOTED([tail -1 stdout], [0], - [Datapath actions: 1,3 + [Datapath actions: 3,1 ]) OVS_VSWITCHD_STOP diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 483a9ded9..17035bbad 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -3424,6 +3424,32 @@ traffic may also be referred to as SPAN or RSPAN, depending on how the mirrored traffic is sent.

+

+ When a packet enters an Open vSwitch bridge, it becomes eligible for + mirroring based on its ingress port and VLAN. As the packet travels + through the flow tables, each time it is output to a port, it becomes + eligible for mirroring based on the egress port and VLAN. In Open + vSwitch 2.5 and later, mirroring occurs just after a packet first becomes + eligible, using the packet as it exists at that point; in Open vSwitch + 2.4 and earlier, mirroring occurs only after a packet has traversed all + the flow tables, using the original packet as it entered the bridge. + This makes a difference only when the flow table modifies the packet: in + Open vSwitch 2.4, the modifications are never visible to mirrors, whereas + in Open vSwitch 2.5 and later modifications made before the first output + that makes it eligible for mirroring to a particular destination are + visible. +

+ +

+ A packet that enters an Open vSwitch bridge is mirrored to a particular + destination only once, even if it is eligible for multiple reasons. For + example, a packet would be mirrored to a particular only once, even if it is selected for mirroring to + that port by and in the same or different + records. +

+ Arbitrary identifier for the . -- 2.20.1