>-----Original Message-----
>From: Ilya Maximets <[email protected]>
>Sent: Monday, August 17, 2020 12:59 PM
>To: Ian Stokes <[email protected]>; [email protected]
>Cc: Eli Britstein <[email protected]>; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH v3 1/1] netdev-offload-dpdk: Fix for broken ethernet
>matching HWOL for XL710NIC.
>
>
>On 8/14/20 3:38 PM, Ian Stokes wrote:
>> From: Emma Finn <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Eli Britstein <[email protected]>
>> Co-authored-by: Eli Britstein <[email protected]>
>> Tested-by: Ian Stokes <[email protected]>
>> ---
>
>Thanks, Ian and Eli.
>This patch looks good to me, but I had some issues backporting it to 2.12 and
>below. We have removed the XL710 workaround from these old branches too,
>so I'm assuming that we need this patch there.
>
>On older branches memory for patterns is statically allocated, so it's not
>easy to
>NULL-ify pointers.
>
>Following version of the code applies to 2.12 and could be easily backported
>down to 2.10:
>
>diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index
>7146b2aab..b824218af 100644
>--- a/lib/netdev-offload-dpdk.c
>+++ b/lib/netdev-offload-dpdk.c
>@@ -446,9 +446,18 @@ netdev_offload_dpdk_add_flow(struct netdev
>*netdev,
> memset(&mask, 0, sizeof mask);
>
> /* Eth */
>- if (match->wc.masks.dl_type ||
>- !eth_addr_is_zero(match->wc.masks.dl_src) ||
>- !eth_addr_is_zero(match->wc.masks.dl_dst)) {
>+ if (match->wc.masks.dl_type == OVS_BE16_MAX && is_ip_any(&match-
>>flow)
>+ && eth_addr_is_zero(match->wc.masks.dl_dst)
>+ && eth_addr_is_zero(match->wc.masks.dl_src)) {
>+ /*
>+ * 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.
>+ */
>+ add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
>+ } else if (match->wc.masks.dl_type ||
>+ !eth_addr_is_zero(match->wc.masks.dl_src) ||
>+ !eth_addr_is_zero(match->wc.masks.dl_dst)) {
> memcpy(&spec.eth.dst, &match->flow.dl_dst, sizeof spec.eth.dst);
> memcpy(&spec.eth.src, &match->flow.dl_src, sizeof spec.eth.src);
> spec.eth.type = match->flow.dl_type;
>---
>
>If it looks good to you, I could apply original patch (below) to master,
>2.14 and 2.13, and a slightly different version (above) to 2.12, 2.11 and 2.10
>once travis builds done. What do you think?
If intel can also do with zero masks (eth type spec 0x0800 type mask 0 /
ipv4...), we don't have to NULL-ify but only zero the mask, but that's already
a logic change than what we know working.
Anyway, the above LGTM.
Your plan is good, but there is also a slightly different approach. We can
adapt the above and use it (or similar) for the 2.13+ branches, instead of the
below, to keep consistent and easier backporting in future.
>
>
>> lib/netdev-offload-dpdk.c | 28 ++++++++++++++++++++--------
>> 1 file changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>> index de6101e4d..2d668275a 100644
>> --- a/lib/netdev-offload-dpdk.c
>> +++ b/lib/netdev-offload-dpdk.c
>> @@ -696,16 +696,28 @@ 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);
>> + /*
>> + * 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.
>> + */
>> + if (match->wc.masks.dl_type == OVS_BE16_MAX && is_ip_any(&match-
>>flow)
>> + && eth_addr_is_zero(match->wc.masks.dl_dst)
>> + && eth_addr_is_zero(match->wc.masks.dl_src)) {
>> + 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);
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
