DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ethdev: expand RSS flows based on last item spec
@ 2019-11-05  9:16 Xiaoyu Min
  2019-11-05 10:11 ` Thomas Monjalon
  2019-11-05 13:42 ` [dpdk-dev] [PATCH v2] ethdev: fix expand RSS flows Xiaoyu Min
  0 siblings, 2 replies; 6+ messages in thread
From: Xiaoyu Min @ 2019-11-05  9:16 UTC (permalink / raw)
  To: orika, viacheslavo, Adrien Mazarguil, Thomas Monjalon,
	Ferruh Yigit, Andrew Rybchenko
  Cc: dev, stable

When rte_flow_expand_rss expands rte_flow item list based on the RSS
types. In another word, it will add some rte_flow items if the user
specified items are not complete, for example:

  ... pattern eth / end actions rss type tcp end ...

above flow will be expaned to:

  ... pattern eth / end actions rss types tcp
  ... pattern eth / ipv4 / tcp / end actions rss types tcp ...
  ... pattern eth / ipv6 / tcp / end actsion rss types tcp ...

However the expansion is just simply expanding items without
checking last items' spec, means the expanded rules could have conflicting
settings which is not reasonable and leads to some HW error, for
example:

  ... pattern eth type is 0x86DD / end actions rss types tcp ...

is expaneded to:

  ... pattern eth type is 0x86DD / ipv4 / tcp end ...

which has conflicting values: 0x86DD vs. ipv4 and HW will refuse create
flow on some NICs.

This patch will fix above by checking the last item's spec and try to
complete the item list.

Currently only support to complete item list based on L2/L3 layer.

Fixes: 4ed05fcd441b ("ethdev: add flow API to expand RSS flows")
Cc: stable@dpdk.org
Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
 lib/librte_ethdev/rte_flow.c | 132 +++++++++++++++++++++++++++++++++--
 1 file changed, 127 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 2f86d1affc..ef5ce1790d 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -173,6 +173,67 @@ flow_err(uint16_t port_id, int ret, struct rte_flow_error *error)
 	return ret;
 }
 
+static enum rte_flow_item_type
+rte_flow_expand_rss_item_complete(const struct rte_flow_item *item)
+{
+	enum rte_flow_item_type ret = RTE_FLOW_ITEM_TYPE_VOID;
+	uint16_t ether_type = 0;
+	uint8_t ip_next_proto = 0;
+
+	if (item == NULL || item->spec == NULL)
+		return ret;
+	switch (item->type) {
+	case RTE_FLOW_ITEM_TYPE_ETH:
+		ether_type = ((const struct rte_flow_item_eth *)
+				(item->spec))->type;
+		if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_IPV4)
+			ret = RTE_FLOW_ITEM_TYPE_IPV4;
+		else if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_IPV6)
+			ret = RTE_FLOW_ITEM_TYPE_IPV6;
+		else if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_VLAN)
+			ret = RTE_FLOW_ITEM_TYPE_VLAN;
+		break;
+	case RTE_FLOW_ITEM_TYPE_VLAN:
+		ether_type = ((const struct rte_flow_item_vlan *)
+				(item->spec))->inner_type;
+		if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_IPV4)
+			ret = RTE_FLOW_ITEM_TYPE_IPV4;
+		else if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_IPV6)
+			ret = RTE_FLOW_ITEM_TYPE_IPV6;
+		else if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_VLAN)
+			ret = RTE_FLOW_ITEM_TYPE_VLAN;
+		break;
+	case RTE_FLOW_ITEM_TYPE_IPV4:
+		ip_next_proto = ((const struct rte_flow_item_ipv4 *)
+				(item->spec))->hdr.next_proto_id;
+		if (ip_next_proto == IPPROTO_UDP)
+			ret = RTE_FLOW_ITEM_TYPE_UDP;
+		else if (ip_next_proto == IPPROTO_TCP)
+			ret = RTE_FLOW_ITEM_TYPE_TCP;
+		else if (ip_next_proto == IPPROTO_IP)
+			ret = RTE_FLOW_ITEM_TYPE_IPV4;
+		else if (ip_next_proto == IPPROTO_IPV6)
+			ret = RTE_FLOW_ITEM_TYPE_IPV6;
+		break;
+	case RTE_FLOW_ITEM_TYPE_IPV6:
+		ip_next_proto = ((const struct rte_flow_item_ipv6 *)
+				(item->spec))->hdr.proto;
+		if (ip_next_proto == IPPROTO_UDP)
+			ret = RTE_FLOW_ITEM_TYPE_UDP;
+		else if (ip_next_proto == IPPROTO_TCP)
+			ret = RTE_FLOW_ITEM_TYPE_TCP;
+		else if (ip_next_proto == IPPROTO_IP)
+			ret = RTE_FLOW_ITEM_TYPE_IPV4;
+		else if (ip_next_proto == IPPROTO_IPV6)
+			ret = RTE_FLOW_ITEM_TYPE_IPV6;
+		break;
+	default:
+		ret = RTE_FLOW_ITEM_TYPE_VOID;
+		break;
+	}
+	return ret;
+}
+
 /* Get generic flow operations structure from a port. */
 const struct rte_flow_ops *
 rte_flow_ops_get(uint16_t port_id, struct rte_flow_error *error)
