From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0071.outbound.protection.outlook.com [104.47.1.71]) by dpdk.org (Postfix) with ESMTP id 59A3D7CEA for ; Wed, 7 Mar 2018 07:00:46 +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; bh=dco5vZD7uyIpjglt9rcK8Q+QzdyQIPZUk4fO5wAMDg8=; b=H0IThQFA7weoedy3mZEHlLRqiJQlwWZvkI9wRoqqaFytNIpRUV75KyMFegka28aetfvMyFhfvNOYsCWmlBP/dZzmfhULS/98vsZLax4MG4atC0MJX1HpI8tid8kqumgjC2rsdnw26fVdDiS/UpI2Co7KaAGRBErsyR3WDxHWNoE= Received: from AM4PR0501MB2657.eurprd05.prod.outlook.com (10.172.215.19) by AM4PR0501MB2675.eurprd05.prod.outlook.com (10.172.215.136) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.548.13; Wed, 7 Mar 2018 06:00:44 +0000 Received: from AM4PR0501MB2657.eurprd05.prod.outlook.com ([fe80::79a1:db23:42bd:25e2]) by AM4PR0501MB2657.eurprd05.prod.outlook.com ([fe80::79a1:db23:42bd:25e2%11]) with mapi id 15.20.0567.012; Wed, 7 Mar 2018 06:00:44 +0000 From: Matan Azrad To: "Tan, Jianfeng" , "Yigit, Ferruh" CC: "Richardson, Bruce" , "Ananyev, Konstantin" , Thomas Monjalon , "maxime.coquelin@redhat.com" , "Burakov, Anatoly" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 3/4] drivers/net: do not allocate rte_eth_dev_data privately Thread-Index: AQHTs82FdkxSJ5ATaEq3udX6FuwTJqPCtKJwgAA1ooCAAV994A== Date: Wed, 7 Mar 2018 06:00:44 +0000 Message-ID: References: <1520177405-59091-1-git-send-email-jianfeng.tan@intel.com> <1520177405-59091-4-git-send-email-jianfeng.tan@intel.com> In-Reply-To: Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM4PR0501MB2675; 7:5NFKx8f8QYx5A2Mws2HHf828VPWq6eSHGOZFomUZvHjsTkCam2NdbB29N1PJpqYfnuWsyVIJ19Hal2NUGuH5FWqch0p29N6MBn4mKP8sHh5wwjzbTeqO1kiXJhczvPapomrnhsyRVAnDovx7q5pBQE8AoKn101yNa1mKzQKUgacBMf2RLSkLFCLFfFM7w6mormtlFXmjep+gvN4iqk57H+ddJwtATjwloMobN2ZHQMYzFc7Dex0PHsC2zn3NnG4T x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: 55d87a39-804e-458c-0b60-08d583f0c420 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(5600026)(4604075)(3008032)(4534165)(4627221)(201703031133081)(201702281549075)(48565401081)(2017052603328)(7153060)(7193020); SRVR:AM4PR0501MB2675; x-ms-traffictypediagnostic: AM4PR0501MB2675: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(189930954265078)(45079756050767)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040501)(2401047)(8121501046)(5005006)(3002001)(3231220)(944501244)(52105095)(93006095)(93001095)(10201501046)(6055026)(6041288)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123560045)(20161123558120)(6072148)(201708071742011); SRVR:AM4PR0501MB2675; BCL:0; PCL:0; RULEID:; SRVR:AM4PR0501MB2675; x-forefront-prvs: 0604AFA86B x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39380400002)(346002)(366004)(39850400004)(396003)(376002)(199004)(13464003)(189003)(305945005)(3660700001)(33656002)(99286004)(106356001)(2950100002)(5250100002)(4326008)(68736007)(86362001)(7736002)(76176011)(3846002)(6116002)(7696005)(66066001)(6436002)(6246003)(53936002)(5660300001)(97736004)(229853002)(9686003)(74316002)(6306002)(55016002)(81166006)(81156014)(14454004)(3280700002)(45080400002)(2906002)(316002)(2900100001)(26005)(102836004)(6506007)(93886005)(53546011)(478600001)(186003)(110136005)(105586002)(25786009)(54906003)(966005)(8936002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR0501MB2675; H:AM4PR0501MB2657.eurprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: cXegKMaMSJVTll87zz8T3BthoZBmZKU9EOENwB1PGpeYt65hLQAkzd1bLk9AMpIYdf0SJGGJMiFx0PYl4iac33m44ISxUXp7WQmNnd3t2RoU6kA1r6vKcm6px9HKlq7VJWpT//U3uYuNfCf0eZYDmyNISB42aQQfRia6nhv0yJTEoIXfr+XksgsG9qmss9PofMIYJq2DRNlThtsLw+Tnl0+I11gu5UXdCInvpO7G+MYqiUUzFDm8buBpS4P2tHTqET/BzIZfwtLxzMZFF7fCnuYqYPUTbprNdfVLHxb49YRiXdm4/ud0yRiZxEgDuaogmWG52ULVSoZG5EZ9o9Z4+Q== spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 55d87a39-804e-458c-0b60-08d583f0c420 X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Mar 2018 06:00:44.7506 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0501MB2675 Subject: Re: [dpdk-dev] [PATCH 3/4] drivers/net: do not allocate rte_eth_dev_data privately X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Mar 2018 06:00:46 -0000 Hi Jianfeng From: Tan, Jianfeng, Sent: Tuesday, March 6, 2018 10:56 AM > > -----Original Message----- > > From: Matan Azrad [mailto:matan@mellanox.com] > > Sent: Tuesday, March 6, 2018 2:08 PM > > To: Tan, Jianfeng; Yigit, Ferruh > > Cc: Richardson, Bruce; Ananyev, Konstantin; Thomas Monjalon; > > maxime.coquelin@redhat.com; Burakov, Anatoly; dev@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH 3/4] drivers/net: do not allocate > > rte_eth_dev_data privately > > > > Hi Jianfeng > > > > Please see a comment below. > > > > > From: Jianfeng Tan, Sent: Sunday, March 4, 2018 5:30 PM We > > > introduced private rte_eth_dev_data to allow vdev to be created both > > in > > > primary process and secondary process(es). This is not friendly to > > > multi- process model, for example, it leads to port id contention > > > issue if two processes both find the data entry is free. > > > > > > And to get stats of primary vdev in secondary, we must allocate from > > > the pre-defined array so that we can find it. > > > > > > Suggested-by: Bruce Richardson > > > Signed-off-by: Jianfeng Tan > > > --- > > > drivers/net/af_packet/rte_eth_af_packet.c | 25 +++++++--------------= ---- > > > drivers/net/kni/rte_eth_kni.c | 13 ++----------- > > > drivers/net/null/rte_eth_null.c | 17 +++-------------- > > > drivers/net/octeontx/octeontx_ethdev.c | 14 ++------------ > > > drivers/net/pcap/rte_eth_pcap.c | 18 +++--------------- > > > drivers/net/tap/rte_eth_tap.c | 9 +-------- > > > drivers/net/vhost/rte_eth_vhost.c | 17 ++--------------- > > > 7 files changed, 20 insertions(+), 93 deletions(-) > > > > > > diff --git a/drivers/net/af_packet/rte_eth_af_packet.c > > > b/drivers/net/af_packet/rte_eth_af_packet.c > > > index 57eccfd..2db692f 100644 > > > --- a/drivers/net/af_packet/rte_eth_af_packet.c > > > +++ b/drivers/net/af_packet/rte_eth_af_packet.c > > > @@ -564,25 +564,17 @@ rte_pmd_init_internals(struct rte_vdev_device > > > *dev, > > > RTE_LOG(ERR, PMD, > > > "%s: no interface specified for AF_PACKET > ethdev\n", > > > name); > > > - goto error_early; > > > + return -1; > > > } > > > > > > RTE_LOG(INFO, PMD, > > > "%s: creating AF_PACKET-backed ethdev on numa socket > %u\n", > > > name, numa_node); > > > > > > - /* > > > - * now do all data allocation - for eth_dev structure, dummy pci > > > driver > > > - * and internal (private) data > > > - */ > > > - data =3D rte_zmalloc_socket(name, sizeof(*data), 0, numa_node); > > > - if (data =3D=3D NULL) > > > - goto error_early; > > > - > > > *internals =3D rte_zmalloc_socket(name, sizeof(**internals), > > > 0, numa_node); > > > if (*internals =3D=3D NULL) > > > - goto error_early; > > > + return -1; > > > > > > for (q =3D 0; q < nb_queues; q++) { > > > (*internals)->rx_queue[q].map =3D MAP_FAILED; @@ -604,24 > > > +596,24 @@ rte_pmd_init_internals(struct rte_vdev_device *dev, > > > RTE_LOG(ERR, PMD, > > > "%s: I/F name too long (%s)\n", > > > name, pair->value); > > > - goto error_early; > > > + return -1; > > > } > > > if (ioctl(sockfd, SIOCGIFINDEX, &ifr) =3D=3D -1) { > > > RTE_LOG(ERR, PMD, > > > "%s: ioctl failed (SIOCGIFINDEX)\n", > > > name); > > > - goto error_early; > > > + return -1; > > > } > > > (*internals)->if_name =3D strdup(pair->value); > > > if ((*internals)->if_name =3D=3D NULL) > > > - goto error_early; > > > + return -1; > > > (*internals)->if_index =3D ifr.ifr_ifindex; > > > > > > if (ioctl(sockfd, SIOCGIFHWADDR, &ifr) =3D=3D -1) { > > > RTE_LOG(ERR, PMD, > > > "%s: ioctl failed (SIOCGIFHWADDR)\n", > > > name); > > > - goto error_early; > > > + return -1; > > > } > > > memcpy(&(*internals)->eth_addr, ifr.ifr_hwaddr.sa_data, > ETH_ALEN); > > > > > > @@ -775,14 +767,13 @@ rte_pmd_init_internals(struct rte_vdev_device > > > *dev, > > > > > > (*internals)->nb_queues =3D nb_queues; > > > > > > - rte_memcpy(data, (*eth_dev)->data, sizeof(*data)); > > > + data =3D (*eth_dev)->data; > > > data->dev_private =3D *internals; > > > data->nb_rx_queues =3D (uint16_t)nb_queues; > > > data->nb_tx_queues =3D (uint16_t)nb_queues; > > > data->dev_link =3D pmd_link; > > > data->mac_addrs =3D &(*internals)->eth_addr; > > > > > > - (*eth_dev)->data =3D data; > > > (*eth_dev)->dev_ops =3D &ops; > > > > > > return 0; > > > @@ -802,8 +793,6 @@ rte_pmd_init_internals(struct rte_vdev_device > > *dev, > > > } > > > free((*internals)->if_name); > > > rte_free(*internals); > > > -error_early: > > > - rte_free(data); > > > return -1; > > > } > > > > > > > I think you should remove the private rte_eth_dev_data freeing in > > rte_pmd_af_packet_remove(). > > This is relevant to all the vdevs here. >=20 > Ah, yes, you are correct. I will fix that in v2. >=20 > > > > Question: > > Does the patch include all the vdevs which allocated private > > rte_eth_dev_data? >=20 > Yes, we are removing all private rte_eth_dev_data. If I missed some devic= e, > welcome to point out. net/ring > > If so, it may solve also part of the issue discussed here: > > > https://emea01.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fdp > d > > > k.org%2Fdev%2Fpatchwork%2Fpatch%2F34047%2F&data=3D02%7C01%7Cmata > n%40mell > > > anox.com%7C4143e70010774a15672708d583401618%7Ca652971c7d2e4d9ba6 > a4d149 > > > 256f461b%7C0%7C0%7C636559233645410291&sdata=3DG1pYHEXENP3low8oziaI > KsxiHB > > mlEjV1f89LMZmnzvc%3D&reserved=3D0 >=20 > Yes, related. We now allocate rte_eth_dev_data which can be indexed by al= l > primary/secondary processes. >=20 > Thanks, > Jianfeng