[ovs-discuss] [ovs-dev] fix the mod_vlan_vid actions with OpenFlow13.

Wed Aug 22 18:59:50 UTC 2018

On Wed, Aug 22, 2018 at 02:17:33AM +0000, Lilijun (Jerry, Cloud Networking) wrote:
> Hi Eric and all,
> 
> Thanks for Eric's advice very much.
> 
> But in my opinion, this issue reported from wangyunjian is something about feature's compatibility. 
> 
> Using the rule: ovs-ofctl  -O OpenFlow13 add-flow ovsbr0 " table=0,priority=2,in_port=1 actions=mod_vlan_vid:3,NORMAL",
> the action mod_vlan_id's  behavior is only to modify a packet's vlan id before we introduce the feature of QinQ. 
> But now it has changed to push an outer vlan unexpectedly.
> 
> The same openflow rules will act with different functions. That maybe bring user's packets being handled with a wrong manner and dropped unexpectedly.
> 
> So, I think we'd better fix it to keep the compatibility instead of asking OVS's users for rules configuration changed to another action.

Unfortunately, I don't know a way to fix it without breaking something
else.

I'd like to reiterate that mod_vlan_vid without qualifying a vlan in the
match only worked pre-QinQ because the PUSH_VLAN would "push" a new tag
by overwriting the existing VLAN - albeit with no observable side effect
due to the follow-up SET_VLAN_VID.

ovs-ofctl has no information other than the match qualifiers. If there
is no vlan being matched it assumes the mod_vlan_vid should also try and
push a VLAN header. set_vlan_vid (OF >= 1.1) does not make this
assumption.

> 
> The first patch from wangyunjian can worked without any compatibility broken although it's a bit complicated.

The initial patch is buggy as I pointed out in my original reply. It
breaks the explicit "push_vlan" action. I would guess that "make
check-kernel" and "make check-system-userspace" fail on the 802.1ad
tests.

The second patch is also buggy. It breaks the mod_vlan_vid action for
non-tagged packets. It won't push a VLAN tag at all.

> Do you have any better and simpler solutions or ideas?

Not at the moment. Maybe someone on the list can come up with something.

> 
> B.R.
> Lilijun
> 
> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org] On Behalf Of Eric Garver
> Sent: Tuesday, August 21, 2018 9:45 PM
> To: wangyunjian <wangyunjian at huawei.com>
> Cc: dev at openvswitch.org; Zhoulei (stone, Cloud Networking) <stone.zhou at huawei.com>; ovs-discuss at openvswitch.org; thomasfherbert at gmail.com
> Subject: Re: [ovs-dev] [ovs-discuss] fix the mod_vlan_vid actions with OpenFlow13.
> 
> On Tue, Aug 21, 2018 at 02:23:33AM +0000, wangyunjian wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Eric Garver [mailto:e at erig.me]
> > > Sent: Monday, August 20, 2018 9:15 PM
> > > To: wangyunjian <wangyunjian at huawei.com>
> > > Cc: dev at openvswitch.org; ovs-discuss at openvswitch.org; Zhoulei 
> > > (stone, Cloud Networking) <stone.zhou at huawei.com>; 
> > > thomasfherbert at gmail.com
> > > Subject: Re: [ovs-discuss] [ovs-dev] fix the mod_vlan_vid actions 
> > > with OpenFlow13.
> > > 
> > > On Mon, Aug 20, 2018 at 02:17:34AM +0000, wangyunjian wrote:
> > > >
> > > >
> > > [..]
> > > > > > 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"
> > > 
> > > What happens if you add a wildcard VLAN match here?
> > > e.g. vlan_tci=0x1000/0x1000
> > 
> > The packet is set vlan_vid ok with adding the wildcard VLAN match.
> > 
> > linux-jrWzwZ:/data/wyj/git/ovs # ovs-ofctl -O OpenFlow13 add-flow ovsbr0 "cookie=0xb043f0d196265635,table=0,priority=2,in_port=1,vlan_tci=0x1000/0x1000 actions=mod_vlan_vid:2,NORMAL"
> > linux-jrWzwZ:/data/wyj/git/ovs # 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
> > 10:06:53.284556 90:17:ac:b0:0a:ff > Broadcast, ethertype 802.1Q 
> > (0x8100), length 64: vlan 2, p 0, ethertype ARP, Request who-has 
> > 3.3.3.18 tell 3.3.3.17, length 46
> > 10:06:54.286542 90:17:ac:b0:0a:ff > Broadcast, ethertype 802.1Q 
> > (0x8100), length 64: vlan 2, p 0, ethertype ARP, Request who-has 
> > 3.3.3.18 tell 3.3.3.17, length 46
> > 10:06:56.283594 90:17:ac:b0:0a:ff > Broadcast, ethertype 802.1Q 
> > (0x8100), length 64: vlan 2, p 0, ethertype ARP, Request who-has 
> > 3.3.3.18 tell 3.3.3.17, length 46
> 
> The mod_vlan_vid will implicitly insert a PUSH_VLAN action if the match/flow does not qualify a vlan regardless of the specified OF version. This is done on the ovs-ofctl side before the flow is sent over to ovs-vswitchd - long before any packets are involved.
> 
> You have two options:
> 
>     1) qualify the VLAN as you've done above
> 
>     2) Use set_vlan_vid action with "-O OpenFlow11" or greater. It will
>        _not_ attempt to push a VLAN.
> 
> This worked before 802.1ad/QinQ support only by accident because the PUSH_VLAN action blindly overwrote the existing VLAN.
> 
> Hope that helps.
> Eric.
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> discuss mailing list
> discuss at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-discuss