Hi Moti,
On Tue, Oct 24, 2017 at 04:11:14PM +0300, Moti Haimovsky wrote:
> This commit addresses the issue of rx interrupts support with
> the new Rx datapath introduced in DPDK version 17.11.
> In order to generate an Rx interrupt an event queue is armed with the
> consumer index of the Rx completion queue. Since version 17.11 this
> index is handled by the PMD so it is now the responsibility of the
> PMD to write this value when enabling Rx interrupts.
>
> Fixes: 6681b845034c ("net/mlx4: add Rx bypassing Verbs")
>
> Signed-off-by: Moti Haimovsky <mo...@mellanox.com>
> ---
> V2:
> * Rebased on top of ff3397e9 ("net/mlx4: relax Rx queue configuration order")
> ---
> drivers/net/mlx4/mlx4_intr.c | 38 +++++++++++++++++++++++++++++++++++++-
> drivers/net/mlx4/mlx4_prm.h | 9 +++++++++
> drivers/net/mlx4/mlx4_rxq.c | 6 +++++-
> 3 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/mlx4/mlx4_intr.c b/drivers/net/mlx4/mlx4_intr.c
> index 3806322..217c591 100644
> --- a/drivers/net/mlx4/mlx4_intr.c
> +++ b/drivers/net/mlx4/mlx4_intr.c
> @@ -53,6 +53,7 @@
> #include <rte_alarm.h>
> #include <rte_errno.h>
> #include <rte_ethdev.h>
> +#include <rte_io.h>
> #include <rte_interrupts.h>
>
> #include "mlx4.h"
> @@ -239,6 +240,40 @@
> }
>
> /**
> + * MLX4 CQ notification .
> + *
> + * @param rxq
> + * Pointer to receive queue structure.
> + * @param solicited
> + * Is request solicited or not.
> + *
> + * @return
> + * Always 0.
Why not setting the return type to void in this case?
> + */
> +static int
> +mlx4_arm_cq(struct rxq *rxq, int solicited)
> +{
> + struct mlx4_cq *cq = &rxq->mcq;
> + uint64_t doorbell;
> + uint32_t sn = cq->arm_sn & MLX4_CQ_DB_GEQ_N_MASK;
> + uint32_t ci = cq->cons_index & MLX4_CQ_DB_CI_MASK;
> + uint32_t cmd = solicited ? MLX4_CQ_DB_REQ_NOT_SOL : MLX4_CQ_DB_REQ_NOT;
> + void *cq_db_reg = (char *)cq->cq_uar + MLX4_CQ_DOORBELL;
use uint8_t* instead of char*.
By the way, why not storing the address to the doorbell directly in the
structure instead of computing it each time it is necessary?
> + *cq->arm_db = rte_cpu_to_be_32(sn << 28 | cmd | ci);
> + /*
> + * Make sure that the doorbell record in host memory is
> + * written before ringing the doorbell via PCI MMIO.
> + */
> + rte_wmb();
> + doorbell = sn << 28 | cmd | cq->cqn;
> + doorbell <<= 32;
> + doorbell |= ci;
> + rte_write64(rte_cpu_to_be_64(doorbell), cq_db_reg);
> + return 0;
> +}
> +
> +/**
> * Uninstall interrupt handler.
> *
> * @param priv
> @@ -333,6 +368,7 @@
> WARN("unable to disable interrupt on rx queue %d",
> idx);
> } else {
> + rxq->mcq.arm_sn++;
> ibv_ack_cq_events(rxq->cq, 1);
> }
> return -ret;
> @@ -358,7 +394,7 @@
> if (!rxq || !rxq->channel)
> ret = EINVAL;
> else
> - ret = ibv_req_notify_cq(rxq->cq, 0);
> + ret = mlx4_arm_cq(rxq, 0);
> if (ret) {
> rte_errno = ret;
> WARN("unable to arm interrupt on rx queue %d", idx);
> diff --git a/drivers/net/mlx4/mlx4_prm.h b/drivers/net/mlx4/mlx4_prm.h
> index 3a77502..92f2fec 100644
> --- a/drivers/net/mlx4/mlx4_prm.h
> +++ b/drivers/net/mlx4/mlx4_prm.h
> @@ -78,6 +78,11 @@ enum {
> MLX4_CQE_L2_TUNNEL_IPOK = (int)(1u << 31),
> };
>
> +/* Completion queue events, numbers and masks. */
> +#define MLX4_CQ_DB_GEQ_N_MASK 0x3
> +#define MLX4_CQ_DB_CI_MASK 0xffffff
You could have replaced all the places where this mask is hard coded by
this new define for consistency.
> +#define MLX4_CQ_DOORBELL 0x20
> +
> /* Send queue information. */
> struct mlx4_sq {
> uint8_t *buf; /**< SQ buffer. */
> @@ -100,6 +105,10 @@ struct mlx4_cq {
> uint32_t cqe_64:1; /**< CQ entry size is 64 bytes. */
> uint32_t cons_index; /**< Last queue entry that was handled. */
> uint32_t *set_ci_db; /**< Pointer to the completion queue doorbell. */
> + uint32_t *arm_db; /**< Pointer to doorbell for arming Rx events. */
> + int arm_sn; /**< Rx event counter. */
> + void *cq_uar; /**< CQ user access region. */
> + uint32_t cqn; /**< CQ number. */
Try to re-arrange the variable order to avoid holes in the structure.
> };
>
> /**
> diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
> index 4c50077..24262cb 100644
> --- a/drivers/net/mlx4/mlx4_rxq.c
> +++ b/drivers/net/mlx4/mlx4_rxq.c
> @@ -510,7 +510,7 @@ void mlx4_rss_detach(struct mlx4_rss *rss)
> struct rte_mbuf *(*elts)[elts_n] = rxq->elts;
> struct mlx4dv_obj mlxdv;
> struct mlx4dv_rwq dv_rwq;
> - struct mlx4dv_cq dv_cq;
> + struct mlx4dv_cq dv_cq = { .comp_mask = MLX4DV_CQ_MASK_UAR, };
> const char *msg;
> struct ibv_cq *cq = NULL;
> struct ibv_wq *wq = NULL;
> @@ -604,6 +604,10 @@ void mlx4_rss_detach(struct mlx4_rss *rss)
> rxq->mcq.cqe_cnt = dv_cq.cqe_cnt;
> rxq->mcq.set_ci_db = dv_cq.set_ci_db;
> rxq->mcq.cqe_64 = (dv_cq.cqe_size & 64) ? 1 : 0;
> + rxq->mcq.arm_db = dv_cq.arm_db;
> + rxq->mcq.arm_sn = dv_cq.arm_sn;
> + rxq->mcq.cqn = dv_cq.cqn;
> + rxq->mcq.cq_uar = dv_cq.cq_uar;
> /* Update doorbell counter. */
> rxq->rq_ci = elts_n / sges_n;
> rte_wmb();
> --
> 1.8.3.1
Thanks,
--
Nélio Laranjeiro
6WIND
