From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 4FEBFA04B8
	for <public@inbox.dpdk.org>; Tue,  5 May 2020 13:56:21 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 259511D5F1;
	Tue,  5 May 2020 13:56:21 +0200 (CEST)
Received: from mga12.intel.com (mga12.intel.com [192.55.52.136])
 by dpdk.org (Postfix) with ESMTP id 82B261D5D4;
 Tue,  5 May 2020 13:56:17 +0200 (CEST)
IronPort-SDR: TT6QKfqe/XixHr+dCm56MiXdnWPxh7oHbt7cpFj6Lo4x+QTdhM7B3n++XsxmzlhoYEh2PARSoW
 mFQTVwOG6eog==
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga005.jf.intel.com ([10.7.209.41])
 by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 05 May 2020 04:56:15 -0700
IronPort-SDR: 8g5fqHdy2/YrhCTRnAZaqp9sh5+n4nJN69XLORL93amepeg0SJPMprvtYIscyRk66mZU5guJSo
 MVvdWUIxsa8A==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.73,354,1583222400"; d="scan'208";a="434456584"
Received: from pgsmsx111.gar.corp.intel.com ([10.108.55.200])
 by orsmga005.jf.intel.com with ESMTP; 05 May 2020 04:56:13 -0700
Received: from pgsmsx112.gar.corp.intel.com ([169.254.3.217]) by
 PGSMSX111.gar.corp.intel.com ([169.254.2.69]) with mapi id 14.03.0439.000;
 Tue, 5 May 2020 19:56:13 +0800
From: "Tummala, Sivaprasad" <sivaprasad.tummala@intel.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>, Flavio Leitner
 <fbl@sysclose.org>
CC: "Wang, Zhihong" <zhihong.wang@intel.com>, "Ye, Xiaolong"
 <xiaolong.ye@intel.com>, "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org"
 <stable@dpdk.org>
Thread-Topic: [PATCH v2] vhost: fix mbuf alloc failure
Thread-Index: AQHWIkrTpvfY9vJu+kqieVUVJg9WoaiY4aOA//+/3QCAAMDk8A==
Date: Tue, 5 May 2020 11:56:12 +0000
Message-ID: <FDDE0085FF3F264C97717D963DBD9EA229F3FD9F@PGSMSX112.gar.corp.intel.com>
References: <20200428095203.64935-1-Sivaprasad.Tummala@intel.com>
 <20200504171118.93782-1-Sivaprasad.Tummala@intel.com>
 <20200504193240.GA92333@p50.lan>
 <FDDE0085FF3F264C97717D963DBD9EA229F3F93B@PGSMSX112.gar.corp.intel.com>
 <cb6a1e09-e6fb-f2e4-8d9b-3b22c60ea398@redhat.com>
In-Reply-To: <cb6a1e09-e6fb-f2e4-8d9b-3b22c60ea398@redhat.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTgwM2UyYTgtNzQzYS00MjMyLWE3NjgtYjIwNzUzNjMwNzJmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiZUFsWXJzd3k4VTVzMUFyRWFBTWJVaDh5RnZXT21cL1BqRTRaWFNveXk1ZjBEcDBhSys0eXViTDNZNmFBRlVvMnIifQ==
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.2.0.6
dlp-reaction: no-action
x-originating-ip: [172.30.20.205]
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-stable] [PATCH v2] vhost: fix mbuf alloc failure
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>

Hi Maxime,=20

Thanks for your comments.=20

SNIPPED

if (allocerr_warned) {
>=20
> =A0=A0=A0 VHOST_LOG_DATA(ERR,
>=20
> =A0=A0=A0 "Failed to allocate memory for mbuf. Packet dropped!\n");
>=20
> =A0=A0=A0 allocerr_warned =3D true;
>=20
> }
>=20
> =A0
>=20
> This is good idea, but having a static variable makes it file scope=20
> making it to =A0entire VHOST devices. Hence if the intention is to=20
> implement device specific
>=20
> log rate limit, should not we resort to `dev->allocerr_warn` counter=20
> mechanism, which resets after n failures `#define LOG_ALLOCFAIL 32`.

I prefer Flavio's proposal, it would have less performance impact than incr=
easing struct virtio_net size. As soon as we can see the error popping once=
 in the log message, it gives some clues on what to investigate. Maybe prov=
iding more details on the failure could help, like printing the pool name a=
nd the requested length.

Agreed. Change in the next patch, sample format as `VHOST_DATA : Failed mbu=
f alloc of size 2054 from mbuf_pool_socket_0 on /tmp/vhost1.`

SNIPPED