[ovs-dev] [PATCH branch-1.3] Do not output priority tagged frames.

Ben Pfaff blp at nicira.com
Fri Nov 18 01:23:21 UTC 2011


Thanks, pushed.

On Thu, Nov 17, 2011 at 05:22:36PM -0800, Justin Pettit wrote:
> Looks fine.
> 
> --Justin
> 
> 
> On Nov 17, 2011, at 5:06 PM, Ben Pfaff wrote:
> 
> > Linux hosts (and probably others) tend to ignore priority-tagged frames, so
> > it's safest not to output them at all.
> > 
> > On "master" we've added a per-port setting for whether priority-tagged
> > frames may be output, but "master" and "branch-1.3" have diverged
> > significantly in how they handle port output, so it's much simpler to
> > port this simpler policy to "branch-1.3".
> > 
> > Reported-by: Michael Mao <mmao at nicira.com>
> > Bug #8320.
> > ---
> > ofproto/ofproto-dpif.c |   40 ++++++++++++++++++------------------
> > tests/ofproto-dpif.at  |   52 ++++++++++++++++++++++-------------------------
> > 2 files changed, 44 insertions(+), 48 deletions(-)
> > 
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 7af9ee2..c41f4db 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -4641,11 +4641,26 @@ compose_mirror_dsts(struct action_xlate_ctx *ctx,
> > }
> > 
> > static void
> > +compose_dst_output_action(struct action_xlate_ctx *ctx, const struct dst *dst)
> > +{
> > +    ovs_be16 tci;
> > +
> > +    tci = htons(dst->vid);
> > +    if (tci) {
> > +        tci |= ctx->flow.vlan_tci & htons(VLAN_PCP_MASK);
> > +        tci |= htons(VLAN_CFI);
> > +    }
> > +    commit_vlan_action(ctx, tci);
> > +
> > +    compose_output_action(ctx, dst->port->odp_port);
> > +}
> > +
> > +static void
> > compose_actions(struct action_xlate_ctx *ctx, uint16_t vlan,
> >                 const struct ofbundle *in_bundle,
> >                 const struct ofbundle *out_bundle)
> > {
> > -    uint16_t initial_vid, cur_vid;
> > +    uint16_t initial_vid;
> >     const struct dst *dst;
> >     struct dst_set set;
> > 
> > @@ -4661,31 +4676,16 @@ compose_actions(struct action_xlate_ctx *ctx, uint16_t vlan,
> >     commit_odp_actions(ctx);
> >     initial_vid = vlan_tci_to_vid(ctx->flow.vlan_tci);
> >     for (dst = set.dsts; dst < &set.dsts[set.n]; dst++) {
> > -        if (dst->vid != initial_vid) {
> > -            continue;
> > +        if (dst->vid == initial_vid) {
> > +            compose_dst_output_action(ctx, dst);
> >         }
> > -        compose_output_action(ctx, dst->port->odp_port);
> >     }
> > 
> >     /* Then output the rest. */
> > -    cur_vid = initial_vid;
> >     for (dst = set.dsts; dst < &set.dsts[set.n]; dst++) {
> > -        if (dst->vid == initial_vid) {
> > -            continue;
> > -        }
> > -        if (dst->vid != cur_vid) {
> > -            ovs_be16 tci;
> > -
> > -            tci = htons(dst->vid);
> > -            tci |= ctx->flow.vlan_tci & htons(VLAN_PCP_MASK);
> > -            if (tci) {
> > -                tci |= htons(VLAN_CFI);
> > -            }
> > -            commit_vlan_action(ctx, tci);
> > -
> > -            cur_vid = dst->vid;
> > +        if (dst->vid != initial_vid) {
> > +            compose_dst_output_action(ctx, dst);
> >         }
> > -        compose_output_action(ctx, dst->port->odp_port);
> >     }
> > 
> >     dst_set_free(&set);
> > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> > index ec5c238..bdb22bd 100644
> > --- a/tests/ofproto-dpif.at
> > +++ b/tests/ofproto-dpif.at
> > @@ -117,29 +117,25 @@ br0=0 p1=$1 p2=$2 p3=$3 p4=$4 p5=$5 p6=$6 p7=$7 p8=$8
> > dnl Each of these specifies an in_port, a VLAN VID (or "none"), a VLAN
> > dnl PCP (used if the VID isn't "none") and the expected set of datapath
> > dnl actions.
> > -dnl
> > -dnl XXX Some of these actually output an 802.1Q header to an access port
> > -dnl (see for example the actions for in_port=p3, vlan=0) to qualify the
> > -dnl packet with a priority.  That should be configurable.
> > for tuple in \
> >         "br0 none 0 drop" \
> >         "br0 0    0 drop" \
> >         "br0 0    1 drop" \
> >         "br0 10   0 p1,p5,p6,p7,p8,pop(vlan),p2" \
> > -        "br0 10   1 p1,p5,p6,p7,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
> > +        "br0 10   1 p1,p5,p6,p7,p8,pop(vlan),p2" \
> >         "br0 11   0 p5,p7" \
> >         "br0 11   1 p5,p7" \
> >         "br0 12   0 p1,p5,p6,pop(vlan),p3,p4,p7,p8" \
> > -        "br0 12   1 p1,p5,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7,p8" \
> > +        "br0 12   1 p1,p5,p6,pop(vlan),p3,p4,p7,p8" \
> >         "p1  none 0 drop" \
> >         "p1  0    0 drop" \
> >         "p1  0    1 drop" \
> >         "p1  10   0 br0,p5,p6,p7,p8,pop(vlan),p2" \
> > -        "p1  10   1 br0,p5,p6,p7,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
> > +        "p1  10   1 br0,p5,p6,p7,p8,pop(vlan),p2" \
> >         "p1  11   0 drop" \
> >         "p1  11   1 drop" \
> >         "p1  12   0 br0,p5,p6,pop(vlan),p3,p4,p7,p8" \
> > -        "p1  12   1 br0,p5,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7,p8" \
> > +        "p1  12   1 br0,p5,p6,pop(vlan),p3,p4,p7,p8" \
> >         "p2  none 0 push(vlan(vid=10,pcp=0)),br0,p1,p5,p6,p7,p8" \
> >         "p2  0    0 pop(vlan),push(vlan(vid=10,pcp=0)),br0,p1,p5,p6,p7,p8" \
> >         "p2  0    1 pop(vlan),push(vlan(vid=10,pcp=1)),br0,p1,p5,p6,p7,p8" \
> > @@ -150,8 +146,8 @@ for tuple in \
> >         "p2  12   0 drop" \
> >         "p2  12   1 drop" \
> >         "p3  none 0 p4,p7,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > -        "p3  0    0 p4,p7,p8,pop(vlan),push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > -        "p3  0    1 p4,p7,p8,pop(vlan),push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
> > +        "p3  0    0 pop(vlan),p4,p7,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > +        "p3  0    1 pop(vlan),p4,p7,p8,push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
> >         "p3  10   0 drop" \
> >         "p3  10   1 drop" \
> >         "p3  11   0 drop" \
> > @@ -159,8 +155,8 @@ for tuple in \
> >         "p3  12   0 drop" \
> >         "p3  12   1 drop" \
> >         "p4  none 0 p3,p7,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > -        "p4  0    0 p3,p7,p8,pop(vlan),push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > -        "p4  0    1 p3,p7,p8,pop(vlan),push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
> > +        "p4  0    0 pop(vlan),p3,p7,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > +        "p4  0    1 pop(vlan),p3,p7,p8,push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
> >         "p4  10   0 drop" \
> >         "p4  10   1 drop" \
> >         "p4  11   0 drop" \
> > @@ -168,41 +164,41 @@ for tuple in \
> >         "p4  12   0 drop" \
> >         "p4  12   1 drop" \
> >         "p5  none 0 p2,push(vlan(vid=10,pcp=0)),br0,p1,p6,p7,p8" \
> > -        "p5  0    0 p2,pop(vlan),push(vlan(vid=10,pcp=0)),br0,p1,p6,p7,p8" \
> > -        "p5  0    1 p2,pop(vlan),push(vlan(vid=10,pcp=1)),br0,p1,p6,p7,p8" \
> > +        "p5  0    0 pop(vlan),p2,push(vlan(vid=10,pcp=0)),br0,p1,p6,p7,p8" \
> > +        "p5  0    1 pop(vlan),p2,push(vlan(vid=10,pcp=1)),br0,p1,p6,p7,p8" \
> >         "p5  10   0 br0,p1,p6,p7,p8,pop(vlan),p2" \
> > -        "p5  10   1 br0,p1,p6,p7,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
> > +        "p5  10   1 br0,p1,p6,p7,p8,pop(vlan),p2" \
> >         "p5  11   0 br0,p7" \
> >         "p5  11   1 br0,p7" \
> >         "p5  12   0 br0,p1,p6,pop(vlan),p3,p4,p7,p8" \
> > -        "p5  12   1 br0,p1,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7,p8" \
> > +        "p5  12   1 br0,p1,p6,pop(vlan),p3,p4,p7,p8" \
> >         "p6  none 0 p2,push(vlan(vid=10,pcp=0)),br0,p1,p5,p7,p8" \
> > -        "p6  0    0 p2,pop(vlan),push(vlan(vid=10,pcp=0)),br0,p1,p5,p7,p8" \
> > -        "p6  0    1 p2,pop(vlan),push(vlan(vid=10,pcp=1)),br0,p1,p5,p7,p8" \
> > +        "p6  0    0 pop(vlan),p2,push(vlan(vid=10,pcp=0)),br0,p1,p5,p7,p8" \
> > +        "p6  0    1 pop(vlan),p2,push(vlan(vid=10,pcp=1)),br0,p1,p5,p7,p8" \
> >         "p6  10   0 br0,p1,p5,p7,p8,pop(vlan),p2" \
> > -        "p6  10   1 br0,p1,p5,p7,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
> > +        "p6  10   1 br0,p1,p5,p7,p8,pop(vlan),p2" \
> >         "p6  11   0 drop" \
> >         "p6  11   1 drop" \
> >         "p6  12   0 br0,p1,p5,pop(vlan),p3,p4,p7,p8" \
> > -        "p6  12   1 br0,p1,p5,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7,p8" \
> > +        "p6  12   1 br0,p1,p5,pop(vlan),p3,p4,p7,p8" \
> >         "p7  none 0 p3,p4,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > -        "p7  0    0 p3,p4,p8,pop(vlan),push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > -        "p7  0    1 p3,p4,p8,pop(vlan),push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
> > +        "p7  0    0 pop(vlan),p3,p4,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > +        "p7  0    1 pop(vlan),p3,p4,p8,push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
> >         "p7  10   0 br0,p1,p5,p6,p8,pop(vlan),p2" \
> > -        "p7  10   1 br0,p1,p5,p6,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
> > +        "p7  10   1 br0,p1,p5,p6,p8,pop(vlan),p2" \
> >         "p7  11   0 br0,p5" \
> >         "p7  11   1 br0,p5" \
> >         "p7  12   0 br0,p1,p5,p6,pop(vlan),p3,p4,p8" \
> > -        "p7  12   1 br0,p1,p5,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p8" \
> > +        "p7  12   1 br0,p1,p5,p6,pop(vlan),p3,p4,p8" \
> >         "p8  none 0 p3,p4,p7,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > -        "p8  0    0 p3,p4,p7,pop(vlan),push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > -        "p8  0    1 p3,p4,p7,pop(vlan),push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
> > +        "p8  0    0 pop(vlan),p3,p4,p7,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
> > +        "p8  0    1 pop(vlan),p3,p4,p7,push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
> >         "p8  10   0 br0,p1,p5,p6,p7,pop(vlan),p2" \
> > -        "p8  10   1 br0,p1,p5,p6,p7,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
> > +        "p8  10   1 br0,p1,p5,p6,p7,pop(vlan),p2" \
> >         "p8  11   0 drop" \
> >         "p8  11   1 drop" \
> >         "p8  12   0 br0,p1,p5,p6,pop(vlan),p3,p4,p7" \
> > -        "p8  12   1 br0,p1,p5,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7"
> > +        "p8  12   1 br0,p1,p5,p6,pop(vlan),p3,p4,p7"
> > do
> >   set $tuple
> >   in_port=$1
> > -- 
> > 1.7.4.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 



More information about the dev mailing list