From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-eopbgr150043.outbound.protection.outlook.com [40.107.15.43]) by dpdk.org (Postfix) with ESMTP id DED39239 for ; Fri, 8 Mar 2019 17:31:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=vM1L95WkpVcroAeqcXZhiMnimRmfyU634FNfdxIRvYQ=; b=PTzQ9jUHPYo9Qdk8uhq1jzvDwI9dSJKcLf4zhgwNsAIzBm3Fpfbvu2VVMEO3/Tdn5vfl//ToFfQ4CBn7/7zV3375bptYGZaDcrHUcnpVBaZBo5ddLSHiRQzBmr3rfGqcwcJ9DkHr/tMlzVT2Gi4Ox4TCzteUvZxHsPcfXJW7dJM= Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com (52.134.72.27) by DB3PR0502MB3962.eurprd05.prod.outlook.com (52.134.67.161) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1665.19; Fri, 8 Mar 2019 16:31:29 +0000 Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::580c:ae7b:8278:cc50]) by DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::580c:ae7b:8278:cc50%3]) with mapi id 15.20.1686.019; Fri, 8 Mar 2019 16:31:29 +0000 From: Yongseok Koh To: Tiwei Bie CC: dpdk stable , Maxime Coquelin , "zhihong.wang@intel.com" , Olivier Matz Thread-Topic: [PATCH 17.11] net/virtio: fix resuming port with Rx vector path Thread-Index: AQHU1ZyFK+PoF8GlqEO8/JXFRjPpgaYB7YiA Date: Fri, 8 Mar 2019 16:31:29 +0000 Message-ID: <9AE9C5A1-D8B9-4968-B5E0-EB360776CD79@mellanox.com> References: <20190308104805.30001-1-tiwei.bie@intel.com> In-Reply-To: <20190308104805.30001-1-tiwei.bie@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=yskoh@mellanox.com; x-originating-ip: [209.116.155.178] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: bdf76ec8-f56c-4194-7905-08d6a3e3846d x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020); SRVR:DB3PR0502MB3962; x-ms-traffictypediagnostic: DB3PR0502MB3962: x-microsoft-exchange-diagnostics: =?us-ascii?Q?1; DB3PR0502MB3962; 23:Lyj8EINKG7obnhCoZxZoFz7SechZ16dXWsrJPEs?= =?us-ascii?Q?lu1uilfFLYtpWn65pRrYe1WcEZtvj38w5lNXhcrKyhhyCpDK3UAj8Y2xi8VR?= =?us-ascii?Q?sdNDLjLkv6K4vVDOJAknDM7vKJ3x2otI8fTfb1aZZ/r3FMZ0GUdlqOwbCCLG?= =?us-ascii?Q?YHc3DcXwUtOFBrIYp4fLNBkKdx/jIB7Yeym9/KihbvFX6xO9RC2fiNoM/8EJ?= =?us-ascii?Q?TX4drvZNcAqi4veisiiAYYPlMN7iiQWkQZXUwo57ltYTEIWpDh8bg4KZUqWp?= =?us-ascii?Q?CkyIjYgMavp2RnOwK1zoBAh15WSfD09goy+h5592FSz0Mz5cLSfP3N4reYpj?= =?us-ascii?Q?KHzUNj2m2SiGQIN7ARKKZM4Hba25e3DI0Tczjqg0HfgkUaq6a4OKAS1doea7?= =?us-ascii?Q?5uMynPS8oHP0bZc8hVUtVOPoJoBtJtHxKjuyiix3uOSdl0ApuVT+3DIvxYxK?= =?us-ascii?Q?8DQ5FlQKxqiaMWPNWD5dzOBPc8JUnFZ/MkBx6xVTbhESsW2t9v0i1HHHdGId?= =?us-ascii?Q?C84OItILwVVHlI+fBBwVYeshCDGxQ5a1tBRv78fdYiMK3JsIcY39FH02Y9P2?= =?us-ascii?Q?RWF5e3E1giVWzfz05iW07tj+3GXEYc85K3RYgPmRmohXJvNWPEP25zMeRJG1?= =?us-ascii?Q?ghqOGNNsA9yJKgnDGs9R3EY39p2oS16NXA+cXf5qQQrTqo8BuZPGUhv7Zakh?= =?us-ascii?Q?LRAaznGjNsC9DtfRiFqJbqid2kFVABdhGOOIMwWN8/HpxGSSwvobJvysnhrI?= =?us-ascii?Q?vJHCHjBGY4uDW9SxwdJ2F4uatElMB9ndOnpLoqbTgKWOP5++LGi+ChxQPC11?= =?us-ascii?Q?1gjoN00rtgq7qicCYPCgRXFACaePtOEjcsUgLfOV+2O7kZUuSmeDYoWodNy3?= =?us-ascii?Q?NJCyhjjUciIHoGGblRGGlJoEkI6M73UjGT8EyWIT+7qiPoBHiNygay36r9U8?= =?us-ascii?Q?K2AJoWbJeGNicCtFr0YuEJWdssgU/MKmKDHZXO6AvNyQkboWzpHDnGNvZHPh?= =?us-ascii?Q?Gis6nEHDXr+0zR2f5ACkA6vHAMQDn5WRs+NifffYeEi5L8pWGr3jc+z6GW+x?= =?us-ascii?Q?Vi171SU6QBBRYfCsTNO3sWEx4xOUJbF4/5lDG3A+bluTAFo5AOuDRfjX8pDf?= =?us-ascii?Q?D0ncuAHoGCSAeaqZyix9GAaj2qnEZk/xOPCvjTGPm/gr6heyKWm/GSDCTlNp?= =?us-ascii?Q?MBwjXYIsMFceULq6yDGtE4cT5X4DHt/5k2rJNkRKaJRC0ln7JMH8N6qThNEu?= =?us-ascii?Q?AHIA+ezGPwOc+edhkSAaG3sKGLrsnPvWy0XcfZJxs?= x-microsoft-antispam-prvs: x-forefront-prvs: 0970508454 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(376002)(39860400002)(136003)(396003)(366004)(199004)(189003)(14444005)(6916009)(6436002)(3846002)(186003)(6512007)(305945005)(66066001)(54906003)(6116002)(8936002)(4326008)(83716004)(229853002)(25786009)(6486002)(7736002)(82746002)(33656002)(316002)(256004)(86362001)(68736007)(71200400001)(14454004)(476003)(71190400001)(5660300002)(81166006)(8676002)(6506007)(36756003)(106356001)(26005)(53546011)(76176011)(102836004)(446003)(11346002)(486006)(81156014)(99286004)(2616005)(2906002)(53936002)(6246003)(478600001)(97736004)(105586002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB3962; H:DB3PR0502MB3980.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: SN9OFmWM3X8aL7b8oQoxBJe3AnH/e6YsxOgQjmb9JFAWekH9GTromCZNnfXYyebCvFcss2Mplwdn2XziFSqHLJ6CdF6f4QYdcc25/I+0nFy9QJUOP3i14uk6igIYGSpgOVOh7Bm/qwGUcS+inTB/RrvQ1QpC0Hhk0mpAi3DZpjIuBhQ0V0S2L0++q4VP81dv0DN1w9lmLJCt6lohYixOYyCdPY0pquOe9HboWr2y3v2XHjnpkrenP8wFd61rizLofkp7Ygw7114xFXA/szqDNf8FIDb590qHiFQa9zUuWsLn+bdMOHEUEokbY90irxLcJE5g6wGHkYU9CE05w8z90OUBUSOQm8DQ9fhi3Uvxs+ymQonGeKt6rb11miTvT4D94hmNWPnawYU1C5UAepK8LeRasJQikvklqqNAHgUV8Nk= Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: bdf76ec8-f56c-4194-7905-08d6a3e3846d X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Mar 2019 16:31:29.3504 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0502MB3962 Subject: Re: [dpdk-stable] [PATCH 17.11] net/virtio: fix resuming port with Rx vector path X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Mar 2019 16:31:31 -0000 > On Mar 8, 2019, at 2:48 AM, Tiwei Bie wrote: >=20 > [ backported from upstream commit 478574706638ff78cbc7e82731a4eae743322ac= 6 ] >=20 > Since commit efc83a1e7fc3 ("net/virtio: fix queue setup consistency"), > when resuming a virtio port, the rx rings are refilled with new mbufs > until they are full (vq->vq_free_cnt =3D=3D 0). This is done without > ensuring that the descriptor index remains a multiple of > RTE_VIRTIO_VPMD_RX_REARM_THRESH, which is a prerequisite when using the > vector mode. This can cause an out of bound access in the rx ring. >=20 > This commit changes the vector refill method from > virtqueue_enqueue_recv_refill_simple() to virtio_rxq_rearm_vec(), which > properly checks that the refill is done by batch of > RTE_VIRTIO_VPMD_RX_REARM_THRESH. >=20 > As virtqueue_enqueue_recv_refill_simple() is no more used, this > patch also removes the function. >=20 > Fixes: efc83a1e7fc3 ("net/virtio: fix queue setup consistency") >=20 > Signed-off-by: Maxime Coquelin > Signed-off-by: Olivier Matz > Signed-off-by: Tiwei Bie > Reviewed-by: Jianfeng Tan > --- > Hi Yongseok, >=20 > This fix patch is backported from v18.02, and targets for v17.11.6. > It fixes a port resuming issue for Rx vector path in virtio PMD. >=20 > Thanks, > Tiwei Applied to stable/17.11. Thanks, Yongseok > drivers/net/virtio/virtio_rxtx.c | 38 ++++++++++++++----------- > drivers/net/virtio/virtio_rxtx.h | 3 -- > drivers/net/virtio/virtio_rxtx_simple.c | 30 +------------------ > drivers/net/virtio/virtio_rxtx_simple.h | 2 +- > 4 files changed, 23 insertions(+), 50 deletions(-) >=20 > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio= _rxtx.c > index 390c137c8..275b65f43 100644 > --- a/drivers/net/virtio/virtio_rxtx.c > +++ b/drivers/net/virtio/virtio_rxtx.c > @@ -59,6 +59,7 @@ > #include "virtio_pci.h" > #include "virtqueue.h" > #include "virtio_rxtx.h" > +#include "virtio_rxtx_simple.h" >=20 > #ifdef RTE_LIBRTE_VIRTIO_DEBUG_DUMP > #define VIRTIO_DUMP_PACKET(m, len) rte_pktmbuf_dump(stdout, m, len) > @@ -465,6 +466,8 @@ virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *= dev, uint16_t queue_idx) > vq->vq_ring.desc[desc_idx].flags =3D > VRING_DESC_F_WRITE; > } > + > + virtio_rxq_vec_setup(rxvq); > } >=20 > memset(&rxvq->fake_mbuf, 0, sizeof(rxvq->fake_mbuf)); > @@ -474,30 +477,31 @@ virtio_dev_rx_queue_setup_finish(struct rte_eth_dev= *dev, uint16_t queue_idx) > &rxvq->fake_mbuf; > } >=20 > - while (!virtqueue_full(vq)) { > - m =3D rte_mbuf_raw_alloc(rxvq->mpool); > - if (m =3D=3D NULL) > - break; > + if (hw->use_simple_rx) { > + while (vq->vq_free_cnt >=3D RTE_VIRTIO_VPMD_RX_REARM_THRESH) { > + virtio_rxq_rearm_vec(rxvq); > + nbufs +=3D RTE_VIRTIO_VPMD_RX_REARM_THRESH; > + } > + } else { > + while (!virtqueue_full(vq)) { > + m =3D rte_mbuf_raw_alloc(rxvq->mpool); > + if (m =3D=3D NULL) > + break; >=20 > - /* Enqueue allocated buffers */ > - if (hw->use_simple_rx) > - error =3D virtqueue_enqueue_recv_refill_simple(vq, m); > - else > + /* Enqueue allocated buffers */ > error =3D virtqueue_enqueue_recv_refill(vq, m); > - > - if (error) { > - rte_pktmbuf_free(m); > - break; > + if (error) { > + rte_pktmbuf_free(m); > + break; > + } > + nbufs++; > } > - nbufs++; > + > + vq_update_avail_idx(vq); > } >=20 > - vq_update_avail_idx(vq); > - > PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs); >=20 > - virtio_rxq_vec_setup(rxvq); > - > VIRTQUEUE_DUMP(vq); >=20 > return 0; > diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio= _rxtx.h > index 54f1e849b..4143551a1 100644 > --- a/drivers/net/virtio/virtio_rxtx.h > +++ b/drivers/net/virtio/virtio_rxtx.h > @@ -88,7 +88,4 @@ struct virtnet_ctl { >=20 > int virtio_rxq_vec_setup(struct virtnet_rx *rxvq); >=20 > -int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq, > - struct rte_mbuf *m); > - > #endif /* _VIRTIO_RXTX_H_ */ > diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio= /virtio_rxtx_simple.c > index b5bc1c49f..86737e416 100644 > --- a/drivers/net/virtio/virtio_rxtx_simple.c > +++ b/drivers/net/virtio/virtio_rxtx_simple.c > @@ -56,34 +56,6 @@ > #pragma GCC diagnostic ignored "-Wcast-qual" > #endif >=20 > -int __attribute__((cold)) > -virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq, > - struct rte_mbuf *cookie) > -{ > - struct vq_desc_extra *dxp; > - struct vring_desc *start_dp; > - uint16_t desc_idx; > - > - cookie->port =3D vq->rxq.port_id; > - > - desc_idx =3D vq->vq_avail_idx & (vq->vq_nentries - 1); > - dxp =3D &vq->vq_descx[desc_idx]; > - dxp->cookie =3D (void *)cookie; > - vq->sw_ring[desc_idx] =3D cookie; > - > - start_dp =3D vq->vq_ring.desc; > - start_dp[desc_idx].addr =3D > - VIRTIO_MBUF_ADDR(cookie, vq) + > - RTE_PKTMBUF_HEADROOM - vq->hw->vtnet_hdr_size; > - start_dp[desc_idx].len =3D cookie->buf_len - > - RTE_PKTMBUF_HEADROOM + vq->hw->vtnet_hdr_size; > - > - vq->vq_free_cnt--; > - vq->vq_avail_idx++; > - > - return 0; > -} > - > uint16_t > virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts, > uint16_t nb_pkts) > @@ -106,7 +78,7 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbu= f **tx_pkts, > rte_compiler_barrier(); >=20 > if (nb_used >=3D VIRTIO_TX_FREE_THRESH) > - virtio_xmit_cleanup(vq); > + virtio_xmit_cleanup_simple(vq); >=20 > nb_commit =3D nb_pkts =3D RTE_MIN((vq->vq_free_cnt >> 1), nb_pkts); > desc_idx =3D (uint16_t)(vq->vq_avail_idx & desc_idx_max); > diff --git a/drivers/net/virtio/virtio_rxtx_simple.h b/drivers/net/virtio= /virtio_rxtx_simple.h > index f531c5428..5f6dabc8a 100644 > --- a/drivers/net/virtio/virtio_rxtx_simple.h > +++ b/drivers/net/virtio/virtio_rxtx_simple.h > @@ -89,7 +89,7 @@ virtio_rxq_rearm_vec(struct virtnet_rx *rxvq) > #define VIRTIO_TX_FREE_NR 32 > /* TODO: vq->tx_free_cnt could mean num of free slots so we could avoid s= hift */ > static inline void > -virtio_xmit_cleanup(struct virtqueue *vq) > +virtio_xmit_cleanup_simple(struct virtqueue *vq) > { > uint16_t i, desc_idx; > uint32_t nb_free =3D 0; > --=20 > 2.17.1 >=20