Thanks Ben, I'll try it.

On Fri, Nov 4, 2016 at 1:39 AM, Ben Pfaff <b...@ovn.org> wrote:

> There's at least one mirroring bug fix in 2.5.1, for this commit:
>
> From: Ben Pfaff <b...@ovn.org>
> Date: Thu, 15 Sep 2016 11:43:46 -0700
> Subject: [PATCH] ofproto-dpif-xlate: Fix treatment of mirrors across patch
>  port.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> When the bridges on both sides of a patch port included mirrors, the
> translation code incorrectly conflated them instead of treating them as
> independent.
>
> Reported-by: Zoltán Balogh <zoltan.bal...@ericsson.com>
> Reported-by: Sugesh Chandran <sugesh.chand...@intel.com>
> Reported-at: http://openvswitch.org/pipermail/discuss/2016-
> September/022689.html
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> Tested-by: Zoltán Balogh <zoltan.bal...@ericsson.com>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
>  ofproto/ofproto-dpif-xlate.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 0f4703f..61b5c81 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2975,7 +2975,6 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
>
>          ofpbuf_use_stub(&ctx->stack, new_stack, sizeof new_stack);
>          ofpbuf_use_stub(&ctx->action_set, actset_stub, sizeof
> actset_stub);
> -        ctx->xbridge = peer->xbridge;
>          flow->in_port.ofp_port = peer->ofp_port;
>          flow->metadata = htonll(0);
>          memset(&flow->tunnel, 0, sizeof flow->tunnel);
> @@ -2984,6 +2983,26 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
>          ctx->conntracked = false;
>          clear_conntrack(flow);
>
> +        /* When the patch port points to a different bridge, then the
> mirrors
> +         * for that bridge clearly apply independently to the packet, so
> we
> +         * reset the mirror bitmap to zero and then restore it after the
> packet
> +         * returns.
> +         *
> +         * When the patch port points to the same bridge, this is more of
> a
> +         * design decision: can mirrors be re-applied to the packet after
> it
> +         * re-enters the bridge, or should we treat that as doubly
> mirroring a
> +         * single packet?  The former may be cleaner, since it respects
> the
> +         * model in which a patch port is like a physical cable plugged
> from
> +         * one switch port to another, but the latter may be less
> surprising to
> +         * users.  We take the latter choice, for now at least.  (To use
> the
> +         * former choice, hard-code 'independent_mirrors' to "true".) */
> +        mirror_mask_t old_mirrors = ctx->mirrors;
> +        bool independent_mirrors = peer->xbridge != ctx->xbridge;
> +        if (independent_mirrors) {
> +            ctx->mirrors = 0;
> +        }
> +        ctx->xbridge = peer->xbridge;
> +
>          /* The bridge is now known so obtain its table version. */
>          ctx->tables_version
>              = ofproto_dpif_get_tables_version(ctx->xbridge->ofproto);
> @@ -3007,10 +3026,10 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
>                   * the learning action look at the packet, then drop it.
> */
>                  struct flow old_base_flow = ctx->base_flow;
>                  size_t old_size = ctx->odp_actions->size;
> -                mirror_mask_t old_mirrors = ctx->mirrors;
> +                mirror_mask_t old_mirrors2 = ctx->mirrors;
>
>                  xlate_table_action(ctx, flow->in_port.ofp_port, 0, true,
> true);
> -                ctx->mirrors = old_mirrors;
> +                ctx->mirrors = old_mirrors2;
>                  ctx->base_flow = old_base_flow;
>                  ctx->odp_actions->size = old_size;
>
> @@ -3023,6 +3042,9 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
>              }
>          }
>
> +        if (independent_mirrors) {
> +            ctx->mirrors = old_mirrors;
> +        }
>          ctx->xin->flow = old_flow;
>          ctx->xbridge = xport->xbridge;
>          ofpbuf_uninit(&ctx->action_set);
> --
> 2.1.3
>
>
>
> On Thu, Nov 03, 2016 at 10:42:09PM +0800, Hui Xiang wrote:
> > Thanks Ben.
> >
> > I am using ovs_version: "2.5.0" , searched below same question/deployment
> > but have not found the answer.
> >
> > [ovs-discuss] port mirroring on openvswitch
> > http://openvswitch.org/pipermail/discuss/2013-October/011413.html
> > [ovs-discuss] problem in mirroring interfaces' traffic (just egress
> packets
> > is mirrored)
> > http://openvswitch.org/pipermail/discuss/2015-December/019665.html
> >
> > Could you show me what kind of mirror bug it might affect?
> >
> > BR.
> > Hui.
> >
> > On Thu, Nov 3, 2016 at 10:11 PM, Ben Pfaff <b...@ovn.org> wrote:
> >
> > > On Thu, Nov 03, 2016 at 05:24:38PM +0800, Hui Xiang wrote:
> > > >   Does port mirroring works on veth device which is connected on
> another
> > > > linux bridge?
> > >
> > > It should.  veth devices aren't special to Open vSwitch.
> > >
> > > You didn't mention what version of OVS you're using.  There have been
> > > bug fixes in mirroring over the last 6 months, so if you're not using
> an
> > > up-to-date version you might consider upgrading.
> > >
>
_______________________________________________
discuss mailing list
discuss@openvswitch.org
http://openvswitch.org/mailman/listinfo/discuss

Reply via email to