From 7c7ae3b9eae10b0dcd99cb13a915647cc710fd6d Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 24 Jul 2015 15:30:58 -0700 Subject: [PATCH] ofproto-dpif-mirror: Fix insane waste of memory and time in checking VLANs. When a mirror was restricted to particular VLANs, this code was allocating, copying, and freeing a 512-byte block of memory just to check the value of a single bit in the block. This fixes the problem. Found by inspection. Signed-off-by: Ben Pfaff Acked-by: Andy Zhou --- ofproto/ofproto-dpif-mirror.c | 18 ++++++++++++------ ofproto/ofproto-dpif-mirror.h | 8 ++++---- ofproto/ofproto-dpif-xlate.c | 3 +-- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c index e0f3dcd49..f3ff5782c 100644 --- a/ofproto/ofproto-dpif-mirror.c +++ b/ofproto/ofproto-dpif-mirror.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc. +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -375,11 +375,17 @@ mirror_update_stats(struct mbridge *mbridge, mirror_mask_t mirrors, } } -/* Retrieves the mirror in 'mbridge' represented by the first bet set of - * 'mirrors'. Returns true if such a mirror exists, false otherwise. - * The caller takes ownership of, and is expected to deallocate, 'vlans' */ +/* 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, + * 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). */ bool -mirror_get(struct mbridge *mbridge, int index, unsigned long **vlans, +mirror_get(struct mbridge *mbridge, int index, const unsigned long **vlans, mirror_mask_t *dup_mirrors, struct ofbundle **out, int *out_vlan) { struct mirror *mirror; @@ -393,7 +399,7 @@ mirror_get(struct mbridge *mbridge, int index, unsigned long **vlans, return false; } - *vlans = vlan_bitmap_clone(mirror->vlans); + *vlans = mirror->vlans; *dup_mirrors = mirror->dup_mirrors; *out = mirror->out ? mirror->out->ofbundle : NULL; *out_vlan = mirror->out_vlan; diff --git a/ofproto/ofproto-dpif-mirror.h b/ofproto/ofproto-dpif-mirror.h index 64c456123..6e0dc88cb 100644 --- a/ofproto/ofproto-dpif-mirror.h +++ b/ofproto/ofproto-dpif-mirror.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2013 Nicira, Inc. +/* Copyright (c) 2013, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -12,8 +12,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#ifndef OFPROT_DPIF_MIRROR_H -#define OFPROT_DPIF_MIRROR_H 1 +#ifndef OFPROTO_DPIF_MIRROR_H +#define OFPROTO_DPIF_MIRROR_H 1 #include @@ -48,7 +48,7 @@ int mirror_get_stats(struct mbridge *, void *aux, uint64_t *packets, uint64_t *bytes); void mirror_update_stats(struct mbridge*, mirror_mask_t, uint64_t packets, uint64_t bytes); -bool mirror_get(struct mbridge *, int index, unsigned long **vlans, +bool mirror_get(struct mbridge *, int index, const unsigned long **vlans, mirror_mask_t *dup_mirrors, struct ofbundle **out, int *out_vlan); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index a002bda64..2390ab295 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1559,7 +1559,7 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct flow *orig_flow) while (mirrors) { mirror_mask_t dup_mirrors; struct ofbundle *out; - unsigned long *vlans; + const unsigned long *vlans; bool vlan_mirrored; bool has_mirror; int out_vlan; @@ -1572,7 +1572,6 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct flow *orig_flow) ctx->xout->wc.masks.vlan_tci |= htons(VLAN_CFI | VLAN_VID_MASK); } vlan_mirrored = !vlans || bitmap_is_set(vlans, vlan); - free(vlans); if (!vlan_mirrored) { mirrors = zero_rightmost_1bit(mirrors); -- 2.20.1