Skip to content

Commit

Permalink
ofproto-dpif-xlate: Rewrite mirroring to better fit flow translation.
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Acked-by: Jarno Rajahalme <[email protected]>
  • Loading branch information
@blp
blp committed Jul 31, 2015
1 parent bef1403 commit 7efbc3b
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 69 deletions.
113 changes: 50 additions & 63 deletions ofproto/ofproto-dpif-xlate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)) {
Expand All @@ -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. */
Expand Down
12 changes: 6 additions & 6 deletions tests/ofproto-dpif.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
26 changes: 26 additions & 0 deletions vswitchd/vswitch.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3424,6 +3424,32 @@
traffic may also be referred to as SPAN or RSPAN, depending on how
the mirrored traffic is sent.</p>

<p>
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.
</p>

<p>
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 <ref
column="output_port"/> only once, even if it is selected for mirroring to
that port by <ref column="select_dst_port"/> and <ref
column="select_src_port"/> in the same or different <ref table="Mirror"/>
records.
</p>

<column name="name">
Arbitrary identifier for the <ref table="Mirror"/>.
</column>
Expand Down

0 comments on commit 7efbc3b

Please sign in to comment.