[ovs-dev] [PATCH] netdev-offload-dpdk: Fix for broken ethernet matching HWOL for XL710 NIC

Tue Aug 11 13:18:35 UTC 2020

On 8/11/20 3:12 PM, Finn, Emma wrote:
> 
> 
>> -----Original Message-----
>> From: Ilya Maximets <i.maximets at ovn.org>
>> Sent: Tuesday 11 August 2020 11:02
>> To: Xing, Beilei <beilei.xing at intel.com>; Stokes, Ian <ian.stokes at intel.com>;
>> Eli Britstein <elibr at mellanox.com>; Finn, Emma <emma.finn at intel.com>;
>> dev at openvswitch.org; Guo, Jia <jia.guo at intel.com>
>> Cc: i.maximets at ovn.org
>> Subject: Re: [PATCH] netdev-offload-dpdk: Fix for broken ethernet matching
>> HWOL for XL710 NIC
>>
>> On 8/11/20 5:35 AM, Xing, Beilei wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Stokes, Ian <ian.stokes at intel.com>
>>>> Sent: Tuesday, August 11, 2020 4:52 AM
>>>> To: Eli Britstein <elibr at mellanox.com>; Finn, Emma
>>>> <emma.finn at intel.com>; dev at openvswitch.org; Xing, Beilei
>>>> <beilei.xing at intel.com>; Guo, Jia <jia.guo at intel.com>
>>>> Cc: i.maximets at ovn.org
>>>> Subject: RE: [PATCH] netdev-offload-dpdk: Fix for broken ethernet
>>>> matching HWOL for XL710 NIC
>>>>
>>>>> On 8/7/2020 7:55 AM, Eli Britstein wrote:
>>>>>>
>>>>>> On 8/6/2020 8:28 PM, Stokes, Ian wrote:
>>>>>>>> On 8/6/2020 6:17 PM, Emma Finn wrote:
>>>>>>>>> The following 2 commits introduced changes which caused a
>>>>>>>>> regression for XL710 devices and functionality ceases for
>>>>>>>>> partial offload as a result.
>>>>>>>>> 864852a0624a ("netdev-offload-dpdk: Fix Ethernet matching for
>>>>>>>>> type
>>>>>>>>> only.")
>>>>>>>>> a79eae87abe4 ("netdev-offload-dpdk: Remove pre-validate of
>>>>> patterns
>>>>>>>>> function.")
>>>>>>>> OVS is vendor agnostic. That kind of workaround belongs in intel
>>>>>>>> PMD in dpdk, not in OVS.
>>>>>>> Hi Eli,
>>>>>>>
>>>>>>> Yes OVS looks to be vendor agnostic, but this code I believe was
>>>>>>> already in place and working for this usecase. As such it's
>>>>>>> removal introduced a regression from an OVS point of view between
>> the releases.
>>>>>>>
>>>>>>> We have had examples in the past where workarounds are
>> permissible
>>>>>>> if there is a clear path to fixing this in the future on the DPDK
>>>>>>> side (which is what I suggest here) (for example scatter gather
>>>>>>> support for MTUs in the past raised similar issue where we hand to
>>>>>>> handle specific NIC until the next DPDK LTS release).
>>>>>>>
>>>>>>> So my suggestion is to re-instate the original workaround and
>>>>>>> remove its when fixed in the next DPDK LTS which supports the
>>>>>>> change for i40e at the PMD layer or if it's backported to the next
>>>>>>> 19.11 stable release which would be validated for use with OVS.
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> There was a bug with this WA.
>>>>>>
>>>>>> Please see
>>>>>> https://patchwork.ozlabs.org/project/openvswitch/patch/1587180266-
>>>>> 28632-1-git-send-email-JackerDune at gmail.com/.
>>
>> I'm worried about this bug.  Current version of a workaround is not correct
>> from the OVS point of view since it wildcards dl_type during offloading that is
>> not expected by higher layers, causing HW flow being much more wide than
>> requested.  In case we going to have this workaround for xl710, we should
>> have another workaround for this issue too.
>>
>>
>>>>>>
>>>>>> Is it possible to address it in DPDK instead of reverting in OVS
>>>>>> and later re-reverting?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Eli
>>>>>>
>>>>>>>
>>>>>>> I've included the i40e DPDK maintainers here for their thoughts also.
>>>>>>>
>>>>>>> @Beilei/Jia Is this something we can look at to introduce in the
>>>>>>> i40e PMD?
>>>>>
>>>>> Hello,
>>>>>
>>>>> Please take a look at:
>>>>>
>>>>> http://mails.dpdk.org/archives/dev/2020-May/166272.html
>>>>>
>>>>> For MLX it was only an optimization. For i40e something similar may
>>>>> be a workaround for this issue.
>>>>
>>>> Thanks for this Eli, let me sync with Beilei on this.
>>>>
>>>> If it's something we can resolve in the PMD then I think we can add
>>>> an errata or known issue for the 2.14 release (and possibly the 2.13
>>>> release as I think the same issue is present there).
>>>>
>>>> If it was fixed in the future we could then remove the issue notice.>
>>>
>>> Hi,
>>>
>>> According to the OVS patch and mlx DPDK patch, is the requirement to
>>> support rte flow pattern like 'pattern IPv4 / UDP src is 32 / end', no
>>> need to use ''pattern ETH / IPv4 / UDP src is 32 / end '?
>>> please correct me if I'm wrong.
>>>
>>> If yes, could you please tell me how OVS adds a flow which doesn’t
>>> include ETH item? I'm not very familiar with OVS, I run some simple
>>> commands before, and find eth will always exist. E.g:
>>> flow_add: ufid:fced09a8-9b8a-420d-9cb6-454ab9bed224
>>> skb_priority(0),skb_mark(0),\
>>> ct_state(-new-est-rel-rpl-inv-trk-snat-dnat),ct_zone(0),ct_mark(0),ct_
>>> label(0),recirc_id(0),\
>>> dp_hash(0),in_port(2),packet_type(ns=0,id=0),eth(src=00:e8:ca:11:ba:80
>>> ,dst=ff:ff:ff:ff:ff:ff),
>>> eth_type(0x0800),ipv4(src=0.0.0.0,dst=255.255.255.255,proto=17,tos=0x1
>>> 0,ttl=128,frag=no),
>>> udp(src=68,dst=67), actions:drop
>>
>> Emma, Ian, could you please provide the pattern that fails to offload on
>> current OVS master so it will be easier for everyone to understand the issue.
>> It's not obvious how to construct the bad pattern by only looking at the i40e
>> pmd code.  Also, please, enable debug logs and provide the testpmd-style
>> arguments constructed by OVS.
>>
>> It might be also good to have all this information in the commit message.
>>
> 
> Following flow pattern is failing from logs:
> 
>   Attributes: ingress=1, egress=0, prio=0, group=0, transfer=1
> rte flow eth pattern:
>   Spec: src=00:00:04:00:0b:00, dst=00:00:04:00:0a:00, type=0x0800
>   Mask: src=00:00:00:00:00:00, dst=00:00:00:00:00:00, type=0xffff
> rte flow ipv4 pattern:
>   Spec: tos=0x0, ttl=40, proto=0x11, src=1.1.0.0, dst=0.0.0.0
>   Mask: tos=0x0, ttl=0, proto=0x0, src=255.255.255.255, dst=0.0.0.0
> rte flow count action:
>   Count: shared=0, id=0
> rte flow port-id action:
>   Port-id: original=0, id=1
> 2020-08-11T10:49:28.002Z|00003|netdev_offload_dpdk(dp_netdev_flow_18)|WARN|dpdk0: rte_flow creation failed: 13 (Unsupported ether_type.).
> 
> Or from dump-flows :
> 
> flow-dump from pmd on cpu core: 23
> ufid:b4ba667d-8f4d-4f45-8896-c541d9fcecc0, skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(dpdk0),packet_type(ns=0,id=0),eth(src=00:00:04:00:0b:00/00:00:00:00:00:00,dst=00:00:04:00:0a:00/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=1.1.0.0,dst=0.0.0.0/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=63/0,dst=63/0), packets:42575487, bytes:2554529220, used:0.000s, offloaded:partial, dp:ovs, actions:dpdk1, dp-extra-info:miniflow_bits(4,2)
> 
> 
> with regards to " provide the testpmd-style arguments constructed by OVS."
> Can you confirm what you mean?

We changed the way of logging in a following commit:
d8ad173fb9c1 ("netdev-offload-dpdk: Log testpmd format for flow create/destroy.")

It's on master and branch-2.14.

> 
> Thanks, 
> Emma
> 
>> Thanks.
>>
>> Best regards, Ilya Maximets.
>>
>>>
>>> Thanks,
>>> Beilei
>>>
>>>>
>>>> Regards
>>>> Ian
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Eli
>>>>>
>>>>>>>
>>>>>>> Regards
>>>>>>> Ian
>>>>>>>
>>>>>>>>> Fixed by partial reversion of these changes.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Emma Finn<emma.finn at intel.com>
>>>
> 