8/13/20 1:46 PM, Ilya Maximets wrote: > > On 8/13/20 1:12 PM, Finn, Emma wrote: > >> > >> > >>> -----Original Message----- > >>> From: Eli Britstein <el...@nvidia.com> > >>> Sent: Thursday 13 August 2020 10:26 > >>> To: Finn, Emma <emma.f...@intel.com>; Ilya Maximets > >>> <i.maxim...@ovn.org>; Xing, Beilei <beilei.x...@intel.com>; Stokes, > >>> Ian <ian.sto...@intel.com>; Eli Britstein <el...@mellanox.com>; > >>> d...@openvswitch.org; Guo, Jia <jia....@intel.com> > >>> Subject: RE: [PATCH] netdev-offload-dpdk: Fix for broken ethernet > >>> matching HWOL for XL710 NIC > >>> > >>> > >>> > >>>> -----Original Message----- > >>>> From: Finn, Emma <emma.f...@intel.com> > >>>> Sent: Thursday, August 13, 2020 11:47 AM > >>>> To: Eli Britstein <el...@nvidia.com>; Ilya Maximets > >>>> <i.maxim...@ovn.org>; Xing, Beilei <beilei.x...@intel.com>; Stokes, > >>>> Ian <ian.sto...@intel.com>; Eli Britstein <el...@mellanox.com>; > >>>> d...@openvswitch.org; Guo, Jia <jia....@intel.com> > >>>> Subject: RE: [PATCH] netdev-offload-dpdk: Fix for broken ethernet > >>>> matching HWOL for XL710 NIC > >>>> > >>>>> -----Original Message----- > >>>>> From: Eli Britstein <el...@nvidia.com> > >>>>> Sent: Tuesday 11 August 2020 16:49 > >>>>> To: Ilya Maximets <i.maxim...@ovn.org>; Finn, Emma > >>>>> <emma.f...@intel.com>; Xing, Beilei <beilei.x...@intel.com>; > >>>>> Stokes, Ian <ian.sto...@intel.com>; Eli Britstein > >>>>> <el...@mellanox.com>; d...@openvswitch.org; Guo, Jia > >>>>> <jia....@intel.com> > >>>>> Subject: RE: [PATCH] netdev-offload-dpdk: Fix for broken ethernet > >>>>> matching HWOL for XL710 NIC > >>>>> > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Ilya Maximets <i.maxim...@ovn.org> > >>>>>> Sent: Tuesday, August 11, 2020 4:19 PM > >>>>>> To: Finn, Emma <emma.f...@intel.com>; Ilya Maximets > >>>>>> <i.maxim...@ovn.org>; Xing, Beilei <beilei.x...@intel.com>; > >>>>>> Stokes, Ian <ian.sto...@intel.com>; Eli Britstein > >>>>>> <el...@mellanox.com>; d...@openvswitch.org; Guo, Jia > >>>>>> <jia....@intel.com> > >>>>>> Subject: Re: [PATCH] netdev-offload-dpdk: Fix for broken ethernet > >>>>>> matching HWOL for XL710 NIC > >>>>>> > >>>>>> On 8/11/20 3:12 PM, Finn, Emma wrote: > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Ilya Maximets <i.maxim...@ovn.org> > >>>>>>>> Sent: Tuesday 11 August 2020 11:02 > >>>>>>>> To: Xing, Beilei <beilei.x...@intel.com>; Stokes, Ian > >>>>>>>> <ian.sto...@intel.com>; Eli Britstein <el...@mellanox.com>; > >>>>>>>> Finn, Emma <emma.f...@intel.com>; d...@openvswitch.org; 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/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://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%25 > >>>>>>>>>>>> 2 > >>>>>>>>>>>> > >>>>> > >>>> > Fpatchwork.ozlabs.org%2Fproject%2Fopenvswitch%2Fpatch%2F158718026 > >>>>>>>>>>>> 6- > >>>>> > >>>>> > &data=02%7C01%7Celibr%40nvidia.com%7C2eb7f5d9553c44e4980a0 > >>> 8 > >>>>>>>>>>>> > >>>>> > >>>>> > d83df912c9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6373 > >>> 2 > >>>>> 7 > >>>>>> 487 > >>>>>>>>>>>> > >>>>> > >>>>> > 263661244&sdata=fgnwXLy3xt%2B8H8h9BJwDzlPmp3dtWv6AMTQ69 > >>> % > >>>>> 2 > >>>>>> B9kb > >>>>>>>>>>>> NM%3D&reserved=0 > >>>>>>>>>>> 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: > >>>>>>>>>>> > >>>>>>>>>>> > >>>>> https://nam03.safelinks.protection.outlook.com/?url=http%3A%2F%2F > >>>>>>>>>>> m > >>>>>>>>>>> ails.dpdk.org%2Farchives%2Fdev%2F2020- > >>>>>> May%2F166272.html&data=0 > >>>>>>>>>>> > >>>>> > >>>>> > 2%7C01%7Celibr%40nvidia.com%7C2eb7f5d9553c44e4980a08d83df912c9 > >>> % > >>>>> 7 > >>>>>> C4 > >>>>>>>>>>> > >>>>> > >>>>> > 3083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637327487263661244 > >>>> & > >>>>> a > >>>>>> mp;s > >>>>>>>>>>> > >>>>> > >>>> > data=g2Yiy07cL4yGIQYsJzNkZqimUibNMXsDQLNdtJ8Evmw%3D&reserv > >>>>> ed= > >>>>>> 0 > >>>>>>>>>>> > >>>>>>>>>>> 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_mar > >>>>>>>>> k( > >>>>>>>>> 0 > >>>>>>>>> ), > >>>>>>>>> c > >>>>>>>>> t_ > >>>>>>>>> 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 > >>>>>>>>> ,t > >>>>>>>>> o > >>>>>>>>> s= > >>>>>>>>> 0 > >>>>>>>>> x1 > >>>>>>>>> 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)|WAR > >>>>> N| > >>>>>> dpdk0: rte_flow creation failed: 13 (Unsupported ether_type.). > >>>>> Hi > >>>>> > >>>>> Maybe another workaround until fixed in i40e PMD is to mask out > >>>>> the ether_type match if there is IPv4 or IPv6. > >>>>> It means that patterns of: > >>>>> eth type is 0x0800 / ipv4 / ... > >>>>> will be > >>>>> eth / ipv4 / ... > >>>>> > >>>>> For IPv6 the same, but for other ether types no removal of the match. > >>>>> eth type is 0x1234 / > >>>>> kept the same. > >>>>> > >>>>> I referred to MLX5 PMD patch. As I mentioned, for MLX5 it's only > >>>>> an optimization, for XL710 it can be used as a workaround. > >>>>> > >>> https://nam03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmai > >>> l > >>>>> s > >>>>> .dpdk.org%2Farchives%2Fdev%2F2020- > >>>> May%2F166272.html&data=02%7C01%7 > >>>>> > >>>> > Celibr%40nvidia.com%7C3ce7437a70c14475299e08d83f657fdd%7C43083d1 > >>> 5 > >>>> 72734 > >>>>> > >>>> > 0c1b7db39efd9ccc17a%7C0%7C0%7C637329052456016934&sdata=D2 > >>> H > >>>> XS8Wo9Si > >>>>> aD3BCtY8%2BvX5gKH8a%2B7VmAVG8YPxkjAE%3D&reserved=0 > >>>>> > >>>>> A similar masking out of protocol type is done today for > >>>>> TCP/UDP/SCTP/ICMP. For example: > >>>>> /* proto == TCP and ITEM_TYPE_TCP, thus no need for proto match. > >>> */ > >>>>> if (next_proto_mask) { > >>>>> *next_proto_mask = 0; > >>>>> } > >>>>> Please take a look at > >>>>> > >>> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa > >>> t > >>>>> c > >>>>> > >>>> > hwork.ozlabs.org%2Fproject%2Fopenvswitch%2Fpatch%2F20200710120718 > >>> .3 > >>>> &am > >>>>> > >>>> > p;data=02%7C01%7Celibr%40nvidia.com%7C3ce7437a70c14475299e08d83f > >>> 6 > >>>> 57fdd > >>>>> > >>>> > %7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637329052456016 > >>>> 934&s > >>>>> > >>>> > data=I8EPuHTyuiP7e3NeEQ86fXIKyAzf5%2BOQ2B8CHo7ilI4%3D&reser > >>> ved > >>>> =0 > >>>>> 8633-3-sriharsha.basavapa...@broadcom.com/ > >>>>> That posted commit suggest not to mask out the protocol type and > >>>>> let the PMDs do it if they wish. > >>>>> > >>>>> The proposed workaround (not tested): > >>>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > >>>>> index de6101e4d..1af7bebcd 100644 > >>>>> --- a/lib/netdev-offload-dpdk.c > >>>>> +++ b/lib/netdev-offload-dpdk.c > >>>>> @@ -676,6 +676,7 @@ static int > >>>>> parse_flow_match(struct flow_patterns *patterns, > >>>>> struct match *match) { > >>>>> + uint8_t *next_eth_type_mask = NULL; > >>>>> uint8_t *next_proto_mask = NULL; > >>>>> struct flow *consumed_masks; > >>>>> uint8_t proto = 0; > >>>>> @@ -712,6 +713,7 @@ parse_flow_match(struct flow_patterns > >>> *patterns, > >>>>> consumed_masks->dl_type = 0; > >>>>> > >>>>> add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, > >>>>> mask); > >>>>> + next_eth_type_mask = &mask->type; > >>>>> } > >>>>> > >>>>> /* VLAN */ > >>>>> @@ -739,6 +741,11 @@ parse_flow_match(struct flow_patterns > >>> *patterns, > >>>>> if (match->flow.dl_type == htons(ETH_TYPE_IP)) { > >>>>> struct rte_flow_item_ipv4 *spec, *mask; > >>>>> > >>>>> + /* IPv4. No need to match ether type. */ > >>>>> + if (next_eth_type_mask) { > >>>>> + *next_eth_type_mask = 0; > >>>>> + } > >>>>> + > >>>>> spec = xzalloc(sizeof *spec); > >>>>> mask = xzalloc(sizeof *mask); > >>>>> > >>>>> @@ -777,6 +784,11 @@ parse_flow_match(struct flow_patterns > >>> *patterns, > >>>>> if (match->flow.dl_type == htons(ETH_TYPE_IPV6)) { > >>>>> struct rte_flow_item_ipv6 *spec, *mask; > >>>>> > >>>>> + /* IPv6. No need to match ether type. */ > >>>>> + if (next_eth_type_mask) { > >>>>> + *next_eth_type_mask = 0; > >>>>> + } > >>>>> + > >>>>> spec = xzalloc(sizeof *spec); > >>>>> mask = xzalloc(sizeof *mask); > >>>>> > >>>>> > >>>>>>> > >>>>>>> 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_ma > >>>>>>> rk > >>>>>>> ( > >>>>>>> 0/ > >>>>>>> 0 > >>>>>>> ),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(dpdk0),packet_ > >>>>>>> ty > >>>>>>> p > >>>>>>> e( > >>>>>>> n > >>>>>>> > >>> s=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=6 > >>>>>>> 3/ > >>>>>>> 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. > >>>>>> > >>>>>>> > >>>> > >>>> Tested both of these patches and neither fixed the issue. > >>>> > >>>> The problem is that parse_flow_match() is setting an ethernet > >>>> pattern that the i40e PMD doesn't support. > >>>> Working Flow: > >>>> rte flow eth pattern: > >>>> Spec = null > >>>> Mask = null > >>>> rte flow ipv4 pattern: > >>>> Spec: tos=0x0, ttl=40, proto=0x11, src=21.2.5.1, dst=0.0.0.0 > >>>> Mask: tos=0x0, ttl=0, proto=0x0, src=240.0.0.0, dst=0.0.0.0 > >>>> > >>>> Broken Flow: > >>>> 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 > >>>> > >>>> I will continue to try rework the patches to fix this. > >>>> > >>>> Thanks, > >>>> Emma > >>>> > >>> Hi Emma, > >>> > >>> My previous patch had a bug (uint8_t instead of uint16_t). Anyway, I > >>> have prepared another one according to your inputs (also not > >>> tested). Please give it a try, and if works, finalize it with comments > >>> etc. > >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > >>> index > >>> de6101e4d..f4b04a880 100644 > >>> --- a/lib/netdev-offload-dpdk.c > >>> +++ b/lib/netdev-offload-dpdk.c > >>> @@ -672,10 +672,19 @@ free_flow_actions(struct flow_actions *actions) > >>> actions->cnt = 0; > >>> } > >>> > >>> +/* write a comment this is a workaround... */ #define > >>> +NULL_ETH_MASK_IF_ZEROS \ > >>> + if (eth_mask && is_all_zeros(ð_mask->src, sizeof eth_mask->src) && > \ > >>> + is_all_zeros(ð_mask->dst, sizeof eth_mask->dst)) { \ > >>> + patterns->items[0].mask = NULL; \ > >>> + free(eth_mask); \ > >>> + } > > > > If we're going with this approach, please, make it a function that > > accepts 'struct rte_flow_item *', i.e. something like: > > > > clear_eth_pattern_if_type_only(&patterns->items[0]); > > > > One extra check that it's actually an eth pattern should be added > > inside the function. > > > > Is it OK to pass spec, but not mask? I do not see anything about this kind > > of configuration in the rte_flow API definition. I think, we should > > clean up spec too in this case, just to be sure that we're not leaking any > resources. > > > >>> + > >>> static int > >>> parse_flow_match(struct flow_patterns *patterns, > >>> struct match *match) { > >>> + struct rte_flow_item_eth *eth_mask = NULL; > > > > In case of a function, there is no need to move this definition > > outside of its block. > > > >>> uint8_t *next_proto_mask = NULL; > >>> struct flow *consumed_masks; > >>> uint8_t proto = 0; > >>> @@ -694,24 +703,24 @@ parse_flow_match(struct flow_patterns > *patterns, > >>> if (match->wc.masks.dl_type || > >>> !eth_addr_is_zero(match->wc.masks.dl_src) || > >>> !eth_addr_is_zero(match->wc.masks.dl_dst)) { > >>> - struct rte_flow_item_eth *spec, *mask; > >>> + struct rte_flow_item_eth *spec; > >>> > >>> spec = xzalloc(sizeof *spec); > >>> - mask = xzalloc(sizeof *mask); > >>> + eth_mask = xzalloc(sizeof *eth_mask); > >>> > >>> memcpy(&spec->dst, &match->flow.dl_dst, sizeof spec->dst); > >>> memcpy(&spec->src, &match->flow.dl_src, sizeof spec->src); > >>> spec->type = match->flow.dl_type; > >>> > >>> - memcpy(&mask->dst, &match->wc.masks.dl_dst, sizeof mask->dst); > >>> - memcpy(&mask->src, &match->wc.masks.dl_src, sizeof mask->src); > >>> - mask->type = match->wc.masks.dl_type; > >>> + memcpy(ð_mask->dst, &match->wc.masks.dl_dst, sizeof > >>> + eth_mask- > >>>> dst); > >>> + memcpy(ð_mask->src, &match->wc.masks.dl_src, sizeof > >>> + eth_mask- > >>>> src); > >>> + eth_mask->type = match->wc.masks.dl_type; > >>> > >>> memset(&consumed_masks->dl_dst, 0, sizeof consumed_masks- > >>>> dl_dst); > >>> memset(&consumed_masks->dl_src, 0, sizeof consumed_masks- > >>>> dl_src); > >>> consumed_masks->dl_type = 0; > >>> > >>> - add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask); > >>> + add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, > >>> + eth_mask); > >>> } > >>> > >>> /* VLAN */ > >>> @@ -739,6 +748,8 @@ parse_flow_match(struct flow_patterns *patterns, > >>> if (match->flow.dl_type == htons(ETH_TYPE_IP)) { > >>> struct rte_flow_item_ipv4 *spec, *mask; > >>> > >>> + NULL_ETH_MASK_IF_ZEROS; > >>> + > >>> spec = xzalloc(sizeof *spec); > >>> mask = xzalloc(sizeof *mask); > >>> > >>> @@ -777,6 +788,8 @@ parse_flow_match(struct flow_patterns *patterns, > >>> if (match->flow.dl_type == htons(ETH_TYPE_IPV6)) { > >>> struct rte_flow_item_ipv6 *spec, *mask; > >>> > >>> + NULL_ETH_MASK_IF_ZEROS; > >>> + > >>> spec = xzalloc(sizeof *spec); > >>> mask = xzalloc(sizeof *mask); > >>> > >>> > >> > >> Great. Thanks Eli, I tested this and it works. I will also have to test > >> this for > backporting to v.2.13.1. > >> I can provide test/review tags for this patch. > > Just for a clarification. > > Emma, will you finalize and send a formal patch for this? > IIUC, Eli expected you to finish this work.
Hi Ilya, Emma is reworking the patch now and will send on this evening, however Emma will be out of office tomorrow, I can help with the testing and re-work if needed. > > We have a tight time frame since we have 2.14 release scheduled on Monday. > It'll be good to have a patch today, so it could be tested. Agreed, once Emma sends the patch the plan is to test and re-work this evening/tomorrow. Thanks Ian > > >> > >> Thanks, > >> Emma > >> > >>> Thanks, > >>> Eli > >>> > >>>>>>> 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.f...@intel.com> > >>>>>>>>> > >>>>>>> > >> > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev