[dpdk-dev,v3,2/7] net/mlx4: associate MR to MP in a short function
Checks
Commit Message
From: Ophir Munk <ophirmu@mellanox.com>
Associate memory region to mempool (on data path)
in a short function.
Handle the less common case of adding a new memory
region to mempool in a separate function.
Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
drivers/net/mlx4/mlx4.h | 2 ++
drivers/net/mlx4/mlx4_mr.c | 46 +++++++++++++++++++++++++++++++++++
drivers/net/mlx4/mlx4_rxtx.c | 57 --------------------------------------------
drivers/net/mlx4/mlx4_rxtx.h | 33 ++++++++++++++++++++++++-
4 files changed, 80 insertions(+), 58 deletions(-)
Comments
On Mon, Oct 30, 2017 at 10:07:24AM +0000, Matan Azrad wrote:
> From: Ophir Munk <ophirmu@mellanox.com>
>
> Associate memory region to mempool (on data path)
> in a short function.
> Handle the less common case of adding a new memory
> region to mempool in a separate function.
>
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
Thanks for moving that function out of the way, a few more comments below.
By the way, commit log lines can be up to 75 characters wide, only the title
line is limited to 50.
> ---
> drivers/net/mlx4/mlx4.h | 2 ++
> drivers/net/mlx4/mlx4_mr.c | 46 +++++++++++++++++++++++++++++++++++
> drivers/net/mlx4/mlx4_rxtx.c | 57 --------------------------------------------
> drivers/net/mlx4/mlx4_rxtx.h | 33 ++++++++++++++++++++++++-
> 4 files changed, 80 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
> index e0a9853..70cf453 100644
> --- a/drivers/net/mlx4/mlx4.h
> +++ b/drivers/net/mlx4/mlx4.h
> @@ -160,5 +160,7 @@ int mlx4_flow_ctrl_set(struct rte_eth_dev *dev,
> /* mlx4_mr.c */
>
> struct ibv_mr *mlx4_mp2mr(struct ibv_pd *pd, struct rte_mempool *mp);
> +uint32_t mlx4_txq_add_mr(struct txq *txq, struct rte_mempool *mp,
> + uint32_t i);
>
> #endif /* RTE_PMD_MLX4_H_ */
> diff --git a/drivers/net/mlx4/mlx4_mr.c b/drivers/net/mlx4/mlx4_mr.c
> index 9700884..ebf6547 100644
> --- a/drivers/net/mlx4/mlx4_mr.c
> +++ b/drivers/net/mlx4/mlx4_mr.c
> @@ -56,6 +56,7 @@
> #include <rte_mempool.h>
>
> #include "mlx4_utils.h"
> +#include "mlx4_rxtx.h"
Alphabetical order is preferred, or so I thought. Nothing seems to enforce
this in the coding rules, it's only a suggestion.
>
> struct mlx4_check_mempool_data {
> int ret;
> @@ -181,3 +182,48 @@ struct ibv_mr *
> rte_errno = errno ? errno : EINVAL;
> return mr;
> }
> +
> +/**
> + * Add memory region (MR) <-> memory pool (MP) association to txq->mp2mr[].
> + * If mp2mr[] is full, remove an entry first.
> + *
> + * @param txq
> + * Pointer to Tx queue structure.
> + * @param[in] mp
> + * Memory pool for which a memory region lkey must be added
Missing "."
> + * @param[in] i
> + * Index in memory pool (MP) where to add memory region (MR)
Same here.
> + *
> + * @return
> + * Added mr->lkey on success, (uint32_t)-1 on failure.
> + */
> +uint32_t mlx4_txq_add_mr(struct txq *txq, struct rte_mempool *mp, uint32_t i)
Coding style again; return type must be on a separate line for definitions
(not declarations).
> +{
> + struct ibv_mr *mr;
> +
> + /* Add a new entry, register MR first. */
> + DEBUG("%p: discovered new memory pool \"%s\" (%p)",
> + (void *)txq, mp->name, (void *)mp);
> + mr = mlx4_mp2mr(txq->priv->pd, mp);
> + if (unlikely(mr == NULL)) {
> + DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.",
> + (void *)txq);
> + return (uint32_t)-1;
> + }
> + if (unlikely(i == RTE_DIM(txq->mp2mr))) {
> + /* Table is full, remove oldest entry. */
> + DEBUG("%p: MR <-> MP table full, dropping oldest entry.",
> + (void *)txq);
> + --i;
> + claim_zero(ibv_dereg_mr(txq->mp2mr[0].mr));
> + memmove(&txq->mp2mr[0], &txq->mp2mr[1],
> + (sizeof(txq->mp2mr) - sizeof(txq->mp2mr[0])));
> + }
> + /* Store the new entry. */
> + txq->mp2mr[i].mp = mp;
> + txq->mp2mr[i].mr = mr;
> + txq->mp2mr[i].lkey = mr->lkey;
> + DEBUG("%p: new MR lkey for MP \"%s\" (%p): 0x%08" PRIU32,
> + (void *)txq, mp->name, (void *)mp, txq->mp2mr[i].lkey);
> + return txq->mp2mr[i].lkey;
> +}
> diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c
> index 4f899ff..f89df46 100644
> --- a/drivers/net/mlx4/mlx4_rxtx.c
> +++ b/drivers/net/mlx4/mlx4_rxtx.c
> @@ -239,63 +239,6 @@ struct pv {
> }
>
> /**
> - * Get memory region (MR) <-> memory pool (MP) association from txq->mp2mr[].
> - * Add MP to txq->mp2mr[] if it's not registered yet. If mp2mr[] is full,
> - * remove an entry first.
> - *
> - * @param txq
> - * Pointer to Tx queue structure.
> - * @param[in] mp
> - * Memory pool for which a memory region lkey must be returned.
> - *
> - * @return
> - * mr->lkey on success, (uint32_t)-1 on failure.
> - */
> -uint32_t
> -mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp)
> -{
> - unsigned int i;
> - struct ibv_mr *mr;
> -
> - for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
> - if (unlikely(txq->mp2mr[i].mp == NULL)) {
> - /* Unknown MP, add a new MR for it. */
> - break;
> - }
> - if (txq->mp2mr[i].mp == mp) {
> - assert(txq->mp2mr[i].lkey != (uint32_t)-1);
> - assert(txq->mp2mr[i].mr->lkey == txq->mp2mr[i].lkey);
> - return txq->mp2mr[i].lkey;
> - }
> - }
> - /* Add a new entry, register MR first. */
> - DEBUG("%p: discovered new memory pool \"%s\" (%p)",
> - (void *)txq, mp->name, (void *)mp);
> - mr = mlx4_mp2mr(txq->priv->pd, mp);
> - if (unlikely(mr == NULL)) {
> - DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.",
> - (void *)txq);
> - return (uint32_t)-1;
> - }
> - if (unlikely(i == RTE_DIM(txq->mp2mr))) {
> - /* Table is full, remove oldest entry. */
> - DEBUG("%p: MR <-> MP table full, dropping oldest entry.",
> - (void *)txq);
> - --i;
> - claim_zero(ibv_dereg_mr(txq->mp2mr[0].mr));
> - memmove(&txq->mp2mr[0], &txq->mp2mr[1],
> - (sizeof(txq->mp2mr) - sizeof(txq->mp2mr[0])));
> - }
> - /* Store the new entry. */
> - txq->mp2mr[i].mp = mp;
> - txq->mp2mr[i].mr = mr;
> - txq->mp2mr[i].lkey = mr->lkey;
> - DEBUG("%p: new MR lkey for MP \"%s\" (%p): 0x%08" PRIu32,
> - (void *)txq, mp->name, (void *)mp, txq->mp2mr[i].lkey);
> - return txq->mp2mr[i].lkey;
> -}
> -
> -/**
> * Posts a single work request to a send queue.
> *
> * @param txq
> diff --git a/drivers/net/mlx4/mlx4_rxtx.h b/drivers/net/mlx4/mlx4_rxtx.h
> index 7d67748..b32fcc9 100644
> --- a/drivers/net/mlx4/mlx4_rxtx.h
> +++ b/drivers/net/mlx4/mlx4_rxtx.h
> @@ -36,6 +36,7 @@
>
> #include <stdint.h>
> #include <sys/queue.h>
> +#include <assert.h>
>
> /* Verbs headers do not support -pedantic. */
> #ifdef PEDANTIC
> @@ -165,7 +166,6 @@ int mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx,
>
> /* mlx4_rxtx.c */
>
> -uint32_t mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp);
> uint16_t mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts,
> uint16_t pkts_n);
> uint16_t mlx4_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
> @@ -182,4 +182,35 @@ int mlx4_tx_queue_setup(struct rte_eth_dev *dev, uint16_t idx,
> const struct rte_eth_txconf *conf);
> void mlx4_tx_queue_release(void *dpdk_txq);
>
> +
Extra empty line, however see below.
> +/**
> + * Get memory region (MR) <-> memory pool (MP) association from txq->mp2mr[].
> + * Call mlx4_txq_add_mr() if MP is not registered yet.
> + *
> + * @param txq
> + * Pointer to Tx queue structure.
> + * @param[in] mp
> + * Memory pool for which a memory region lkey must be returned.
> + *
> + * @return
> + * mr->lkey on success, (uint32_t)-1 on failure.
> + */
> +static inline uint32_t
> +mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp)
> +{
> + unsigned int i;
> +
> + for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
> + if (unlikely(txq->mp2mr[i].mp == NULL)) {
> + /* Unknown MP, add a new MR for it. */
> + break;
> + }
> + if (txq->mp2mr[i].mp == mp) {
> + assert(txq->mp2mr[i].lkey != (uint32_t)-1);
> + assert(txq->mp2mr[i].mr->lkey == txq->mp2mr[i].lkey);
> + return txq->mp2mr[i].lkey;
> + }
> + }
> + return mlx4_txq_add_mr(txq, mp, i);
> +}
I'm OK with keeping this function as a static inline defined in a header
file, however I think mlx4_rxtx.h is not the right place for it given the
remaining MR-handling functions are declared by mlx4.h. You should move it
to that file as well (likely below mlx4_txq_add_mr()).
Note: RTE_DIM() and unlikely() require rte_common.h, NULL requires stddef.h.
While there, I think you should drop both assert() statements and the
reliance on assert.h because such statements shouldn't be part of a static
inline function not supposed to fail. Let's assume mp2mr[] is correctly
updated upstream.
(Also missing empty line here.)
> #endif /* MLX4_RXTX_H_ */
> --
> 1.8.3.1
>
Hi,
Please see inline
On Monday, October 30, 2017 4:23 PM, Adrien Mazarguil wrote:
>
> On Mon, Oct 30, 2017 at 10:07:24AM +0000, Matan Azrad wrote:
> > From: Ophir Munk <ophirmu@mellanox.com>
> >
> > Associate memory region to mempool (on data path) in a short function.
> > Handle the less common case of adding a new memory region to mempool
> > in a separate function.
> >
> > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
>
> Thanks for moving that function out of the way, a few more comments
> below.
>
> By the way, commit log lines can be up to 75 characters wide, only the title
> line is limited to 50.
>
Commit message updated to be longer
> > ---
> > drivers/net/mlx4/mlx4.h | 2 ++
> > drivers/net/mlx4/mlx4_mr.c | 46
> +++++++++++++++++++++++++++++++++++
> > drivers/net/mlx4/mlx4_rxtx.c | 57
> > --------------------------------------------
> > drivers/net/mlx4/mlx4_rxtx.h | 33 ++++++++++++++++++++++++-
> > 4 files changed, 80 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h index
> > e0a9853..70cf453 100644
> > --- a/drivers/net/mlx4/mlx4.h
> > +++ b/drivers/net/mlx4/mlx4.h
> > @@ -160,5 +160,7 @@ int mlx4_flow_ctrl_set(struct rte_eth_dev *dev,
> > /* mlx4_mr.c */
> >
> > struct ibv_mr *mlx4_mp2mr(struct ibv_pd *pd, struct rte_mempool *mp);
> > +uint32_t mlx4_txq_add_mr(struct txq *txq, struct rte_mempool *mp,
> > + uint32_t i);
> >
> > #endif /* RTE_PMD_MLX4_H_ */
> > diff --git a/drivers/net/mlx4/mlx4_mr.c b/drivers/net/mlx4/mlx4_mr.c
> > index 9700884..ebf6547 100644
> > --- a/drivers/net/mlx4/mlx4_mr.c
> > +++ b/drivers/net/mlx4/mlx4_mr.c
> > @@ -56,6 +56,7 @@
> > #include <rte_mempool.h>
> >
> > #include "mlx4_utils.h"
> > +#include "mlx4_rxtx.h"
>
> Alphabetical order is preferred, or so I thought. Nothing seems to enforce
> this in the coding rules, it's only a suggestion.
Suggestion accepted
>
> >
> > struct mlx4_check_mempool_data {
> > int ret;
> > @@ -181,3 +182,48 @@ struct ibv_mr *
> > rte_errno = errno ? errno : EINVAL;
> > return mr;
> > }
> > +
> > +/**
> > + * Add memory region (MR) <-> memory pool (MP) association to txq-
> >mp2mr[].
> > + * If mp2mr[] is full, remove an entry first.
> > + *
> > + * @param txq
> > + * Pointer to Tx queue structure.
> > + * @param[in] mp
> > + * Memory pool for which a memory region lkey must be added
>
> Missing "."
Added "."
>
> > + * @param[in] i
> > + * Index in memory pool (MP) where to add memory region (MR)
>
> Same here.
Done
>
> > + *
> > + * @return
> > + * Added mr->lkey on success, (uint32_t)-1 on failure.
> > + */
> > +uint32_t mlx4_txq_add_mr(struct txq *txq, struct rte_mempool *mp,
> > +uint32_t i)
>
> Coding style again; return type must be on a separate line for definitions (not
> declarations).
>
> > +{
> > + struct ibv_mr *mr;
> > +
> > + /* Add a new entry, register MR first. */
> > + DEBUG("%p: discovered new memory pool \"%s\" (%p)",
> > + (void *)txq, mp->name, (void *)mp);
> > + mr = mlx4_mp2mr(txq->priv->pd, mp);
> > + if (unlikely(mr == NULL)) {
> > + DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.",
> > + (void *)txq);
> > + return (uint32_t)-1;
> > + }
> > + if (unlikely(i == RTE_DIM(txq->mp2mr))) {
> > + /* Table is full, remove oldest entry. */
> > + DEBUG("%p: MR <-> MP table full, dropping oldest entry.",
> > + (void *)txq);
> > + --i;
> > + claim_zero(ibv_dereg_mr(txq->mp2mr[0].mr));
> > + memmove(&txq->mp2mr[0], &txq->mp2mr[1],
> > + (sizeof(txq->mp2mr) - sizeof(txq->mp2mr[0])));
> > + }
> > + /* Store the new entry. */
> > + txq->mp2mr[i].mp = mp;
> > + txq->mp2mr[i].mr = mr;
> > + txq->mp2mr[i].lkey = mr->lkey;
> > + DEBUG("%p: new MR lkey for MP \"%s\" (%p): 0x%08" PRIU32,
> > + (void *)txq, mp->name, (void *)mp, txq->mp2mr[i].lkey);
> > + return txq->mp2mr[i].lkey;
> > +}
> > diff --git a/drivers/net/mlx4/mlx4_rxtx.c
> > b/drivers/net/mlx4/mlx4_rxtx.c index 4f899ff..f89df46 100644
> > --- a/drivers/net/mlx4/mlx4_rxtx.c
> > +++ b/drivers/net/mlx4/mlx4_rxtx.c
> > @@ -239,63 +239,6 @@ struct pv {
> > }
> >
> > /**
> > - * Get memory region (MR) <-> memory pool (MP) association from txq-
> >mp2mr[].
> > - * Add MP to txq->mp2mr[] if it's not registered yet. If mp2mr[] is
> > full,
> > - * remove an entry first.
> > - *
> > - * @param txq
> > - * Pointer to Tx queue structure.
> > - * @param[in] mp
> > - * Memory pool for which a memory region lkey must be returned.
> > - *
> > - * @return
> > - * mr->lkey on success, (uint32_t)-1 on failure.
> > - */
> > -uint32_t
> > -mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp) -{
> > - unsigned int i;
> > - struct ibv_mr *mr;
> > -
> > - for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
> > - if (unlikely(txq->mp2mr[i].mp == NULL)) {
> > - /* Unknown MP, add a new MR for it. */
> > - break;
> > - }
> > - if (txq->mp2mr[i].mp == mp) {
> > - assert(txq->mp2mr[i].lkey != (uint32_t)-1);
> > - assert(txq->mp2mr[i].mr->lkey == txq-
> >mp2mr[i].lkey);
> > - return txq->mp2mr[i].lkey;
> > - }
> > - }
> > - /* Add a new entry, register MR first. */
> > - DEBUG("%p: discovered new memory pool \"%s\" (%p)",
> > - (void *)txq, mp->name, (void *)mp);
> > - mr = mlx4_mp2mr(txq->priv->pd, mp);
> > - if (unlikely(mr == NULL)) {
> > - DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.",
> > - (void *)txq);
> > - return (uint32_t)-1;
> > - }
> > - if (unlikely(i == RTE_DIM(txq->mp2mr))) {
> > - /* Table is full, remove oldest entry. */
> > - DEBUG("%p: MR <-> MP table full, dropping oldest entry.",
> > - (void *)txq);
> > - --i;
> > - claim_zero(ibv_dereg_mr(txq->mp2mr[0].mr));
> > - memmove(&txq->mp2mr[0], &txq->mp2mr[1],
> > - (sizeof(txq->mp2mr) - sizeof(txq->mp2mr[0])));
> > - }
> > - /* Store the new entry. */
> > - txq->mp2mr[i].mp = mp;
> > - txq->mp2mr[i].mr = mr;
> > - txq->mp2mr[i].lkey = mr->lkey;
> > - DEBUG("%p: new MR lkey for MP \"%s\" (%p): 0x%08" PRIu32,
> > - (void *)txq, mp->name, (void *)mp, txq->mp2mr[i].lkey);
> > - return txq->mp2mr[i].lkey;
> > -}
> > -
> > -/**
> > * Posts a single work request to a send queue.
> > *
> > * @param txq
> > diff --git a/drivers/net/mlx4/mlx4_rxtx.h
> > b/drivers/net/mlx4/mlx4_rxtx.h index 7d67748..b32fcc9 100644
> > --- a/drivers/net/mlx4/mlx4_rxtx.h
> > +++ b/drivers/net/mlx4/mlx4_rxtx.h
> > @@ -36,6 +36,7 @@
> >
> > #include <stdint.h>
> > #include <sys/queue.h>
> > +#include <assert.h>
> >
> > /* Verbs headers do not support -pedantic. */ #ifdef PEDANTIC @@
> > -165,7 +166,6 @@ int mlx4_rx_queue_setup(struct rte_eth_dev *dev,
> > uint16_t idx,
> >
> > /* mlx4_rxtx.c */
> >
> > -uint32_t mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp);
> > uint16_t mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts,
> > uint16_t pkts_n);
> > uint16_t mlx4_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, @@
> > -182,4 +182,35 @@ int mlx4_tx_queue_setup(struct rte_eth_dev *dev,
> uint16_t idx,
> > const struct rte_eth_txconf *conf); void
> > mlx4_tx_queue_release(void *dpdk_txq);
> >
> > +
>
> Extra empty line, however see below.
>
Extra line removed
> > +/**
> > + * Get memory region (MR) <-> memory pool (MP) association from txq-
> >mp2mr[].
> > + * Call mlx4_txq_add_mr() if MP is not registered yet.
> > + *
> > + * @param txq
> > + * Pointer to Tx queue structure.
> > + * @param[in] mp
> > + * Memory pool for which a memory region lkey must be returned.
> > + *
> > + * @return
> > + * mr->lkey on success, (uint32_t)-1 on failure.
> > + */
> > +static inline uint32_t
> > +mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp) {
> > + unsigned int i;
> > +
> > + for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
> > + if (unlikely(txq->mp2mr[i].mp == NULL)) {
> > + /* Unknown MP, add a new MR for it. */
> > + break;
> > + }
> > + if (txq->mp2mr[i].mp == mp) {
> > + assert(txq->mp2mr[i].lkey != (uint32_t)-1);
> > + assert(txq->mp2mr[i].mr->lkey == txq-
> >mp2mr[i].lkey);
> > + return txq->mp2mr[i].lkey;
> > + }
> > + }
> > + return mlx4_txq_add_mr(txq, mp, i);
> > +}
>
> I'm OK with keeping this function as a static inline defined in a header file,
> however I think mlx4_rxtx.h is not the right place for it given the remaining
> MR-handling functions are declared by mlx4.h. You should move it to that file
> as well (likely below mlx4_txq_add_mr()).
>
> Note: RTE_DIM() and unlikely() require rte_common.h, NULL requires
> stddef.h.
>
Please note that moving mlx4_txq_mp2mr() to mlx4.h requires to "#include mlx4_rxtx.h" in mlx4.h
(because of accessing struct txq fields).
At the same time mlx4_rxtx.h requires to "#include mlx4.h" because it uses definitions such as
MLX4_RSS_HASH_KEY_SIZE.
End result is that mlx4.h and mlx4_rxtx.h include each other.
It does not compile without rewriting code (domino effect) which I don't think is a desirable result
of this review (unless you specify otherwise).
Therefore for v4 I have left mlx4_txq_mp2mr() in mlx4_rxtx.h
> While there, I think you should drop both assert() statements and the reliance
> on assert.h because such statements shouldn't be part of a static inline
> function not supposed to fail. Let's assume mp2mr[] is correctly updated
> upstream.
>
assert() - removed
> (Also missing empty line here.)
>
Empty line added
> > #endif /* MLX4_RXTX_H_ */
> > --
> > 1.8.3.1
> >
>
> --
> Adrien Mazarguil
> 6WIND
@@ -160,5 +160,7 @@ int mlx4_flow_ctrl_set(struct rte_eth_dev *dev,
/* mlx4_mr.c */
struct ibv_mr *mlx4_mp2mr(struct ibv_pd *pd, struct rte_mempool *mp);
+uint32_t mlx4_txq_add_mr(struct txq *txq, struct rte_mempool *mp,
+ uint32_t i);
#endif /* RTE_PMD_MLX4_H_ */
@@ -56,6 +56,7 @@
#include <rte_mempool.h>
#include "mlx4_utils.h"
+#include "mlx4_rxtx.h"
struct mlx4_check_mempool_data {
int ret;
@@ -181,3 +182,48 @@ struct ibv_mr *
rte_errno = errno ? errno : EINVAL;
return mr;
}
+
+/**
+ * Add memory region (MR) <-> memory pool (MP) association to txq->mp2mr[].
+ * If mp2mr[] is full, remove an entry first.
+ *
+ * @param txq
+ * Pointer to Tx queue structure.
+ * @param[in] mp
+ * Memory pool for which a memory region lkey must be added
+ * @param[in] i
+ * Index in memory pool (MP) where to add memory region (MR)
+ *
+ * @return
+ * Added mr->lkey on success, (uint32_t)-1 on failure.
+ */
+uint32_t mlx4_txq_add_mr(struct txq *txq, struct rte_mempool *mp, uint32_t i)
+{
+ struct ibv_mr *mr;
+
+ /* Add a new entry, register MR first. */
+ DEBUG("%p: discovered new memory pool \"%s\" (%p)",
+ (void *)txq, mp->name, (void *)mp);
+ mr = mlx4_mp2mr(txq->priv->pd, mp);
+ if (unlikely(mr == NULL)) {
+ DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.",
+ (void *)txq);
+ return (uint32_t)-1;
+ }
+ if (unlikely(i == RTE_DIM(txq->mp2mr))) {
+ /* Table is full, remove oldest entry. */
+ DEBUG("%p: MR <-> MP table full, dropping oldest entry.",
+ (void *)txq);
+ --i;
+ claim_zero(ibv_dereg_mr(txq->mp2mr[0].mr));
+ memmove(&txq->mp2mr[0], &txq->mp2mr[1],
+ (sizeof(txq->mp2mr) - sizeof(txq->mp2mr[0])));
+ }
+ /* Store the new entry. */
+ txq->mp2mr[i].mp = mp;
+ txq->mp2mr[i].mr = mr;
+ txq->mp2mr[i].lkey = mr->lkey;
+ DEBUG("%p: new MR lkey for MP \"%s\" (%p): 0x%08" PRIU32,
+ (void *)txq, mp->name, (void *)mp, txq->mp2mr[i].lkey);
+ return txq->mp2mr[i].lkey;
+}
@@ -239,63 +239,6 @@ struct pv {
}
/**
- * Get memory region (MR) <-> memory pool (MP) association from txq->mp2mr[].
- * Add MP to txq->mp2mr[] if it's not registered yet. If mp2mr[] is full,
- * remove an entry first.
- *
- * @param txq
- * Pointer to Tx queue structure.
- * @param[in] mp
- * Memory pool for which a memory region lkey must be returned.
- *
- * @return
- * mr->lkey on success, (uint32_t)-1 on failure.
- */
-uint32_t
-mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp)
-{
- unsigned int i;
- struct ibv_mr *mr;
-
- for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
- if (unlikely(txq->mp2mr[i].mp == NULL)) {
- /* Unknown MP, add a new MR for it. */
- break;
- }
- if (txq->mp2mr[i].mp == mp) {
- assert(txq->mp2mr[i].lkey != (uint32_t)-1);
- assert(txq->mp2mr[i].mr->lkey == txq->mp2mr[i].lkey);
- return txq->mp2mr[i].lkey;
- }
- }
- /* Add a new entry, register MR first. */
- DEBUG("%p: discovered new memory pool \"%s\" (%p)",
- (void *)txq, mp->name, (void *)mp);
- mr = mlx4_mp2mr(txq->priv->pd, mp);
- if (unlikely(mr == NULL)) {
- DEBUG("%p: unable to configure MR, ibv_reg_mr() failed.",
- (void *)txq);
- return (uint32_t)-1;
- }
- if (unlikely(i == RTE_DIM(txq->mp2mr))) {
- /* Table is full, remove oldest entry. */
- DEBUG("%p: MR <-> MP table full, dropping oldest entry.",
- (void *)txq);
- --i;
- claim_zero(ibv_dereg_mr(txq->mp2mr[0].mr));
- memmove(&txq->mp2mr[0], &txq->mp2mr[1],
- (sizeof(txq->mp2mr) - sizeof(txq->mp2mr[0])));
- }
- /* Store the new entry. */
- txq->mp2mr[i].mp = mp;
- txq->mp2mr[i].mr = mr;
- txq->mp2mr[i].lkey = mr->lkey;
- DEBUG("%p: new MR lkey for MP \"%s\" (%p): 0x%08" PRIu32,
- (void *)txq, mp->name, (void *)mp, txq->mp2mr[i].lkey);
- return txq->mp2mr[i].lkey;
-}
-
-/**
* Posts a single work request to a send queue.
*
* @param txq
@@ -36,6 +36,7 @@
#include <stdint.h>
#include <sys/queue.h>
+#include <assert.h>
/* Verbs headers do not support -pedantic. */
#ifdef PEDANTIC
@@ -165,7 +166,6 @@ int mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx,
/* mlx4_rxtx.c */
-uint32_t mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp);
uint16_t mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts,
uint16_t pkts_n);
uint16_t mlx4_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
@@ -182,4 +182,35 @@ int mlx4_tx_queue_setup(struct rte_eth_dev *dev, uint16_t idx,
const struct rte_eth_txconf *conf);
void mlx4_tx_queue_release(void *dpdk_txq);
+
+/**
+ * Get memory region (MR) <-> memory pool (MP) association from txq->mp2mr[].
+ * Call mlx4_txq_add_mr() if MP is not registered yet.
+ *
+ * @param txq
+ * Pointer to Tx queue structure.
+ * @param[in] mp
+ * Memory pool for which a memory region lkey must be returned.
+ *
+ * @return
+ * mr->lkey on success, (uint32_t)-1 on failure.
+ */
+static inline uint32_t
+mlx4_txq_mp2mr(struct txq *txq, struct rte_mempool *mp)
+{
+ unsigned int i;
+
+ for (i = 0; (i != RTE_DIM(txq->mp2mr)); ++i) {
+ if (unlikely(txq->mp2mr[i].mp == NULL)) {
+ /* Unknown MP, add a new MR for it. */
+ break;
+ }
+ if (txq->mp2mr[i].mp == mp) {
+ assert(txq->mp2mr[i].lkey != (uint32_t)-1);
+ assert(txq->mp2mr[i].mr->lkey == txq->mp2mr[i].lkey);
+ return txq->mp2mr[i].lkey;
+ }
+ }
+ return mlx4_txq_add_mr(txq, mp, i);
+}
#endif /* MLX4_RXTX_H_ */