@@ -932,6 +993,11 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 	size_t lsize;
 	size_t user_pattern_size = 0;
 	void *addr = NULL;
+	const struct rte_flow_expand_node *next = NULL;
+	struct rte_flow_item missed_item;
+	int missed = 0;
+	int elt = 0;
+	const struct rte_flow_item *last_item = NULL;
 
 	lsize = offsetof(struct rte_flow_expand_rss, entry) +
 		elt_n * sizeof(buf->entry[0]);
@@ -942,8 +1008,8 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 		addr = buf->entry[0].pattern;
 	}
 	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
-		const struct rte_flow_expand_node *next = NULL;
-
+		if (item->type != RTE_FLOW_ITEM_TYPE_VOID)
+			last_item = item;
 		for (i = 0; node->next && node->next[i]; ++i) {
 			next = &graph[node->next[i]];
 			if (next->type == item->type)
@@ -964,6 +1030,41 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 	/* Start expanding. */
 	memset(flow_items, 0, sizeof(flow_items));
 	user_pattern_size -= sizeof(*item);
+	/*
+	 * Check if the last valid item has spec set
+	 * and need complete pattern.
+	 */
+	missed_item.type = rte_flow_expand_rss_item_complete(last_item);
+	if (missed_item.type != RTE_FLOW_ITEM_TYPE_VOID) {
+		next = NULL;
+		missed = 1;
+		for (i = 0; node->next && node->next[i]; ++i) {
+			next = &graph[node->next[i]];
+			if (next->type == missed_item.type) {
+				flow_items[0].type = missed_item.type;
+				flow_items[1].type = RTE_FLOW_ITEM_TYPE_END;
+				break;
+			}
+			next = NULL;
+		}
+	}
+	if (next && missed) {
+		elt = 2; /* missed item + item end. */
+		node = next;
+		lsize += elt * sizeof(*item) + user_pattern_size;
+		if ((node->rss_types & types) && lsize <= size) {
+			buf->entry[buf->entries].priority = 1;
+			buf->entry[buf->entries].pattern = addr;
+			buf->entries++;
+			rte_memcpy(addr, buf->entry[0].pattern,
+				   user_pattern_size);
+			addr = (void *)(((uintptr_t)addr) + user_pattern_size);
+			rte_memcpy(addr, flow_items, elt * sizeof(*item));
+			addr = (void *)(((uintptr_t)addr) +
+					elt * sizeof(*item));
+		}
+	}
+	memset(flow_items, 0, sizeof(flow_items));
 	next_node = node->next;
 	stack[stack_pos] = next_node;
 	node = next_node ? &graph[*next_node] : NULL;
@@ -976,21 +1077,24 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 			 * When the stack_pos is 0, there are 1 element in it,
 			 * plus the addition END item.
 			 */
-			int elt = stack_pos + 2;
-
+			elt = stack_pos + 2;
 			flow_items[stack_pos + 1].type = RTE_FLOW_ITEM_TYPE_END;
 			lsize += elt * sizeof(*item) + user_pattern_size;
 			if (lsize <= size) {
 				size_t n = elt * sizeof(*item);
 
 				buf->entry[buf->entries].priority =
-					stack_pos + 1;
+					stack_pos + 1 + missed;
 				buf->entry[buf->entries].pattern = addr;
 				buf->entries++;
 				rte_memcpy(addr, buf->entry[0].pattern,
 					   user_pattern_size);
 				addr = (void *)(((uintptr_t)addr) +
 						user_pattern_size);
+				rte_memcpy(addr, &missed_item,
+					   missed * sizeof(*item));
+				addr = (void *)(((uintptr_t)addr) +
+					missed * sizeof(*item));
 				rte_memcpy(addr, flow_items, n);
 				addr = (void *)(((uintptr_t)addr) + n);
 			}
@@ -1015,5 +1119,23 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 		}
 		node = *next_node ? &graph[*next_node] : NULL;
 	};
