Skip to content

Commit

Permalink
ofproto-dpif-mirror: Fix insane waste of memory and time in checking …
Browse files Browse the repository at this point in the history
…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 <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
  • Loading branch information
blp committed Jul 27, 2015
1 parent 9809048 commit a454754
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
18 changes: 12 additions & 6 deletions 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.
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions 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.
Expand All @@ -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 <stdint.h>

Expand Down Expand Up @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions ofproto/ofproto-dpif-xlate.c
Expand Up @@ -1564,7 +1564,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;
Expand All @@ -1577,7 +1577,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);
Expand Down

0 comments on commit a454754

Please sign in to comment.