On 7/8/24 20:27, 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 > v10: > - Changed rcu memory order > - Updated documentation to refer to packets instead of flows > - Updated comments > --- > Documentation/ref/ovs-tcpdump.8.rst | 8 +- > NEWS | 6 + > lib/flow.h | 9 ++ > ofproto/ofproto-dpif-mirror.c | 106 +++++++++++++++++- > ofproto/ofproto-dpif-mirror.h | 8 +- > ofproto/ofproto-dpif-xlate.c | 15 ++- > ofproto/ofproto-dpif.c | 12 +- > ofproto/ofproto.h | 3 + > tests/ofproto-dpif.at | 167 ++++++++++++++++++++++++++++ > utilities/ovs-tcpdump.in | 13 ++- > vswitchd/bridge.c | 13 ++- > vswitchd/vswitch.ovsschema | 7 +- > vswitchd/vswitch.xml | 16 +++ > 13 files changed, 368 insertions(+), 15 deletions(-) > > diff --git a/Documentation/ref/ovs-tcpdump.8.rst > b/Documentation/ref/ovs-tcpdump.8.rst > index b9f8cdf6f..e7bd5e9e4 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 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 > index e0359b759..cbe777a71 100644 > --- a/NEWS > +++ b/NEWS > @@ -16,6 +16,12 @@ Post-v3.3.0 > per interface 'options:dpdk-lsc-interrupt' to 'false'. > - Python: > * Added custom transaction support to the Idl via add_op(). > + - 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. > > > 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..a14f85843 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,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,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)) {
This is a strange check, it only fails if both are NULL. Since we do not actually allow ports anymore in the match, the plain string comparison is enough. So, I removed the second line from this check. With that, applied. Thanks! Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