+	/* no expanded flows but we have missed item, create one rule for it */
+	if (buf->entries == 1 && missed != 0) {
+		elt = 2;
+		lsize += elt * sizeof(*item) + user_pattern_size;
+		if (lsize <= size) {
+			buf->entry[buf->entries].priority = 1;
+			buf->entry[buf->entries].pattern = addr;
+			buf->entries++;
+			flow_items[0].type = missed_item.type;
+			flow_items[1].type = RTE_FLOW_ITEM_TYPE_END;
+			rte_memcpy(addr, buf->entry[0].pattern,
+				   user_pattern_size);
+			addr = (void *)(((uintptr_t)addr) + user_pattern_size);
+			rte_memcpy(addr, flow_items, elt * sizeof(*item));
+			addr = (void *)(((uintptr_t)addr) +
+					elt * sizeof(*item));
+		}
+	}
 	return lsize;
 }
-- 
2.24.0.rc0.3.g12a4aeaad8


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: expand RSS flows based on last item spec
  2019-11-05  9:16 [dpdk-dev] [PATCH] ethdev: expand RSS flows based on last item spec Xiaoyu Min
@ 2019-11-05 10:11 ` Thomas Monjalon
  2019-11-05 12:08   ` Jack Min
  2019-11-05 13:42 ` [dpdk-dev] [PATCH v2] ethdev: fix expand RSS flows Xiaoyu Min
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2019-11-05 10:11 UTC (permalink / raw)
  To: Xiaoyu Min
  Cc: dev, orika, viacheslavo, Adrien Mazarguil, Ferruh Yigit,
	Andrew Rybchenko, stable

Hi Jack,

If it is a fix, please change your title.

05/11/2019 10:16, Xiaoyu Min:
> When rte_flow_expand_rss expands rte_flow item list based on the RSS
> types.

There is no verb in this sentence.

> In another word, it will add some rte_flow items if the user

What is "it"?

> specified items are not complete, for example:
> 
>   ... pattern eth / end actions rss type tcp end ...

Please explain why it is not complete.

> 
> above flow will be expaned to:
> 
>   ... pattern eth / end actions rss types tcp
>   ... pattern eth / ipv4 / tcp / end actions rss types tcp ...
>   ... pattern eth / ipv6 / tcp / end actsion rss types tcp ...

There are several typos in this text. Please check.

> However the expansion is just simply expanding items without
> checking last items' spec, means the expanded rules could have conflicting
> settings which is not reasonable and leads to some HW error, for
> example:

This wording is really not clear.
Please make short sentences.

>   ... pattern eth type is 0x86DD / end actions rss types tcp ...
> 
> is expaneded to:
> 
>   ... pattern eth type is 0x86DD / ipv4 / tcp end ...
> 
> which has conflicting values: 0x86DD vs. ipv4 and HW will refuse create
> flow on some NICs.
> 
> This patch will fix above by checking the last item's spec and try to
> complete the item list.
> 
> Currently only support to complete item list based on L2/L3 layer.
> 
> Fixes: 4ed05fcd441b ("ethdev: add flow API to expand RSS flows")
> Cc: stable@dpdk.org

Missing empty line here to separate blocks.

> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
>  lib/librte_ethdev/rte_flow.c | 132 +++++++++++++++++++++++++++++++++--
>  1 file changed, 127 insertions(+), 5 deletions(-)

It's a big change. It needs to be carefully reviewed.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: expand RSS flows based on last item spec
  2019-11-05 10:11 ` Thomas Monjalon
