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 <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>
ovsrobot · Jul 15, 2024 · 3b18822 · 3b18822
1 parent 04c090c
commit 3b18822
Show file tree

Hide file tree

Showing 13 changed files with 365 additions and 15 deletions.
diff --git 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)``.
diff --git a/NEWS b/NEWS
@@ -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

diff --git 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)

diff --git 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,14 +498,17 @@ 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.
  */
 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;
 }
 

diff --git 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);

diff --git 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);
 

diff --git 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;

diff --git 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,