[ovs-dev] [PATCH] netdev-offload-dpdk: Fix for broken ethernet matching HWOL for XL710 NIC
Ilya Maximets
i.maximets at ovn.org
Tue Aug 11 11:51:13 UTC 2020
On 8/6/20 5: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.")
>
> Fixed by partial reversion of these changes.
>
> Signed-off-by: Emma Finn <emma.finn at intel.com>
> ---
> lib/netdev-offload-dpdk.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index de6101e..b300884 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -691,8 +691,7 @@ parse_flow_match(struct flow_patterns *patterns,
> consumed_masks->packet_type = 0;
>
> /* Eth */
> - if (match->wc.masks.dl_type ||
> - !eth_addr_is_zero(match->wc.masks.dl_src) ||
> + if (!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;
>
> @@ -712,6 +711,16 @@ parse_flow_match(struct flow_patterns *patterns,
> consumed_masks->dl_type = 0;
>
> add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
> + } else {
> + /*
> + * If user specifies a flow (like UDP flow) without L2 patterns,
> + * OVS will at least set the dl_type. Normally, it's enough to
> + * create an eth pattern just with it. Unluckily, some Intel's
> + * NIC (such as XL710) doesn't support that. Below is a workaround,
> + * which simply matches any L2 pkts.
> + */
> + consumed_masks->dl_type = 0;
> + add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
> }
>
> /* VLAN */
>
What about alternative workaround (not even compiled):
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 44ebf96da..dfb803c98 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -5198,6 +5198,22 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
return flow;
}
+struct int
+netdev_dpdk_rte_flow_validate(struct netdev *netdev,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item *items,
+ const struct rte_flow_action *actions,
+ struct rte_flow_error *error)
+{
+ struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+ int res;
+
+ ovs_mutex_lock(&dev->mutex);
+ res = rte_flow_validate(dev->port_id, attr, items, actions, error);
+ ovs_mutex_unlock(&dev->mutex);
+ return res;
+}
+
int
netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
struct rte_flow *rte_flow,
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index 848346cb4..49e3d2bfa 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -48,6 +48,13 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
const struct rte_flow_item *items,
const struct rte_flow_action *actions,
struct rte_flow_error *error);
+
+struct int
+netdev_dpdk_rte_flow_validate(struct netdev *netdev,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item *items,
+ const struct rte_flow_action *actions,
+ struct rte_flow_error *error);
int
netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
struct rte_flow *rte_flow,
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index de6101e4d..5df1d06de 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -553,6 +553,59 @@ dump_flow(struct ds *s, struct ds *s_extra,
return s;
}
+/* XXX: Comment why this function needed and what it does. */
+static struct flow *
+try_workaround_eth_pattern__(struct netdev *netdev,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item *items,
+ const struct rte_flow_action *actions,
+ struct rte_flow_error *error)
+{
+ struct rte_flow_item_eth *mask = items[0].mask;
+ if (items[0].type != RTE_FLOW_ITEM_TYPE_ETH
+ || mask->type != 0xffff
+ || !eth_addr_is_zero(mask->src)
+ || !eth_addr_is_zero(mask->dst)) {
+ return NULL;
+ }
+
+ struct rte_flow_item_eth *eth_spec = xzalloc(sizeof *eth_spec);
+ struct rte_flow_item_eth *eth_mask = xzalloc(sizeof *eth_mask);
+ const struct rte_flow_attr dummy_attr = { .ingress = 1, };
+ const struct rte_flow_action dummy_actions[] = {
+ { .type = RTE_FLOW_ACTION_TYPE_DROP, },
+ { .type = RTE_FLOW_ACTION_TYPE_END, },
+ };
+ const struct rte_flow_item bad_items[] = {
+ {
+ .type = RTE_FLOW_ITEM_TYPE_ETH,
+ .spec = eth_spec,
+ .mask = eth_mask,
+ },
+ /* XXX: Add extra items to trigger the issue here. */
+ };
+ bool may_need_workaround = false;
+ static struct flow *flow = NULL;
+
+ eth_spec->type = htons(ETH_TYPE_IP);
+ eth_mask->type = 0xffff;
+
+ if (netdev_dpdk_rte_flow_validate(netdev, &dummy_attr, bad_items,
+ dummy_actions, NULL) == /* XXX: Error type */) {
+ /* Seems PMD doesn't support ETH pattern with only 'type' set. */
+ free(items[0].spec);
+ free(items[0].mask);
+ items[0].spec = NULL;
+ items[0].mask = NULL;
+ flow = netdev_dpdk_rte_flow_create(netdev, attr, items,
+ actions, error);
+ }
+ free(eth_spec);
+ free(eth_mask);
+
+ return flow;
+}
+
static struct rte_flow *
netdev_offload_dpdk_flow_create(struct netdev *netdev,
const struct rte_flow_attr *attr,
@@ -566,6 +619,10 @@ netdev_offload_dpdk_flow_create(struct netdev *netdev,
char *extra_str;
flow = netdev_dpdk_rte_flow_create(netdev, attr, items, actions, error);
+ if (!flow && error->type == /* XXX: Put correct error type here */ ) {
+ flow = try_workaround_eth_pattern__(netdev, attr, items,
+ actions, error);
+ }
if (flow) {
if (!VLOG_DROP_DBG(&rl)) {
dump_flow(&s, &s_extra, attr, items, actions);
---
This should re-introduce the workaround for XL710 while keeping other NICs that
correctly supports ETH paterns untouched, i.e. avoid bugs with too wide matching
pattern for them. With a small additional overhead for the failed flows. A bit
higher overhead for XL710, but better than not having offloading at all. Overhead
will be removed along with the workaround once DPDK i40e PMD fixed.
Thoughts?
For the implementation, we could try to remember which netdev requires a workaround,
but this will require more changes.
Best regards, Ilya Maximets.
More information about the dev
mailing list