Currently a bridge mirror will collect all packets and tools like
ovs-tcpdump can apply additional filters after they have already been
duplicated by vswitchd. This can result in inefficient collection.
This patch adds support to apply pre-selection to bridge mirrors, which
can limit which packets are mirrored based on flow metadata. This
significantly improves overall vswitchd performance during mirroring if
only a subset of traffic is required.
I benchmarked this with two setups. A netlink based test with two veth
pairs connected to a single bridge, and a netdev based test involving a
mix of DPDK nics, and netdev-linux interfaces. Both tests involved
saturating the link with iperf3 and then sending an icmp ping every
second. I then measured the throughput on the link with no mirroring,
icmp pre-selected mirroring, and full mirroring. The results, below,
indicate a significant reduction to impact of mirroring when only a
subset of the traffic on a port is selected for collection.
Test No Mirror | No Filter | Filter | No Filter | Filter |
+-----------+-----------+-----------+-----------+----------+
netlink | 39.0 Gbps | 36.1 Gbps | 38.2 Gbps | 7% | 2% |
netdev | 7.39 Gbps | 4.95 Gbps | 6.24 Gbps | 33% | 15% |
The ratios above are the percent reduction in total throughput when
mirroring is used either with or without a filter.
Signed-off-by: Mike Pattrick <m...@redhat.com>
---
Documentation/ref/ovs-tcpdump.8.rst | 4 ++
NEWS | 2 +
lib/flow.h | 11 ++++++
ofproto/ofproto-dpif-mirror.c | 60 +++++++++++++++++++++++++++--
ofproto/ofproto-dpif-mirror.h | 9 ++++-
ofproto/ofproto-dpif-xlate.c | 6 ++-
ofproto/ofproto-dpif.c | 2 +-
ofproto/ofproto.h | 3 ++
tests/ofproto-dpif.at | 43 +++++++++++++++++++++
utilities/ovs-tcpdump.in | 13 ++++++-
vswitchd/bridge.c | 8 ++++
vswitchd/vswitch.ovsschema | 4 +-
vswitchd/vswitch.xml | 6 +++
13 files changed, 160 insertions(+), 11 deletions(-)
diff --git a/Documentation/ref/ovs-tcpdump.8.rst
b/Documentation/ref/ovs-tcpdump.8.rst
index b9f8cdf6f..9163c8a5e 100644
--- a/Documentation/ref/ovs-tcpdump.8.rst
+++ b/Documentation/ref/ovs-tcpdump.8.rst
@@ -61,6 +61,10 @@ Options
If specified, mirror all ports (optional).
+* ``--filter <flow>``
+
+ If specified, only mirror flows that match the provided filter.
+
See Also
========
diff --git a/NEWS b/NEWS
index 7a852427e..26797ca56 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
Post-v3.2.0
--------------------
+ - ovs-tcpdump:
+ * Added new --filter parameter to apply pre-selection on mirrored flows.
v3.2.0 - xx xxx xxxx
diff --git a/lib/flow.h b/lib/flow.h
index a9d026e1c..c2e67dfc5 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -939,6 +939,17 @@ flow_union_with_miniflow(struct flow *dst, const struct
miniflow *src)
flow_union_with_miniflow_subset(dst, src, src->map);
}
+/* Perform a bitwise OR of minimask 'src' mask data with the equivalent
+ * fields in 'dst', storing the result in 'dst'. */
+static inline void
+flow_wildcards_union_with_minimask(struct flow_wildcards *dst,
+ const struct minimask *src)
+{
+ flow_union_with_miniflow_subset(&dst->masks,
+ &src->masks,
+ src->masks.map);
+}
+
static inline bool is_ct_valid(const struct flow *flow,
const struct flow_wildcards *mask,
struct flow_wildcards *wc)
diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c
index 343b75f0e..e4174a564 100644
--- a/ofproto/ofproto-dpif-mirror.c
+++ b/ofproto/ofproto-dpif-mirror.c
@@ -21,6 +21,7 @@
#include "cmap.h"
#include "hmapx.h"
#include "ofproto.h"
+#include "ofproto-dpif-trace.h"
#include "vlan-bitmap.h"
#include "openvswitch/vlog.h"
@@ -57,6 +58,10 @@ struct mirror {
struct hmapx srcs; /* Contains "struct mbundle*"s. */
struct hmapx dsts; /* Contains "struct mbundle*"s. */
+ /* Filter criteria. */
+ struct miniflow *filter;
+ struct minimask *mask;
+
/* This is accessed by handler threads assuming RCU protection (see
* mirror_get()), but can be manipulated by mirror_set() without any
* explicit synchronization. */
@@ -212,7 +217,9 @@ mirror_set(struct mbridge *mbridge, void *aux, const char
*name,
struct ofbundle **dsts, size_t n_dsts,
unsigned long *src_vlans, struct ofbundle *out_bundle,
uint16_t snaplen,
- uint16_t out_vlan)
+ uint16_t out_vlan,
+ const char *filter,
+ const struct ofproto *ofproto)
{
struct mbundle *mbundle, *out;
mirror_mask_t mirror_bit;
@@ -285,6 +292,33 @@ mirror_set(struct mbridge *mbridge, void *aux, const char
*name,
mirror->out_vlan = out_vlan;
mirror->snaplen = snaplen;
+ if (mirror->filter) {
+ ovsrcu_postpone(free, mirror->filter);
+ ovsrcu_postpone(free, mirror->mask);
+ mirror->filter = NULL;
+ mirror->mask = NULL;
+ }
+ if (filter) {
+ struct ofputil_port_map map = OFPUTIL_PORT_MAP_INITIALIZER(&map);
+ struct flow_wildcards wc;
+ struct flow flow;
+ char *err;
+
+ ofproto_append_ports_to_map(&map, ofproto->ports);
+ err = parse_ofp_exact_flow(&flow, &wc,
+ ofproto_get_tun_tab(ofproto),
+ filter, &map);
+ ofputil_port_map_destroy(&map);
+ if (err) {
+ VLOG_WARN("filter is invalid: %s", err);
+ free(err);
+ return EINVAL;
+ }
+
+ mirror->mask = minimask_create(&wc);
+ mirror->filter = miniflow_create(&flow);
+ }
+
/* Update mbundles. */
mirror_bit = MIRROR_MASK_C(1) << mirror->idx;
CMAP_FOR_EACH (mbundle, cmap_node, &mirror->mbridge->mbundles) {
@@ -340,6 +374,13 @@ mirror_destroy(struct mbridge *mbridge, void *aux)
ovsrcu_postpone(free, vlans);
}
+ if (mirror->filter) {
+ ovsrcu_postpone(free, mirror->filter);
+ ovsrcu_postpone(free, mirror->mask);
+ mirror->filter = NULL;
+ mirror->mask = NULL;
+ }
+
mbridge->mirrors[mirror->idx] = NULL;
/* mirror_get() might have just read the pointer, so we must postpone the
* free. */
@@ -411,14 +452,17 @@ mirror_update_stats(struct mbridge *mbridge,
mirror_mask_t mirrors,
* in which a 1-bit indicates that the mirror includes a particular VLAN,
* '*dup_mirrors' receives a bitmap of mirrors whose output duplicates mirror
* 'index', '*out' receives the output ofbundle (if any), and '*out_vlan'
- * receives the output VLAN (if any).
+ * receives the output VLAN (if any). In cases where the mirror has a filter
+ * configured and the flow matches that filter, '*wc' is modified to include
+ * that filters mask.
*
* Everything returned here is assumed to be RCU protected.
*/
bool
mirror_get(struct mbridge *mbridge, int index, const unsigned long **vlans,
mirror_mask_t *dup_mirrors, struct ofbundle **out,
- int *snaplen, int *out_vlan)
+ int *snaplen, int *out_vlan, struct flow *flow,
+ struct flow_wildcards *wc)
{
struct mirror *mirror;
@@ -430,6 +474,16 @@ mirror_get(struct mbridge *mbridge, int index, const
unsigned long **vlans,
if (!mirror) {
return false;
}
+
+ if (wc && mirror->filter) {
+ if (miniflow_equal_flow_in_minimask(mirror->filter, flow,
+ mirror->mask)) {
+ flow_wildcards_union_with_minimask(wc, mirror->mask);
+ } else {
+ return false;
+ }
+ }
+
/* Assume 'mirror' is RCU protected, i.e., it will not be freed until this
* thread quiesces. */
diff --git a/ofproto/ofproto-dpif-mirror.h b/ofproto/ofproto-dpif-mirror.h
index eed63ec4a..6de742cbf 100644
--- a/ofproto/ofproto-dpif-mirror.h
+++ b/ofproto/ofproto-dpif-mirror.h
@@ -24,6 +24,9 @@ typedef uint32_t mirror_mask_t;
struct ofproto_dpif;
struct ofbundle;
+struct ofproto;
+struct flow;
+struct flow_wildcards;
/* The following functions are used by handler threads without any locking,
* assuming RCU protection. */
@@ -40,7 +43,8 @@ void mirror_update_stats(struct mbridge*, mirror_mask_t,
uint64_t packets,
uint64_t bytes);
bool mirror_get(struct mbridge *, int index, const unsigned long **vlans,
mirror_mask_t *dup_mirrors, struct ofbundle **out,
- int *snaplen, int *out_vlan);
+ int *snaplen, int *out_vlan, struct flow *flow,
+ struct flow_wildcards *wc);
/* The remaining functions are assumed to be called by the main thread only. */
@@ -54,7 +58,8 @@ int mirror_set(struct mbridge *, void *aux, const char *name,
struct ofbundle **srcs, size_t n_srcs,
struct ofbundle **dsts, size_t n_dsts,
unsigned long *src_vlans, struct ofbundle *out_bundle,
- uint16_t snaplen, uint16_t out_vlan);
+ uint16_t snaplen, uint16_t out_vlan, const char *filter,
+ const struct ofproto *ofproto);
void mirror_destroy(struct mbridge *, void *aux);
int mirror_get_stats(struct mbridge *, void *aux, uint64_t *packets,
uint64_t *bytes);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4928ea99c..4547caa31 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2246,8 +2246,10 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle
*xbundle,
/* Get the details of the mirror represented by the rightmost 1-bit. */
if (OVS_UNLIKELY(!mirror_get(xbridge->mbridge, raw_ctz(mirrors),
&vlans, &dup_mirrors,
- &out, &snaplen, &out_vlan))) {
- /* The mirror got reconfigured before we got to read it's
+ &out, &snaplen, &out_vlan,
+ &ctx->xin->flow,
+ ctx->wc))) {
+ /* The mirror doesn't currently exist, we cannot retrieve it's
* configuration. */
mirrors = zero_rightmost_1bit(mirrors);
continue;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index fad7342b0..c03bcea60 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3662,7 +3662,7 @@ mirror_set__(struct ofproto *ofproto_, void *aux,
error = mirror_set(ofproto->mbridge, aux, s->name, srcs, s->n_srcs, dsts,
s->n_dsts, s->src_vlans,
bundle_lookup(ofproto, s->out_bundle),
- s->snaplen, s->out_vlan);
+ s->snaplen, s->out_vlan, s->filter, ofproto_);
free(srcs);
free(dsts);
return error;
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 8efdb20a0..372e0e3fc 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -497,6 +497,9 @@ struct ofproto_mirror_settings {
uint16_t out_vlan; /* Output VLAN, only if out_bundle is NULL. */
uint16_t snaplen; /* Max packet size of a mirrored packet
in byte, set to 0 equals 65535. */
+
+ /* Output filter */
+ char *filter;
};
int ofproto_mirror_register(struct ofproto *, void *aux,
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 6824ce0bb..e55c480d1 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5182,6 +5182,49 @@ OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - mirroring, filter])
+AT_KEYWORDS([mirror mirrors mirroring])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2 3
+ovs-vsctl \
+ set Bridge br0 mirrors=@m --\
+ --id=@p3 get Port p3 --\
+ --id=@m create Mirror name=mymirror select_all=true output_port=@p3
filter="icmp"
+
+AT_DATA([flows.txt], [dnl
+in_port=1 actions=output:2
+in_port=2 actions=output:1
+])
+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,2
+])
+
+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=6,tos=0,ttl=128,frag=no),tcp()"
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+ [Datapath actions: 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: 3,1
+])
+
+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=6,tos=0,ttl=128,frag=no),tcp()"
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+ [Datapath actions: 1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
AT_SETUP([ofproto-dpif - mirroring, select_all])
AT_KEYWORDS([mirror mirrors mirroring])
OVS_VSWITCHD_START
diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index 420c11eb8..590f02c77 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -138,6 +138,7 @@ The following options are available:
--mirror-to The name for the mirror port to use (optional)
Default 'miINTERFACE'
--span If specified, mirror all ports (optional)
+ --filter Set a preselection filter
""" % {'prog': sys.argv[0]})
sys.exit(0)
@@ -350,7 +351,7 @@ class OVSDB(object):
return result
def bridge_mirror(self, intf_name, mirror_intf_name, br_name,
- mirror_select_all=False):
+ mirror_select_all=False, mirror_filter=None):
txn = self._start_txn()
mirror = txn.insert(self.get_table('Mirror'))
@@ -358,6 +359,9 @@ class OVSDB(object):
mirror.select_all = mirror_select_all
+ if mirror_filter is not None:
+ mirror.filter = mirror_filter
+
mirrored_port = self._find_row_by_name('Port', intf_name)
mirror.verify('select_dst_port')
@@ -441,6 +445,7 @@ def main():
mirror_interface = None
mirror_select_all = False
dump_cmd = 'tcpdump'
+ mirror_filter = None
for cur, nxt in argv_tuples(sys.argv[1:]):
if skip_next:
@@ -470,6 +475,10 @@ def main():
elif cur in ['--span']:
mirror_select_all = True
continue
+ elif cur in ['--filter']:
+ mirror_filter = nxt
+ skip_next = True
+ continue
tcpdargs.append(cur)
if interface is None:
@@ -522,7 +531,7 @@ def main():
ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
ovsdb.bridge_mirror(interface, mirror_interface,
ovsdb.port_bridge(interface),
- mirror_select_all)
+ mirror_select_all, mirror_filter=mirror_filter)
except OVSDBException as oe:
print("ERROR: Unable to properly setup the mirror: %s." % str(oe))
sys.exit(1)
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index e9110c1d8..50213a1f9 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -5166,6 +5166,13 @@ mirror_configure(struct mirror *m)
/* Get VLAN selection. */
s.src_vlans = vlan_bitmap_from_array(cfg->select_vlan, cfg->n_select_vlan);
+ /* Set filter, this value is not retained after mirror_set() is called. */
+ if (cfg->filter && strlen(cfg->filter)) {
+ s.filter = xstrdup(cfg->filter);
+ } else {
+ s.filter = NULL;
+ }
+
/* Configure. */
ofproto_mirror_register(m->bridge->ofproto, m, &s);
@@ -5175,6 +5182,7 @@ mirror_configure(struct mirror *m)
}
free(s.srcs);
free(s.src_vlans);
+ free(s.filter);
return true;
}
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 2d395ff95..5400a5bed 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
{"name": "Open_vSwitch",
"version": "8.4.0",
- "cksum": "2738838700 27127",
+ "cksum": "59426185 27174",
"tables": {
"Open_vSwitch": {
"columns": {
@@ -461,6 +461,8 @@
"type": {"key": "string", "value": "integer",
"min": 0, "max": "unlimited"},
"ephemeral": true},
+ "filter": {
+ "type": "string"},
"external_ids": {
"type": {"key": "string", "value": "string",
"min": 0, "max": "unlimited"}}}},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index cfcde34ff..3965054d5 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -5237,6 +5237,12 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch
options:peer=p1 \
are not truncated.
</p>
</column>
+ <column name="filter">
+ <p>Filter to apply to mirror.</p>
+ <p>Flows that match <ref column="filter"/> will flow through this
+ mirror, flows that do not match will be ignored.
+ </p>
+ </column>
</group>
<group title="Statistics: Mirror counters">
--
2.39.3
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
