[dpdk-dev] fix ceph async messenger runtime bug based on spdk/dpdk
Checks
Commit Message
From: chunmei <chunmei.liu@intel.com>
ceph async messenger has some run time error with this dpdk library,
1) need set rxm->next= null at end of packet since not init it to null
when allocate a buffer other wise rte_mbuf_sanity_check will report error.
2) when check the size, can't calculate mbuf_data_room_size
because async messenger dpdk will allocate this part later
not at create mempool.
Signed-off-by: chunmei Liu <chunmei.liu@intel.com>
---
drivers/net/ixgbe/ixgbe_rxtx.c | 3 ++-
lib/librte_mbuf/rte_mbuf.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
Comments
14/12/2017 00:44, chunmei Liu:
> From: chunmei <chunmei.liu@intel.com>
>
> ceph async messenger has some run time error with this dpdk library,
> 1) need set rxm->next= null at end of packet since not init it to null
> when allocate a buffer other wise rte_mbuf_sanity_check will report error.
> 2) when check the size, can't calculate mbuf_data_room_size
> because async messenger dpdk will allocate this part later
> not at create mempool.
>
> Signed-off-by: chunmei Liu <chunmei.liu@intel.com>
Hi Chunmei,
Several comments about the formatting:
I've already notified you that you should Cc Olivier for mbuf changes.
You should also Cc Wenzhuo and Konstantin for ixgbe change.
Please add them in your next emails.
The title should show what is fixed, not the use-case.
When sending a new revision of the patch, you should increment
the version number and add a changelog below the ---.
Please use --in-reply-to when sending a new version, in order
to keep track of all versions in the same thread.
Your author name and SoB name should be the same
(your full name with uppercases):
Chunmei Liu <chunmei.liu@intel.com>
For more informations, you should read this:
http://dpdk.org/dev#send
Thank you
Hi Chunmei,
On Wed, Dec 13, 2017 at 03:44:55PM -0800, chunmei Liu wrote:
> From: chunmei <chunmei.liu@intel.com>
>
> ceph async messenger has some run time error with this dpdk library,
> 1) need set rxm->next= null at end of packet since not init it to null
> when allocate a buffer other wise rte_mbuf_sanity_check will report error.
> 2) when check the size, can't calculate mbuf_data_room_size
> because async messenger dpdk will allocate this part later
> not at create mempool.
>
> Signed-off-by: chunmei Liu <chunmei.liu@intel.com>
> ---
> drivers/net/ixgbe/ixgbe_rxtx.c | 3 ++-
> lib/librte_mbuf/rte_mbuf.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 1e07895..918946b 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2190,7 +2190,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
> rxm->next = next_rxe->mbuf;
> next_sc_entry->fbuf = first_seg;
> goto next_desc;
> - }
> + } else
> + rxm->next = NULL;
>
> /* Initialize the first mbuf of the returned packet */
> ixgbe_fill_cluster_head_buf(first_seg, &rxd, rxq, staterr);
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 0e3e36a..c01da19 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -102,7 +102,6 @@ rte_pktmbuf_pool_init(struct rte_mempool *mp, void *opaque_arg)
> }
>
> RTE_ASSERT(mp->elt_size >= sizeof(struct rte_mbuf) +
> - user_mbp_priv->mbuf_data_room_size +
> user_mbp_priv->mbuf_priv_size);
>
> mbp_priv = rte_mempool_get_priv(mp);
I don't see the point here.
Many parts of dpdk rely on this code without issue. I enabled the
asserts and see no issue with testpmd. Can you give more details?
> @@ -233,6 +232,8 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
> }
> if (nb_segs != 0)
> rte_panic("bad nb_segs\n");
> + if (m_seg != NULL)
> + rte_panic("bad m_seg\n");
> }
A similar check was recently sumbitted in another patch. Please see:
https://dpdk.org/dev/patchwork/patch/32057/
Hi Chunmei,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of chunmei Liu
> Sent: Wednesday, December 13, 2017 11:45 PM
> To: dev@dpdk.org
> Cc: Liu, Chunmei <chunmei.liu@intel.com>
> Subject: [dpdk-dev] [PATCH] fix ceph async messenger runtime bug based on spdk/dpdk
>
> From: chunmei <chunmei.liu@intel.com>
>
> ceph async messenger has some run time error with this dpdk library,
> 1) need set rxm->next= null at end of packet since not init it to null
> when allocate a buffer other wise rte_mbuf_sanity_check will report error.
> 2) when check the size, can't calculate mbuf_data_room_size
> because async messenger dpdk will allocate this part later
> not at create mempool.
>
> Signed-off-by: chunmei Liu <chunmei.liu@intel.com>
> ---
> drivers/net/ixgbe/ixgbe_rxtx.c | 3 ++-
> lib/librte_mbuf/rte_mbuf.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 1e07895..918946b 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2190,7 +2190,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
> rxm->next = next_rxe->mbuf;
> next_sc_entry->fbuf = first_seg;
> goto next_desc;
> - }
> + } else
> + rxm->next = NULL;
Shouldn't be a problem I think, but wonder why do we need it at all?
As I remember, rte_pktmbuf_free_seg() always sets m->next to NULL.
Konstantin
>
> /* Initialize the first mbuf of the returned packet */
> ixgbe_fill_cluster_head_buf(first_seg, &rxd, rxq, staterr);
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 0e3e36a..c01da19 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -102,7 +102,6 @@ rte_pktmbuf_pool_init(struct rte_mempool *mp, void *opaque_arg)
> }
>
> RTE_ASSERT(mp->elt_size >= sizeof(struct rte_mbuf) +
> - user_mbp_priv->mbuf_data_room_size +
> user_mbp_priv->mbuf_priv_size);
>
> mbp_priv = rte_mempool_get_priv(mp);
> @@ -233,6 +232,8 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
> }
> if (nb_segs != 0)
> rte_panic("bad nb_segs\n");
> + if (m_seg != NULL)
> + rte_panic("bad m_seg\n");
> }
>
> /* dump a mbuf on console */
> --
> 2.7.4
@@ -2190,7 +2190,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
rxm->next = next_rxe->mbuf;
next_sc_entry->fbuf = first_seg;
goto next_desc;
- }
+ } else
+ rxm->next = NULL;
/* Initialize the first mbuf of the returned packet */
ixgbe_fill_cluster_head_buf(first_seg, &rxd, rxq, staterr);
@@ -102,7 +102,6 @@ rte_pktmbuf_pool_init(struct rte_mempool *mp, void *opaque_arg)
}
RTE_ASSERT(mp->elt_size >= sizeof(struct rte_mbuf) +
- user_mbp_priv->mbuf_data_room_size +
user_mbp_priv->mbuf_priv_size);
mbp_priv = rte_mempool_get_priv(mp);
@@ -233,6 +232,8 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
}
if (nb_segs != 0)
rte_panic("bad nb_segs\n");
+ if (m_seg != NULL)
+ rte_panic("bad m_seg\n");
}
/* dump a mbuf on console */