diff mbox

[dpdk-dev] fix ceph async messenger runtime bug based on spdk/dpdk

Message ID 1513193552-159561-1-git-send-email-chunmei.liu@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

chunmei Liu Dec. 13, 2017, 7:32 p.m. UTC 
  From: chunmei <chunmei.liu@intel.com>

ceph async messenger has some run time error with this dpdk library,
1) need init mb->next= 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 | 1 +
 lib/librte_mbuf/rte_mbuf.c     | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Thomas Monjalon Dec. 13, 2017, 9:18 p.m. UTC | #1 
13/12/2017 20:32, chunmei Liu:
>  drivers/net/ixgbe/ixgbe_rxtx.c | 1 +
>  lib/librte_mbuf/rte_mbuf.c     | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)

+Cc Olivier

Please note that such a change in lib/librte_mbuf/ cannot be applied
without the approval of Olivier Matz.
Stephen Hemminger Dec. 13, 2017, 9:31 p.m. UTC | #2 
On Wed, 13 Dec 2017 11:32:32 -0800
chunmei Liu <chunmei.liu@intel.com> wrote:

> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 1e07895..30fdbc1 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -1589,6 +1589,7 @@ ixgbe_rx_alloc_bufs(struct ixgbe_rx_queue *rxq, bool reset_mbuf)
>  		}
>  
>  		rte_mbuf_refcnt_set(mb, 1);
> +		mb->next = NULL;
>  		mb->data_off = RTE_PKTMBUF_HEADROOM;
>  

This will have measurable performance impact because the next pointer is
on the second cache line.
chunmei Liu Dec. 13, 2017, 10:34 p.m. UTC | #3 
But if not initialize allocated mbuf->next = NULL, when do ip package assemble, the last mbuf->next maybe not be null, all the logical has error, the application use null pointer determine a packaged end. DPDK NIC doesn't work now.

-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org] 
Sent: Wednesday, December 13, 2017 1:31 PM
To: Liu, Chunmei <chunmei.liu@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] fix ceph async messenger runtime bug based on spdk/dpdk

On Wed, 13 Dec 2017 11:32:32 -0800
chunmei Liu <chunmei.liu@intel.com> wrote:

> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c 
> b/drivers/net/ixgbe/ixgbe_rxtx.c index 1e07895..30fdbc1 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -1589,6 +1589,7 @@ ixgbe_rx_alloc_bufs(struct ixgbe_rx_queue *rxq, bool reset_mbuf)
>  		}
>  
>  		rte_mbuf_refcnt_set(mb, 1);
> +		mb->next = NULL;
>  		mb->data_off = RTE_PKTMBUF_HEADROOM;
>  

This will have measurable performance impact because the next pointer is on the second cache line.
chunmei Liu Dec. 13, 2017, 10:46 p.m. UTC | #4 
Hold on, Let me try another place to fix this issue. 

-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org] 
Sent: Wednesday, December 13, 2017 1:31 PM
To: Liu, Chunmei <chunmei.liu@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] fix ceph async messenger runtime bug based on spdk/dpdk

On Wed, 13 Dec 2017 11:32:32 -0800
chunmei Liu <chunmei.liu@intel.com> wrote:

> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c 
> b/drivers/net/ixgbe/ixgbe_rxtx.c index 1e07895..30fdbc1 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -1589,6 +1589,7 @@ ixgbe_rx_alloc_bufs(struct ixgbe_rx_queue *rxq, bool reset_mbuf)
>  		}
>  
>  		rte_mbuf_refcnt_set(mb, 1);
> +		mb->next = NULL;
>  		mb->data_off = RTE_PKTMBUF_HEADROOM;
>  

This will have measurable performance impact because the next pointer is on the second cache line.
Stephen Hemminger Dec. 13, 2017, 10:51 p.m. UTC | #5 
On Wed, 13 Dec 2017 22:46:30 +0000
"Liu, Chunmei" <chunmei.liu@intel.com> wrote:

> Hold on, Let me try another place to fix this issue. 
> 
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org] 
> Sent: Wednesday, December 13, 2017 1:31 PM
> To: Liu, Chunmei <chunmei.liu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] fix ceph async messenger runtime bug based on spdk/dpdk
> 
> On Wed, 13 Dec 2017 11:32:32 -0800
> chunmei Liu <chunmei.liu@intel.com> wrote:
> 
> > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c 
> > b/drivers/net/ixgbe/ixgbe_rxtx.c index 1e07895..30fdbc1 100644
> > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > @@ -1589,6 +1589,7 @@ ixgbe_rx_alloc_bufs(struct ixgbe_rx_queue *rxq, bool reset_mbuf)
> >  		}
> >  
> >  		rte_mbuf_refcnt_set(mb, 1);
> > +		mb->next = NULL;
> >  		mb->data_off = RTE_PKTMBUF_HEADROOM;
> >    
> 
> This will have measurable performance impact because the next pointer is on the second cache line.

Please do not do top posting on open source mailing lists.
diff mbox

Patch

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 1e07895..30fdbc1 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1589,6 +1589,7 @@  ixgbe_rx_alloc_bufs(struct ixgbe_rx_queue *rxq, bool reset_mbuf)
 		}
 
 		rte_mbuf_refcnt_set(mb, 1);
+		mb->next = NULL;
 		mb->data_off = RTE_PKTMBUF_HEADROOM;
 
 		/* populate the descriptors */
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 */