Re: [ovs-dev] [PATCH v2] ofproto-dpif-mirror: Add support for pre-selection filter
On Fri, Aug 25, 2023 at 10:28 AM Adrian Moreno wrote:
>
>
>
> On 7/18/23 21:38, 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.
> >
> > 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.
> >
>
> Looks really promising!
>
> > Signed-off-by: Mike Pattrick
> > ---
> > 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 ``
> > +
> > + 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.
> >
>
> Maybe also comment on the new column in the Mirror table of the OVSDB?
>
> >
> > v3.2.0 - xx xxx
> > 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(>masks,
> > +>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
Re: [ovs-dev] [PATCH v2] ofproto-dpif-mirror: Add support for pre-selection filter
On 7/18/23 21:38, 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.
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.
Looks really promising!
Signed-off-by: Mike Pattrick
---
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 ``
+
+ 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.
Maybe also comment on the new column in the Mirror table of the OVSDB?
v3.2.0 - xx xxx
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(>masks,
+>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
