DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [PATCH] net/mlx5: implement drop action in hardware classifier
       [not found] <1495717559-13005-1-git-send-email-shacharbe@mellanox.com>
@ 2017-05-26  6:56 ` Nélio Laranjeiro
  0 siblings, 0 replies; 6+ messages in thread
From: Nélio Laranjeiro @ 2017-05-26  6:56 UTC (permalink / raw)
  To: Shachar Beiser; +Cc: adrien.mazarguil, dev

Shachar,

Please see the few comments above,

On Thu, May 25, 2017 at 01:05:58PM +0000, Shachar Beiser wrote:
> The current drop action is implemented as a queue tail drop,
> requiring to instantiate multiple WQs to maintain high drop rate.
> This commit, implements the drop action in hardware classifier.
> This enables to reduce the amount of contexts needed for the drop,
> without affecting the drop rate.
> 
> Signed-off-by: Shachar Beiser <shacharbe@mellanox.com>
> ---
>  drivers/net/mlx5/Makefile    |  5 ++++
>  drivers/net/mlx5/mlx5_flow.c | 56 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> index c079959..daf8013 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -101,6 +101,11 @@ mlx5_autoconf.h.new: FORCE
>  mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
>  	$Q $(RM) -f -- '$@'
>  	$Q sh -- '$<' '$@' \
> +		HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP \
> +		infiniband/verbs_exp.h \
> +		enum IBV_EXP_FLOW_SPEC_ACTION_DROP \
> +		$(AUTOCONF_OUTPUT)
> +	$Q sh -- '$<' '$@' \
>  		HAVE_VERBS_IBV_EXP_CQ_COMPRESSED_CQE \
>  		infiniband/verbs_exp.h \
>  		enum IBV_EXP_CQ_COMPRESSED_CQE \
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index adcbe3f..d132d85 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -994,6 +994,12 @@ struct mlx5_flow_action {
>  {
>  	struct rte_flow *rte_flow;
>  
> +#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
> +
> +	struct ibv_exp_flow_spec_action_drop *drop;
> +
> +	unsigned int size = sizeof(struct ibv_exp_flow_spec_action_drop);
> +#endif

Remove those extra lines, it does not respect the file coding style.

>  	assert(priv->pd);
>  	assert(priv->ctx);
>  	rte_flow = rte_calloc(__func__, 1, sizeof(*rte_flow), 0);
> @@ -1007,6 +1013,15 @@ struct mlx5_flow_action {
>  	rte_flow->qp = priv->flow_drop_queue->qp;
>  	if (!priv->started)
>  		return rte_flow;
> +#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
> +	drop = (void *)((uintptr_t)flow->ibv_attr + flow->offset);
> +	*drop = (struct ibv_exp_flow_spec_action_drop){
> +			.type = IBV_EXP_FLOW_SPEC_ACTION_DROP,
> +			.size = size,
> +	};
> +	++flow->ibv_attr->num_of_specs;
> +	flow->offset += sizeof(struct ibv_exp_flow_spec_action_drop);
> +#endif
>  	rte_flow->ibv_flow = ibv_exp_create_flow(rte_flow->qp,
>  						 rte_flow->ibv_attr);
>  	if (!rte_flow->ibv_flow) {
> @@ -1370,8 +1385,10 @@ struct rte_flow *
>  priv_flow_create_drop_queue(struct priv *priv)
>  {
>  	struct rte_flow_drop *fdq = NULL;
> +#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
>  	unsigned int i;
>  
> +#endif

The empty line should be after the #endif not before.

>  	assert(priv->pd);
>  	assert(priv->ctx);
>  	fdq = rte_calloc(__func__, 1, sizeof(*fdq), 0);
> @@ -1387,6 +1404,7 @@ struct rte_flow *
>  		WARN("cannot allocate CQ for drop queue");
>  		goto error;
>  	}
> +#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
>  	for (i = 0; i != MLX5_DROP_WQ_N; ++i) {
>  		fdq->wqs[i] = ibv_exp_create_wq(priv->ctx,
>  				&(struct ibv_exp_wq_init_attr){
> @@ -1412,6 +1430,31 @@ struct rte_flow *
>  		WARN("cannot allocate indirection table for drop queue");
>  		goto error;
>  	}
> +#else
> +	fdq->wqs[0] = ibv_exp_create_wq(priv->ctx,
> +			&(struct ibv_exp_wq_init_attr){
> +			.wq_type = IBV_EXP_WQT_RQ,
> +			.max_recv_wr = 1,
> +			.max_recv_sge = 1,
> +			.pd = priv->pd,
> +			.cq = fdq->cq,
> +			});
> +	if (!fdq->wqs[0]) {
> +		WARN("cannot allocate WQ for drop queue");
> +		goto error;
> +	}

>From here...

> +	fdq->ind_table = ibv_exp_create_rwq_ind_table(priv->ctx,
> +			&(struct ibv_exp_rwq_ind_table_init_attr){
> +			.pd = priv->pd,
> +			.log_ind_tbl_size = 0,
> +			.ind_tbl = fdq->wqs,
> +			.comp_mask = 0,
> +			});
> +	if (!fdq->ind_table) {
> +		WARN("cannot allocate indirection table for drop queue");
> +		goto error;
> +	}
> +#endif
>[...]

To this point, the code is copy/paste from the block above, it should
not be present.  Please keep the diff as small as possible.

Thanks,

-- 
Nélio Laranjeiro
6WIND

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

* Re: [dpdk-dev] [PATCH] net/mlx5: implement drop action in hardware classifier
  2017-05-29 13:35     ` Shachar Beiser
