> >-----Original Message----- > >From: Ilya Maximets <i.maxim...@ovn.org> > >Sent: Thursday, August 13, 2020 9:26 PM > >To: Emma Finn <emma.f...@intel.com>; d...@openvswitch.org > >Cc: ian.sto...@intel.com; i.maxim...@ovn.org; Eli Britstein > ><el...@nvidia.com>; beilei.x...@intel.com; i.maxim...@ovn.org > >Subject: Re: [PATCH] netdev-offload-dpdk: Fix for broken ethernet > >matching HWOL for XL710 NIC > > > > > >On 8/13/20 6:13 PM, Emma Finn wrote: > >> This patch introduces a temporary work around to fix partial hardware > >> offload for XL710 devices. Currently the incorrect ethernet pattern > >> is being set. This patch will be removed once this issue is fixed > >> within the i40e PMD. > >> > >> Signed-off-by: Emma Finn <emma.f...@intel.com> > >> Signed-off-by: Eli Britstein <el...@nvidia.com> > >> Co-authored-by: Eli Britstein <el...@nvidia.com> > >> --- > >> lib/netdev-offload-dpdk.c | 35 +++++++++++++++++++++++++---------- > >> 1 file changed, 25 insertions(+), 10 deletions(-) > >> > >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > >> index de6101e..ede2077 100644 > >> --- a/lib/netdev-offload-dpdk.c > >> +++ b/lib/netdev-offload-dpdk.c > >> @@ -672,10 +672,24 @@ free_flow_actions(struct flow_actions *actions) > >> actions->cnt = 0; > >> } > >> > >> +/* > >> +* This is a temporary work around to fix ethernet pattern for > >> +partial hardware > >> +* offload for X710 devices. This fix will be reverted once the issue > >> +is fixed > >> +* within the i40e PMD driver. > >> +*/ > >> +#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); \ > >> + } > > > >Could you please address my comments from this e-mail: > > > >https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail. > >op > >envswitch.org%2Fpipermail%2Fovs-dev%2F2020- > >August%2F373873.html&data=02%7C01%7Celibr%40nvidia.com%7C08c8f > >a0338e14858343d08d83fb664bf%7C43083d15727340c1b7db39efd9ccc17a% > >7C0%7C1%7C637329399898256323&sdata=bM8dq1Da0%2F%2FL7m7y > >m470T1aClwH9ZE%2BhlNcED1m7sdQ%3D&reserved=0 > >? > > > >i.e. convert this macro definition into function. > My previous patch was a POC. Here is a more "nicer" one. No macro/function, > and no redundant allocation/free: > Need to test and obviously finalize.
Thanks for this Eli, just a heads up Emma is out of office today but I can test this and submit the v2 if you prefer? @Ilya, what are your thoughts on this approach below? BR Ian > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index > de6101e4d..c6d293af3 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -696,16 +696,26 @@ parse_flow_match(struct flow_patterns *patterns, > !eth_addr_is_zero(match->wc.masks.dl_dst)) { > struct rte_flow_item_eth *spec, *mask; > > - spec = xzalloc(sizeof *spec); > - mask = xzalloc(sizeof *mask); > + /* WRITE A COMMENT ABOUT THIS WORKAROUND. */ > + if (is_all_zeros(&match->wc.masks.dl_dst, sizeof mask->dst) && > + is_all_zeros(&match->wc.masks.dl_src, sizeof mask->src) && > + match->wc.masks.dl_type == 0xFFFF && > + (match->flow.dl_type == htons(ETH_TYPE_IP) || > + match->flow.dl_type == htons(ETH_TYPE_IPV6))) { > + spec = NULL; > + mask = NULL; > + } else { > + spec = xzalloc(sizeof *spec); > + mask = xzalloc(sizeof *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(&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 mask->dst); > + memcpy(&mask->src, &match->wc.masks.dl_src, sizeof mask->src); > + 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); > > > >> + > >> static int > >> parse_flow_match(struct flow_patterns *patterns, > >> struct match *match) { > >> + struct rte_flow_item_eth *eth_mask = NULL; > >> + struct rte_flow_item_eth *eth_spec = NULL; > >> uint8_t *next_proto_mask = NULL; > >> struct flow *consumed_masks; > >> uint8_t proto = 0; > >> @@ -694,24 +708,23 @@ 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; > >> > >> - spec = xzalloc(sizeof *spec); > >> - mask = xzalloc(sizeof *mask); > >> + eth_spec = xzalloc(sizeof *eth_spec); > >> + 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(ð_spec->dst, &match->flow.dl_dst, sizeof eth_spec->dst); > >> + memcpy(ð_spec->src, &match->flow.dl_src, sizeof eth_spec->src); > >> + eth_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, eth_spec, > >> + eth_mask); > >> } > >> > >> /* VLAN */ > >> @@ -738,6 +751,7 @@ parse_flow_match(struct flow_patterns *patterns, > >> /* IP v4 */ > >> if (match->flow.dl_type == htons(ETH_TYPE_IP)) { > >> struct rte_flow_item_ipv4 *spec, *mask; > > > >Empty line needed. > > > >> + NULL_ETH_MASK_IF_ZEROS; > >> > >> spec = xzalloc(sizeof *spec); > >> mask = xzalloc(sizeof *mask); @@ -776,6 +790,7 @@ > >> parse_flow_match(struct flow_patterns *patterns, > >> /* IP v6 */ > >> if (match->flow.dl_type == htons(ETH_TYPE_IPV6)) { > >> struct rte_flow_item_ipv6 *spec, *mask; > > > >ditto. > > > >> + NULL_ETH_MASK_IF_ZEROS; > >> > >> spec = xzalloc(sizeof *spec); > >> mask = xzalloc(sizeof *mask); > >> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev