Message ID | 20231004194050.387760-1-mkp@redhat.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,v5,1/2] ofproto-dpif-mirror: Reduce number of function parameters. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/intel-ovs-compilation | success | test: success |
On Wed, Oct 04, 2023 at 03:40:49PM -0400, Mike Pattrick wrote: > 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 <mkp@redhat.com> Thanks for addressing this Mike, much appreciated. Acked-by: Simon Horman <horms@ovn.org>
On 4 Oct 2023, at 21:40, Mike Pattrick wrote: > 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 <mkp@redhat.com> Hi Mike, Small nit below, if fixed, please add my ‘Acked-by: Eelco Chaudron <echaudro@redhat.com>’ in the next rev. //Eelco > --- > ofproto/ofproto-dpif-mirror.c | 61 ++++++++++++++++++----------------- > ofproto/ofproto-dpif-mirror.h | 30 ++++++++++++----- > ofproto/ofproto-dpif-xlate.c | 25 ++++++-------- > ofproto/ofproto-dpif.c | 6 ++-- > 4 files changed, 66 insertions(+), 56 deletions(-) > > diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c > index 343b75f0e..17ba6f799 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, struct ofbundle **srcs, > + struct ofbundle **dsts, struct ofbundle *bundle) > + > { > 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 (bundle) { > + out = mbundle_lookup(mbridge, 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, srcs, ms->n_srcs, &srcs_map); > + mbundle_lookup_multiple(mbridge, dsts, ms->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->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..b19b9e69f 100644 > --- a/ofproto/ofproto-dpif-mirror.h > +++ b/ofproto/ofproto-dpif-mirror.h > @@ -22,9 +22,25 @@ > #define MAX_MIRRORS 32 > typedef uint32_t mirror_mask_t; > > +struct ofproto_mirror_settings; > struct ofproto_dpif; > struct ofbundle; > > +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 *bundle; /* A registered ofbundle handle or NULL. */ > + 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. */ Missing s in byte(s). In addition is the Set 0 equals 65535 true? Does it not mean the original size of the packet? Not sure if we support more than 64K in TSO or the datapath in general. > +}; > + > + > /* The following functions are used by handler threads without any locking, > * assuming RCU protection. */ > > @@ -38,9 +54,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 +65,10 @@ 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, > + struct ofbundle **srcs, struct ofbundle **dsts, > + struct ofbundle *bundle); > 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 be4bd6657..3746edf06 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -2247,16 +2247,11 @@ 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))) { > + &mc))) { > /* The mirror got reconfigured before we got to read it's > * configuration. */ > mirrors = zero_rightmost_1bit(mirrors); > @@ -2266,10 +2261,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; > } > @@ -2281,21 +2276,21 @@ 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.bundle) { > + struct xbundle *out_xbundle = xbundle_lookup(ctx->xcfg, mc.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 ba5706f6a..5a5a8ea86 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -3663,10 +3663,8 @@ mirror_set__(struct ofproto *ofproto_, void *aux, > 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); > + error = mirror_set(ofproto->mbridge, aux, s, srcs, dsts, > + bundle_lookup(ofproto, s->out_bundle)); > free(srcs); > free(dsts); > return error; > -- > 2.39.3 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c index 343b75f0e..17ba6f799 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, struct ofbundle **srcs, + struct ofbundle **dsts, struct ofbundle *bundle) + { 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 (bundle) { + out = mbundle_lookup(mbridge, 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, srcs, ms->n_srcs, &srcs_map); + mbundle_lookup_multiple(mbridge, dsts, ms->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->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..b19b9e69f 100644 --- a/ofproto/ofproto-dpif-mirror.h +++ b/ofproto/ofproto-dpif-mirror.h @@ -22,9 +22,25 @@ #define MAX_MIRRORS 32 typedef uint32_t mirror_mask_t; +struct ofproto_mirror_settings; struct ofproto_dpif; struct ofbundle; +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 *bundle; /* A registered ofbundle handle or NULL. */ + 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. */ +}; + + /* The following functions are used by handler threads without any locking, * assuming RCU protection. */ @@ -38,9 +54,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 +65,10 @@ 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, + struct ofbundle **srcs, struct ofbundle **dsts, + struct ofbundle *bundle); 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 be4bd6657..3746edf06 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2247,16 +2247,11 @@ 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))) { + &mc))) { /* The mirror got reconfigured before we got to read it's * configuration. */ mirrors = zero_rightmost_1bit(mirrors); @@ -2266,10 +2261,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; } @@ -2281,21 +2276,21 @@ 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.bundle) { + struct xbundle *out_xbundle = xbundle_lookup(ctx->xcfg, mc.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 ba5706f6a..5a5a8ea86 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3663,10 +3663,8 @@ mirror_set__(struct ofproto *ofproto_, void *aux, 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); + error = mirror_set(ofproto->mbridge, aux, s, srcs, dsts, + bundle_lookup(ofproto, s->out_bundle)); free(srcs); free(dsts); return error;
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 <mkp@redhat.com> --- ofproto/ofproto-dpif-mirror.c | 61 ++++++++++++++++++----------------- ofproto/ofproto-dpif-mirror.h | 30 ++++++++++++----- ofproto/ofproto-dpif-xlate.c | 25 ++++++-------- ofproto/ofproto-dpif.c | 6 ++-- 4 files changed, 66 insertions(+), 56 deletions(-)