@ 2019-11-05 12:08   ` Jack Min
  0 siblings, 0 replies; 6+ messages in thread
From: Jack Min @ 2019-11-05 12:08 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, Ori Kam, Slava Ovsiienko, Adrien Mazarguil, Ferruh Yigit,
	Andrew Rybchenko, stable

On Tue, 19-11-05, 11:11, Thomas Monjalon wrote:
> Hi Jack,
> 
> If it is a fix, please change your title.
> 
Ok, I will add 'fix' in title.

> 05/11/2019 10:16, Xiaoyu Min:
> > When rte_flow_expand_rss expands rte_flow item list based on the RSS
> > types.
> 
> There is no verb in this sentence.
> 
It seems I missed some words here. I'll update it.

> > In another word, it will add some rte_flow items if the user
> 
> What is "it"?
> 
I mean rte_flow_expande_rss.

> > specified items are not complete, for example:
> > 
> >   ... pattern eth / end actions rss type tcp end ...
> 
> Please explain why it is not complete.
> 
What I mean is user provides above instead of:

  ... pattern eth / ipv6 / tcp / end actions rss type tcp end ...

This one is complete.
> > 
> > above flow will be expaned to:
> > 
> >   ... pattern eth / end actions rss types tcp
> >   ... pattern eth / ipv4 / tcp / end actions rss types tcp ...
> >   ... pattern eth / ipv6 / tcp / end actsion rss types tcp ...
> 
> There are several typos in this text. Please check.
> 
Sure.

> > However the expansion is just simply expanding items without
> > checking last items' spec, means the expanded rules could have conflicting
> > settings which is not reasonable and leads to some HW error, for
> > example:
> 
> This wording is really not clear.
> Please make short sentences.
> 
OK, I'll make it shorter.

> >   ... pattern eth type is 0x86DD / end actions rss types tcp ...
> > 
> > is expaneded to:
> > 
> >   ... pattern eth type is 0x86DD / ipv4 / tcp end ...
> > 
> > which has conflicting values: 0x86DD vs. ipv4 and HW will refuse create
> > flow on some NICs.
> > 
> > This patch will fix above by checking the last item's spec and try to
> > complete the item list.
> > 
> > Currently only support to complete item list based on L2/L3 layer.
> > 
> > Fixes: 4ed05fcd441b ("ethdev: add flow API to expand RSS flows")
> > Cc: stable@dpdk.org
> 
> Missing empty line here to separate blocks.
> 
Ok, I'll add empty line here.

> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > ---
> >  lib/librte_ethdev/rte_flow.c | 132 +++++++++++++++++++++++++++++++++--
> >  1 file changed, 127 insertions(+), 5 deletions(-)
> 
> It's a big change. It needs to be carefully reviewed.
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-dev] [PATCH v2] ethdev: fix expand RSS flows
  2019-11-05  9:16 [dpdk-dev] [PATCH] ethdev: expand RSS flows based on last item spec Xiaoyu Min
  2019-11-05 10:11 ` Thomas Monjalon
@ 2019-11-05 13:42 ` Xiaoyu Min
  2019-11-06 10:12   ` Ori Kam
  1 sibling, 1 reply; 6+ messages in thread
