ofproto-dpif-mirror: Add support for pre-selection filter.

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. Signed-off-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
2025-08-22 01:51:26 +00:00 · 2024-07-08 14:27:42 -04:00 · 2024-07-08 14:27:42 -04:00 · 3b1882261c
commit 3b1882261c
parent 04c090c61e
13 changed files with 365 additions and 15 deletions
--- a/Documentation/ref/ovs-tcpdump.8.rst
+++ b/Documentation/ref/ovs-tcpdump.8.rst
@ -61,8 +61,14 @@ Options

  If specified, mirror all ports (optional).

+* ``--filter <flow>``
+
+  If specified, only mirror packets that match the provided OpenFlow filter.
+  The available fields are documented in ``ovs-fields(7)``.
+
 See Also
 ========

 ``ovs-appctl(8)``, ``ovs-vswitchd(8)``, ``ovs-pcap(1)``,
-``ovs-tcpundump(1)``, ``tcpdump(8)``, ``wireshark(8)``.
+``ovs-fields(7)``, ``ovs-tcpundump(1)``, ``tcpdump(8)``,
+``wireshark(8)``.
--- a/6
+++ b/6
@ -9,6 +9,12 @@ Post-v3.3.0
     * 'dpif/show' and 'list-commands' now support output in JSON format.
     * Added 'ofproto/detrace' command that outputs the set of OpenFlow rules
       and groups that contributed to the creation of a specific datapath flow.
+   - ovs-vsctl:
+     * Added a new filter column in the Mirror table which can be used to
+       apply filters to mirror ports.
+   - ovs-tcpdump:
+     * Added command line parameter --filter to enable filtering the packets
+       that are captured by tcpdump.
   - Userspace datapath:
     * Conntrack now supports 'random' flag for selecting ports in a range
       while natting and 'persistent' flag for selection of the IP address
--- a/lib/flow.h
+++ b/lib/flow.h
@ -939,6 +939,15 @@ 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)
--- 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"

@ -48,6 +49,11 @@ struct mbundle {
    mirror_mask_t mirror_out;   /* Mirrors that output to this mbundle. */
 };

