On Mon, Oct 2, 2023 at 6:07 AM Simon Horman <[email protected]> wrote:
>
> On Fri, Sep 08, 2023 at 12:28:24PM -0400, 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.
> >
> > Signed-off-by: Mike Pattrick <[email protected]>
>
> ...
>
> > diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c
>
> ...
>
> > @@ -212,7 +218,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 ofproto *ofproto)
>
> Hi Mike,
>
> I am concerned about the implications of passing large numbers of
> function arguments, which seems to happen several places in this patch.
Hi Simon,
I agree, I'll send in a new patch to reduce the number of arguments we
pass in these functions. I think pre-patch we already exceeded a
reasonable number of parameters for these functions anyways.
Thank you,
Mike
>
> > {
> > struct mbundle *mbundle, *out;
> > mirror_mask_t mirror_bit;
>
> ...
>
> > @@ -430,6 +482,7 @@ mirror_get(struct mbridge *mbridge, int index, const
> > unsigned long **vlans,
> > if (!mirror) {
> > return false;
> > }
> > +
> > /* Assume 'mirror' is RCU protected, i.e., it will not be freed until
> > this
> > * thread quiesces. */
> >
>
> nit: this hunk seems unrelated to the subject of this patch.
>
> ...
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
