From patchwork Mon Feb 26 21:16:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Pattrick X-Patchwork-Id: 1904738 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=LAcROzE1; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (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 4TkD2n2tH6z1yX0 for ; Tue, 27 Feb 2024 08:17:05 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 64C4B40840; Mon, 26 Feb 2024 21:17:03 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id M2k12yBYRRUO; Mon, 26 Feb 2024 21:17:01 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.9.56; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 147EC40546 Authentication-Results: smtp4.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=LAcROzE1 Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 147EC40546; Mon, 26 Feb 2024 21:17:00 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 958A7C0077; Mon, 26 Feb 2024 21:17: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 DA3D8C0037 for ; Mon, 26 Feb 2024 21:16:59 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id C475C81F38 for ; Mon, 26 Feb 2024 21:16:59 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xzkiy-8a5dWF for ; Mon, 26 Feb 2024 21:16: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 7F34F81EE5 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 7F34F81EE5 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=LAcROzE1 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 7F34F81EE5 for ; Mon, 26 Feb 2024 21:16:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708982217; 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=JDo0eUBYodv98ARapIu71uoXnTehUav7NxER2sLr1EM=; b=LAcROzE1vjkwVzKh44k+ZA8eOTmoq6tDANfVJpTRDePY/EDDbdWL15NMsdv8p4451tBedz KLRA31nrOxI279JIbHMnYlxUbdJVomvaODRygACx4CDvfcw5CssT4nXhcwNxpasFNN6pMd t24pIOVpFiMc4S242tP2OhnSthB/kIc= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-323-X5Qf4nR8N8Csd75G0Ee0kw-1; Mon, 26 Feb 2024 16:16:55 -0500 X-MC-Unique: X5Qf4nR8N8Csd75G0Ee0kw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id 297E13C13A87; Mon, 26 Feb 2024 21:16:55 +0000 (UTC) Received: from mpattric.remote.csb (unknown [10.22.32.230]) by smtp.corp.redhat.com (Postfix) with ESMTP id B3EA2492BC7; Mon, 26 Feb 2024 21:16:53 +0000 (UTC) From: Mike Pattrick To: dev@openvswitch.org Date: Mon, 26 Feb 2024 16:16:48 -0500 Message-Id: <20240226211649.553204-1-mkp@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v7 1/2] ofproto-dpif-mirror: Reduce number of function parameters. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 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. Signed-off-by: Mike Pattrick Acked-by: Simon Horman Acked-by: Eelco Chaudron Acked-by: Eelco Chaudron --- ofproto/ofproto-dpif-mirror.c | 61 ++++++++++++++++++----------------- ofproto/ofproto-dpif-mirror.h | 42 +++++++++++++++++++----- ofproto/ofproto-dpif-xlate.c | 29 ++++++++--------- ofproto/ofproto-dpif.c | 23 ++++++------- 4 files changed, 91 insertions(+), 64 deletions(-) diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c index 343b75f0e..a84c843b3 100644 --- a/ofproto/ofproto-dpif-mirror.c +++ b/ofproto/ofproto-dpif-mirror.c @@ -207,19 +207,23 @@ 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 +231,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 +246,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 +256,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 +279,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 +410,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 +436,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..a983b3876 100644 --- a/ofproto/ofproto-dpif-mirror.h +++ b/ofproto/ofproto-dpif-mirror.h @@ -22,9 +22,38 @@ #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 +67,8 @@ 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 *mbridge, int index, + struct mirror_config *mc); /* The remaining functions are assumed to be called by the main thread only. */ @@ -50,11 +78,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 *mbridge, void *aux, + const struct ofproto_mirror_settings *ms, + const struct mirror_bundles *mb); 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 89f183182..aa2a514f9 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 f59d69c4d..4f31f5ebb 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3645,7 +3645,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; @@ -3654,23 +3654,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; }