From: Xiaoyu Min @ 2019-11-05 13:42 UTC (permalink / raw)
  To: orika, viacheslavo, thomas, Adrien Mazarguil, Ferruh Yigit,
	Andrew Rybchenko
  Cc: dev, stable

rte_flow_expand_rss expands rte_flow item list based on the RSS
types. In another word, some additional rules are added if the user
specified items are not complete enough according to the RSS type,
for example:

  ... pattern eth / end actions rss type tcp end ...

User only provides item eth but want to do RSS on tcp traffic.
The pattern is not complete enough to filter TCP traffic only.
This will be a problem for some HWs.
So some PMDs use rte_flow_expand_rss to expand above user provided
flow to:

  ... pattern eth / end actions rss types tcp
  ... pattern eth / ipv4 / tcp / end actions rss types tcp ...
  ... pattern eth / ipv6 / tcp / end actions rss types tcp ...

in order to filter TCP traffic only and do RSS correctly.

However the current expansion cannot handle pattern as below, which
provides ethertype or ip next proto instead of providing an item:

  ... pattern eth type is 0x86DD / end actions rss types tcp ...

rte_flow_expand_rss will expand above flow to:

  ... pattern eth type is 0x86DD / ipv4 / tcp end ...

which has conflicting values: 0x86DD vs. ipv4 and some HWs will refuse
to create flow.

This patch will fix above by checking the last item's spec and to
expand RSS flows correctly.

Currently only support to complete item list based on ether type or ip
next proto.

Fixes: 4ed05fcd441b ("ethdev: add flow API to expand RSS flows")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
 v2:
   * more clear commit message 
---
 lib/librte_ethdev/rte_flow.c | 132 +++++++++++++++++++++++++++++++++--
 1 file changed, 127 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 2f86d1affc..ef5ce1790d 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -173,6 +173,67 @@ flow_err(uint16_t port_id, int ret, struct rte_flow_error *error)
 	return ret;
 }
 
+static enum rte_flow_item_type
+rte_flow_expand_rss_item_complete(const struct rte_flow_item *item)
+{
+	enum rte_flow_item_type ret = RTE_FLOW_ITEM_TYPE_VOID;
+	uint16_t ether_type = 0;
+	uint8_t ip_next_proto = 0;
+
+	if (item == NULL || item->spec == NULL)
+		return ret;
+	switch (item->type) {
+	case RTE_FLOW_ITEM_TYPE_ETH:
+		ether_type = ((const struct rte_flow_item_eth *)
+				(item->spec))->type;
+		if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_IPV4)
+			ret = RTE_FLOW_ITEM_TYPE_IPV4;
+		else if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_IPV6)
+			ret = RTE_FLOW_ITEM_TYPE_IPV6;
+		else if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_VLAN)
+			ret = RTE_FLOW_ITEM_TYPE_VLAN;
+		break;
+	case RTE_FLOW_ITEM_TYPE_VLAN:
+		ether_type = ((const struct rte_flow_item_vlan *)
+				(item->spec))->inner_type;
+		if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_IPV4)
+			ret = RTE_FLOW_ITEM_TYPE_IPV4;
+		else if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_IPV6)
+			ret = RTE_FLOW_ITEM_TYPE_IPV6;
+		else if (rte_be_to_cpu_16(ether_type) == RTE_ETHER_TYPE_VLAN)
+			ret = RTE_FLOW_ITEM_TYPE_VLAN;
+		break;
+	case RTE_FLOW_ITEM_TYPE_IPV4:
+		ip_next_proto = ((const struct rte_flow_item_ipv4 *)
+				(item->spec))->hdr.next_proto_id;
+		if (ip_next_proto == IPPROTO_UDP)
+			ret = RTE_FLOW_ITEM_TYPE_UDP;
+		else if (ip_next_proto == IPPROTO_TCP)
+			ret = RTE_FLOW_ITEM_TYPE_TCP;
+		else if (ip_next_proto == IPPROTO_IP)
+			ret = RTE_FLOW_ITEM_TYPE_IPV4;
+		else if (ip_next_proto == IPPROTO_IPV6)
+			ret = RTE_FLOW_ITEM_TYPE_IPV6;
+		break;
+	case RTE_FLOW_ITEM_TYPE_IPV6:
+		ip_next_proto = ((const struct rte_flow_item_ipv6 *)
+				(item->spec))->hdr.proto;
+		if (ip_next_proto == IPPROTO_UDP)
+			ret = RTE_FLOW_ITEM_TYPE_UDP;
+		else if (ip_next_proto == IPPROTO_TCP)
+			ret = RTE_FLOW_ITEM_TYPE_TCP;
+		else if (ip_next_proto == IPPROTO_IP)
+			ret = RTE_FLOW_ITEM_TYPE_IPV4;
+		else if (ip_next_proto == IPPROTO_IPV6)
+			ret = RTE_FLOW_ITEM_TYPE_IPV6;
+		break;
+	default:
+		ret = RTE_FLOW_ITEM_TYPE_VOID;
+		break;
+	}
+	return ret;
+}
+
 /* Get generic flow operations structure from a port. */
 const struct rte_flow_ops *
 rte_flow_ops_get(uint16_t port_id, struct rte_flow_error *error)
