On 1 May 2024, at 16:54, Mike Pattrick wrote:

> 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]>
> ---
> v8:
>  - Corrected code from v7 related to sequence and in_port. Mirrors
>    reject filters with an in_port set as this could cause confusion.
>  - Combined ovsrcu pointers into a new struct, minimatch wasn't used
>    because the minimatch_* functions didn't fit the usage here.
>  - Added a test to check for modifying filters when partially
>    overlapping flows already exist.
>  - Corrected documentation.
> v9:
>  - Explicitly cleared mirror_config.filter* when not set
> ---
>  Documentation/ref/ovs-tcpdump.8.rst |   8 +-
>  NEWS                                |   6 +
>  lib/flow.h                          |   9 ++
>  ofproto/ofproto-dpif-mirror.c       | 104 +++++++++++++++++-
>  ofproto/ofproto-dpif-mirror.h       |   9 +-
>  ofproto/ofproto-dpif-xlate.c        |  15 ++-
>  ofproto/ofproto-dpif.c              |  12 +-
>  ofproto/ofproto.h                   |   3 +
>  tests/ofproto-dpif.at               | 165 ++++++++++++++++++++++++++++
>  utilities/ovs-tcpdump.in            |  13 ++-
>  vswitchd/bridge.c                   |  13 ++-
>  vswitchd/vswitch.ovsschema          |   7 +-
>  vswitchd/vswitch.xml                |  16 +++
>  13 files changed, 365 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/ref/ovs-tcpdump.8.rst 
> b/Documentation/ref/ovs-tcpdump.8.rst
> index b9f8cdf6f..e21e61211 100644
> --- 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 flows 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
> index b92cec532..f3a4bf076 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -7,6 +7,12 @@ Post-v3.3.0
>     - The primary development branch has been renamed from 'master' to 'main'.
>       The OVS tree remains hosted on GitHub.
> https://github.com/openvswitch/ovs.git
> +   - 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 flows
> +       that are captured by tcpdump.
>
>
>  v3.3.0 - 16 Feb 2024
> diff --git a/lib/flow.h b/lib/flow.h
> index 75a9be3c1..60ec4b0d7 100644
> --- 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
> index 4967ecc9a..6d89d13a5 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"
>
> @@ -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,23 @@ 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 = xmalloc(sizeof *fm);
> +    fm->flow = miniflow_create(flow);
> +    fm->mask = minimask_create(wc);
> +    return fm;
> +}
> +
>  struct mbridge *
>  mbridge_create(void)
>  {
> @@ -207,8 +234,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 +291,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 +317,51 @@ 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) ||
> +        (mirror->filter_str || ms->filter)) {
> +        if (mirror->filter_str) {
> +            ovsrcu_postpone(filtermask_free,
> +                            ovsrcu_get_protected(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 +417,15 @@ mirror_destroy(struct mbridge *mbridge, void *aux)
>          ovsrcu_postpone(free, vlans);
>      }
>
> +    if (mirror->filter_str) {
> +        ovsrcu_postpone(filtermask_free,
> +                        ovsrcu_get_protected(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 +497,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.
>   */
> @@ -423,6 +508,7 @@ mirror_get(struct mbridge *mbridge, int index,
>             struct mirror_config *mc)
>  {
>      struct mirror *mirror;
> +    struct filtermask *fm;
>
>      if (!mc || !mbridge) {
>          return false;
> @@ -440,6 +526,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
> index 37d57463c..90331ede5 100644
> --- 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,11 @@ struct mirror_config {
>      /* VLANs of packets to select for mirroring. */
>      unsigned long *vlans;           /* vlan_bitmap, NULL selects all VLANs. 
> */
>
> +    /* The flow if a filter is used, or NULL. */
> +    struct miniflow *filter_flow;
> +    /* The filter's flow mask, or NULL. */
> +    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 +81,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
> index 55846fe98..88015f817 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2250,7 +2250,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 flows wildcard will be modified. */
>  static void
>  mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle,
>                mirror_mask_t mirrors)
> @@ -2302,6 +2303,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
> index 6f5830e40..7a3df1a47 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3754,7 +3754,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
> index 1c07df275..655caa14e 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -501,6 +501,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 3eaccb13a..4e7913352 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -5199,6 +5199,171 @@ 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 dpif/dump-flows -m br0 | grep -cE "actions:p2$"], [0], 
> [1
> +])
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 $icmp_flow])
> +AT_CHECK([ovs-appctl dpif/dump-flows -m br0 | grep -cE "actions:p3,p2$"], 
> [0], [1
> +])
> +
> +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])
>  OVS_VSWITCHD_START
> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index eada803bb..07fc435fa 100755
> --- 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)
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 95a65fcdc..39f8fba49 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -5144,6 +5144,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)) {
> @@ -5212,8 +5213,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) {
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index e2d5e2e85..f42f09121 100644
> --- a/vswitchd/vswitch.ovsschema
> +++ b/vswitchd/vswitch.ovsschema
> @@ -1,6 +1,6 @@
>  {"name": "Open_vSwitch",
> - "version": "8.5.0",
> - "cksum": "4040946650 27557",
> + "version": "8.6.0",
> + "cksum": "401789815 27661",
>   "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"}}}},
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 8a1b607d7..2aea5c9fd 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -5256,6 +5256,22 @@ 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, support for the <code>
> +            in_port</code> field is not supported; <ref
> +            column="select_src_port"/> should be used to limit the mirror to
> +            an 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">
> -- 
> 2.39.3
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to