@ 2017-05-29 14:05       ` Nélio Laranjeiro
  0 siblings, 0 replies; 6+ messages in thread
From: Nélio Laranjeiro @ 2017-05-29 14:05 UTC (permalink / raw)
  To: Shachar Beiser; +Cc: dev, Adrien Mazarguil

>> From: Nélio Laranjeiro [mailto:nelio.laranjeiro@6wind.com] 
>> Sent: Monday, May 29, 2017 4:08 PM
>> To: Shachar Beiser <shacharbe@mellanox.com>
>> Cc: dev@dpdk.org; Adrien Mazarguil <adrien.mazarguil@6wind.com>
>> Subject: Re: [PATCH] net/mlx5: implement drop action in hardware classifier
>> 
>> On Sun, May 28, 2017 at 06:49:20AM +0000, Shachar Beiser wrote:
>> > The current drop action is implemented as a queue tail drop,
>> > requiring to instantiate multiple WQs to maintain high drop rate.
>> > This commit, implements the drop action in hardware classifier.
>> > This enables to reduce the amount of contexts needed for the drop,
>> > without affecting the drop rate.
>> > 
>> > Signed-off-by: Shachar Beiser <shacharbe@mellanox.com>
>> > ---
>> >  drivers/net/mlx5/Makefile    |  5 +++++
>> >  drivers/net/mlx5/mlx5_flow.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 48 insertions(+)
>> > 
>> > diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
>> > index c079959..daf8013 100644
>> > --- a/drivers/net/mlx5/Makefile
>> > +++ b/drivers/net/mlx5/Makefile
>> > @@ -101,6 +101,11 @@ mlx5_autoconf.h.new: FORCE
>> >  mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
>> >  	$Q $(RM) -f -- '$@'
>> >  	$Q sh -- '$<' '$@' \
>> > +		HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP \
>> > +		infiniband/verbs_exp.h \
>> > +		enum IBV_EXP_FLOW_SPEC_ACTION_DROP \
>> > +		$(AUTOCONF_OUTPUT)
>> > +	$Q sh -- '$<' '$@' \
>> >  		HAVE_VERBS_IBV_EXP_CQ_COMPRESSED_CQE \
>> >  		infiniband/verbs_exp.h \
>> >  		enum IBV_EXP_CQ_COMPRESSED_CQE \
>> > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
>> > index adcbe3f..e243d39 100644
>> > --- a/drivers/net/mlx5/mlx5_flow.c
>> > +++ b/drivers/net/mlx5/mlx5_flow.c
>> > @@ -994,6 +994,11 @@ struct mlx5_flow_action {
>> >  {
>> >  	struct rte_flow *rte_flow;
>> >  
>> > +#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
>> > +	struct ibv_exp_flow_spec_action_drop *drop;
>> > +	unsigned int size = sizeof(struct ibv_exp_flow_spec_action_drop);
>> > +#endif
>> 
>> Extra empty line before the #ifdef
>> 
>> > +
>> >  	assert(priv->pd);
>> >  	assert(priv->ctx);
>> >  	rte_flow = rte_calloc(__func__, 1, sizeof(*rte_flow), 0);
>> > @@ -1007,6 +1012,15 @@ struct mlx5_flow_action {
>> >  	rte_flow->qp = priv->flow_drop_queue->qp;
>> >  	if (!priv->started)
>> >  		return rte_flow;
>> > +#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
>> > +	drop = (void *)((uintptr_t)flow->ibv_attr + flow->offset);
>> > +	*drop = (struct ibv_exp_flow_spec_action_drop){
>> > +			.type = IBV_EXP_FLOW_SPEC_ACTION_DROP,
>> > +			.size = size,
>> > +	};
>> > +	++flow->ibv_attr->num_of_specs;
>> > +	flow->offset += sizeof(struct ibv_exp_flow_spec_action_drop);
>> > +#endif
>> >  	rte_flow->ibv_flow = ibv_exp_create_flow(rte_flow->qp,
>> >  						 rte_flow->ibv_attr);
>> >  	if (!rte_flow->ibv_flow) {
>> >[...]
>> 
>> From what I see by just changing the value of MLX5_DROP_WQ_N when
>> HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP (in the same source file)
>> limits the patch to this point.
>> 
>> Am I missing something?
>> 
>> Regards,
> On Mon, May 29, 2017 at 01:35:57PM +0000, Shachar Beiser wrote:
> The current firmware version does not implement queue tail drop
> therefore queue allocation is not required by firmware. Unfortunately,
> the ibverbs APIs requires to allocate WQ & CQ  .In the future this
> allocation will be saved . Right now, you are right. 

What I mean is, in your patch, what you need is the snippet I let above,
and at compilation time you can update the MLX5_DROP_WQ_N to 1 (defined
at 4 in the same file), this way the remaining code will only create a
single CQ/WQ as you expect.

This small snippet above plus 3 lines handles the job.

Regards,

PS: Avoid top answering on this mailing list.

-- 
Nélio Laranjeiro
6WIND

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

* Re: [dpdk-dev] [PATCH] net/mlx5: implement drop action in hardware classifier
  2017-05-29 13:07   ` Nélio Laranjeiro