@@ -932,6 +993,11 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 	size_t lsize;
 	size_t user_pattern_size = 0;
 	void *addr = NULL;
+	const struct rte_flow_expand_node *next = NULL;
+	struct rte_flow_item missed_item;
+	int missed = 0;
+	int elt = 0;
+	const struct rte_flow_item *last_item = NULL;
 
 	lsize = offsetof(struct rte_flow_expand_rss, entry) +
 		elt_n * sizeof(buf->entry[0]);
@@ -942,8 +1008,8 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 		addr = buf->entry[0].pattern;
 	}
 	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
-		const struct rte_flow_expand_node *next = NULL;
-
+		if (item->type != RTE_FLOW_ITEM_TYPE_VOID)
+			last_item = item;
 		for (i = 0; node->next && node->next[i]; ++i) {
 			next = &graph[node->next[i]];
 			if (next->type == item->type)
@@ -964,6 +1030,41 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 	/* Start expanding. */
 	memset(flow_items, 0, sizeof(flow_items));
 	user_pattern_size -= sizeof(*item);
+	/*
+	 * Check if the last valid item has spec set
+	 * and need complete pattern.
+	 */
+	missed_item.type = rte_flow_expand_rss_item_complete(last_item);
+	if (missed_item.type != RTE_FLOW_ITEM_TYPE_VOID) {
+		next = NULL;
+		missed = 1;
+		for (i = 0; node->next && node->next[i]; ++i) {
+			next = &graph[node->next[i]];
+			if (next->type == missed_item.type) {
+				flow_items[0].type = missed_item.type;
+				flow_items[1].type = RTE_FLOW_ITEM_TYPE_END;
+				break;
+			}
+			next = NULL;
+		}
+	}
+	if (next && missed) {
+		elt = 2; /* missed item + item end. */
+		node = next;
+		lsize += elt * sizeof(*item) + user_pattern_size;
+		if ((node->rss_types & types) && lsize <= size) {
+			buf->entry[buf->entries].priority = 1;
+			buf->entry[buf->entries].pattern = addr;
+			buf->entries++;
+			rte_memcpy(addr, buf->entry[0].pattern,
+				   user_pattern_size);
+			addr = (void *)(((uintptr_t)addr) + user_pattern_size);
+			rte_memcpy(addr, flow_items, elt * sizeof(*item));
+			addr = (void *)(((uintptr_t)addr) +
+					elt * sizeof(*item));
+		}
+	}
+	memset(flow_items, 0, sizeof(flow_items));
 	next_node = node->next;
 	stack[stack_pos] = next_node;
 	node = next_node ? &graph[*next_node] : NULL;
@@ -976,21 +1077,24 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 			 * When the stack_pos is 0, there are 1 element in it,
 			 * plus the addition END item.
 			 */
-			int elt = stack_pos + 2;
-
+			elt = stack_pos + 2;
 			flow_items[stack_pos + 1].type = RTE_FLOW_ITEM_TYPE_END;
 			lsize += elt * sizeof(*item) + user_pattern_size;
 			if (lsize <= size) {
 				size_t n = elt * sizeof(*item);
 
 				buf->entry[buf->entries].priority =
-					stack_pos + 1;
+					stack_pos + 1 + missed;
 				buf->entry[buf->entries].pattern = addr;
 				buf->entries++;
 				rte_memcpy(addr, buf->entry[0].pattern,
 					   user_pattern_size);
 				addr = (void *)(((uintptr_t)addr) +
 						user_pattern_size);
+				rte_memcpy(addr, &missed_item,
+					   missed * sizeof(*item));
+				addr = (void *)(((uintptr_t)addr) +
+					missed * sizeof(*item));
 				rte_memcpy(addr, flow_items, n);
 				addr = (void *)(((uintptr_t)addr) + n);
 			}
@@ -1015,5 +1119,23 @@ rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
 		}
 		node = *next_node ? &graph[*next_node] : NULL;
 	};
