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

Wed Aug 22 13:44:35 UTC 2018

On 8/21/18 10:17 PM, 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.
>
> The first patch from wangyunjian can worked without any compatibility broken although it's a bit complicated.
> Do you have any better and simpler solutions or ideas?
>
> 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.
Open Flow version 1.0 specified ambiguous actions on vlans. The action 
of mod_vlan would result in an implicit push on an untagged packet. This 
was fixed in version 1.1 and above. Please see Section B.9.3 and B.6.18. 
in OpenFlow version 1.3.
I agree with Eric that it is better to use set_vlan action and specify 
Open Flow version 1.1 or greater to avoid unexpected push vlan actions.
>
> Hope that helps.
> Eric.
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev