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?


>  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

Reply via email to