+	/* no expanded flows but we have missed item, create one rule for it */
+	if (buf->entries == 1 && missed != 0) {
+		elt = 2;
+		lsize += elt * sizeof(*item) + user_pattern_size;
+		if (lsize <= size) {
+			buf->entry[buf->entries].priority = 1;
+			buf->entry[buf->entries].pattern = addr;
+			buf->entries++;
+			flow_items[0].type = missed_item.type;
+			flow_items[1].type = RTE_FLOW_ITEM_TYPE_END;
+			rte_memcpy(addr, buf->entry[0].pattern,
+				   user_pattern_size);
+			addr = (void *)(((uintptr_t)addr) + user_pattern_size);
+			rte_memcpy(addr, flow_items, elt * sizeof(*item));
+			addr = (void *)(((uintptr_t)addr) +
+					elt * sizeof(*item));
+		}
+	}
 	return lsize;
 }
-- 
2.24.0.rc0.3.g12a4aeaad8


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ethdev: fix expand RSS flows
  2019-11-05 13:42 ` [dpdk-dev] [PATCH v2] ethdev: fix expand RSS flows Xiaoyu Min
@ 2019-11-06 10:12   ` Ori Kam
  2019-11-07 17:02     ` Ferruh Yigit
  0 siblings, 1 reply; 6+ messages in thread
From: Ori Kam @ 2019-11-06 10:12 UTC (permalink / raw)
  To: Jack Min, Slava Ovsiienko, Thomas Monjalon, Adrien Mazarguil,
	Ferruh Yigit, Andrew Rybchenko
  Cc: dev, stable



> -----Original Message-----
> From: Xiaoyu Min <jackmin@mellanox.com>
> Sent: Tuesday, November 5, 2019 3:43 PM
> To: Ori Kam <orika@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> Adrien Mazarguil <adrien.mazarguil@6wind.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH v2] ethdev: fix expand RSS flows
> 
> rte_flow_expand_rss expands rte_flow item list based on the RSS
> types. In another word, some additional rules are added if the user
> specified items are not complete enough according to the RSS type,
> for example:
> 
>   ... pattern eth / end actions rss type tcp end ...
> 
> User only provides item eth but want to do RSS on tcp traffic.
> The pattern is not complete enough to filter TCP traffic only.
> This will be a problem for some HWs.
> So some PMDs use rte_flow_expand_rss to expand above user provided
> flow to:
> 
>   ... pattern eth / end actions rss types tcp
>   ... pattern eth / ipv4 / tcp / end actions rss types tcp ...
>   ... pattern eth / ipv6 / tcp / end actions rss types tcp ...
> 
> in order to filter TCP traffic only and do RSS correctly.
> 
> However the current expansion cannot handle pattern as below, which
> provides ethertype or ip next proto instead of providing an item:
> 
>   ... pattern eth type is 0x86DD / end actions rss types tcp ...
> 
> rte_flow_expand_rss will expand above flow to:
> 
>   ... pattern eth type is 0x86DD / ipv4 / tcp end ...
> 
> which has conflicting values: 0x86DD vs. ipv4 and some HWs will refuse
> to create flow.
> 
> This patch will fix above by checking the last item's spec and to
> expand RSS flows correctly.
> 
> Currently only support to complete item list based on ether type or ip
> next proto.
> 
> Fixes: 4ed05fcd441b ("ethdev: add flow API to expand RSS flows")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
>  v2:
>    * more clear commit message
> ---

Acked-by: Ori Kam <orika@mellanox.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ethdev: fix expand RSS flows
  2019-11-06 10:12   ` Ori Kam
