vhost: return error message for mbuf allocation failure

Message ID 20191016143107.10424-1-i.maximets@ovn.org (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: return error message for mbuf allocation failure |

Checks

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

Commit Message

Ilya Maximets Oct. 16, 2019, 2:31 p.m. UTC
  mbuf allocation failure is a hard failure that highlights some
significant issues with memory pool size or a mbuf leak.

We still have the message for subsequent chained mbufs, but not
for the first one.  It was removed while introducing extbuf
support for large buffers.  But it was useful for catching
mempool issues and needs to be returned back.

Cc: Flavio Leitner <fbl@sysclose.org>

Fixes: 5005bcda7123 ("vhost: add support for large buffers")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 lib/librte_vhost/virtio_net.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
  

Comments

Flavio Leitner Oct. 16, 2019, 3:02 p.m. UTC | #1
On Wed, 16 Oct 2019 16:31:07 +0200
Ilya Maximets <i.maximets@ovn.org> wrote:

> mbuf allocation failure is a hard failure that highlights some
> significant issues with memory pool size or a mbuf leak.
> 
> We still have the message for subsequent chained mbufs, but not
> for the first one.  It was removed while introducing extbuf
> support for large buffers.  But it was useful for catching
> mempool issues and needs to be returned back.
> 
> Cc: Flavio Leitner <fbl@sysclose.org>
> 
> Fixes: 5005bcda7123 ("vhost: add support for large buffers")
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>


Reviewed-by: Flavio Leitner <fbl@sysclose.org>
Thanks Ilya
fbl

> ---
>  lib/librte_vhost/virtio_net.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/virtio_net.c
> b/lib/librte_vhost/virtio_net.c index 66f0c7206..f8af4e0b3 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -1354,8 +1354,11 @@ virtio_dev_pktmbuf_alloc(struct virtio_net
> *dev, struct rte_mempool *mp, {
>  	struct rte_mbuf *pkt = rte_pktmbuf_alloc(mp);
>  
> -	if (unlikely(pkt == NULL))
> +	if (unlikely(pkt == NULL)) {
> +		RTE_LOG(ERR, VHOST_DATA,
> +			"Failed to allocate memory for mbuf.\n");
>  		return NULL;
> +	}
>  
>  	if (rte_pktmbuf_tailroom(pkt) >= data_len)
>  		return pkt;
  
Tiwei Bie Oct. 17, 2019, 4:43 a.m. UTC | #2
On Wed, Oct 16, 2019 at 04:31:07PM +0200, Ilya Maximets wrote:
> mbuf allocation failure is a hard failure that highlights some
> significant issues with memory pool size or a mbuf leak.
> 
> We still have the message for subsequent chained mbufs, but not
> for the first one.  It was removed while introducing extbuf
> support for large buffers.  But it was useful for catching
> mempool issues and needs to be returned back.
> 
> Cc: Flavio Leitner <fbl@sysclose.org>
> 
> Fixes: 5005bcda7123 ("vhost: add support for large buffers")
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
>  lib/librte_vhost/virtio_net.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

Thanks,
Tiwei
  
Maxime Coquelin Oct. 24, 2019, 9:39 a.m. UTC | #3
On 10/16/19 4:31 PM, Ilya Maximets wrote:
> mbuf allocation failure is a hard failure that highlights some
> significant issues with memory pool size or a mbuf leak.
> 
> We still have the message for subsequent chained mbufs, but not
> for the first one.  It was removed while introducing extbuf
> support for large buffers.  But it was useful for catching
> mempool issues and needs to be returned back.
> 
> Cc: Flavio Leitner <fbl@sysclose.org>
> 
> Fixes: 5005bcda7123 ("vhost: add support for large buffers")
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
>  lib/librte_vhost/virtio_net.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks!
Maxime
  
Maxime Coquelin Oct. 24, 2019, 10:18 a.m. UTC | #4
On 10/24/19 11:39 AM, Maxime Coquelin wrote:
> 
> 
> On 10/16/19 4:31 PM, Ilya Maximets wrote:
>> mbuf allocation failure is a hard failure that highlights some
>> significant issues with memory pool size or a mbuf leak.
>>
>> We still have the message for subsequent chained mbufs, but not
>> for the first one.  It was removed while introducing extbuf
>> support for large buffers.  But it was useful for catching
>> mempool issues and needs to be returned back.
>>
>> Cc: Flavio Leitner <fbl@sysclose.org>
>>
>> Fixes: 5005bcda7123 ("vhost: add support for large buffers")
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>> ---
>>  lib/librte_vhost/virtio_net.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks!
> Maxime
> 


Applied to dpdk-next-virtio/master.

Thanks,
Maxime
  

Patch

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 66f0c7206..f8af4e0b3 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -1354,8 +1354,11 @@  virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,
 {
 	struct rte_mbuf *pkt = rte_pktmbuf_alloc(mp);
 
-	if (unlikely(pkt == NULL))
+	if (unlikely(pkt == NULL)) {
+		RTE_LOG(ERR, VHOST_DATA,
+			"Failed to allocate memory for mbuf.\n");
 		return NULL;
+	}
 
 	if (rte_pktmbuf_tailroom(pkt) >= data_len)
 		return pkt;