DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/sfc: avoid unnecessary flow pattern item traversal
@ 2023-02-14 13:27 Ivan Malov
  2023-02-14 13:39 ` Andrew Rybchenko
  0 siblings, 1 reply; 2+ messages in thread
From: Ivan Malov @ 2023-02-14 13:27 UTC (permalink / raw)
  To: dev; +Cc: Andrew Rybchenko, Viacheslav Galaktionov, Andy Moreton

Currently, the code tries to look up a user-provided item
by traversing those known to the PMD. Avoid the traversal
since it is easier to access items directly by their IDs,
with necessary checks to avoid reading past the buffer.

Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/net/sfc/sfc_flow.c | 60 +++++++++++---------------------------
 drivers/net/sfc/sfc_flow.h |  1 -
 drivers/net/sfc/sfc_mae.c  | 39 +++++++++----------------
 3 files changed, 30 insertions(+), 70 deletions(-)

diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
index fe1f5ba55f..9e1b746d51 100644
--- a/drivers/net/sfc/sfc_flow.c
+++ b/drivers/net/sfc/sfc_flow.c
@@ -1130,96 +1130,84 @@ sfc_flow_parse_pppoex(const struct rte_flow_item *item,
 }
 
 static const struct sfc_flow_item sfc_flow_items[] = {
-	{
-		.type = RTE_FLOW_ITEM_TYPE_VOID,
+	[RTE_FLOW_ITEM_TYPE_VOID] = {
 		.name = "VOID",
 		.prev_layer = SFC_FLOW_ITEM_ANY_LAYER,
 		.layer = SFC_FLOW_ITEM_ANY_LAYER,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_void,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_ETH,
+	[RTE_FLOW_ITEM_TYPE_ETH] = {
 		.name = "ETH",
 		.prev_layer = SFC_FLOW_ITEM_START_LAYER,
 		.layer = SFC_FLOW_ITEM_L2,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_eth,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_VLAN,
+	[RTE_FLOW_ITEM_TYPE_VLAN] = {
 		.name = "VLAN",
 		.prev_layer = SFC_FLOW_ITEM_L2,
 		.layer = SFC_FLOW_ITEM_L2,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_vlan,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_PPPOED,
+	[RTE_FLOW_ITEM_TYPE_PPPOED] = {
 		.name = "PPPOED",
 		.prev_layer = SFC_FLOW_ITEM_L2,
 		.layer = SFC_FLOW_ITEM_L2,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_pppoex,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_PPPOES,
+	[RTE_FLOW_ITEM_TYPE_PPPOES] = {
 		.name = "PPPOES",
 		.prev_layer = SFC_FLOW_ITEM_L2,
 		.layer = SFC_FLOW_ITEM_L2,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_pppoex,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_IPV4,
+	[RTE_FLOW_ITEM_TYPE_IPV4] = {
 		.name = "IPV4",
 		.prev_layer = SFC_FLOW_ITEM_L2,
 		.layer = SFC_FLOW_ITEM_L3,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_ipv4,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_IPV6,
+	[RTE_FLOW_ITEM_TYPE_IPV6] = {
 		.name = "IPV6",
 		.prev_layer = SFC_FLOW_ITEM_L2,
 		.layer = SFC_FLOW_ITEM_L3,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_ipv6,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_TCP,
+	[RTE_FLOW_ITEM_TYPE_TCP] = {
 		.name = "TCP",
 		.prev_layer = SFC_FLOW_ITEM_L3,
 		.layer = SFC_FLOW_ITEM_L4,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_tcp,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_UDP,
+	[RTE_FLOW_ITEM_TYPE_UDP] = {
 		.name = "UDP",
 		.prev_layer = SFC_FLOW_ITEM_L3,
 		.layer = SFC_FLOW_ITEM_L4,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_udp,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_VXLAN,
+	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
 		.name = "VXLAN",
 		.prev_layer = SFC_FLOW_ITEM_L4,
 		.layer = SFC_FLOW_ITEM_START_LAYER,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_vxlan,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_GENEVE,
+	[RTE_FLOW_ITEM_TYPE_GENEVE] = {
 		.name = "GENEVE",
 		.prev_layer = SFC_FLOW_ITEM_L4,
 		.layer = SFC_FLOW_ITEM_START_LAYER,
 		.ctx_type = SFC_FLOW_PARSE_CTX_FILTER,
 		.parse = sfc_flow_parse_geneve,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_NVGRE,
+	[RTE_FLOW_ITEM_TYPE_NVGRE] = {
 		.name = "NVGRE",
 		.prev_layer = SFC_FLOW_ITEM_L3,
 		.layer = SFC_FLOW_ITEM_START_LAYER,
@@ -1300,21 +1288,6 @@ sfc_flow_parse_attr(struct sfc_adapter *sa,
 	return 0;
 }
 
-/* Get item from array sfc_flow_items */
-static const struct sfc_flow_item *
-sfc_flow_get_item(const struct sfc_flow_item *items,
-		  unsigned int nb_items,
-		  enum rte_flow_item_type type)
-{
-	unsigned int i;
-
-	for (i = 0; i < nb_items; i++)
-		if (items[i].type == type)
-			return &items[i];
-
-	return NULL;
-}
-
 int
 sfc_flow_parse_pattern(struct sfc_adapter *sa,
 		       const struct sfc_flow_item *flow_items,
@@ -1336,15 +1309,16 @@ sfc_flow_parse_pattern(struct sfc_adapter *sa,
 	}
 
 	for (; pattern->type != RTE_FLOW_ITEM_TYPE_END; pattern++) {
-		item = sfc_flow_get_item(flow_items, nb_flow_items,
-					 pattern->type);
-		if (item == NULL) {
+		if (pattern->type >= nb_flow_items ||
+		    flow_items[pattern->type].parse == NULL) {
 			rte_flow_error_set(error, ENOTSUP,
 					   RTE_FLOW_ERROR_TYPE_ITEM, pattern,
 					   "Unsupported pattern item");
 			return -rte_errno;
 		}
 
+		item = &flow_items[pattern->type];
+
 		/*
 		 * Omitting one or several protocol layers at the beginning
 		 * of pattern is supported
@@ -1362,7 +1336,7 @@ sfc_flow_parse_pattern(struct sfc_adapter *sa,
 		 * Allow only VOID and ETH pattern items in the inner frame.
 		 * Also check that there is only one tunneling protocol.
 		 */
-		switch (item->type) {
+		switch (pattern->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
 		case RTE_FLOW_ITEM_TYPE_ETH:
 			break;
diff --git a/drivers/net/sfc/sfc_flow.h b/drivers/net/sfc/sfc_flow.h
index 12875344b5..a433c3a0df 100644
--- a/drivers/net/sfc/sfc_flow.h
+++ b/drivers/net/sfc/sfc_flow.h
@@ -141,7 +141,6 @@ typedef int (sfc_flow_item_parse)(const struct rte_flow_item *item,
 				  struct rte_flow_error *error);
 
 struct sfc_flow_item {
-	enum rte_flow_item_type type;		/* Type of item */
 	const char *name;			/* Item name */
 	enum sfc_flow_item_layers layer;	/* Layer of item */
 	enum sfc_flow_item_layers prev_layer;	/* Previous layer of item */
diff --git a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c
index 3daeed81b9..83bfa3bbeb 100644
--- a/drivers/net/sfc/sfc_mae.c
+++ b/drivers/net/sfc/sfc_mae.c
@@ -2377,16 +2377,14 @@ sfc_mae_rule_parse_item_tunnel(const struct rte_flow_item *item,
 }
 
 static const struct sfc_flow_item sfc_flow_items[] = {
-	{
-		.type = RTE_FLOW_ITEM_TYPE_MARK,
+	[RTE_FLOW_ITEM_TYPE_MARK] = {
 		.name = "MARK",
 		.prev_layer = SFC_FLOW_ITEM_ANY_LAYER,
 		.layer = SFC_FLOW_ITEM_ANY_LAYER,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_mark,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_PORT_ID,
+	[RTE_FLOW_ITEM_TYPE_PORT_ID] = {
 		.name = "PORT_ID",
 		/*
 		 * In terms of RTE flow, this item is a META one,
@@ -2397,8 +2395,7 @@ static const struct sfc_flow_item sfc_flow_items[] = {
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_port_id,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR,
+	[RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR] = {
 		.name = "PORT_REPRESENTOR",
 		/*
 		 * In terms of RTE flow, this item is a META one,
@@ -2409,8 +2406,7 @@ static const struct sfc_flow_item sfc_flow_items[] = {
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_ethdev_based,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT,
+	[RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT] = {
 		.name = "REPRESENTED_PORT",
 		/*
 		 * In terms of RTE flow, this item is a META one,
@@ -2421,72 +2417,63 @@ static const struct sfc_flow_item sfc_flow_items[] = {
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_ethdev_based,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_ETH,
+	[RTE_FLOW_ITEM_TYPE_ETH] = {
 		.name = "ETH",
 		.prev_layer = SFC_FLOW_ITEM_START_LAYER,
 		.layer = SFC_FLOW_ITEM_L2,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_eth,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_VLAN,
+	[RTE_FLOW_ITEM_TYPE_VLAN] = {
 		.name = "VLAN",
 		.prev_layer = SFC_FLOW_ITEM_L2,
 		.layer = SFC_FLOW_ITEM_L2,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_vlan,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_IPV4,
+	[RTE_FLOW_ITEM_TYPE_IPV4] = {
 		.name = "IPV4",
 		.prev_layer = SFC_FLOW_ITEM_L2,
 		.layer = SFC_FLOW_ITEM_L3,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_ipv4,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_IPV6,
+	[RTE_FLOW_ITEM_TYPE_IPV6] = {
 		.name = "IPV6",
 		.prev_layer = SFC_FLOW_ITEM_L2,
 		.layer = SFC_FLOW_ITEM_L3,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_ipv6,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_TCP,
+	[RTE_FLOW_ITEM_TYPE_TCP] = {
 		.name = "TCP",
 		.prev_layer = SFC_FLOW_ITEM_L3,
 		.layer = SFC_FLOW_ITEM_L4,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_tcp,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_UDP,
+	[RTE_FLOW_ITEM_TYPE_UDP] = {
 		.name = "UDP",
 		.prev_layer = SFC_FLOW_ITEM_L3,
 		.layer = SFC_FLOW_ITEM_L4,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_udp,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_VXLAN,
+	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
 		.name = "VXLAN",
 		.prev_layer = SFC_FLOW_ITEM_L4,
 		.layer = SFC_FLOW_ITEM_START_LAYER,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_tunnel,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_GENEVE,
+	[RTE_FLOW_ITEM_TYPE_GENEVE] = {
 		.name = "GENEVE",
 		.prev_layer = SFC_FLOW_ITEM_L4,
 		.layer = SFC_FLOW_ITEM_START_LAYER,
 		.ctx_type = SFC_FLOW_PARSE_CTX_MAE,
 		.parse = sfc_mae_rule_parse_item_tunnel,
 	},
-	{
-		.type = RTE_FLOW_ITEM_TYPE_NVGRE,
+	[RTE_FLOW_ITEM_TYPE_NVGRE] = {
 		.name = "NVGRE",
 		.prev_layer = SFC_FLOW_ITEM_L3,
 		.layer = SFC_FLOW_ITEM_START_LAYER,
-- 
2.17.1


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

* Re: [PATCH] net/sfc: avoid unnecessary flow pattern item traversal
  2023-02-14 13:27 [PATCH] net/sfc: avoid unnecessary flow pattern item traversal Ivan Malov
@ 2023-02-14 13:39 ` Andrew Rybchenko
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Rybchenko @ 2023-02-14 13:39 UTC (permalink / raw)
  To: Ivan Malov, dev; +Cc: Viacheslav Galaktionov, Andy Moreton