@ 2019-11-07 17:02     ` Ferruh Yigit
  0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2019-11-07 17:02 UTC (permalink / raw)
  To: Ori Kam, Jack Min, Slava Ovsiienko, Thomas Monjalon,
	Adrien Mazarguil, Andrew Rybchenko
  Cc: dev, stable

On 11/6/2019 10:12 AM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: Xiaoyu Min <jackmin@mellanox.com>
>> Sent: Tuesday, November 5, 2019 3:43 PM
>> To: Ori Kam <orika@mellanox.com>; Slava Ovsiienko
>> <viacheslavo@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
>> Adrien Mazarguil <adrien.mazarguil@6wind.com>; Ferruh Yigit
>> <ferruh.yigit@intel.com>; Andrew Rybchenko <arybchenko@solarflare.com>
>> Cc: dev@dpdk.org; stable@dpdk.org
>> Subject: [PATCH v2] ethdev: fix expand RSS flows
>>
>> rte_flow_expand_rss expands rte_flow item list based on the RSS
>> types. In another word, some additional rules are added if the user
>> specified items are not complete enough according to the RSS type,
>> for example:
>>
>>   ... pattern eth / end actions rss type tcp end ...
>>
>> User only provides item eth but want to do RSS on tcp traffic.
>> The pattern is not complete enough to filter TCP traffic only.
>> This will be a problem for some HWs.
>> So some PMDs use rte_flow_expand_rss to expand above user provided
>> flow to:
>>
>>   ... pattern eth / end actions rss types tcp
>>   ... pattern eth / ipv4 / tcp / end actions rss types tcp ...
>>   ... pattern eth / ipv6 / tcp / end actions rss types tcp ...
>>
>> in order to filter TCP traffic only and do RSS correctly.
>>
>> However the current expansion cannot handle pattern as below, which
>> provides ethertype or ip next proto instead of providing an item:
>>
>>   ... pattern eth type is 0x86DD / end actions rss types tcp ...
>>
>> rte_flow_expand_rss will expand above flow to:
>>
>>   ... pattern eth type is 0x86DD / ipv4 / tcp end ...
>>
>> which has conflicting values: 0x86DD vs. ipv4 and some HWs will refuse
>> to create flow.
>>
>> This patch will fix above by checking the last item's spec and to
>> expand RSS flows correctly.
>>
>> Currently only support to complete item list based on ether type or ip
>> next proto.
>>
>> Fixes: 4ed05fcd441b ("ethdev: add flow API to expand RSS flows")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
>> ---
>>  v2:
>>    * more clear commit message
>> ---
> 
> Acked-by: Ori Kam <orika@mellanox.com>
> 

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-11-07 17:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05  9:16 [dpdk-dev] [PATCH] ethdev: expand RSS flows based on last item spec Xiaoyu Min
2019-11-05 10:11 ` Thomas Monjalon
2019-11-05 12:08   ` Jack Min
2019-11-05 13:42 ` [dpdk-dev] [PATCH v2] ethdev: fix expand RSS flows Xiaoyu Min
2019-11-06 10:12   ` Ori Kam
2019-11-07 17:02     ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).