* [dpdk-dev] [PATCH] net/mlx5: allow flow rule with attribute egress
@ 2018-09-27 14:25 Dekel Peled
2018-09-29 9:24 ` Yongseok Koh
2018-10-07 14:01 ` [dpdk-dev] [PATCH v2] " Dekel Peled
0 siblings, 2 replies; 8+ messages in thread
From: Dekel Peled @ 2018-09-27 14:25 UTC (permalink / raw)
To: dev, shahafs, yskoh; +Cc: orika
This patch complements [1], adding to MLX5 PMD the option to set
flow rule for egress traffic.
[1] "net/mlx5: support metadata as flow rule criteria"
http://mails.dpdk.org/archives/dev/2018-September/113275.html
Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
drivers/net/mlx5/mlx5_flow.c | 54 ++++++++++++++++++++++++++++++++++++++
drivers/net/mlx5/mlx5_flow.h | 6 +++++
drivers/net/mlx5/mlx5_flow_dv.c | 24 ++++++++++-------
drivers/net/mlx5/mlx5_flow_verbs.c | 7 ++++-
4 files changed, 80 insertions(+), 11 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 9581691..79a06df 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -644,6 +644,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
*
* @param[in] action_flags
* Bit-fields that holds the actions detected until now.
+ * @param[in] attr
+ * Attributes of flow that includes this action.
* @param[out] error
* Pointer to error structure.
*
@@ -652,6 +654,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
*/
int
mlx5_flow_validate_action_flag(uint64_t action_flags,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error)
{
@@ -668,6 +671,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
"can't have 2 flag"
" actions in same flow");
+ if (attr->egress)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+ NULL,
+ "flag action not supported for "
+ "egress");
return 0;
}
@@ -678,6 +687,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
* Pointer to the queue action.
* @param[in] action_flags
* Bit-fields that holds the actions detected until now.
+ * @param[in] attr
+ * Attributes of flow that includes this action.
* @param[out] error
* Pointer to error structure.
*
@@ -687,6 +698,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
int
mlx5_flow_validate_action_mark(const struct rte_flow_action *action,
uint64_t action_flags,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error)
{
const struct rte_flow_action_mark *mark = action->conf;
@@ -715,6 +727,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
"can't have 2 flag actions in same"
" flow");
+ if (attr->egress)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+ NULL,
+ "mark action not supported for "
+ "egress");
return 0;
}
@@ -723,6 +741,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
*
* @param[in] action_flags
* Bit-fields that holds the actions detected until now.
+ * @param[in] attr
+ * Attributes of flow that includes this action.
* @param[out] error
* Pointer to error structure.
*
@@ -731,6 +751,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
*/
int
mlx5_flow_validate_action_drop(uint64_t action_flags,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error)
{
if (action_flags & MLX5_ACTION_FLAG)
@@ -747,6 +768,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
"can't have 2 fate actions in"
" same flow");
+ if (attr->egress)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+ NULL,
+ "drop action not supported for "
+ "egress");
return 0;
}
@@ -759,6 +786,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
* Bit-fields that holds the actions detected until now.
* @param[in] dev
* Pointer to the Ethernet device structure.
+ * @param[in] attr
+ * Attributes of flow that includes this action.
* @param[out] error
* Pointer to error structure.
*
@@ -769,6 +798,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
mlx5_flow_validate_action_queue(const struct rte_flow_action *action,
uint64_t action_flags,
struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error)
{
struct priv *priv = dev->data->dev_private;
@@ -790,6 +820,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
RTE_FLOW_ERROR_TYPE_ACTION_CONF,
&queue->index,
"queue is not configured");
+ if (attr->egress)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+ NULL,
+ "queue action not supported for "
+ "egress");
return 0;
}
@@ -802,6 +838,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
* Bit-fields that holds the actions detected until now.
* @param[in] dev
* Pointer to the Ethernet device structure.
+ * @param[in] attr
+ * Attributes of flow that includes this action.
* @param[out] error
* Pointer to error structure.
*
@@ -812,6 +850,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
mlx5_flow_validate_action_rss(const struct rte_flow_action *action,
uint64_t action_flags,
struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error)
{
struct priv *priv = dev->data->dev_private;
@@ -866,6 +905,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION_CONF,
&rss->queue[i], "queue is not configured");
}
+ if (attr->egress)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+ NULL,
+ "rss action not supported for "
+ "egress");
return 0;
}
@@ -874,6 +919,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
*
* @param[in] dev
* Pointer to the Ethernet device structure.
+ * @param[in] attr
+ * Attributes of flow that includes this action.
* @param[out] error
* Pointer to error structure.
*
@@ -882,6 +929,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
*/
int
mlx5_flow_validate_action_count(struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error)
{
struct priv *priv = dev->data->dev_private;
@@ -890,6 +938,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
"flow counters are not supported.");
+ if (attr->egress)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+ NULL,
+ "count action not supported for "
+ "egress");
return 0;
}
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index d91ae17..a353d07 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -283,21 +283,27 @@ uint64_t mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow, int tunnel,
uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
uint32_t subpriority);
int mlx5_flow_validate_action_count(struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error);
int mlx5_flow_validate_action_drop(uint64_t action_flags,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error);
int mlx5_flow_validate_action_flag(uint64_t action_flags,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error);
int mlx5_flow_validate_action_mark(const struct rte_flow_action *action,
uint64_t action_flags,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error);
int mlx5_flow_validate_action_queue(const struct rte_flow_action *action,
uint64_t action_flags,
struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error);
int mlx5_flow_validate_action_rss(const struct rte_flow_action *action,
uint64_t action_flags,
struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
struct rte_flow_error *error);
int mlx5_flow_validate_attributes(struct rte_eth_dev *dev,
const struct rte_flow_attr *attributes,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 2439f5e..1f3fcb8 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -117,21 +117,17 @@
RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
NULL,
"priority out of range");
- if (attributes->egress)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- NULL,
- "egress is not supported");
if (attributes->transfer)
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
NULL,
"transfer is not supported");
- if (!attributes->ingress)
+ if (!(attributes->egress ^ attributes->ingress))
return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
+ RTE_FLOW_ERROR_TYPE_ATTR,
NULL,
- "ingress attribute is mandatory");
+ "must specify exactly one of "
+ "ingress or egress");
return 0;
}
@@ -288,6 +284,7 @@
break;
case RTE_FLOW_ACTION_TYPE_FLAG:
ret = mlx5_flow_validate_action_flag(action_flags,
+ attr,
error);
if (ret < 0)
return ret;
@@ -297,6 +294,7 @@
case RTE_FLOW_ACTION_TYPE_MARK:
ret = mlx5_flow_validate_action_mark(actions,
action_flags,
+ attr,
error);
if (ret < 0)
return ret;
@@ -305,6 +303,7 @@
break;
case RTE_FLOW_ACTION_TYPE_DROP:
ret = mlx5_flow_validate_action_drop(action_flags,
+ attr,
error);
if (ret < 0)
return ret;
@@ -314,6 +313,7 @@
case RTE_FLOW_ACTION_TYPE_QUEUE:
ret = mlx5_flow_validate_action_queue(actions,
action_flags, dev,
+ attr,
error);
if (ret < 0)
return ret;
@@ -323,6 +323,7 @@
case RTE_FLOW_ACTION_TYPE_RSS:
ret = mlx5_flow_validate_action_rss(actions,
action_flags, dev,
+ attr,
error);
if (ret < 0)
return ret;
@@ -330,7 +331,7 @@
++actions_n;
break;
case RTE_FLOW_ACTION_TYPE_COUNT:
- ret = mlx5_flow_validate_action_count(dev, error);
+ ret = mlx5_flow_validate_action_count(dev, attr, error);
if (ret < 0)
return ret;
action_flags |= MLX5_ACTION_COUNT;
@@ -1080,6 +1081,7 @@
queue = action->conf;
flow->rss.queue_num = 1;
(*flow->queue)[0] = queue->index;
+ flow->actions |= MLX5_ACTION_QUEUE;
break;
case RTE_FLOW_ACTION_TYPE_RSS:
rss = action->conf;
@@ -1091,6 +1093,7 @@
flow->rss.types = rss->types;
flow->rss.level = rss->level;
/* Added to array only in apply since we need the QP */
+ flow->actions |= MLX5_ACTION_RSS;
break;
default:
break;
@@ -1301,7 +1304,8 @@
dv->actions[n].type = MLX5DV_FLOW_ACTION_DEST_IBV_QP;
dv->actions[n].qp = dv->hrxq->qp;
n++;
- } else {
+ } else if (flow->actions &
+ (MLX5_ACTION_QUEUE | MLX5_ACTION_RSS)) {
struct mlx5_hrxq *hrxq;
hrxq = mlx5_hrxq_get(dev, flow->key,
MLX5_RSS_HASH_KEY_LEN,
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 05ab5fd..b1aa704 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -1112,6 +1112,7 @@
break;
case RTE_FLOW_ACTION_TYPE_FLAG:
ret = mlx5_flow_validate_action_flag(action_flags,
+ attr,
error);
if (ret < 0)
return ret;
@@ -1120,6 +1121,7 @@
case RTE_FLOW_ACTION_TYPE_MARK:
ret = mlx5_flow_validate_action_mark(actions,
action_flags,
+ attr,
error);
if (ret < 0)
return ret;
@@ -1127,6 +1129,7 @@
break;
case RTE_FLOW_ACTION_TYPE_DROP:
ret = mlx5_flow_validate_action_drop(action_flags,
+ attr,
error);
if (ret < 0)
return ret;
@@ -1135,6 +1138,7 @@
case RTE_FLOW_ACTION_TYPE_QUEUE:
ret = mlx5_flow_validate_action_queue(actions,
action_flags, dev,
+ attr,
error);
if (ret < 0)
return ret;
@@ -1143,13 +1147,14 @@
case RTE_FLOW_ACTION_TYPE_RSS:
ret = mlx5_flow_validate_action_rss(actions,
action_flags, dev,
+ attr,
error);
if (ret < 0)
return ret;
action_flags |= MLX5_ACTION_RSS;
break;
case RTE_FLOW_ACTION_TYPE_COUNT:
- ret = mlx5_flow_validate_action_count(dev, error);
+ ret = mlx5_flow_validate_action_count(dev, attr, error);
if (ret < 0)
return ret;
action_flags |= MLX5_ACTION_COUNT;
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] net/mlx5: allow flow rule with attribute egress 2018-09-27 14:25 [dpdk-dev] [PATCH] net/mlx5: allow flow rule with attribute egress Dekel Peled @ 2018-09-29 9:24 ` Yongseok Koh 2018-10-03 6:06 ` Dekel Peled 2018-10-07 14:01 ` [dpdk-dev] [PATCH v2] " Dekel Peled 1 sibling, 1 reply; 8+ messages in thread From: Yongseok Koh @ 2018-09-29 9:24 UTC (permalink / raw) To: Dekel Peled; +Cc: dev, Shahaf Shuler, Ori Kam On Thu, Sep 27, 2018 at 05:25:30PM +0300, Dekel Peled wrote: > This patch complements [1], adding to MLX5 PMD the option to set > flow rule for egress traffic. > > [1] "net/mlx5: support metadata as flow rule criteria" > http://mails.dpdk.org/archives/dev/2018-September/113275.html > > Signed-off-by: Dekel Peled <dekelp@mellanox.com> > --- > drivers/net/mlx5/mlx5_flow.c | 54 ++++++++++++++++++++++++++++++++++++++ > drivers/net/mlx5/mlx5_flow.h | 6 +++++ > drivers/net/mlx5/mlx5_flow_dv.c | 24 ++++++++++------- > drivers/net/mlx5/mlx5_flow_verbs.c | 7 ++++- > 4 files changed, 80 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c > index 9581691..79a06df 100644 > --- a/drivers/net/mlx5/mlx5_flow.c > +++ b/drivers/net/mlx5/mlx5_flow.c > @@ -644,6 +644,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * > * @param[in] action_flags > * Bit-fields that holds the actions detected until now. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -652,6 +654,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > */ > int > mlx5_flow_validate_action_flag(uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > > @@ -668,6 +671,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > RTE_FLOW_ERROR_TYPE_ACTION, NULL, > "can't have 2 flag" > " actions in same flow"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > + NULL, These two lines could be one line. > + "flag action not supported for " > + "egress"); > return 0; > } > > @@ -678,6 +687,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * Pointer to the queue action. > * @param[in] action_flags > * Bit-fields that holds the actions detected until now. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -687,6 +698,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > int > mlx5_flow_validate_action_mark(const struct rte_flow_action *action, > uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > const struct rte_flow_action_mark *mark = action->conf; > @@ -715,6 +727,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > RTE_FLOW_ERROR_TYPE_ACTION, NULL, > "can't have 2 flag actions in same" > " flow"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > + NULL, Ditto. There are a few more occurrences below. > + "mark action not supported for " > + "egress"); > return 0; > } > > @@ -723,6 +741,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * > * @param[in] action_flags > * Bit-fields that holds the actions detected until now. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -731,6 +751,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > */ > int > mlx5_flow_validate_action_drop(uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > if (action_flags & MLX5_ACTION_FLAG) > @@ -747,6 +768,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > RTE_FLOW_ERROR_TYPE_ACTION, NULL, > "can't have 2 fate actions in" > " same flow"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > + NULL, > + "drop action not supported for " > + "egress"); > return 0; > } > > @@ -759,6 +786,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * Bit-fields that holds the actions detected until now. > * @param[in] dev > * Pointer to the Ethernet device structure. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -769,6 +798,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > mlx5_flow_validate_action_queue(const struct rte_flow_action *action, > uint64_t action_flags, > struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > struct priv *priv = dev->data->dev_private; > @@ -790,6 +820,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > RTE_FLOW_ERROR_TYPE_ACTION_CONF, > &queue->index, > "queue is not configured"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > + NULL, > + "queue action not supported for " > + "egress"); > return 0; > } > > @@ -802,6 +838,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * Bit-fields that holds the actions detected until now. > * @param[in] dev > * Pointer to the Ethernet device structure. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -812,6 +850,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > mlx5_flow_validate_action_rss(const struct rte_flow_action *action, > uint64_t action_flags, > struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > struct priv *priv = dev->data->dev_private; > @@ -866,6 +905,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > (error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION_CONF, > &rss->queue[i], "queue is not configured"); > } > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > + NULL, > + "rss action not supported for " > + "egress"); > return 0; > } > > @@ -874,6 +919,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * > * @param[in] dev > * Pointer to the Ethernet device structure. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -882,6 +929,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > */ > int > mlx5_flow_validate_action_count(struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > struct priv *priv = dev->data->dev_private; > @@ -890,6 +938,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > return rte_flow_error_set(error, ENOTSUP, > RTE_FLOW_ERROR_TYPE_ACTION, NULL, > "flow counters are not supported."); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > + NULL, > + "count action not supported for " > + "egress"); > return 0; > } > > diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h > index d91ae17..a353d07 100644 > --- a/drivers/net/mlx5/mlx5_flow.h > +++ b/drivers/net/mlx5/mlx5_flow.h > @@ -283,21 +283,27 @@ uint64_t mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow, int tunnel, > uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > uint32_t subpriority); > int mlx5_flow_validate_action_count(struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_drop(uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_flag(uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_mark(const struct rte_flow_action *action, > uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_queue(const struct rte_flow_action *action, > uint64_t action_flags, > struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_rss(const struct rte_flow_action *action, > uint64_t action_flags, > struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_attributes(struct rte_eth_dev *dev, > const struct rte_flow_attr *attributes, > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c > index 2439f5e..1f3fcb8 100644 > --- a/drivers/net/mlx5/mlx5_flow_dv.c > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > @@ -117,21 +117,17 @@ > RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, > NULL, > "priority out of range"); > - if (attributes->egress) > - return rte_flow_error_set(error, ENOTSUP, > - RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > - NULL, > - "egress is not supported"); > if (attributes->transfer) > return rte_flow_error_set(error, ENOTSUP, > RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER, > NULL, > "transfer is not supported"); > - if (!attributes->ingress) > + if (!(attributes->egress ^ attributes->ingress)) > return rte_flow_error_set(error, ENOTSUP, > - RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > + RTE_FLOW_ERROR_TYPE_ATTR, > NULL, > - "ingress attribute is mandatory"); > + "must specify exactly one of " > + "ingress or egress"); > return 0; > } > > @@ -288,6 +284,7 @@ > break; > case RTE_FLOW_ACTION_TYPE_FLAG: > ret = mlx5_flow_validate_action_flag(action_flags, > + attr, > error); Merge into one line as much as you can. Same comment for the rest. > if (ret < 0) > return ret; > @@ -297,6 +294,7 @@ > case RTE_FLOW_ACTION_TYPE_MARK: > ret = mlx5_flow_validate_action_mark(actions, > action_flags, > + attr, > error); > if (ret < 0) > return ret; > @@ -305,6 +303,7 @@ > break; > case RTE_FLOW_ACTION_TYPE_DROP: > ret = mlx5_flow_validate_action_drop(action_flags, > + attr, > error); > if (ret < 0) > return ret; > @@ -314,6 +313,7 @@ > case RTE_FLOW_ACTION_TYPE_QUEUE: > ret = mlx5_flow_validate_action_queue(actions, > action_flags, dev, > + attr, > error); > if (ret < 0) > return ret; > @@ -323,6 +323,7 @@ > case RTE_FLOW_ACTION_TYPE_RSS: > ret = mlx5_flow_validate_action_rss(actions, > action_flags, dev, > + attr, > error); > if (ret < 0) > return ret; > @@ -330,7 +331,7 @@ > ++actions_n; > break; > case RTE_FLOW_ACTION_TYPE_COUNT: > - ret = mlx5_flow_validate_action_count(dev, error); > + ret = mlx5_flow_validate_action_count(dev, attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_ACTION_COUNT; > @@ -1080,6 +1081,7 @@ > queue = action->conf; > flow->rss.queue_num = 1; > (*flow->queue)[0] = queue->index; > + flow->actions |= MLX5_ACTION_QUEUE; > break; > case RTE_FLOW_ACTION_TYPE_RSS: > rss = action->conf; > @@ -1091,6 +1093,7 @@ > flow->rss.types = rss->types; > flow->rss.level = rss->level; > /* Added to array only in apply since we need the QP */ > + flow->actions |= MLX5_ACTION_RSS; Let's do the same for the rest of actions. > break; > default: > break; > @@ -1301,7 +1304,8 @@ > dv->actions[n].type = MLX5DV_FLOW_ACTION_DEST_IBV_QP; > dv->actions[n].qp = dv->hrxq->qp; > n++; > - } else { > + } else if (flow->actions & > + (MLX5_ACTION_QUEUE | MLX5_ACTION_RSS)) { The previous code is not wrong but definitely error-prone. Can you please make the same change for the verbs code? You can create a separate patch for this fix. > struct mlx5_hrxq *hrxq; > hrxq = mlx5_hrxq_get(dev, flow->key, > MLX5_RSS_HASH_KEY_LEN, > diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c > index 05ab5fd..b1aa704 100644 > --- a/drivers/net/mlx5/mlx5_flow_verbs.c > +++ b/drivers/net/mlx5/mlx5_flow_verbs.c > @@ -1112,6 +1112,7 @@ > break; > case RTE_FLOW_ACTION_TYPE_FLAG: > ret = mlx5_flow_validate_action_flag(action_flags, > + attr, > error); > if (ret < 0) > return ret; > @@ -1120,6 +1121,7 @@ > case RTE_FLOW_ACTION_TYPE_MARK: > ret = mlx5_flow_validate_action_mark(actions, > action_flags, > + attr, > error); > if (ret < 0) > return ret; > @@ -1127,6 +1129,7 @@ > break; > case RTE_FLOW_ACTION_TYPE_DROP: > ret = mlx5_flow_validate_action_drop(action_flags, > + attr, > error); > if (ret < 0) > return ret; > @@ -1135,6 +1138,7 @@ > case RTE_FLOW_ACTION_TYPE_QUEUE: > ret = mlx5_flow_validate_action_queue(actions, > action_flags, dev, > + attr, > error); > if (ret < 0) > return ret; > @@ -1143,13 +1147,14 @@ > case RTE_FLOW_ACTION_TYPE_RSS: > ret = mlx5_flow_validate_action_rss(actions, > action_flags, dev, > + attr, > error); > if (ret < 0) > return ret; > action_flags |= MLX5_ACTION_RSS; > break; > case RTE_FLOW_ACTION_TYPE_COUNT: > - ret = mlx5_flow_validate_action_count(dev, error); > + ret = mlx5_flow_validate_action_count(dev, attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_ACTION_COUNT; > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx5: allow flow rule with attribute egress 2018-09-29 9:24 ` Yongseok Koh @ 2018-10-03 6:06 ` Dekel Peled 2018-10-03 7:28 ` Yongseok Koh 0 siblings, 1 reply; 8+ messages in thread From: Dekel Peled @ 2018-10-03 6:06 UTC (permalink / raw) To: Yongseok Koh; +Cc: dev, Shahaf Shuler, Ori Kam Thanks, PSB. > -----Original Message----- > From: Yongseok Koh > Sent: Saturday, September 29, 2018 12:24 PM > To: Dekel Peled <dekelp@mellanox.com> > Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>; Ori Kam > <orika@mellanox.com> > Subject: Re: [PATCH] net/mlx5: allow flow rule with attribute egress > > On Thu, Sep 27, 2018 at 05:25:30PM +0300, Dekel Peled wrote: > > This patch complements [1], adding to MLX5 PMD the option to set flow > > rule for egress traffic. > > > > [1] "net/mlx5: support metadata as flow rule criteria" > > http://mails.dpdk.org/archives/dev/2018-September/113275.html > > > > Signed-off-by: Dekel Peled <dekelp@mellanox.com> > > --- > > drivers/net/mlx5/mlx5_flow.c | 54 > ++++++++++++++++++++++++++++++++++++++ > > drivers/net/mlx5/mlx5_flow.h | 6 +++++ > > drivers/net/mlx5/mlx5_flow_dv.c | 24 ++++++++++------- > > drivers/net/mlx5/mlx5_flow_verbs.c | 7 ++++- > > 4 files changed, 80 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5_flow.c > > b/drivers/net/mlx5/mlx5_flow.c index 9581691..79a06df 100644 > > --- a/drivers/net/mlx5/mlx5_flow.c > > +++ b/drivers/net/mlx5/mlx5_flow.c > > @@ -644,6 +644,8 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > * > > * @param[in] action_flags > > * Bit-fields that holds the actions detected until now. > > + * @param[in] attr > > + * Attributes of flow that includes this action. > > * @param[out] error > > * Pointer to error structure. > > * > > @@ -652,6 +654,7 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > */ > > int > > mlx5_flow_validate_action_flag(uint64_t action_flags, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error) { > > > > @@ -668,6 +671,12 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > RTE_FLOW_ERROR_TYPE_ACTION, > NULL, > > "can't have 2 flag" > > " actions in same flow"); > > + if (attr->egress) > > + return rte_flow_error_set(error, ENOTSUP, > > + > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > + NULL, > > These two lines could be one line. Can't do, It exceeds the 80 characters limit. > > > + "flag action not supported for " > > + "egress"); > > return 0; > > } > > > > @@ -678,6 +687,8 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > * Pointer to the queue action. > > * @param[in] action_flags > > * Bit-fields that holds the actions detected until now. > > + * @param[in] attr > > + * Attributes of flow that includes this action. > > * @param[out] error > > * Pointer to error structure. > > * > > @@ -687,6 +698,7 @@ uint32_t mlx5_flow_adjust_priority(struct > > rte_eth_dev *dev, int32_t priority, int > > mlx5_flow_validate_action_mark(const struct rte_flow_action *action, > > uint64_t action_flags, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error) { > > const struct rte_flow_action_mark *mark = action->conf; @@ -715,6 > > +727,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, > int32_t priority, > > RTE_FLOW_ERROR_TYPE_ACTION, > NULL, > > "can't have 2 flag actions in same" > > " flow"); > > + if (attr->egress) > > + return rte_flow_error_set(error, ENOTSUP, > > + > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > + NULL, > > Ditto. There are a few more occurrences below. Can't do, It exceeds the 80 characters limit. > > > + "mark action not supported for " > > + "egress"); > > return 0; > > } > > > > @@ -723,6 +741,8 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > * > > * @param[in] action_flags > > * Bit-fields that holds the actions detected until now. > > + * @param[in] attr > > + * Attributes of flow that includes this action. > > * @param[out] error > > * Pointer to error structure. > > * > > @@ -731,6 +751,7 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > */ > > int > > mlx5_flow_validate_action_drop(uint64_t action_flags, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error) { > > if (action_flags & MLX5_ACTION_FLAG) @@ -747,6 +768,12 @@ > uint32_t > > mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > > RTE_FLOW_ERROR_TYPE_ACTION, > NULL, > > "can't have 2 fate actions in" > > " same flow"); > > + if (attr->egress) > > + return rte_flow_error_set(error, ENOTSUP, > > + > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > + NULL, > > + "drop action not supported for " > > + "egress"); > > return 0; > > } > > > > @@ -759,6 +786,8 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > * Bit-fields that holds the actions detected until now. > > * @param[in] dev > > * Pointer to the Ethernet device structure. > > + * @param[in] attr > > + * Attributes of flow that includes this action. > > * @param[out] error > > * Pointer to error structure. > > * > > @@ -769,6 +798,7 @@ uint32_t mlx5_flow_adjust_priority(struct > > rte_eth_dev *dev, int32_t priority, > mlx5_flow_validate_action_queue(const struct rte_flow_action *action, > > uint64_t action_flags, > > struct rte_eth_dev *dev, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error) > > { > > struct priv *priv = dev->data->dev_private; @@ -790,6 +820,12 @@ > > uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t > priority, > > > RTE_FLOW_ERROR_TYPE_ACTION_CONF, > > &queue->index, > > "queue is not configured"); > > + if (attr->egress) > > + return rte_flow_error_set(error, ENOTSUP, > > + > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > + NULL, > > + "queue action not supported for " > > + "egress"); > > return 0; > > } > > > > @@ -802,6 +838,8 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > * Bit-fields that holds the actions detected until now. > > * @param[in] dev > > * Pointer to the Ethernet device structure. > > + * @param[in] attr > > + * Attributes of flow that includes this action. > > * @param[out] error > > * Pointer to error structure. > > * > > @@ -812,6 +850,7 @@ uint32_t mlx5_flow_adjust_priority(struct > > rte_eth_dev *dev, int32_t priority, mlx5_flow_validate_action_rss(const > struct rte_flow_action *action, > > uint64_t action_flags, > > struct rte_eth_dev *dev, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error) { > > struct priv *priv = dev->data->dev_private; @@ -866,6 +905,12 @@ > > uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t > priority, > > (error, EINVAL, > RTE_FLOW_ERROR_TYPE_ACTION_CONF, > > &rss->queue[i], "queue is not configured"); > > } > > + if (attr->egress) > > + return rte_flow_error_set(error, ENOTSUP, > > + > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > + NULL, > > + "rss action not supported for " > > + "egress"); > > return 0; > > } > > > > @@ -874,6 +919,8 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > * > > * @param[in] dev > > * Pointer to the Ethernet device structure. > > + * @param[in] attr > > + * Attributes of flow that includes this action. > > * @param[out] error > > * Pointer to error structure. > > * > > @@ -882,6 +929,7 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > */ > > int > > mlx5_flow_validate_action_count(struct rte_eth_dev *dev, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error) > > { > > struct priv *priv = dev->data->dev_private; @@ -890,6 +938,12 @@ > > uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t > priority, > > return rte_flow_error_set(error, ENOTSUP, > > RTE_FLOW_ERROR_TYPE_ACTION, > NULL, > > "flow counters are not supported."); > > + if (attr->egress) > > + return rte_flow_error_set(error, ENOTSUP, > > + > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > + NULL, > > + "count action not supported for " > > + "egress"); > > return 0; > > } > > > > diff --git a/drivers/net/mlx5/mlx5_flow.h > > b/drivers/net/mlx5/mlx5_flow.h index d91ae17..a353d07 100644 > > --- a/drivers/net/mlx5/mlx5_flow.h > > +++ b/drivers/net/mlx5/mlx5_flow.h > > @@ -283,21 +283,27 @@ uint64_t mlx5_flow_hashfields_adjust(struct > > mlx5_flow *dev_flow, int tunnel, uint32_t > mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > > uint32_t subpriority); > > int mlx5_flow_validate_action_count(struct rte_eth_dev *dev, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error); int > > mlx5_flow_validate_action_drop(uint64_t action_flags, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error); int > > mlx5_flow_validate_action_flag(uint64_t action_flags, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error); int > > mlx5_flow_validate_action_mark(const struct rte_flow_action *action, > > uint64_t action_flags, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error); int > > mlx5_flow_validate_action_queue(const struct rte_flow_action *action, > > uint64_t action_flags, > > struct rte_eth_dev *dev, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error); int > > mlx5_flow_validate_action_rss(const struct rte_flow_action *action, > > uint64_t action_flags, > > struct rte_eth_dev *dev, > > + const struct rte_flow_attr *attr, > > struct rte_flow_error *error); > > int mlx5_flow_validate_attributes(struct rte_eth_dev *dev, > > const struct rte_flow_attr *attributes, diff -- > git > > a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c > > index 2439f5e..1f3fcb8 100644 > > --- a/drivers/net/mlx5/mlx5_flow_dv.c > > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > > @@ -117,21 +117,17 @@ > > > RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, > > NULL, > > "priority out of range"); > > - if (attributes->egress) > > - return rte_flow_error_set(error, ENOTSUP, > > - > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > - NULL, > > - "egress is not supported"); > > if (attributes->transfer) > > return rte_flow_error_set(error, ENOTSUP, > > > RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER, > > NULL, > > "transfer is not supported"); > > - if (!attributes->ingress) > > + if (!(attributes->egress ^ attributes->ingress)) > > return rte_flow_error_set(error, ENOTSUP, > > - > RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > > + RTE_FLOW_ERROR_TYPE_ATTR, > > NULL, > > - "ingress attribute is mandatory"); > > + "must specify exactly one of " > > + "ingress or egress"); > > return 0; > > } > > > > @@ -288,6 +284,7 @@ > > break; > > case RTE_FLOW_ACTION_TYPE_FLAG: > > ret = mlx5_flow_validate_action_flag(action_flags, > > + attr, > > error); > > Merge into one line as much as you can. Same comment for the rest. Done. > > > if (ret < 0) > > return ret; > > @@ -297,6 +294,7 @@ > > case RTE_FLOW_ACTION_TYPE_MARK: > > ret = mlx5_flow_validate_action_mark(actions, > > action_flags, > > + attr, > > error); > > if (ret < 0) > > return ret; > > @@ -305,6 +303,7 @@ > > break; > > case RTE_FLOW_ACTION_TYPE_DROP: > > ret = mlx5_flow_validate_action_drop(action_flags, > > + attr, > > error); > > if (ret < 0) > > return ret; > > @@ -314,6 +313,7 @@ > > case RTE_FLOW_ACTION_TYPE_QUEUE: > > ret = mlx5_flow_validate_action_queue(actions, > > action_flags, dev, > > + attr, > > error); > > if (ret < 0) > > return ret; > > @@ -323,6 +323,7 @@ > > case RTE_FLOW_ACTION_TYPE_RSS: > > ret = mlx5_flow_validate_action_rss(actions, > > action_flags, dev, > > + attr, > > error); > > if (ret < 0) > > return ret; > > @@ -330,7 +331,7 @@ > > ++actions_n; > > break; > > case RTE_FLOW_ACTION_TYPE_COUNT: > > - ret = mlx5_flow_validate_action_count(dev, error); > > + ret = mlx5_flow_validate_action_count(dev, attr, > error); > > if (ret < 0) > > return ret; > > action_flags |= MLX5_ACTION_COUNT; @@ -1080,6 > +1081,7 @@ > > queue = action->conf; > > flow->rss.queue_num = 1; > > (*flow->queue)[0] = queue->index; > > + flow->actions |= MLX5_ACTION_QUEUE; > > break; > > case RTE_FLOW_ACTION_TYPE_RSS: > > rss = action->conf; > > @@ -1091,6 +1093,7 @@ > > flow->rss.types = rss->types; > > flow->rss.level = rss->level; > > /* Added to array only in apply since we need the QP */ > > + flow->actions |= MLX5_ACTION_RSS; > > Let's do the same for the rest of actions. Done. > > > break; > > default: > > break; > > @@ -1301,7 +1304,8 @@ > > dv->actions[n].type = > MLX5DV_FLOW_ACTION_DEST_IBV_QP; > > dv->actions[n].qp = dv->hrxq->qp; > > n++; > > - } else { > > + } else if (flow->actions & > > + (MLX5_ACTION_QUEUE | MLX5_ACTION_RSS)) { > > The previous code is not wrong but definitely error-prone. Can you please > make the same change for the verbs code? You can create a separate patch > for this fix. I will do it. > > > struct mlx5_hrxq *hrxq; > > hrxq = mlx5_hrxq_get(dev, flow->key, > > MLX5_RSS_HASH_KEY_LEN, > > diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c > > b/drivers/net/mlx5/mlx5_flow_verbs.c > > index 05ab5fd..b1aa704 100644 > > --- a/drivers/net/mlx5/mlx5_flow_verbs.c > > +++ b/drivers/net/mlx5/mlx5_flow_verbs.c > > @@ -1112,6 +1112,7 @@ > > break; > > case RTE_FLOW_ACTION_TYPE_FLAG: > > ret = mlx5_flow_validate_action_flag(action_flags, > > + attr, > > error); > > if (ret < 0) > > return ret; > > @@ -1120,6 +1121,7 @@ > > case RTE_FLOW_ACTION_TYPE_MARK: > > ret = mlx5_flow_validate_action_mark(actions, > > action_flags, > > + attr, > > error); > > if (ret < 0) > > return ret; > > @@ -1127,6 +1129,7 @@ > > break; > > case RTE_FLOW_ACTION_TYPE_DROP: > > ret = mlx5_flow_validate_action_drop(action_flags, > > + attr, > > error); > > if (ret < 0) > > return ret; > > @@ -1135,6 +1138,7 @@ > > case RTE_FLOW_ACTION_TYPE_QUEUE: > > ret = mlx5_flow_validate_action_queue(actions, > > action_flags, dev, > > + attr, > > error); > > if (ret < 0) > > return ret; > > @@ -1143,13 +1147,14 @@ > > case RTE_FLOW_ACTION_TYPE_RSS: > > ret = mlx5_flow_validate_action_rss(actions, > > action_flags, dev, > > + attr, > > error); > > if (ret < 0) > > return ret; > > action_flags |= MLX5_ACTION_RSS; > > break; > > case RTE_FLOW_ACTION_TYPE_COUNT: > > - ret = mlx5_flow_validate_action_count(dev, error); > > + ret = mlx5_flow_validate_action_count(dev, attr, > error); > > if (ret < 0) > > return ret; > > action_flags |= MLX5_ACTION_COUNT; > > -- > > 1.8.3.1 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx5: allow flow rule with attribute egress 2018-10-03 6:06 ` Dekel Peled @ 2018-10-03 7:28 ` Yongseok Koh 2018-10-03 13:02 ` Dekel Peled 0 siblings, 1 reply; 8+ messages in thread From: Yongseok Koh @ 2018-10-03 7:28 UTC (permalink / raw) To: Dekel Peled; +Cc: dev, Shahaf Shuler, Ori Kam On Tue, Oct 02, 2018 at 11:06:07PM -0700, Dekel Peled wrote: > Thanks, PSB. > > > -----Original Message----- > > From: Yongseok Koh > > Sent: Saturday, September 29, 2018 12:24 PM > > To: Dekel Peled <dekelp@mellanox.com> > > Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>; Ori Kam > > <orika@mellanox.com> > > Subject: Re: [PATCH] net/mlx5: allow flow rule with attribute egress > > > > On Thu, Sep 27, 2018 at 05:25:30PM +0300, Dekel Peled wrote: > > > This patch complements [1], adding to MLX5 PMD the option to set flow > > > rule for egress traffic. > > > > > > [1] "net/mlx5: support metadata as flow rule criteria" > > > http://mails.dpdk.org/archives/dev/2018-September/113275.html > > > > > > Signed-off-by: Dekel Peled <dekelp@mellanox.com> > > > --- > > > drivers/net/mlx5/mlx5_flow.c | 54 > > ++++++++++++++++++++++++++++++++++++++ > > > drivers/net/mlx5/mlx5_flow.h | 6 +++++ > > > drivers/net/mlx5/mlx5_flow_dv.c | 24 ++++++++++------- > > > drivers/net/mlx5/mlx5_flow_verbs.c | 7 ++++- > > > 4 files changed, 80 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/net/mlx5/mlx5_flow.c > > > b/drivers/net/mlx5/mlx5_flow.c index 9581691..79a06df 100644 > > > --- a/drivers/net/mlx5/mlx5_flow.c > > > +++ b/drivers/net/mlx5/mlx5_flow.c > > > @@ -644,6 +644,8 @@ uint32_t mlx5_flow_adjust_priority(struct > > rte_eth_dev *dev, int32_t priority, > > > * > > > * @param[in] action_flags > > > * Bit-fields that holds the actions detected until now. > > > + * @param[in] attr > > > + * Attributes of flow that includes this action. > > > * @param[out] error > > > * Pointer to error structure. > > > * > > > @@ -652,6 +654,7 @@ uint32_t mlx5_flow_adjust_priority(struct > > rte_eth_dev *dev, int32_t priority, > > > */ > > > int > > > mlx5_flow_validate_action_flag(uint64_t action_flags, > > > + const struct rte_flow_attr *attr, > > > struct rte_flow_error *error) { > > > > > > @@ -668,6 +671,12 @@ uint32_t mlx5_flow_adjust_priority(struct > > rte_eth_dev *dev, int32_t priority, > > > RTE_FLOW_ERROR_TYPE_ACTION, > > NULL, > > > "can't have 2 flag" > > > " actions in same flow"); > > > + if (attr->egress) > > > + return rte_flow_error_set(error, ENOTSUP, > > > + > > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > > + NULL, > > > > These two lines could be one line. > Can't do, It exceeds the 80 characters limit. if (attr->egress) return rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, 01234567890123456789012345678901234567890123456789012345678901234567890123456789 "flag action not supported for " "egress"); Exactly 80 :-) Thanks, Yongseok ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx5: allow flow rule with attribute egress 2018-10-03 7:28 ` Yongseok Koh @ 2018-10-03 13:02 ` Dekel Peled 0 siblings, 0 replies; 8+ messages in thread From: Dekel Peled @ 2018-10-03 13:02 UTC (permalink / raw) To: Yongseok Koh; +Cc: dev, Shahaf Shuler, Ori Kam Thanks, PSB. > -----Original Message----- > From: Yongseok Koh > Sent: Wednesday, October 3, 2018 10:28 AM > To: Dekel Peled <dekelp@mellanox.com> > Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>; Ori Kam > <orika@mellanox.com> > Subject: Re: [PATCH] net/mlx5: allow flow rule with attribute egress > > On Tue, Oct 02, 2018 at 11:06:07PM -0700, Dekel Peled wrote: > > Thanks, PSB. > > > > > -----Original Message----- > > > From: Yongseok Koh > > > Sent: Saturday, September 29, 2018 12:24 PM > > > To: Dekel Peled <dekelp@mellanox.com> > > > Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>; Ori Kam > > > <orika@mellanox.com> > > > Subject: Re: [PATCH] net/mlx5: allow flow rule with attribute egress > > > > > > On Thu, Sep 27, 2018 at 05:25:30PM +0300, Dekel Peled wrote: > > > > This patch complements [1], adding to MLX5 PMD the option to set > > > > flow rule for egress traffic. > > > > > > > > [1] "net/mlx5: support metadata as flow rule criteria" > > > > http://mails.dpdk.org/archives/dev/2018-September/113275.html > > > > > > > > Signed-off-by: Dekel Peled <dekelp@mellanox.com> > > > > --- > > > > drivers/net/mlx5/mlx5_flow.c | 54 > > > ++++++++++++++++++++++++++++++++++++++ > > > > drivers/net/mlx5/mlx5_flow.h | 6 +++++ > > > > drivers/net/mlx5/mlx5_flow_dv.c | 24 ++++++++++------- > > > > drivers/net/mlx5/mlx5_flow_verbs.c | 7 ++++- > > > > 4 files changed, 80 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/drivers/net/mlx5/mlx5_flow.c > > > > b/drivers/net/mlx5/mlx5_flow.c index 9581691..79a06df 100644 > > > > --- a/drivers/net/mlx5/mlx5_flow.c > > > > +++ b/drivers/net/mlx5/mlx5_flow.c > > > > @@ -644,6 +644,8 @@ uint32_t mlx5_flow_adjust_priority(struct > > > rte_eth_dev *dev, int32_t priority, > > > > * > > > > * @param[in] action_flags > > > > * Bit-fields that holds the actions detected until now. > > > > + * @param[in] attr > > > > + * Attributes of flow that includes this action. > > > > * @param[out] error > > > > * Pointer to error structure. > > > > * > > > > @@ -652,6 +654,7 @@ uint32_t mlx5_flow_adjust_priority(struct > > > rte_eth_dev *dev, int32_t priority, > > > > */ > > > > int > > > > mlx5_flow_validate_action_flag(uint64_t action_flags, > > > > + const struct rte_flow_attr *attr, > > > > struct rte_flow_error *error) { > > > > > > > > @@ -668,6 +671,12 @@ uint32_t mlx5_flow_adjust_priority(struct > > > rte_eth_dev *dev, int32_t priority, > > > > RTE_FLOW_ERROR_TYPE_ACTION, > > > NULL, > > > > "can't have 2 flag" > > > > " actions in same flow"); > > > > + if (attr->egress) > > > > + return rte_flow_error_set(error, ENOTSUP, > > > > + > > > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > > > > + NULL, > > > > > > These two lines could be one line. > > Can't do, It exceeds the 80 characters limit. > > if (attr->egress) > return rte_flow_error_set(error, ENOTSUP, > > RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, > 0123456789012345678901234567890123456789012345678901234567890123456 > 7890123456789 > "flag action not supported for " > "egress"); > > Exactly 80 :-) Done. > > Thanks, > Yongseok ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH v2] net/mlx5: allow flow rule with attribute egress 2018-09-27 14:25 [dpdk-dev] [PATCH] net/mlx5: allow flow rule with attribute egress Dekel Peled 2018-09-29 9:24 ` Yongseok Koh @ 2018-10-07 14:01 ` Dekel Peled 2018-10-08 18:55 ` Yongseok Koh 1 sibling, 1 reply; 8+ messages in thread From: Dekel Peled @ 2018-10-07 14:01 UTC (permalink / raw) To: yskoh, shahafs; +Cc: dev, orika This patch complements [1], adding to MLX5 PMD the option to set flow rule for egress traffic. [1] "net/mlx5: support metadata as flow rule criteria" http://mails.dpdk.org/archives/dev/2018-September/113275.html Signed-off-by: Dekel Peled <dekelp@mellanox.com> --- V2: * Rebase on tip. * Apply code review comments. --- drivers/net/mlx5/mlx5_flow.c | 48 ++++++++++++++++++++++++++++++++++++++ drivers/net/mlx5/mlx5_flow.h | 6 +++++ drivers/net/mlx5/mlx5_flow_dv.c | 36 ++++++++++++++-------------- drivers/net/mlx5/mlx5_flow_verbs.c | 7 +++++- 4 files changed, 78 insertions(+), 19 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 6e4a651..1087f67 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -645,6 +645,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, * * @param[in] action_flags * Bit-fields that holds the actions detected until now. + * @param[in] attr + * Attributes of flow that includes this action. * @param[out] error * Pointer to error structure. * @@ -653,6 +655,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, */ int mlx5_flow_validate_action_flag(uint64_t action_flags, + const struct rte_flow_attr *attr, struct rte_flow_error *error) { @@ -669,6 +672,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, RTE_FLOW_ERROR_TYPE_ACTION, NULL, "can't have 2 flag" " actions in same flow"); + if (attr->egress) + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, + "flag action not supported for " + "egress"); return 0; } @@ -679,6 +687,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, * Pointer to the queue action. * @param[in] action_flags * Bit-fields that holds the actions detected until now. + * @param[in] attr + * Attributes of flow that includes this action. * @param[out] error * Pointer to error structure. * @@ -688,6 +698,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, int mlx5_flow_validate_action_mark(const struct rte_flow_action *action, uint64_t action_flags, + const struct rte_flow_attr *attr, struct rte_flow_error *error) { const struct rte_flow_action_mark *mark = action->conf; @@ -716,6 +727,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, RTE_FLOW_ERROR_TYPE_ACTION, NULL, "can't have 2 mark actions in same" " flow"); + if (attr->egress) + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, + "mark action not supported for " + "egress"); return 0; } @@ -724,6 +740,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, * * @param[in] action_flags * Bit-fields that holds the actions detected until now. + * @param[in] attr + * Attributes of flow that includes this action. * @param[out] error * Pointer to error structure. * @@ -732,6 +750,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, */ int mlx5_flow_validate_action_drop(uint64_t action_flags, + const struct rte_flow_attr *attr, struct rte_flow_error *error) { if (action_flags & MLX5_FLOW_ACTION_FLAG) @@ -747,6 +766,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, RTE_FLOW_ERROR_TYPE_ACTION, NULL, "can't have 2 fate actions in" " same flow"); + if (attr->egress) + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, + "drop action not supported for " + "egress"); return 0; } @@ -759,6 +783,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, * Bit-fields that holds the actions detected until now. * @param[in] dev * Pointer to the Ethernet device structure. + * @param[in] attr + * Attributes of flow that includes this action. * @param[out] error * Pointer to error structure. * @@ -769,6 +795,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, mlx5_flow_validate_action_queue(const struct rte_flow_action *action, uint64_t action_flags, struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, struct rte_flow_error *error) { struct priv *priv = dev->data->dev_private; @@ -789,6 +816,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, RTE_FLOW_ERROR_TYPE_ACTION_CONF, &queue->index, "queue is not configured"); + if (attr->egress) + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, + "queue action not supported for " + "egress"); return 0; } @@ -801,6 +833,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, * Bit-fields that holds the actions detected until now. * @param[in] dev * Pointer to the Ethernet device structure. + * @param[in] attr + * Attributes of flow that includes this action. * @param[out] error * Pointer to error structure. * @@ -811,6 +845,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, mlx5_flow_validate_action_rss(const struct rte_flow_action *action, uint64_t action_flags, struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, struct rte_flow_error *error) { struct priv *priv = dev->data->dev_private; @@ -864,6 +899,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, (error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION_CONF, &rss->queue[i], "queue is not configured"); } + if (attr->egress) + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, + "rss action not supported for " + "egress"); return 0; } @@ -872,6 +912,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, * * @param[in] dev * Pointer to the Ethernet device structure. + * @param[in] attr + * Attributes of flow that includes this action. * @param[out] error * Pointer to error structure. * @@ -880,6 +922,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, */ int mlx5_flow_validate_action_count(struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, struct rte_flow_error *error) { struct priv *priv = dev->data->dev_private; @@ -888,6 +931,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, return rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, NULL, "flow counters are not supported."); + if (attr->egress) + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, + "count action not supported for " + "egress"); return 0; } diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index bfc077d..690c597 100644 --- a/drivers/net/mlx5/mlx5_flow.h +++ b/drivers/net/mlx5/mlx5_flow.h @@ -280,21 +280,27 @@ uint64_t mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow, int tunnel, uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, uint32_t subpriority); int mlx5_flow_validate_action_count(struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, struct rte_flow_error *error); int mlx5_flow_validate_action_drop(uint64_t action_flags, + const struct rte_flow_attr *attr, struct rte_flow_error *error); int mlx5_flow_validate_action_flag(uint64_t action_flags, + const struct rte_flow_attr *attr, struct rte_flow_error *error); int mlx5_flow_validate_action_mark(const struct rte_flow_action *action, uint64_t action_flags, + const struct rte_flow_attr *attr, struct rte_flow_error *error); int mlx5_flow_validate_action_queue(const struct rte_flow_action *action, uint64_t action_flags, struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, struct rte_flow_error *error); int mlx5_flow_validate_action_rss(const struct rte_flow_action *action, uint64_t action_flags, struct rte_eth_dev *dev, + const struct rte_flow_attr *attr, struct rte_flow_error *error); int mlx5_flow_validate_attributes(struct rte_eth_dev *dev, const struct rte_flow_attr *attributes, diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 3b46edc..80e7b24 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -116,21 +116,16 @@ RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, NULL, "priority out of range"); - if (attributes->egress) - return rte_flow_error_set(error, ENOTSUP, - RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, - NULL, - "egress is not supported"); if (attributes->transfer) return rte_flow_error_set(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER, NULL, "transfer is not supported"); - if (!attributes->ingress) - return rte_flow_error_set(error, EINVAL, - RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, - NULL, - "ingress attribute is mandatory"); + if (!(attributes->egress ^ attributes->ingress)) + return rte_flow_error_set(error, ENOTSUP, + RTE_FLOW_ERROR_TYPE_ATTR, NULL, + "must specify exactly one of " + "ingress or egress"); return 0; } @@ -287,7 +282,7 @@ break; case RTE_FLOW_ACTION_TYPE_FLAG: ret = mlx5_flow_validate_action_flag(action_flags, - error); + attr, error); if (ret < 0) return ret; action_flags |= MLX5_FLOW_ACTION_FLAG; @@ -296,7 +291,7 @@ case RTE_FLOW_ACTION_TYPE_MARK: ret = mlx5_flow_validate_action_mark(actions, action_flags, - error); + attr, error); if (ret < 0) return ret; action_flags |= MLX5_FLOW_ACTION_MARK; @@ -304,7 +299,7 @@ break; case RTE_FLOW_ACTION_TYPE_DROP: ret = mlx5_flow_validate_action_drop(action_flags, - error); + attr, error); if (ret < 0) return ret; action_flags |= MLX5_FLOW_ACTION_DROP; @@ -313,7 +308,7 @@ case RTE_FLOW_ACTION_TYPE_QUEUE: ret = mlx5_flow_validate_action_queue(actions, action_flags, dev, - error); + attr, error); if (ret < 0) return ret; action_flags |= MLX5_FLOW_ACTION_QUEUE; @@ -322,14 +317,14 @@ case RTE_FLOW_ACTION_TYPE_RSS: ret = mlx5_flow_validate_action_rss(actions, action_flags, dev, - error); + attr, error); if (ret < 0) return ret; action_flags |= MLX5_FLOW_ACTION_RSS; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_COUNT: - ret = mlx5_flow_validate_action_count(dev, error); + ret = mlx5_flow_validate_action_count(dev, attr, error); if (ret < 0) return ret; action_flags |= MLX5_FLOW_ACTION_COUNT; @@ -342,7 +337,7 @@ "action not supported"); } } - if (!(action_flags & MLX5_FLOW_FATE_ACTIONS)) + if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr->ingress) return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION, actions, "no fate action is found"); @@ -1065,12 +1060,14 @@ dev_flow->dv.actions[actions_n].tag_value = MLX5_FLOW_MARK_DEFAULT; actions_n++; + flow->actions |= MLX5_FLOW_ACTION_FLAG; break; case RTE_FLOW_ACTION_TYPE_MARK: dev_flow->dv.actions[actions_n].type = MLX5DV_FLOW_ACTION_TAG; dev_flow->dv.actions[actions_n].tag_value = ((const struct rte_flow_action_mark *) (action->conf))->id; + flow->actions |= MLX5_FLOW_ACTION_MARK; actions_n++; break; case RTE_FLOW_ACTION_TYPE_DROP: @@ -1081,6 +1078,7 @@ queue = action->conf; flow->rss.queue_num = 1; (*flow->queue)[0] = queue->index; + flow->actions |= MLX5_FLOW_ACTION_QUEUE; break; case RTE_FLOW_ACTION_TYPE_RSS: rss = action->conf; @@ -1092,6 +1090,7 @@ flow->rss.types = rss->types; flow->rss.level = rss->level; /* Added to array only in apply since we need the QP */ + flow->actions |= MLX5_FLOW_ACTION_RSS; break; default: break; @@ -1302,7 +1301,8 @@ dv->actions[n].type = MLX5DV_FLOW_ACTION_DEST_IBV_QP; dv->actions[n].qp = dv->hrxq->qp; n++; - } else { + } else if (flow->actions & + (MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)) { struct mlx5_hrxq *hrxq; hrxq = mlx5_hrxq_get(dev, flow->key, MLX5_RSS_HASH_KEY_LEN, diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c index 6964476..ad8f7ac 100644 --- a/drivers/net/mlx5/mlx5_flow_verbs.c +++ b/drivers/net/mlx5/mlx5_flow_verbs.c @@ -1113,6 +1113,7 @@ break; case RTE_FLOW_ACTION_TYPE_FLAG: ret = mlx5_flow_validate_action_flag(action_flags, + attr, error); if (ret < 0) return ret; @@ -1121,6 +1122,7 @@ case RTE_FLOW_ACTION_TYPE_MARK: ret = mlx5_flow_validate_action_mark(actions, action_flags, + attr, error); if (ret < 0) return ret; @@ -1128,6 +1130,7 @@ break; case RTE_FLOW_ACTION_TYPE_DROP: ret = mlx5_flow_validate_action_drop(action_flags, + attr, error); if (ret < 0) return ret; @@ -1136,6 +1139,7 @@ case RTE_FLOW_ACTION_TYPE_QUEUE: ret = mlx5_flow_validate_action_queue(actions, action_flags, dev, + attr, error); if (ret < 0) return ret; @@ -1144,13 +1148,14 @@ case RTE_FLOW_ACTION_TYPE_RSS: ret = mlx5_flow_validate_action_rss(actions, action_flags, dev, + attr, error); if (ret < 0) return ret; action_flags |= MLX5_FLOW_ACTION_RSS; break; case RTE_FLOW_ACTION_TYPE_COUNT: - ret = mlx5_flow_validate_action_count(dev, error); + ret = mlx5_flow_validate_action_count(dev, attr, error); if (ret < 0) return ret; action_flags |= MLX5_FLOW_ACTION_COUNT; -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/mlx5: allow flow rule with attribute egress 2018-10-07 14:01 ` [dpdk-dev] [PATCH v2] " Dekel Peled @ 2018-10-08 18:55 ` Yongseok Koh 2018-10-11 8:14 ` Shahaf Shuler 0 siblings, 1 reply; 8+ messages in thread From: Yongseok Koh @ 2018-10-08 18:55 UTC (permalink / raw) To: Dekel Peled; +Cc: Shahaf Shuler, dev, Ori Kam On Sun, Oct 07, 2018 at 05:01:05PM +0300, Dekel Peled wrote: > This patch complements [1], adding to MLX5 PMD the option to set > flow rule for egress traffic. > > [1] "net/mlx5: support metadata as flow rule criteria" > http://mails.dpdk.org/archives/dev/2018-September/113275.html > > Signed-off-by: Dekel Peled <dekelp@mellanox.com> > --- Acked-by: Yongseok Koh <yskoh@mellanox.com> Thanks > V2: > * Rebase on tip. > * Apply code review comments. > --- > drivers/net/mlx5/mlx5_flow.c | 48 ++++++++++++++++++++++++++++++++++++++ > drivers/net/mlx5/mlx5_flow.h | 6 +++++ > drivers/net/mlx5/mlx5_flow_dv.c | 36 ++++++++++++++-------------- > drivers/net/mlx5/mlx5_flow_verbs.c | 7 +++++- > 4 files changed, 78 insertions(+), 19 deletions(-) > > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c > index 6e4a651..1087f67 100644 > --- a/drivers/net/mlx5/mlx5_flow.c > +++ b/drivers/net/mlx5/mlx5_flow.c > @@ -645,6 +645,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * > * @param[in] action_flags > * Bit-fields that holds the actions detected until now. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -653,6 +655,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > */ > int > mlx5_flow_validate_action_flag(uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > > @@ -669,6 +672,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > RTE_FLOW_ERROR_TYPE_ACTION, NULL, > "can't have 2 flag" > " actions in same flow"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, > + "flag action not supported for " > + "egress"); > return 0; > } > > @@ -679,6 +687,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * Pointer to the queue action. > * @param[in] action_flags > * Bit-fields that holds the actions detected until now. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -688,6 +698,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > int > mlx5_flow_validate_action_mark(const struct rte_flow_action *action, > uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > const struct rte_flow_action_mark *mark = action->conf; > @@ -716,6 +727,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > RTE_FLOW_ERROR_TYPE_ACTION, NULL, > "can't have 2 mark actions in same" > " flow"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, > + "mark action not supported for " > + "egress"); > return 0; > } > > @@ -724,6 +740,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * > * @param[in] action_flags > * Bit-fields that holds the actions detected until now. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -732,6 +750,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > */ > int > mlx5_flow_validate_action_drop(uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > if (action_flags & MLX5_FLOW_ACTION_FLAG) > @@ -747,6 +766,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > RTE_FLOW_ERROR_TYPE_ACTION, NULL, > "can't have 2 fate actions in" > " same flow"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, > + "drop action not supported for " > + "egress"); > return 0; > } > > @@ -759,6 +783,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * Bit-fields that holds the actions detected until now. > * @param[in] dev > * Pointer to the Ethernet device structure. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -769,6 +795,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > mlx5_flow_validate_action_queue(const struct rte_flow_action *action, > uint64_t action_flags, > struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > struct priv *priv = dev->data->dev_private; > @@ -789,6 +816,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > RTE_FLOW_ERROR_TYPE_ACTION_CONF, > &queue->index, > "queue is not configured"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, > + "queue action not supported for " > + "egress"); > return 0; > } > > @@ -801,6 +833,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * Bit-fields that holds the actions detected until now. > * @param[in] dev > * Pointer to the Ethernet device structure. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -811,6 +845,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > mlx5_flow_validate_action_rss(const struct rte_flow_action *action, > uint64_t action_flags, > struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > struct priv *priv = dev->data->dev_private; > @@ -864,6 +899,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > (error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION_CONF, > &rss->queue[i], "queue is not configured"); > } > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, > + "rss action not supported for " > + "egress"); > return 0; > } > > @@ -872,6 +912,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > * > * @param[in] dev > * Pointer to the Ethernet device structure. > + * @param[in] attr > + * Attributes of flow that includes this action. > * @param[out] error > * Pointer to error structure. > * > @@ -880,6 +922,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > */ > int > mlx5_flow_validate_action_count(struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error) > { > struct priv *priv = dev->data->dev_private; > @@ -888,6 +931,11 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > return rte_flow_error_set(error, ENOTSUP, > RTE_FLOW_ERROR_TYPE_ACTION, NULL, > "flow counters are not supported."); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL, > + "count action not supported for " > + "egress"); > return 0; > } > > diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h > index bfc077d..690c597 100644 > --- a/drivers/net/mlx5/mlx5_flow.h > +++ b/drivers/net/mlx5/mlx5_flow.h > @@ -280,21 +280,27 @@ uint64_t mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow, int tunnel, > uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority, > uint32_t subpriority); > int mlx5_flow_validate_action_count(struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_drop(uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_flag(uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_mark(const struct rte_flow_action *action, > uint64_t action_flags, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_queue(const struct rte_flow_action *action, > uint64_t action_flags, > struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_action_rss(const struct rte_flow_action *action, > uint64_t action_flags, > struct rte_eth_dev *dev, > + const struct rte_flow_attr *attr, > struct rte_flow_error *error); > int mlx5_flow_validate_attributes(struct rte_eth_dev *dev, > const struct rte_flow_attr *attributes, > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c > index 3b46edc..80e7b24 100644 > --- a/drivers/net/mlx5/mlx5_flow_dv.c > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > @@ -116,21 +116,16 @@ > RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, > NULL, > "priority out of range"); > - if (attributes->egress) > - return rte_flow_error_set(error, ENOTSUP, > - RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > - NULL, > - "egress is not supported"); > if (attributes->transfer) > return rte_flow_error_set(error, ENOTSUP, > RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER, > NULL, > "transfer is not supported"); > - if (!attributes->ingress) > - return rte_flow_error_set(error, EINVAL, > - RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > - NULL, > - "ingress attribute is mandatory"); > + if (!(attributes->egress ^ attributes->ingress)) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR, NULL, > + "must specify exactly one of " > + "ingress or egress"); > return 0; > } > > @@ -287,7 +282,7 @@ > break; > case RTE_FLOW_ACTION_TYPE_FLAG: > ret = mlx5_flow_validate_action_flag(action_flags, > - error); > + attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_FLOW_ACTION_FLAG; > @@ -296,7 +291,7 @@ > case RTE_FLOW_ACTION_TYPE_MARK: > ret = mlx5_flow_validate_action_mark(actions, > action_flags, > - error); > + attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_FLOW_ACTION_MARK; > @@ -304,7 +299,7 @@ > break; > case RTE_FLOW_ACTION_TYPE_DROP: > ret = mlx5_flow_validate_action_drop(action_flags, > - error); > + attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_FLOW_ACTION_DROP; > @@ -313,7 +308,7 @@ > case RTE_FLOW_ACTION_TYPE_QUEUE: > ret = mlx5_flow_validate_action_queue(actions, > action_flags, dev, > - error); > + attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_FLOW_ACTION_QUEUE; > @@ -322,14 +317,14 @@ > case RTE_FLOW_ACTION_TYPE_RSS: > ret = mlx5_flow_validate_action_rss(actions, > action_flags, dev, > - error); > + attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_FLOW_ACTION_RSS; > ++actions_n; > break; > case RTE_FLOW_ACTION_TYPE_COUNT: > - ret = mlx5_flow_validate_action_count(dev, error); > + ret = mlx5_flow_validate_action_count(dev, attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_FLOW_ACTION_COUNT; > @@ -342,7 +337,7 @@ > "action not supported"); > } > } > - if (!(action_flags & MLX5_FLOW_FATE_ACTIONS)) > + if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr->ingress) > return rte_flow_error_set(error, EINVAL, > RTE_FLOW_ERROR_TYPE_ACTION, actions, > "no fate action is found"); > @@ -1065,12 +1060,14 @@ > dev_flow->dv.actions[actions_n].tag_value = > MLX5_FLOW_MARK_DEFAULT; > actions_n++; > + flow->actions |= MLX5_FLOW_ACTION_FLAG; > break; > case RTE_FLOW_ACTION_TYPE_MARK: > dev_flow->dv.actions[actions_n].type = MLX5DV_FLOW_ACTION_TAG; > dev_flow->dv.actions[actions_n].tag_value = > ((const struct rte_flow_action_mark *) > (action->conf))->id; > + flow->actions |= MLX5_FLOW_ACTION_MARK; > actions_n++; > break; > case RTE_FLOW_ACTION_TYPE_DROP: > @@ -1081,6 +1078,7 @@ > queue = action->conf; > flow->rss.queue_num = 1; > (*flow->queue)[0] = queue->index; > + flow->actions |= MLX5_FLOW_ACTION_QUEUE; > break; > case RTE_FLOW_ACTION_TYPE_RSS: > rss = action->conf; > @@ -1092,6 +1090,7 @@ > flow->rss.types = rss->types; > flow->rss.level = rss->level; > /* Added to array only in apply since we need the QP */ > + flow->actions |= MLX5_FLOW_ACTION_RSS; > break; > default: > break; > @@ -1302,7 +1301,8 @@ > dv->actions[n].type = MLX5DV_FLOW_ACTION_DEST_IBV_QP; > dv->actions[n].qp = dv->hrxq->qp; > n++; > - } else { > + } else if (flow->actions & > + (MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)) { > struct mlx5_hrxq *hrxq; > hrxq = mlx5_hrxq_get(dev, flow->key, > MLX5_RSS_HASH_KEY_LEN, > diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c > index 6964476..ad8f7ac 100644 > --- a/drivers/net/mlx5/mlx5_flow_verbs.c > +++ b/drivers/net/mlx5/mlx5_flow_verbs.c > @@ -1113,6 +1113,7 @@ > break; > case RTE_FLOW_ACTION_TYPE_FLAG: > ret = mlx5_flow_validate_action_flag(action_flags, > + attr, > error); > if (ret < 0) > return ret; > @@ -1121,6 +1122,7 @@ > case RTE_FLOW_ACTION_TYPE_MARK: > ret = mlx5_flow_validate_action_mark(actions, > action_flags, > + attr, > error); > if (ret < 0) > return ret; > @@ -1128,6 +1130,7 @@ > break; > case RTE_FLOW_ACTION_TYPE_DROP: > ret = mlx5_flow_validate_action_drop(action_flags, > + attr, > error); > if (ret < 0) > return ret; > @@ -1136,6 +1139,7 @@ > case RTE_FLOW_ACTION_TYPE_QUEUE: > ret = mlx5_flow_validate_action_queue(actions, > action_flags, dev, > + attr, > error); > if (ret < 0) > return ret; > @@ -1144,13 +1148,14 @@ > case RTE_FLOW_ACTION_TYPE_RSS: > ret = mlx5_flow_validate_action_rss(actions, > action_flags, dev, > + attr, > error); > if (ret < 0) > return ret; > action_flags |= MLX5_FLOW_ACTION_RSS; > break; > case RTE_FLOW_ACTION_TYPE_COUNT: > - ret = mlx5_flow_validate_action_count(dev, error); > + ret = mlx5_flow_validate_action_count(dev, attr, error); > if (ret < 0) > return ret; > action_flags |= MLX5_FLOW_ACTION_COUNT; > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/mlx5: allow flow rule with attribute egress 2018-10-08 18:55 ` Yongseok Koh @ 2018-10-11 8:14 ` Shahaf Shuler 0 siblings, 0 replies; 8+ messages in thread From: Shahaf Shuler @ 2018-10-11 8:14 UTC (permalink / raw) To: Yongseok Koh, Dekel Peled; +Cc: dev, Ori Kam Monday, October 8, 2018 9:56 PM, Yongseok Koh: > Subject: Re: [PATCH v2] net/mlx5: allow flow rule with attribute egress > > On Sun, Oct 07, 2018 at 05:01:05PM +0300, Dekel Peled wrote: > > This patch complements [1], adding to MLX5 PMD the option to set flow > > rule for egress traffic. > > > > [1] "net/mlx5: support metadata as flow rule criteria" > > http://mails.dpdk.org/archives/dev/2018-September/113275.html > > > > Signed-off-by: Dekel Peled <dekelp@mellanox.com> > > --- > Acked-by: Yongseok Koh <yskoh@mellanox.com> Applied to next-net-mlx, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-10-11 8:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-27 14:25 [dpdk-dev] [PATCH] net/mlx5: allow flow rule with attribute egress Dekel Peled 2018-09-29 9:24 ` Yongseok Koh 2018-10-03 6:06 ` Dekel Peled 2018-10-03 7:28 ` Yongseok Koh 2018-10-03 13:02 ` Dekel Peled 2018-10-07 14:01 ` [dpdk-dev] [PATCH v2] " Dekel Peled 2018-10-08 18:55 ` Yongseok Koh 2018-10-11 8:14 ` Shahaf Shuler
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).