From patchwork Mon Jul 8 18:27:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Pattrick X-Patchwork-Id: 1958100 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EMEyISgT; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WHt0N2Sg7z1xpP for ; Tue, 9 Jul 2024 04:28:04 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id ECD33608D9; Mon, 8 Jul 2024 18:28:01 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id 88kAFr00F7Jt; Mon, 8 Jul 2024 18:28:00 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 4BCF4607B2 Authentication-Results: smtp3.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EMEyISgT Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 4BCF4607B2; Mon, 8 Jul 2024 18:28:00 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 199EEC0A97; Mon, 8 Jul 2024 18:28:00 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 057C9C0A96 for ; Mon, 8 Jul 2024 18:27:59 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id DF84D811E0 for ; Mon, 8 Jul 2024 18:27:58 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id k1G5zJMI38Zp for ; Mon, 8 Jul 2024 18:27:57 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.133.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=mkp@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp1.osuosl.org 71CC3811CE Authentication-Results: smtp1.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 71CC3811CE Authentication-Results: smtp1.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EMEyISgT Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 71CC3811CE for ; Mon, 8 Jul 2024 18:27:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1720463276; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=ps8poY9xSQSL3vEXbwgx7dYX952Cg5nTftOhq/akPew=; b=EMEyISgT4dIVzjvlYM4C3DzPKjlilc3fn02IGm6GPYRiTgMvhgJ1VCNIKgfQdKLKYmK8yI 4BWUyOyjuCahpRlKWR+4TWkGutNOf6ZNNBeV87PpgaPnUwVNhQhQfXPwCckVNOBsKeuwzS 1Cu+g98sVfJNj6Sl5GrcDTO095pRFRM= Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-588-7sUglcFyMKm4NaMPI0bWcQ-1; Mon, 08 Jul 2024 14:27:54 -0400 X-MC-Unique: 7sUglcFyMKm4NaMPI0bWcQ-1 Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id C0A521955F30; Mon, 8 Jul 2024 18:27:53 +0000 (UTC) Received: from mpattric.remote.csb (unknown [10.22.18.114]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 3BD601955F3B; Mon, 8 Jul 2024 18:27:52 +0000 (UTC) From: Mike Pattrick To: dev@openvswitch.org Date: Mon, 8 Jul 2024 14:27:41 -0400 Message-Id: <20240708182742.1268032-1-mkp@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v10 1/2] ofproto-dpif-mirror: Reduce number of function parameters. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Previously the mirror_set() and mirror_get() functions took a large number of parameters, which was inefficient and difficult to read and extend. This patch moves most of the parameters into a struct. Acked-by: Simon Horman Acked-by: Eelco Chaudron Signed-off-by: Mike Pattrick --- ofproto/ofproto-dpif-mirror.c | 60 ++++++++++++++++++----------------- ofproto/ofproto-dpif-mirror.h | 40 ++++++++++++++++++----- ofproto/ofproto-dpif-xlate.c | 29 ++++++++--------- ofproto/ofproto-dpif.c | 23 +++++++------- 4 files changed, 88 insertions(+), 64 deletions(-) diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c index 343b75f0e..4967ecc9a 100644 --- a/ofproto/ofproto-dpif-mirror.c +++ b/ofproto/ofproto-dpif-mirror.c @@ -207,19 +207,22 @@ mirror_bundle_dst(struct mbridge *mbridge, struct ofbundle *ofbundle) } int -mirror_set(struct mbridge *mbridge, void *aux, const char *name, - struct ofbundle **srcs, size_t n_srcs, - struct ofbundle **dsts, size_t n_dsts, - unsigned long *src_vlans, struct ofbundle *out_bundle, - uint16_t snaplen, - uint16_t out_vlan) +mirror_set(struct mbridge *mbridge, void *aux, + const struct ofproto_mirror_settings *ms, + const struct mirror_bundles *mb) { struct mbundle *mbundle, *out; mirror_mask_t mirror_bit; struct mirror *mirror; struct hmapx srcs_map; /* Contains "struct ofbundle *"s. */ struct hmapx dsts_map; /* Contains "struct ofbundle *"s. */ + uint16_t out_vlan; + if (!ms || !mbridge) { + return EINVAL; + } + + out_vlan = ms->out_vlan; mirror = mirror_lookup(mbridge, aux); if (!mirror) { int idx; @@ -227,7 +230,7 @@ mirror_set(struct mbridge *mbridge, void *aux, const char *name, idx = mirror_scan(mbridge); if (idx < 0) { VLOG_WARN("maximum of %d port mirrors reached, cannot create %s", - MAX_MIRRORS, name); + MAX_MIRRORS, ms->name); return EFBIG; } @@ -242,8 +245,8 @@ mirror_set(struct mbridge *mbridge, void *aux, const char *name, unsigned long *vlans = ovsrcu_get(unsigned long *, &mirror->vlans); /* Get the new configuration. */ - if (out_bundle) { - out = mbundle_lookup(mbridge, out_bundle); + if (mb->out_bundle) { + out = mbundle_lookup(mbridge, mb->out_bundle); if (!out) { mirror_destroy(mbridge, mirror->aux); return EINVAL; @@ -252,16 +255,16 @@ mirror_set(struct mbridge *mbridge, void *aux, const char *name, } else { out = NULL; } - mbundle_lookup_multiple(mbridge, srcs, n_srcs, &srcs_map); - mbundle_lookup_multiple(mbridge, dsts, n_dsts, &dsts_map); + mbundle_lookup_multiple(mbridge, mb->srcs, mb->n_srcs, &srcs_map); + mbundle_lookup_multiple(mbridge, mb->dsts, mb->n_dsts, &dsts_map); /* If the configuration has not changed, do nothing. */ if (hmapx_equals(&srcs_map, &mirror->srcs) && hmapx_equals(&dsts_map, &mirror->dsts) - && vlan_bitmap_equal(vlans, src_vlans) + && vlan_bitmap_equal(vlans, ms->src_vlans) && mirror->out == out && mirror->out_vlan == out_vlan - && mirror->snaplen == snaplen) + && mirror->snaplen == ms->snaplen) { hmapx_destroy(&srcs_map); hmapx_destroy(&dsts_map); @@ -275,15 +278,15 @@ mirror_set(struct mbridge *mbridge, void *aux, const char *name, hmapx_swap(&dsts_map, &mirror->dsts); hmapx_destroy(&dsts_map); - if (vlans || src_vlans) { + if (vlans || ms->src_vlans) { ovsrcu_postpone(free, vlans); - vlans = vlan_bitmap_clone(src_vlans); + vlans = vlan_bitmap_clone(ms->src_vlans); ovsrcu_set(&mirror->vlans, vlans); } mirror->out = out; mirror->out_vlan = out_vlan; - mirror->snaplen = snaplen; + mirror->snaplen = ms->snaplen; /* Update mbundles. */ mirror_bit = MIRROR_MASK_C(1) << mirror->idx; @@ -406,23 +409,22 @@ mirror_update_stats(struct mbridge *mbridge, mirror_mask_t mirrors, /* Retrieves the mirror numbered 'index' in 'mbridge'. Returns true if such a * mirror exists, false otherwise. * - * If successful, '*vlans' receives the mirror's VLAN membership information, + * If successful 'mc->vlans' receives the mirror's VLAN membership information, * either a null pointer if the mirror includes all VLANs or a 4096-bit bitmap * in which a 1-bit indicates that the mirror includes a particular VLAN, - * '*dup_mirrors' receives a bitmap of mirrors whose output duplicates mirror - * 'index', '*out' receives the output ofbundle (if any), and '*out_vlan' - * receives the output VLAN (if any). + * '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). * * Everything returned here is assumed to be RCU protected. */ bool -mirror_get(struct mbridge *mbridge, int index, const unsigned long **vlans, - mirror_mask_t *dup_mirrors, struct ofbundle **out, - int *snaplen, int *out_vlan) +mirror_get(struct mbridge *mbridge, int index, + struct mirror_config *mc) { struct mirror *mirror; - if (!mbridge) { + if (!mc || !mbridge) { return false; } @@ -433,11 +435,11 @@ mirror_get(struct mbridge *mbridge, int index, const unsigned long **vlans, /* Assume 'mirror' is RCU protected, i.e., it will not be freed until this * thread quiesces. */ - *vlans = ovsrcu_get(unsigned long *, &mirror->vlans); - *dup_mirrors = mirror->dup_mirrors; - *out = mirror->out ? mirror->out->ofbundle : NULL; - *out_vlan = mirror->out_vlan; - *snaplen = mirror->snaplen; + mc->vlans = ovsrcu_get(unsigned long *, &mirror->vlans); + mc->dup_mirrors = mirror->dup_mirrors; + mc->out_bundle = mirror->out ? mirror->out->ofbundle : NULL; + mc->out_vlan = mirror->out_vlan; + mc->snaplen = mirror->snaplen; return true; } diff --git a/ofproto/ofproto-dpif-mirror.h b/ofproto/ofproto-dpif-mirror.h index eed63ec4a..37d57463c 100644 --- a/ofproto/ofproto-dpif-mirror.h +++ b/ofproto/ofproto-dpif-mirror.h @@ -22,9 +22,37 @@ #define MAX_MIRRORS 32 typedef uint32_t mirror_mask_t; +struct ofproto_mirror_settings; struct ofproto_dpif; struct ofbundle; +struct mirror_bundles { + struct ofbundle **srcs; + size_t n_srcs; + + struct ofbundle **dsts; + size_t n_dsts; + + struct ofbundle *out_bundle; +}; + +struct mirror_config { + /* A bitmap of mirrors that duplicate the current mirror. */ + mirror_mask_t dup_mirrors; + + /* VLANs of packets to select for mirroring. */ + unsigned long *vlans; /* vlan_bitmap, NULL selects all VLANs. */ + + /* 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 + set. */ + + /* Max size of a mirrored packet in bytes, if set to zero then no + * truncation will occur. */ + uint16_t snaplen; +}; + /* The following functions are used by handler threads without any locking, * assuming RCU protection. */ @@ -38,9 +66,7 @@ mirror_mask_t mirror_bundle_dst(struct mbridge *, struct ofbundle *); void mirror_update_stats(struct mbridge*, mirror_mask_t, uint64_t packets, uint64_t bytes); -bool mirror_get(struct mbridge *, int index, const unsigned long **vlans, - mirror_mask_t *dup_mirrors, struct ofbundle **out, - int *snaplen, int *out_vlan); +bool mirror_get(struct mbridge *, int index, struct mirror_config *); /* The remaining functions are assumed to be called by the main thread only. */ @@ -50,11 +76,9 @@ 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, const char *name, - struct ofbundle **srcs, size_t n_srcs, - struct ofbundle **dsts, size_t n_dsts, - unsigned long *src_vlans, struct ofbundle *out_bundle, - uint16_t snaplen, uint16_t out_vlan); +int mirror_set(struct mbridge *, void *aux, + const struct ofproto_mirror_settings *, + const struct mirror_bundles *); void mirror_destroy(struct mbridge *, void *aux); int mirror_get_stats(struct mbridge *, void *aux, uint64_t *packets, uint64_t *bytes); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 7c4950895..55846fe98 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2279,16 +2279,12 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle, * 'used_mirrors', as long as some candidates remain. */ mirror_mask_t used_mirrors = 0; while (mirrors) { - const unsigned long *vlans; - mirror_mask_t dup_mirrors; - struct ofbundle *out; - int out_vlan; - int snaplen; + struct mirror_config mc; /* Get the details of the mirror represented by the rightmost 1-bit. */ - if (OVS_UNLIKELY(!mirror_get(xbridge->mbridge, raw_ctz(mirrors), - &vlans, &dup_mirrors, - &out, &snaplen, &out_vlan))) { + if (OVS_UNLIKELY(!mirror_get(xbridge->mbridge, + raw_ctz(mirrors), + &mc))) { /* The mirror got reconfigured before we got to read it's * configuration. */ mirrors = zero_rightmost_1bit(mirrors); @@ -2298,10 +2294,10 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle, /* If this mirror selects on the basis of VLAN, and it does not select * 'vlan', then discard this mirror and go on to the next one. */ - if (vlans) { + if (mc.vlans) { ctx->wc->masks.vlans[0].tci |= htons(VLAN_CFI | VLAN_VID_MASK); } - if (vlans && !bitmap_is_set(vlans, xvlan.v[0].vid)) { + if (mc.vlans && !bitmap_is_set(mc.vlans, xvlan.v[0].vid)) { mirrors = zero_rightmost_1bit(mirrors); continue; } @@ -2313,21 +2309,22 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle, * destination, so that we don't mirror to them again. This must be * done now to ensure that output_normal(), below, doesn't recursively * output to the same mirrors. */ - ctx->mirrors |= dup_mirrors; - ctx->mirror_snaplen = snaplen; + ctx->mirrors |= mc.dup_mirrors; + ctx->mirror_snaplen = mc.snaplen; /* Send the packet to the mirror. */ - if (out) { - struct xbundle *out_xbundle = xbundle_lookup(ctx->xcfg, out); + if (mc.out_bundle) { + struct xbundle *out_xbundle = xbundle_lookup(ctx->xcfg, + mc.out_bundle); if (out_xbundle) { output_normal(ctx, out_xbundle, &xvlan); } - } else if (xvlan.v[0].vid != out_vlan + } else if (xvlan.v[0].vid != mc.out_vlan && !eth_addr_is_reserved(ctx->xin->flow.dl_dst)) { struct xbundle *xb; uint16_t old_vid = xvlan.v[0].vid; - xvlan.v[0].vid = out_vlan; + xvlan.v[0].vid = mc.out_vlan; LIST_FOR_EACH (xb, list_node, &xbridge->xbundles) { if (xbundle_includes_vlan(xb, &xvlan) && !xbundle_mirror_out(xbridge, xb)) { diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index fcd7cd753..d5069e2cf 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3730,7 +3730,7 @@ mirror_set__(struct ofproto *ofproto_, void *aux, const struct ofproto_mirror_settings *s) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); - struct ofbundle **srcs, **dsts; + struct mirror_bundles mb; int error; size_t i; @@ -3739,23 +3739,24 @@ mirror_set__(struct ofproto *ofproto_, void *aux, return 0; } - srcs = xmalloc(s->n_srcs * sizeof *srcs); - dsts = xmalloc(s->n_dsts * sizeof *dsts); + mb.srcs = xmalloc(s->n_srcs * sizeof *mb.srcs); + mb.dsts = xmalloc(s->n_dsts * sizeof *mb.dsts); for (i = 0; i < s->n_srcs; i++) { - srcs[i] = bundle_lookup(ofproto, s->srcs[i]); + mb.srcs[i] = bundle_lookup(ofproto, s->srcs[i]); } for (i = 0; i < s->n_dsts; i++) { - dsts[i] = bundle_lookup(ofproto, s->dsts[i]); + mb.dsts[i] = bundle_lookup(ofproto, s->dsts[i]); } - error = mirror_set(ofproto->mbridge, aux, s->name, srcs, s->n_srcs, dsts, - s->n_dsts, s->src_vlans, - bundle_lookup(ofproto, s->out_bundle), - s->snaplen, s->out_vlan); - free(srcs); - free(dsts); + mb.n_srcs = s->n_srcs; + mb.n_dsts = s->n_dsts; + mb.out_bundle = bundle_lookup(ofproto, s->out_bundle); + + error = mirror_set(ofproto->mbridge, aux, s, &mb); + free(mb.srcs); + free(mb.dsts); return error; } From patchwork Mon Jul 8 18:27:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Pattrick X-Patchwork-Id: 1958101 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=U/w9EJ04; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WHt0V6R2tz1xpP for ; Tue, 9 Jul 2024 04:28:10 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 926D7811F4; Mon, 8 Jul 2024 18:28:08 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id EOy83pQYU-gf; Mon, 8 Jul 2024 18:28:05 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org D05BE811EC Authentication-Results: smtp1.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=U/w9EJ04 Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id D05BE811EC; Mon, 8 Jul 2024 18:28:04 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7609CC0A98; Mon, 8 Jul 2024 18:28:04 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 27C33C0A98 for ; Mon, 8 Jul 2024 18:28:01 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 3C60A811DE for ; Mon, 8 Jul 2024 18:28:00 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id t9umtYGhnPOt for ; Mon, 8 Jul 2024 18:27:58 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.129.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=mkp@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp1.osuosl.org 725D3811DF Authentication-Results: smtp1.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 725D3811DF Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 725D3811DF for ; Mon, 8 Jul 2024 18:27:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1720463277; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WEj3FlEqKg56L5NK0Ak0vj8eIV6GwBJesEhvBwEhrro=; b=U/w9EJ04gn4maSuRPxpfOiHH2r82imnl1fYD5XCwlIP27NqsEt59WfTXkjmyd7EU3NN8fG Q1zTxBp0OCWtXvtWKXYyNrcn7sgnKiaj0uAk9amE4c4kxt5jP83nwAJVniHkdJ0Qlg9jNQ x8L2XBnv99cLesQFwkj58hjsMDJWkQI= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-139-wEMhCb-7M9Cw1ytG04nlcQ-1; Mon, 08 Jul 2024 14:27:55 -0400 X-MC-Unique: wEMhCb-7M9Cw1ytG04nlcQ-1 Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id C10BD1956048 for ; Mon, 8 Jul 2024 18:27:54 +0000 (UTC) Received: from mpattric.remote.csb (unknown [10.22.18.114]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id D02A71955E83; Mon, 8 Jul 2024 18:27:53 +0000 (UTC) From: Mike Pattrick To: dev@openvswitch.org Date: Mon, 8 Jul 2024 14:27:42 -0400 Message-Id: <20240708182742.1268032-2-mkp@redhat.com> In-Reply-To: <20240708182742.1268032-1-mkp@redhat.com> References: <20240708182742.1268032-1-mkp@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v10 2/2] ofproto-dpif-mirror: Add support for pre-selection filter. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" 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 --- 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 `` + + 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)) { + 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 +419,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,7 +499,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. */ @@ -422,6 +509,7 @@ bool mirror_get(struct mbridge *mbridge, int index, struct mirror_config *mc) { + struct filtermask *fm; struct mirror *mirror; if (!mc || !mbridge) { @@ -440,6 +528,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..a03dd8235 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,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 index 55846fe98..888f4d4b3 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 wildcard for the flow's current filter is 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 d5069e2cf..6bdf69b02 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 0b23fd6c5..9b7895ddd 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -5221,6 +5221,173 @@ 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 netdev-dummy/receive p1 $icmp_flow]) +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:1, bytes:106, used:0.0s, actions:p3,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:2, bytes:236, used:0.0s, actions:p2 +]) + +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 e3afb78a4..ecc72fb04 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. + +

+ When set, only packets that match are + selected for mirroring. Packets that do not match are ignored + by thie mirror. The syntax is described + in ovs-fields(7). However, support for the + in_port field is not supported; should be used to limit the mirror to + an source port. +

+

+ This filter is applied after , , , and + . +

+