* [dpdk-dev] [PATCH v2] net/mlx5: fix default mark copy flow
@ 2019-11-27 13:36 Viacheslav Ovsiienko
2019-11-27 13:45 ` Ori Kam
0 siblings, 1 reply; 3+ messages in thread
From: Viacheslav Ovsiienko @ 2019-11-27 13:36 UTC (permalink / raw)
To: dev; +Cc: matan, rasland, orika
In extensive metadata mode the MARK copy table is engaged,
if the application creates the flow with zero MARK ID action:
flow create 1 ingress pattern eth / ... / end actions mark id 0 / .. end
And then destroys that, the traffic to the port stops. This happens
due to default flow for the copy table has the zero ID and is removed
with the application rule. The patch extends internal ID variable
to 64 bits and provide the UINT64_MAX ID for the copy table default
rule.
Fixes: dd3c774f6ffb ("net/mlx5: add metadata register copy table")
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
drivers/net/mlx5/mlx5_defs.h | 1 +
drivers/net/mlx5/mlx5_flow.c | 13 +++++++------
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index 9e113c5..042e1f3 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -174,6 +174,7 @@
/* Size of the simple hash table for metadata register table. */
#define MLX5_FLOW_MREG_HTABLE_SZ 4096
#define MLX5_FLOW_MREG_HNAME "MARK_COPY_TABLE"
+#define MLX5_DEFAULT_COPY_ID UINT32_MAX
/* Definition of static_assert found in /usr/include/assert.h */
#ifndef HAVE_STATIC_ASSERT
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 5c78ea7..0087163 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2947,16 +2947,16 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
mcp_res = (void *)mlx5_hlist_lookup(priv->mreg_cp_tbl, mark_id);
if (mcp_res) {
/* For non-default rule. */
- if (mark_id)
+ if (mark_id != MLX5_DEFAULT_COPY_ID)
mcp_res->refcnt++;
- assert(mark_id || mcp_res->refcnt == 1);
+ assert(mark_id != MLX5_DEFAULT_COPY_ID || mcp_res->refcnt == 1);
return mcp_res;
}
/* Provide the full width of FLAG specific value. */
if (mark_id == (priv->sh->dv_regc0_mask & MLX5_FLOW_MARK_DEFAULT))
tag_spec.data = MLX5_FLOW_MARK_DEFAULT;
/* Build a new flow. */
- if (mark_id) {
+ if (mark_id != MLX5_DEFAULT_COPY_ID) {
items[0] = (struct rte_flow_item){
.type = MLX5_RTE_FLOW_ITEM_TYPE_TAG,
.spec = &tag_spec,
@@ -3054,7 +3054,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
}
/*
* We do not check availability of metadata registers here,
- * because copy resources are allocated in this case.
+ * because copy resources are not allocated in this case.
*/
if (--mcp_res->refcnt)
return;
@@ -3133,7 +3133,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
/* Check if default flow is registered. */
if (!priv->mreg_cp_tbl)
return;
- mcp_res = (void *)mlx5_hlist_lookup(priv->mreg_cp_tbl, 0ULL);
+ mcp_res = (void *)mlx5_hlist_lookup(priv->mreg_cp_tbl,
+ MLX5_DEFAULT_COPY_ID);
if (!mcp_res)
return;
assert(mcp_res->flow);
@@ -3166,7 +3167,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
!mlx5_flow_ext_mreg_supported(dev) ||
!priv->sh->dv_regc0_mask)
return 0;
- mcp_res = flow_mreg_add_copy_action(dev, 0, error);
+ mcp_res = flow_mreg_add_copy_action(dev, MLX5_DEFAULT_COPY_ID, error);
if (!mcp_res)
return -rte_errno;
return 0;
--
1.8.3.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix default mark copy flow
2019-11-27 13:36 [dpdk-dev] [PATCH v2] net/mlx5: fix default mark copy flow Viacheslav Ovsiienko
@ 2019-11-27 13:45 ` Ori Kam
2019-11-27 15:16 ` Thomas Monjalon
0 siblings, 1 reply; 3+ messages in thread
From: Ori Kam @ 2019-11-27 13:45 UTC (permalink / raw)
To: Slava Ovsiienko, dev; +Cc: Matan Azrad, Raslan Darawsheh
> -----Original Message-----
> From: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Sent: Wednesday, November 27, 2019 3:37 PM
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Ori Kam <orika@mellanox.com>
> Subject: [PATCH v2] net/mlx5: fix default mark copy flow
>
> In extensive metadata mode the MARK copy table is engaged,
> if the application creates the flow with zero MARK ID action:
>
> flow create 1 ingress pattern eth / ... / end actions mark id 0 / .. end
>
> And then destroys that, the traffic to the port stops. This happens
> due to default flow for the copy table has the zero ID and is removed
> with the application rule. The patch extends internal ID variable
> to 64 bits and provide the UINT64_MAX ID for the copy table default
> rule.
>
> Fixes: dd3c774f6ffb ("net/mlx5: add metadata register copy table")
>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
> drivers/net/mlx5/mlx5_defs.h | 1 +
> drivers/net/mlx5/mlx5_flow.c | 13 +++++++------
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
> index 9e113c5..042e1f3 100644
> --- a/drivers/net/mlx5/mlx5_defs.h
> +++ b/drivers/net/mlx5/mlx5_defs.h
> @@ -174,6 +174,7 @@
> /* Size of the simple hash table for metadata register table. */
> #define MLX5_FLOW_MREG_HTABLE_SZ 4096
> #define MLX5_FLOW_MREG_HNAME "MARK_COPY_TABLE"
> +#define MLX5_DEFAULT_COPY_ID UINT32_MAX
>
> /* Definition of static_assert found in /usr/include/assert.h */
> #ifndef HAVE_STATIC_ASSERT
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 5c78ea7..0087163 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -2947,16 +2947,16 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> mcp_res = (void *)mlx5_hlist_lookup(priv->mreg_cp_tbl, mark_id);
> if (mcp_res) {
> /* For non-default rule. */
> - if (mark_id)
> + if (mark_id != MLX5_DEFAULT_COPY_ID)
> mcp_res->refcnt++;
> - assert(mark_id || mcp_res->refcnt == 1);
> + assert(mark_id != MLX5_DEFAULT_COPY_ID || mcp_res-
> >refcnt == 1);
> return mcp_res;
> }
> /* Provide the full width of FLAG specific value. */
> if (mark_id == (priv->sh->dv_regc0_mask &
> MLX5_FLOW_MARK_DEFAULT))
> tag_spec.data = MLX5_FLOW_MARK_DEFAULT;
> /* Build a new flow. */
> - if (mark_id) {
> + if (mark_id != MLX5_DEFAULT_COPY_ID) {
> items[0] = (struct rte_flow_item){
> .type = MLX5_RTE_FLOW_ITEM_TYPE_TAG,
> .spec = &tag_spec,
> @@ -3054,7 +3054,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> }
> /*
> * We do not check availability of metadata registers here,
> - * because copy resources are allocated in this case.
> + * because copy resources are not allocated in this case.
> */
> if (--mcp_res->refcnt)
> return;
> @@ -3133,7 +3133,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> /* Check if default flow is registered. */
> if (!priv->mreg_cp_tbl)
> return;
> - mcp_res = (void *)mlx5_hlist_lookup(priv->mreg_cp_tbl, 0ULL);
> + mcp_res = (void *)mlx5_hlist_lookup(priv->mreg_cp_tbl,
> + MLX5_DEFAULT_COPY_ID);
> if (!mcp_res)
> return;
> assert(mcp_res->flow);
> @@ -3166,7 +3167,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> !mlx5_flow_ext_mreg_supported(dev) ||
> !priv->sh->dv_regc0_mask)
> return 0;
> - mcp_res = flow_mreg_add_copy_action(dev, 0, error);
> + mcp_res = flow_mreg_add_copy_action(dev,
> MLX5_DEFAULT_COPY_ID, error);
> if (!mcp_res)
> return -rte_errno;
> return 0;
> --
> 1.8.3.1
Acked-by: Ori Kam <orika@mellanox.com>
Best,
Ori
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix default mark copy flow
2019-11-27 13:45 ` Ori Kam
@ 2019-11-27 15:16 ` Thomas Monjalon
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Monjalon @ 2019-11-27 15:16 UTC (permalink / raw)
To: Slava Ovsiienko; +Cc: dev, Ori Kam, Matan Azrad, Raslan Darawsheh
27/11/2019 14:45, Ori Kam:
> From: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Sent: Wednesday, November 27, 2019 3:37 PM
>
> > In extensive metadata mode the MARK copy table is engaged,
> > if the application creates the flow with zero MARK ID action:
> >
> > flow create 1 ingress pattern eth / ... / end actions mark id 0 / .. end
> >
> > And then destroys that, the traffic to the port stops. This happens
> > due to default flow for the copy table has the zero ID and is removed
> > with the application rule. The patch extends internal ID variable
> > to 64 bits and provide the UINT64_MAX ID for the copy table default
> > rule.
> >
> > Fixes: dd3c774f6ffb ("net/mlx5: add metadata register copy table")
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Acked-by: Ori Kam <orika@mellanox.com>
Applied, thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-11-27 15:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 13:36 [dpdk-dev] [PATCH v2] net/mlx5: fix default mark copy flow Viacheslav Ovsiienko
2019-11-27 13:45 ` Ori Kam
2019-11-27 15:16 ` Thomas Monjalon
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).