@ 2017-05-29 13:35     ` Shachar Beiser
  2017-05-29 14:05       ` Nélio Laranjeiro
  0 siblings, 1 reply; 6+ messages in thread
From: Shachar Beiser @ 2017-05-29 13:35 UTC (permalink / raw)
  To: Nélio Laranjeiro; +Cc: dev, Adrien Mazarguil

The current firmware version does not implement queue tail drop therefore queue allocation is not required by firmware. Unfortunately, the ibverbs APIs requires to allocate WQ & CQ  .In the future this allocation will be saved . Right now, you are right. 

-----Original Message-----
From: Nélio Laranjeiro [mailto:nelio.laranjeiro@6wind.com] 
Sent: Monday, May 29, 2017 4:08 PM
To: Shachar Beiser <shacharbe@mellanox.com>
Cc: dev@dpdk.org; Adrien Mazarguil <adrien.mazarguil@6wind.com>
Subject: Re: [PATCH] net/mlx5: implement drop action in hardware classifier

On Sun, May 28, 2017 at 06:49:20AM +0000, Shachar Beiser wrote:
> The current drop action is implemented as a queue tail drop,
> requiring to instantiate multiple WQs to maintain high drop rate.
> This commit, implements the drop action in hardware classifier.
> This enables to reduce the amount of contexts needed for the drop,
> without affecting the drop rate.
> 
> Signed-off-by: Shachar Beiser <shacharbe@mellanox.com>
> ---
>  drivers/net/mlx5/Makefile    |  5 +++++
>  drivers/net/mlx5/mlx5_flow.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> index c079959..daf8013 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -101,6 +101,11 @@ mlx5_autoconf.h.new: FORCE
>  mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
>  	$Q $(RM) -f -- '$@'
>  	$Q sh -- '$<' '$@' \
> +		HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP \
> +		infiniband/verbs_exp.h \
> +		enum IBV_EXP_FLOW_SPEC_ACTION_DROP \
> +		$(AUTOCONF_OUTPUT)
> +	$Q sh -- '$<' '$@' \
>  		HAVE_VERBS_IBV_EXP_CQ_COMPRESSED_CQE \
>  		infiniband/verbs_exp.h \
>  		enum IBV_EXP_CQ_COMPRESSED_CQE \
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index adcbe3f..e243d39 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -994,6 +994,11 @@ struct mlx5_flow_action {
>  {
>  	struct rte_flow *rte_flow;
>  
> +#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
> +	struct ibv_exp_flow_spec_action_drop *drop;
> +	unsigned int size = sizeof(struct ibv_exp_flow_spec_action_drop);
> +#endif

Extra empty line before the #ifdef

> +
>  	assert(priv->pd);
>  	assert(priv->ctx);
>  	rte_flow = rte_calloc(__func__, 1, sizeof(*rte_flow), 0);
> @@ -1007,6 +1012,15 @@ struct mlx5_flow_action {
>  	rte_flow->qp = priv->flow_drop_queue->qp;
>  	if (!priv->started)
>  		return rte_flow;
> +#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
> +	drop = (void *)((uintptr_t)flow->ibv_attr + flow->offset);
> +	*drop = (struct ibv_exp_flow_spec_action_drop){
> +			.type = IBV_EXP_FLOW_SPEC_ACTION_DROP,
> +			.size = size,
> +	};
> +	++flow->ibv_attr->num_of_specs;
> +	flow->offset += sizeof(struct ibv_exp_flow_spec_action_drop);
> +#endif
>  	rte_flow->ibv_flow = ibv_exp_create_flow(rte_flow->qp,
>  						 rte_flow->ibv_attr);
>  	if (!rte_flow->ibv_flow) {
>[...]

>From what I see by just changing the value of MLX5_DROP_WQ_N when
HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP (in the same source file)
limits the patch to this point.

Am I missing something?

Regards,

-- 
Nélio Laranjeiro
6WIND

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

* Re: [dpdk-dev] [PATCH] net/mlx5: implement drop action in hardware classifier
  2017-05-28  6:49 ` Shachar Beiser
@ 2017-05-29 13:07   ` Nélio Laranjeiro
  2017-05-29 13:35     ` Shachar Beiser
  0 siblings, 1 reply; 6+ messages in thread
From: Nélio Laranjeiro @ 2017-05-29 13:07 UTC (permalink / raw)
  To: Shachar Beiser; +Cc: dev, Adrien Mazarguil

On Sun, May 28, 2017 at 06:49:20AM +0000, Shachar Beiser wrote:
> The current drop action is implemented as a queue tail drop,
> requiring to instantiate multiple WQs to maintain high drop rate.
> This commit, implements the drop action in hardware classifier.
> This enables to reduce the amount of contexts needed for the drop,
> without affecting the drop rate.
> 
> Signed-off-by: Shachar Beiser <shacharbe@mellanox.com>
> ---
>  drivers/net/mlx5/Makefile    |  5 +++++
>  drivers/net/mlx5/mlx5_flow.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> index c079959..daf8013 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -101,6 +101,11 @@ mlx5_autoconf.h.new: FORCE
>  mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
>  	$Q $(RM) -f -- '$@'
>  	$Q sh -- '$<' '$@' \
> +		HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP \
> +		infiniband/verbs_exp.h \
> +		enum IBV_EXP_FLOW_SPEC_ACTION_DROP \
> +		$(AUTOCONF_OUTPUT)
> +	$Q sh -- '$<' '$@' \
>  		HAVE_VERBS_IBV_EXP_CQ_COMPRESSED_CQE \
>  		infiniband/verbs_exp.h \
>  		enum IBV_EXP_CQ_COMPRESSED_CQE \
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index adcbe3f..e243d39 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -994,6 +994,11 @@ struct mlx5_flow_action {
>  {
>  	struct rte_flow *rte_flow;
>  
> +#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
> +	struct ibv_exp_flow_spec_action_drop *drop;
> +	unsigned int size = sizeof(struct ibv_exp_flow_spec_action_drop);
> +#endif

Extra empty line before the #ifdef

> +
>  	assert(priv->pd);
>  	assert(priv->ctx);
>  	rte_flow = rte_calloc(__func__, 1, sizeof(*rte_flow), 0);
> @@ -1007,6 +1012,15 @@ struct mlx5_flow_action {
>  	rte_flow->qp = priv->flow_drop_queue->qp;
>  	if (!priv->started)
>  		return rte_flow;
> +#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
> +	drop = (void *)((uintptr_t)flow->ibv_attr + flow->offset);
> +	*drop = (struct ibv_exp_flow_spec_action_drop){
> +			.type = IBV_EXP_FLOW_SPEC_ACTION_DROP,
> +			.size = size,
> +	};
> +	++flow->ibv_attr->num_of_specs;
> +	flow->offset += sizeof(struct ibv_exp_flow_spec_action_drop);
> +#endif
>  	rte_flow->ibv_flow = ibv_exp_create_flow(rte_flow->qp,
>  						 rte_flow->ibv_attr);
>  	if (!rte_flow->ibv_flow) {
>[...]

>From what I see by just changing the value of MLX5_DROP_WQ_N when
HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP (in the same source file)
limits the patch to this point.

Am I missing something?

Regards,

-- 
Nélio Laranjeiro
6WIND

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

* [dpdk-dev] [PATCH] net/mlx5: implement drop action in hardware classifier
  2017-05-28  6:49 Shachar Beiser
@ 2017-05-28  6:49 ` Shachar Beiser
  2017-05-29 13:07   ` Nélio Laranjeiro
  0 siblings, 1 reply; 6+ messages in thread
From: Shachar Beiser @ 2017-05-28  6:49 UTC (permalink / raw)
  To: dev; +Cc: Adrien Mazarguil, Nelio Laranjeiro, Shachar Beiser

The current drop action is implemented as a queue tail drop,
requiring to instantiate multiple WQs to maintain high drop rate.
This commit, implements the drop action in hardware classifier.
This enables to reduce the amount of contexts needed for the drop,
without affecting the drop rate.

Signed-off-by: Shachar Beiser <shacharbe@mellanox.com>
---
 drivers/net/mlx5/Makefile    |  5 +++++
 drivers/net/mlx5/mlx5_flow.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index c079959..daf8013 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -101,6 +101,11 @@ mlx5_autoconf.h.new: FORCE
 mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
 	$Q $(RM) -f -- '$@'
 	$Q sh -- '$<' '$@' \
+		HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP \
+		infiniband/verbs_exp.h \
+		enum IBV_EXP_FLOW_SPEC_ACTION_DROP \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
 		HAVE_VERBS_IBV_EXP_CQ_COMPRESSED_CQE \
 		infiniband/verbs_exp.h \
 		enum IBV_EXP_CQ_COMPRESSED_CQE \
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index adcbe3f..e243d39 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -994,6 +994,11 @@ struct mlx5_flow_action {
 {
 	struct rte_flow *rte_flow;
 
+#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
+	struct ibv_exp_flow_spec_action_drop *drop;
+	unsigned int size = sizeof(struct ibv_exp_flow_spec_action_drop);
+#endif
+
 	assert(priv->pd);
 	assert(priv->ctx);
 	rte_flow = rte_calloc(__func__, 1, sizeof(*rte_flow), 0);
@@ -1007,6 +1012,15 @@ struct mlx5_flow_action {
 	rte_flow->qp = priv->flow_drop_queue->qp;
 	if (!priv->started)
 		return rte_flow;
+#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
+	drop = (void *)((uintptr_t)flow->ibv_attr + flow->offset);
+	*drop = (struct ibv_exp_flow_spec_action_drop){
+			.type = IBV_EXP_FLOW_SPEC_ACTION_DROP,
+			.size = size,
+	};
+	++flow->ibv_attr->num_of_specs;
+	flow->offset += sizeof(struct ibv_exp_flow_spec_action_drop);
+#endif
 	rte_flow->ibv_flow = ibv_exp_create_flow(rte_flow->qp,
 						 rte_flow->ibv_attr);
 	if (!rte_flow->ibv_flow) {
@@ -1370,7 +1384,9 @@ struct rte_flow *
 priv_flow_create_drop_queue(struct priv *priv)
 {
 	struct rte_flow_drop *fdq = NULL;
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	unsigned int i;
+#endif
 
 	assert(priv->pd);
 	assert(priv->ctx);
@@ -1387,6 +1403,7 @@ struct rte_flow *
 		WARN("cannot allocate CQ for drop queue");
 		goto error;
 	}
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	for (i = 0; i != MLX5_DROP_WQ_N; ++i) {
 		fdq->wqs[i] = ibv_exp_create_wq(priv->ctx,
 				&(struct ibv_exp_wq_init_attr){
@@ -1401,6 +1418,20 @@ struct rte_flow *
 			goto error;
 		}
 	}
+#else
+	fdq->wqs[0] = ibv_exp_create_wq(priv->ctx,
+			&(struct ibv_exp_wq_init_attr){
+			.wq_type = IBV_EXP_WQT_RQ,
+			.max_recv_wr = 1,
+			.max_recv_sge = 1,
+			.pd = priv->pd,
+			.cq = fdq->cq,
+			});
+	if (!fdq->wqs[0]) {
+		WARN("cannot allocate WQ for drop queue");
+		goto error;
+	}
+#endif
 	fdq->ind_table = ibv_exp_create_rwq_ind_table(priv->ctx,
 			&(struct ibv_exp_rwq_ind_table_init_attr){
 			.pd = priv->pd,
@@ -1441,10 +1472,15 @@ struct rte_flow *
 		claim_zero(ibv_destroy_qp(fdq->qp));
 	if (fdq->ind_table)
 		claim_zero(ibv_exp_destroy_rwq_ind_table(fdq->ind_table));
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	for (i = 0; i != MLX5_DROP_WQ_N; ++i) {
 		if (fdq->wqs[i])
 			claim_zero(ibv_exp_destroy_wq(fdq->wqs[i]));
 	}
+#else
+	if (fdq->wqs[0])
+		claim_zero(ibv_exp_destroy_wq(fdq->wqs[0]));
+#endif
 	if (fdq->cq)
 		claim_zero(ibv_destroy_cq(fdq->cq));
 	if (fdq)
@@ -1463,7 +1499,9 @@ struct rte_flow *
 priv_flow_delete_drop_queue(struct priv *priv)
 {
 	struct rte_flow_drop *fdq = priv->flow_drop_queue;
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	unsigned int i;
+#endif
 
 	if (!fdq)
 		return;
@@ -1471,10 +1509,15 @@ struct rte_flow *
 		claim_zero(ibv_destroy_qp(fdq->qp));
 	if (fdq->ind_table)
 		claim_zero(ibv_exp_destroy_rwq_ind_table(fdq->ind_table));
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	for (i = 0; i != MLX5_DROP_WQ_N; ++i) {
 		if (fdq->wqs[i])
 			claim_zero(ibv_exp_destroy_wq(fdq->wqs[i]));
 	}
+#else
+	if (fdq->wqs[0])
+		claim_zero(ibv_exp_destroy_wq(fdq->wqs[0]));
+#endif
 	if (fdq->cq)
 		claim_zero(ibv_destroy_cq(fdq->cq));
 	rte_free(fdq);
-- 
1.8.3.1

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

* [dpdk-dev] [PATCH] net/mlx5: implement drop action in hardware classifier
@ 2017-05-28  6:49 Shachar Beiser
  2017-05-28  6:49 ` Shachar Beiser
  0 siblings, 1 reply; 6+ messages in thread
From: Shachar Beiser @ 2017-05-28  6:49 UTC (permalink / raw)
  To: dev; +Cc: Adrien Mazarguil, Nelio Laranjeiro, Shachar Beiser

The current drop action is implemented as a queue tail drop,
requiring to instantiate multiple WQs to maintain high drop rate.
This commit, implements the drop action in hardware classifier.
This enables to reduce the amount of contexts needed for the drop,
without affecting the drop rate.

Signed-off-by: Shachar Beiser <shacharbe@mellanox.com>
---
 drivers/net/mlx5/Makefile    |  5 +++++
 drivers/net/mlx5/mlx5_flow.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index c079959..daf8013 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -101,6 +101,11 @@ mlx5_autoconf.h.new: FORCE
 mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
 	$Q $(RM) -f -- '$@'
 	$Q sh -- '$<' '$@' \
+		HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP \
+		infiniband/verbs_exp.h \
+		enum IBV_EXP_FLOW_SPEC_ACTION_DROP \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
 		HAVE_VERBS_IBV_EXP_CQ_COMPRESSED_CQE \
 		infiniband/verbs_exp.h \
 		enum IBV_EXP_CQ_COMPRESSED_CQE \
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index adcbe3f..e243d39 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -994,6 +994,11 @@ struct mlx5_flow_action {
 {
 	struct rte_flow *rte_flow;
 
+#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
+	struct ibv_exp_flow_spec_action_drop *drop;
+	unsigned int size = sizeof(struct ibv_exp_flow_spec_action_drop);
+#endif
+
 	assert(priv->pd);
 	assert(priv->ctx);
 	rte_flow = rte_calloc(__func__, 1, sizeof(*rte_flow), 0);
@@ -1007,6 +1012,15 @@ struct mlx5_flow_action {
 	rte_flow->qp = priv->flow_drop_queue->qp;
 	if (!priv->started)
 		return rte_flow;
+#ifdef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
+	drop = (void *)((uintptr_t)flow->ibv_attr + flow->offset);
+	*drop = (struct ibv_exp_flow_spec_action_drop){
+			.type = IBV_EXP_FLOW_SPEC_ACTION_DROP,
+			.size = size,
+	};
+	++flow->ibv_attr->num_of_specs;
+	flow->offset += sizeof(struct ibv_exp_flow_spec_action_drop);
+#endif
 	rte_flow->ibv_flow = ibv_exp_create_flow(rte_flow->qp,
 						 rte_flow->ibv_attr);
 	if (!rte_flow->ibv_flow) {
@@ -1370,7 +1384,9 @@ struct rte_flow *
 priv_flow_create_drop_queue(struct priv *priv)
 {
 	struct rte_flow_drop *fdq = NULL;
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	unsigned int i;
+#endif
 
 	assert(priv->pd);
 	assert(priv->ctx);
@@ -1387,6 +1403,7 @@ struct rte_flow *
 		WARN("cannot allocate CQ for drop queue");
 		goto error;
 	}
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	for (i = 0; i != MLX5_DROP_WQ_N; ++i) {
 		fdq->wqs[i] = ibv_exp_create_wq(priv->ctx,
 				&(struct ibv_exp_wq_init_attr){
@@ -1401,6 +1418,20 @@ struct rte_flow *
 			goto error;
 		}
 	}
+#else
+	fdq->wqs[0] = ibv_exp_create_wq(priv->ctx,
+			&(struct ibv_exp_wq_init_attr){
+			.wq_type = IBV_EXP_WQT_RQ,
+			.max_recv_wr = 1,
+			.max_recv_sge = 1,
+			.pd = priv->pd,
+			.cq = fdq->cq,
+			});
+	if (!fdq->wqs[0]) {
+		WARN("cannot allocate WQ for drop queue");
+		goto error;
+	}
+#endif
 	fdq->ind_table = ibv_exp_create_rwq_ind_table(priv->ctx,
 			&(struct ibv_exp_rwq_ind_table_init_attr){
 			.pd = priv->pd,
@@ -1441,10 +1472,15 @@ struct rte_flow *
 		claim_zero(ibv_destroy_qp(fdq->qp));
 	if (fdq->ind_table)
 		claim_zero(ibv_exp_destroy_rwq_ind_table(fdq->ind_table));
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	for (i = 0; i != MLX5_DROP_WQ_N; ++i) {
 		if (fdq->wqs[i])
 			claim_zero(ibv_exp_destroy_wq(fdq->wqs[i]));
 	}
+#else
+	if (fdq->wqs[0])
+		claim_zero(ibv_exp_destroy_wq(fdq->wqs[0]));
+#endif
 	if (fdq->cq)
 		claim_zero(ibv_destroy_cq(fdq->cq));
 	if (fdq)
@@ -1463,7 +1499,9 @@ struct rte_flow *
 priv_flow_delete_drop_queue(struct priv *priv)
 {
 	struct rte_flow_drop *fdq = priv->flow_drop_queue;
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	unsigned int i;
+#endif
 
 	if (!fdq)
 		return;
@@ -1471,10 +1509,15 @@ struct rte_flow *
 		claim_zero(ibv_destroy_qp(fdq->qp));
 	if (fdq->ind_table)
 		claim_zero(ibv_exp_destroy_rwq_ind_table(fdq->ind_table));
+#ifndef HAVE_VERBS_IBV_EXP_FLOW_SPEC_ACTION_DROP
 	for (i = 0; i != MLX5_DROP_WQ_N; ++i) {
 		if (fdq->wqs[i])
 			claim_zero(ibv_exp_destroy_wq(fdq->wqs[i]));
 	}
+#else
+	if (fdq->wqs[0])
+		claim_zero(ibv_exp_destroy_wq(fdq->wqs[0]));
+#endif
 	if (fdq->cq)
 		claim_zero(ibv_destroy_cq(fdq->cq));
 	rte_free(fdq);
-- 
1.8.3.1

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

end of thread, other threads:[~2017-05-29 14:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1495717559-13005-1-git-send-email-shacharbe@mellanox.com>
2017-05-26  6:56 ` [dpdk-dev] [PATCH] net/mlx5: implement drop action in hardware classifier Nélio Laranjeiro
2017-05-28  6:49 Shachar Beiser
2017-05-28  6:49 ` Shachar Beiser
2017-05-29 13:07   ` Nélio Laranjeiro
2017-05-29 13:35     ` Shachar Beiser
2017-05-29 14:05       ` Nélio Laranjeiro

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