+struct filtermask {
+    struct miniflow *flow;
+    struct minimask *mask;
+};
+
 struct mirror {
    struct mbridge *mbridge;    /* Owning ofproto. */
    size_t idx;                 /* In ofproto's "mirrors" array. */
@ -57,6 +63,10 @@ struct mirror {
    struct hmapx srcs;          /* Contains "struct mbundle*"s. */
    struct hmapx dsts;          /* Contains "struct mbundle*"s. */

+    /* Filter criteria. */
+    OVSRCU_TYPE(struct filtermask *) filter_mask;
+    char *filter_str;
+
    /* This is accessed by handler threads assuming RCU protection (see
     * mirror_get()), but can be manipulated by mirror_set() without any
     * explicit synchronization. */
@ -83,6 +93,25 @@ static void mbundle_lookup_multiple(const struct mbridge *, struct ofbundle **,
 static int mirror_scan(struct mbridge *);
 static void mirror_update_dups(struct mbridge *);

+static void
+filtermask_free(struct filtermask *fm)
+{
+    free(fm->flow);
+    free(fm->mask);
+    free(fm);
+}
+
+static struct filtermask *
+filtermask_create(struct flow *flow, struct flow_wildcards *wc)
+{
+    struct filtermask *fm;
+
+    fm = xmalloc(sizeof *fm);
+    fm->flow = miniflow_create(flow);
+    fm->mask = minimask_create(wc);
+    return fm;
+}
+
 struct mbridge *
 mbridge_create(void)
 {
@ -207,8 +236,8 @@ mirror_bundle_dst(struct mbridge *mbridge, struct ofbundle *ofbundle)
 }

 int
-mirror_set(struct mbridge *mbridge, void *aux,
-           const struct ofproto_mirror_settings *ms,
+mirror_set(struct mbridge *mbridge, const struct ofproto *ofproto,
+           void *aux, const struct ofproto_mirror_settings *ms,
           const struct mirror_bundles *mb)
 {
    struct mbundle *mbundle, *out;
@ -264,11 +293,13 @@ mirror_set(struct mbridge *mbridge, void *aux,
        && vlan_bitmap_equal(vlans, ms->src_vlans)
        && mirror->out == out
        && mirror->out_vlan == out_vlan
-        && mirror->snaplen == ms->snaplen)
+        && mirror->snaplen == ms->snaplen
+        && nullable_string_is_equal(mirror->filter_str, ms->filter)
+        && !ms->filter)
    {
        hmapx_destroy(&srcs_map);
        hmapx_destroy(&dsts_map);
-        return 0;
+        return ECANCELED;
    }

    /* XXX: Not sure if these need to be thread safe. */
@ -288,6 +319,50 @@ mirror_set(struct mbridge *mbridge, void *aux,
    mirror->out_vlan = out_vlan;
    mirror->snaplen = ms->snaplen;

+    if (!nullable_string_is_equal(mirror->filter_str, ms->filter)) {
+        if (mirror->filter_str) {
+            ovsrcu_postpone(filtermask_free,
+                            ovsrcu_get(struct filtermask *,
+                                       &mirror->filter_mask));
+            free(mirror->filter_str);
+            mirror->filter_str = NULL;
+            ovsrcu_set(&mirror->filter_mask, NULL);
+        }
+
+        if (ms->filter && strlen(ms->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),
+                                       ms->filter, &map);
+            ofputil_port_map_destroy(&map);
+            if (err) {
+                VLOG_WARN("filter is invalid: %s", err);
+                free(err);
+                mirror_destroy(mbridge, mirror->aux);
+                return EINVAL;
+            }
+
+            /* If the user wants to filter on in_port, they should use the srcs
+             * bundle.  Users setting in_port could experience unexpected
+             * behavior, and it would be overly complex to detect all possible
+             * issues.  So instead we attempt to extract the in_port and error
+             * if successful. */
+            if (wc.masks.in_port.ofp_port) {
+                VLOG_WARN("filter is invalid due to in_port field.");
+                mirror_destroy(mbridge, mirror->aux);
+                return EINVAL;
+            }
+
+            mirror->filter_str = xstrdup(ms->filter);
+            ovsrcu_set(&mirror->filter_mask, filtermask_create(&flow, &wc));
+        }
+    }
+
    /* Update mbundles. */
    mirror_bit = MIRROR_MASK_C(1) << mirror->idx;
    CMAP_FOR_EACH (mbundle, cmap_node, &mirror->mbridge->mbundles) {
@ -343,6 +418,15 @@ mirror_destroy(struct mbridge *mbridge, void *aux)
        ovsrcu_postpone(free, vlans);
    }

+    if (mirror->filter_str) {
+        ovsrcu_postpone(filtermask_free,
+                        ovsrcu_get(struct filtermask *,
+                                   &mirror->filter_mask));
+        free(mirror->filter_str);
+        mirror->filter_str = NULL;
+        ovsrcu_set(&mirror->filter_mask, NULL);
+    }
+
    mbridge->mirrors[mirror->idx] = NULL;
    /* mirror_get() might have just read the pointer, so we must postpone the
     * free. */
@ -414,7 +498,9 @@ mirror_update_stats(struct mbridge *mbridge, mirror_mask_t mirrors,
 * in which a 1-bit indicates that the mirror includes a particular VLAN,
 * 'mc->dup_mirrors' receives a bitmap of mirrors whose output duplicates
 * mirror 'index', 'mc->out' receives the output ofbundle (if any),
- * and 'mc->out_vlan' receives the output VLAN (if any).
+ * and 'mc->out_vlan' receives the output VLAN (if any).  In cases where the
+ * mirror has a filter configured 'mc->filter_flow' and 'mc->filter_mask'
+ * receives the flow and mask that this mirror should collect.
 *
 * Everything returned here is assumed to be RCU protected.
 */
@ -422,6 +508,7 @@ bool
 mirror_get(struct mbridge *mbridge, int index,
           struct mirror_config *mc)
 {
+    struct filtermask *fm;
    struct mirror *mirror;

    if (!mc || !mbridge) {
@ -440,6 +527,14 @@ mirror_get(struct mbridge *mbridge, int index,
    mc->out_bundle = mirror->out ? mirror->out->ofbundle : NULL;
    mc->out_vlan = mirror->out_vlan;
    mc->snaplen = mirror->snaplen;
+    fm = ovsrcu_get(struct filtermask *, &mirror->filter_mask);
+    if (fm) {
+        mc->filter_flow = fm->flow;
+        mc->filter_mask = fm->mask;
+    } else {
+        mc->filter_flow = NULL;
+        mc->filter_mask = NULL;
+    }
    return true;
 }

--- a/ofproto/ofproto-dpif-mirror.h
+++ b/ofproto/ofproto-dpif-mirror.h
@ -23,8 +23,8 @@
 typedef uint32_t mirror_mask_t;

 struct ofproto_mirror_settings;
-struct ofproto_dpif;
 struct ofbundle;
+struct ofproto;

 struct mirror_bundles {
    struct ofbundle **srcs;
@ -43,6 +43,10 @@ struct mirror_config {
    /* VLANs of packets to select for mirroring. */
    unsigned long *vlans;           /* vlan_bitmap, NULL selects all VLANs. */

+    /* Miniflow and minimask if a filter is configured, else both are NULL. */
+    struct miniflow *filter_flow;
+    struct minimask *filter_mask;
+
    /* Output (mutually exclusive). */
    struct ofbundle *out_bundle;    /* A registered ofbundle handle or NULL. */
    uint16_t out_vlan;              /* Output VLAN, not used if out_bundle is
@ -76,7 +80,7 @@ bool mbridge_need_revalidate(struct mbridge *);
 void mbridge_register_bundle(struct mbridge *, struct ofbundle *);
 void mbridge_unregister_bundle(struct mbridge *, struct ofbundle *);

-int mirror_set(struct mbridge *, void *aux,
+int mirror_set(struct mbridge *, const struct ofproto *, void *aux,
               const struct ofproto_mirror_settings *,
               const struct mirror_bundles *);
 void mirror_destroy(struct mbridge *, void *aux);
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@ -2262,7 +2262,8 @@ lookup_input_bundle(const struct xlate_ctx *ctx,

 /* Mirrors the packet represented by 'ctx' to appropriate mirror destinations,
 * given the packet is ingressing or egressing on 'xbundle', which has ingress
- * or egress (as appropriate) mirrors 'mirrors'. */
+ * or egress (as appropriate) mirrors 'mirrors'.  In cases where a mirror is
+ * filtered, the current wildcard for the flow's current filter is modified. */
 static void
 mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle,
              mirror_mask_t mirrors)
@ -2313,6 +2314,18 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle,
            continue;
        }

+        /* After the VLAN check, apply a flow mask if a filter is specified. */
+        if (ctx->wc && mc.filter_flow) {
+            flow_wildcards_union_with_minimask(ctx->wc, mc.filter_mask);
+            if (!OVS_UNLIKELY(
+                 miniflow_equal_flow_in_minimask(mc.filter_flow,
+                                                 &ctx->xin->flow,
+                                                 mc.filter_mask))) {
+                mirrors = zero_rightmost_1bit(mirrors);
+                continue;
+            }
+        }
+
        /* We sent a packet to this mirror. */
        used_mirrors |= rightmost_1bit(mirrors);

--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@ -3843,7 +3843,17 @@ mirror_set__(struct ofproto *ofproto_, void *aux,
    mb.n_dsts = s->n_dsts;
    mb.out_bundle = bundle_lookup(ofproto, s->out_bundle);

-    error = mirror_set(ofproto->mbridge, aux, s, &mb);
+    error = mirror_set(ofproto->mbridge, ofproto_, aux, s, &mb);
+
+    if (!error) {
+        ofproto->backer->need_revalidate = REV_RECONFIGURE;
+    } else if (error == ECANCELED) {
+        /* The user requested a change that is identical to the current state,
+         * the reconfiguration is canceled, but don't log an error message
+         * about that. */
+        error = 0;
+    }
+
    free(mb.srcs);
    free(mb.dsts);
    return error;
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@ -511,6 +511,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,
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@ -5220,6 +5220,172 @@ AT_CHECK([tail -1 stdout], [0],
 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
+AT_CHECK([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"], [0], [ignore])
+
+icmp_flow="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)"
+tcp_flow1="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(dst=443)"
+tcp_flow2="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(dst=80)"
+
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl add-flow br0 'actions=normal' ])
+
+dnl Add non-matching flows, then change the mirror to match one of the flows,
+dnl then add a matching flow.
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 $icmp_flow])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 $tcp_flow1])
+AT_CHECK([ovs-vsctl set mirror mymirror filter="tcp"], [0])
+AT_CHECK([ovs-appctl revalidator/wait])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 $tcp_flow2])
+AT_CHECK([ovs-appctl dpif/dump-flows --names br0 | strip_ufid | strip_used | sort], [0], [dnl
+recirc_id(0),in_port(p1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),dnl
+eth_type(0x0800),ipv4(proto=1,frag=no), packets:0, bytes:0, used:never, actions:br0,p2
+recirc_id(0),in_port(p1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),dnl
+eth_type(0x0800),ipv4(proto=6,frag=no), packets:1, bytes:118, used:0.0s, actions:p3,br0,p2
+])
+AT_CHECK([ovs-appctl dpctl/dump-flows --names | strip_ufid | strip_used | sort], [0], [dnl
+flow-dump from the main thread:
+recirc_id(0),in_port(p1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),dnl
+eth_type(0x0800),ipv4(proto=1,frag=no), packets:0, bytes:0, used:never, actions:br0,p2
+recirc_id(0),in_port(p1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),dnl
+eth_type(0x0800),ipv4(proto=6,frag=no), packets:1, bytes:118, used:0.0s, actions:p3,br0,p2
+])
+
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=1 actions=output:2"])
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=2 actions=output:1"])
+
+dnl Add mirrored flow after non-mirrored flow.
+AT_CHECK([ovs-vsctl set mirror mymirror filter="icmp"], [0])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 $tcp_flow1])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 $icmp_flow])
+AT_CHECK([ovs-appctl dpif/dump-flows --names br0 | strip_ufid | strip_used | sort], [0], [dnl
+recirc_id(0),in_port(p1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),dnl
+eth_type(0x0800),ipv4(proto=1,frag=no), packets:1, bytes:106, used:0.0s, actions:p3,p2
+recirc_id(0),in_port(p1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),dnl
+eth_type(0x0800),ipv4(proto=6,frag=no), packets:2, bytes:236, used:0.0s, actions:p2
+])
+
+dnl Check one direction, only icmp should mirror.
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$icmp_flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,2
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 2
+])
+
+dnl Check other direction, only icmp should mirror.
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(2),$icmp_flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,1
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(2),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 1
+])
+
+dnl Change filter to tcp, only tcp should mirror.
+AT_CHECK([ovs-vsctl set mirror mymirror filter="tcp"], [0])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$icmp_flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 2
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,2
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(2),$icmp_flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 1
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(2),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,1
+])
+
+dnl Invalid filter. Nothing should mirror, error should be logged.
+AT_CHECK([ovs-vsctl set mirror mymirror filter="invalid"], [0])
+dnl Setting an in_port is also invalid.
+AT_CHECK([ovs-vsctl set mirror mymirror filter="\"in_port=p1\""], [0])
+
+dnl Each of the above two lines should produce two log messages.
+OVS_WAIT_UNTIL([test $(grep -Ec "filter is invalid|mirror mymirror configuration is invalid" ovs-vswitchd.log) -eq 4])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$icmp_flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 2
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 2
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(2),$icmp_flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 1
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(2),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 1
+])
+
+dnl Check more complex filter cases with partially overlapping default wildcards.
+AT_CHECK([ovs-vsctl set mirror mymirror filter="\"tcp,tcp_dst=80\""], [0])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 2
+])
+
+dnl Change port number.
+AT_CHECK([ovs-appctl dpif-dummy/change-port-number ovs-dummy p1 8])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(8),$tcp_flow2"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,2
+])
+
+dnl Empty filter, all traffic should mirror.
+AT_CHECK([ovs-vsctl clear mirror mymirror filter], [0])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(8),$icmp_flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,2
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(8),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,2
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(2),$icmp_flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,8
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(2),$tcp_flow1"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+  [Datapath actions: 3,8
+])
+
+OVS_VSWITCHD_STOP(["/filter is invalid: invalid: unknown field invalid/d
+/filter is invalid due to in_port field/d
+/mirror mymirror configuration is invalid/d"])
+AT_CLEANUP

 AT_SETUP([ofproto-dpif - mirroring, select_all])
 AT_KEYWORDS([mirror mirrors mirroring])
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@ -142,6 +142,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 an OpenFlow formatted preselection filter
 """ % {'prog': sys.argv[0]})
    sys.exit(0)

@ -354,7 +355,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'))
@ -362,6 +363,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')
@ -445,6 +449,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:
@ -474,6 +479,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:
@ -526,7 +535,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)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@ -5217,6 +5217,7 @@ mirror_configure(struct mirror *m)
 {
    const struct ovsrec_mirror *cfg = m->cfg;
    struct ofproto_mirror_settings s;
+    int ret;

    /* Set name. */
    if (strcmp(cfg->name, m->name)) {
@ -5285,8 +5286,18 @@ mirror_configure(struct mirror *m)
    /* Get VLAN selection. */
    s.src_vlans = vlan_bitmap_from_array(cfg->select_vlan, cfg->n_select_vlan);

+    /* Set the filter, mirror_set() will strdup this pointer. */
+    s.filter = cfg->filter;
+
    /* Configure. */
-    ofproto_mirror_register(m->bridge->ofproto, m, &s);
+    ret = ofproto_mirror_register(m->bridge->ofproto, m, &s);
+    if (ret == EOPNOTSUPP) {
+        VLOG_ERR("ofproto %s: does not support mirroring",
+                 m->bridge->ofproto->name);
+    } else if (ret) {
+        VLOG_ERR("bridge %s: mirror %s configuration is invalid",
+                 m->bridge->name, m->name);
+    }

    /* Clean up. */
    if (s.srcs != s.dsts) {
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "8.6.0",
- "cksum": "1543805939 27765",
+ "version": "8.7.0",
+ "cksum": "3751637058 27869",
 "tables": {
   "Open_vSwitch": {
     "columns": {
@ -461,6 +461,9 @@
         "type": {"key": "string", "value": "integer",
                  "min": 0, "max": "unlimited"},
         "ephemeral": true},
+       "filter": {
+         "type": {"key": {"type": "string"},
+                  "min": 0, "max": 1}},
       "external_ids": {
         "type": {"key": "string", "value": "string",
                  "min": 0, "max": "unlimited"}}}},
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@ -5286,6 +5286,21 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
        VLANs on which packets are selected for mirroring.  An empty set
        selects packets on all VLANs.
      </column>
+      <column name="filter">
+        <p>
+          When set, only packets that match <ref column="filter"/> are
+          selected for mirroring.  Packets that do not match are ignored
+          by thie mirror.  The <ref column="filter"/> syntax is described
+          in <code>ovs-fields</code>(7).  However, the <code>in_port</code>
+          field is not supported; <ref column="select_src_port"/> should be
+          used to limit the mirror to a source port.
+        </p>
+        <p>
+          This filter is applied after <ref column="select_all"/>, <ref
+          column="select_dst_port"/>, <ref column="select_src_port"/>, and
+          <ref column="select_vlan"/>.
+        </p>
+      </column>
    </group>

    <group title="Mirroring Destination Configuration">