On 2/14/23 16:27, Ivan Malov wrote:
> Currently, the code tries to look up a user-provided item
> by traversing those known to the PMD. Avoid the traversal
> since it is easier to access items directly by their IDs,
> with necessary checks to avoid reading past the buffer.
> 
> Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
> Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
>   drivers/net/sfc/sfc_flow.c | 60 +++++++++++---------------------------
>   drivers/net/sfc/sfc_flow.h |  1 -
>   drivers/net/sfc/sfc_mae.c  | 39 +++++++++----------------
>   3 files changed, 30 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
> index fe1f5ba55f..9e1b746d51 100644
> --- a/drivers/net/sfc/sfc_flow.c
> +++ b/drivers/net/sfc/sfc_flow.c
> @@ -1130,96 +1130,84 @@ sfc_flow_parse_pppoex(const struct rte_flow_item *item,
>   }
>   
>   static const struct sfc_flow_item sfc_flow_items[] = {
> -	{
> -		.type = RTE_FLOW_ITEM_TYPE_VOID,
> +	[RTE_FLOW_ITEM_TYPE_VOID] = {

Doesn't it make these arrays sparse and potentially too big?
I'd like to see mitigation of the question in the changeset
description.


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

end of thread, other threads:[~2023-02-14 13:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14 13:27 [PATCH] net/sfc: avoid unnecessary flow pattern item traversal Ivan Malov
2023-02-14 13:39 ` Andrew Rybchenko

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).