[ovs-discuss] the wrong mod_vlan_vid actions with OpenFlow13.
wangyunjian
wangyunjian at huawei.com
Sat Aug 18 03:33:56 UTC 2018
> -----Original Message-----
> From: Eric Garver [mailto:e at erig.me]
> Sent: Friday, August 17, 2018 8:49 PM
> To: wangyunjian <wangyunjian at huawei.com>
> Cc: ovs-discuss at openvswitch.org; Zhoulei (stone, Cloud Networking)
> <stone.zhou at huawei.com>; blp at nicira.com; thomasfherbert at gmail.com
> Subject: Re: [ovs-discuss] the wrong mod_vlan_vid actions with OpenFlow13.
>
> On Fri, Aug 17, 2018 at 12:15:30PM +0000, wangyunjian wrote:
> > The datapath flow which pushs double vlan was found using ovs-appctl
> > dpctl/dump-flows, but the flow was set mod_vlan_vid actions.
> > This problem is discovered from "Add support for 802.1ad (QinQ tunneling)".
>
> Thanks for reporting. Can you say what version of OVS you're using?
> Including any extra patches you may have applied.
The version of OVS is master branch(git log " be5e6d6822e60b5b84ac65dcd1b249145356a809
ofp-ed-props: Fix hang for crafted OpenFlow encap/decap properties".).
>
> > My test steps:
> >
> > 1) ovs-vsctl add-br ovsbr0 -- set bridge ovsbr0 datapath_type=netdev
> > ovs-vsctl add-port ovsbr0 eth2 -- set Interface eth2 type=dpdk
> options:dpdk-devargs=0000:03:00.0
> > ovs-ofctl -O OpenFlow13 add-flow ovsbr0 " table=0,priority=2,in_port=1
> actions=mod_vlan_vid:3,NORMAL"
> > ovs-vsctl show
> > 25502611-fa90-492c-bb72-305f19a63224
> > Bridge "ovsbr0"
> > Port "eth2"
> > Interface "eth2"
> > type: dpdk
> > options: {dpdk-devargs="0000:03:00.0"}
> > Port "ovsbr0"
> > Interface "ovsbr0"
> > type: internal
> >
> > 2) tcpdump -i ovsbr0 -ne
> > tcpdump: verbose output suppressed, use -v or -vv for full protocol
> > decode listening on ovsbr0, link-type EN10MB (Ethernet), capture size
> > 65535 bytes
> > 19:40:36.418484 90:17:ac:b0:0a:ff > Broadcast, ethertype 802.1Q
> > (0x8100), length 68: vlan 3, p 0, ethertype 802.1Q, vlan 2, p 0,
> > ethertype ARP, Request who-has 3.3.3.18 tell 3.3.3.17, length 46
> > 19:40:37.420464 90:17:ac:b0:0a:ff > Broadcast, ethertype 802.1Q
> > (0x8100), length 68: vlan 3, p 0, ethertype 802.1Q, vlan 2, p 0,
> > ethertype ARP, Request who-has 3.3.3.18 tell 3.3.3.17, length 46
> >
> > 3) ovs-appctl dpctl/dump-flows netdev at ovs-netdev flow-dump from pmd
> on
> > cpu core: 1
> > recirc_id(0),in_port(2),packet_type(ns=0,id=0),eth(src=90:17:ac:b0:0a:
> > ff,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8100),vlan(vid=2,pcp=0),encap(eth
> > _type(0x0806),arp(sip=3.3.3.17,tip=3.3.3.18,op=1/0xff)), packets:612,
> > bytes:39168, used:0.148s, actions:push_vlan(vid=3,pcp=0),1
> >
> > 4) ovs-appctl ofproto/trace
> "recirc_id(0),in_port(2),packet_type(ns=0,id=0),eth(src=90:17:ac:b0:0a:ff,dst
> =ff:ff:ff:ff:ff:ff),eth_type(0x8100),vlan(vid=2,pcp=0),encap(eth_type(0x0806)
> ,arp(sip=3.3.3.17,tip=3.3.3.18,op=1/0xff))"
> > Flow:
> > arp,in_port=1,dl_vlan=2,dl_vlan_pcp=0,vlan_tci1=0x0000,dl_src=90:17:ac
> > :b0:0a:ff,dl_dst=ff:ff:ff:ff:ff:ff,arp_spa=3.3.3.17,arp_tpa=3.3.3.18,a
> > rp_op=1,arp_sha=00:00:00:00:00:00,arp_tha=00:00:00:00:00:00
> >
> > bridge("ovsbr0")
> > ----------------
> > 0. in_port=1, priority 2, cookie 0xb043f0d196265635
> > push_vlan:0x8100
> > set_field:4099->vlan_vid
> > NORMAL
> > -> no learned MAC for destination, flooding
> >
> > Final flow:
> > arp,in_port=1,dl_vlan=3,dl_vlan_pcp=0,dl_vlan1=2,dl_vlan_pcp1=0,dl_src
> > =90:17:ac:b0:0a:ff,dl_dst=ff:ff:ff:ff:ff:ff,arp_spa=3.3.3.17,arp_tpa=3
> > .3.3.18,arp_op=1,arp_sha=00:00:00:00:00:00,arp_tha=00:00:00:00:00:00
> > Megaflow:
> > recirc_id=0,eth,arp,in_port=1,dl_vlan=2,dl_vlan_pcp=0,dl_src=90:17:ac:
> > b0:0a:ff,dl_dst=ff:ff:ff:ff:ff:ff,arp_spa=3.3.3.17,arp_tpa=3.3.3.18,ar
> > p_op=1 Datapath actions: push_vlan(vid=3,pcp=0),1
> >
> > Fix patch:
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c
> > b/ofproto/ofproto-dpif-xlate.c index e26f6c8..e610422 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -6366,6 +6366,8 @@ do_xlate_actions(const struct ofpact *ofpacts,
> size_t ofpacts_len,
> > struct flow_wildcards *wc = ctx->wc;
> > struct flow *flow = &ctx->xin->flow;
> > const struct ofpact *a;
> > + struct xbundle *in_xbundle;
> > + struct xport *in_port;
> > /* dl_type already in the mask, not set below. */ @@ -6479,7
> > +6481,15 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
> ofpacts_len,
> > break;
> > case OFPACT_PUSH_VLAN:
> > - flow_push_vlan_uninit(flow, wc);
> > + in_xbundle = lookup_input_bundle(ctx, flow->in_port.ofp_port,
> &in_port);
> > + if (!in_xbundle) {
> > + xlate_report(ctx, OFT_WARN, "no input bundle, dropping");
> > + return;
> > + }
> > +
> > + if (in_xbundle->vlan_mode == PORT_VLAN_DOT1Q_TUNNEL) {
> > + flow_push_vlan_uninit(flow, wc);
> > + }
> > flow->vlans[0].tpid = ofpact_get_PUSH_VLAN(a)->ethertype;
> > flow->vlans[0].tci = htons(VLAN_CFI);
> > break;
>
> This patch doesn't look right to me. push_vlan can be used explicitly without
> dot1q tunnel, so we should always honor it.
Do you have a good idea to fix it?
>
> mod_vlan_vid corresponds to OFPACT_SET_VLAN_VID, so I'm not sure why
> you're attempting to modify OFPACT_PUSH_VLAN.
When using OpenFlow13, the mod_vlan_vid corresponds to OFPACT_PUSH_VLAN.
The code:
static void
encode_SET_VLAN_VID(const struct ofpact_vlan_vid *vlan_vid,
enum ofp_version ofp_version, struct ofpbuf *out)
{
uint16_t vid = vlan_vid->vlan_vid;
/* Push a VLAN tag, if none is present and this form of the action calls
* for such a feature. */
if (ofp_version > OFP10_VERSION
&& vlan_vid->push_vlan_if_needed
&& !vlan_vid->flow_has_vlan) {
put_OFPAT11_PUSH_VLAN(out, htons(ETH_TYPE_VLAN_8021Q)); ---> //Currently walking this branch
}
if (ofp_version == OFP10_VERSION) {
put_OFPAT10_SET_VLAN_VID(out, vid);
} else if (ofp_version == OFP11_VERSION) {
put_OFPAT11_SET_VLAN_VID(out, vid);
} else {
put_set_field(out, ofp_version, MFF_VLAN_VID, vid | OFPVID12_PRESENT);
}
}
More information about the discuss
mailing list