On 8/11/20 5:35 AM, Xing, Beilei wrote:
> 
> 
>> -----Original Message-----
>> From: Stokes, Ian <ian.sto...@intel.com>
>> Sent: Tuesday, August 11, 2020 4:52 AM
>> To: Eli Britstein <el...@mellanox.com>; Finn, Emma <emma.f...@intel.com>;
>> d...@openvswitch.org; Xing, Beilei <beilei.x...@intel.com>; Guo, Jia
>> <jia....@intel.com>
>> Cc: i.maxim...@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-jackerd...@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=0x10,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.

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.f...@intel.com>
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to