From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <matan@mellanox.com>
Received: from EUR01-HE1-obe.outbound.protection.outlook.com
(mail-he1eur01on0060.outbound.protection.outlook.com [104.47.0.60])
by dpdk.org (Postfix) with ESMTP id 01CE37CFA
for <dev@dpdk.org>; Wed, 7 Mar 2018 07:10:11 +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=B4OxanYpkPk2DZqA1YKX/z/Zrs/56BEuHbwMY9PEDwE=;
b=AWucBwpNm3Xj5rdQ0xW5Z1uu6F8ZOEV233coCmbk1ecxCfhJF9fQj+ItKvPzNVK6MZdekKhIibHzPyVC6rWchgWJQ++3SDgcgq950eyi4LqOToIHQJJWG9RUIuZDIrAS33S8BDUbCxFOeuifg6FXXPpdlPcnlrksqUDqW1MP748=
Received: from AM4PR0501MB2657.eurprd05.prod.outlook.com (10.172.215.19) by
AM4PR0501MB2162.eurprd05.prod.outlook.com (10.165.45.155) 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:10:09 +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:10:09 +0000
From: Matan Azrad <matan@mellanox.com>
To: "Tan, Jianfeng" <jianfeng.tan@intel.com>, "Yigit, Ferruh"
<ferruh.yigit@intel.com>
CC: "Richardson, Bruce" <bruce.richardson@intel.com>, "Ananyev, Konstantin"
<konstantin.ananyev@intel.com>, Thomas Monjalon <thomas@monjalon.net>,
"maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>, "Burakov, Anatoly"
<anatoly.burakov@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH 3/4] drivers/net: do not allocate
rte_eth_dev_data privately
Thread-Index: AQHTtdryxPGaBBVegE+NtOA/aA66UA==
Date: Wed, 7 Mar 2018 06:10:09 +0000
Message-ID: <AM4PR0501MB265768E34CDF92F1BDF1A832D2D80@AM4PR0501MB2657.eurprd05.prod.outlook.com>
References: <1520177405-59091-1-git-send-email-jianfeng.tan@intel.com>
<1520177405-59091-4-git-send-email-jianfeng.tan@intel.com>
<AM4PR0501MB26578466FF367876668DCE40D2D90@AM4PR0501MB2657.eurprd05.prod.outlook.com>
<ED26CBA2FAD1BF48A8719AEF02201E3651463A53@SHSMSX103.ccr.corp.intel.com>
<AM4PR0501MB265753064BC2B7DDE6397B71D2D80@AM4PR0501MB2657.eurprd05.prod.outlook.com>
In-Reply-To: <AM4PR0501MB265753064BC2B7DDE6397B71D2D80@AM4PR0501MB2657.eurprd05.prod.outlook.com>
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; AM4PR0501MB2162;
7:xp9z3zmmOEF4zC4qdJbCJSKbnKgD+YSFe1krqFPMeFGJxyjpeJ+7Rr6XIo71bYAeC1eYxLgMNUhYKmc84UKvUPQDhEi2T6DLeZyfQ2SJ0U+Z52tf1JpD1/CObzEtsXU8ymcKI11R2JRuPkqQBbkrPLLwc5YDYJGUoK7ppPKXjwpzduzpy2b1t5vNr6KmarBJfGnKQe4iat6P4SKVQzJPDlzw0iuJqDMVbds4b0Cvf9fF63qWOU781GOnj9TNTaQ/
x-ms-exchange-antispam-srfa-diagnostics: SSOS;
x-ms-office365-filtering-correlation-id: 40411f80-992b-4aa4-b65b-08d583f2149f
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:AM4PR0501MB2162;
x-ms-traffictypediagnostic: AM4PR0501MB2162:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <AM4PR0501MB216243126D9DCED5094F145FD2D80@AM4PR0501MB2162.eurprd05.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(189930954265078)(45079756050767)(228905959029699);
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0;
RULEID:(8211001083)(6040501)(2401047)(5005006)(8121501046)(10201501046)(93006095)(93001095)(3002001)(3231220)(944501244)(52105095)(6055026)(6041288)(20161123558120)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123562045)(6072148)(201708071742011);
SRVR:AM4PR0501MB2162; BCL:0; PCL:0; RULEID:; SRVR:AM4PR0501MB2162;
x-forefront-prvs: 0604AFA86B
x-forefront-antispam-report: SFV:NSPM;
SFS:(10009020)(396003)(376002)(39850400004)(346002)(366004)(39380400002)(189003)(199004)(13464003)(99286004)(74316002)(53936002)(186003)(316002)(33656002)(305945005)(7696005)(26005)(6246003)(9686003)(7736002)(102836004)(3280700002)(6306002)(97736004)(68736007)(6506007)(53546011)(5660300001)(55016002)(4326008)(6436002)(66066001)(54906003)(5250100002)(25786009)(110136005)(105586002)(2950100002)(478600001)(2900100001)(3846002)(6116002)(2906002)(106356001)(966005)(2940100002)(14454004)(86362001)(76176011)(81166006)(3660700001)(229853002)(45080400002)(93886005)(81156014)(8936002);
DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR0501MB2162;
H:AM4PR0501MB2657.eurprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords;
MX:1; A:1; LANG:en;
received-spf: None (protection.outlook.com: mellanox.com does not designate
permitted sender hosts)
x-microsoft-antispam-message-info: /iQwMUOrZPL2HnxZwavqg/KnmaZDusTOvmVSj6BRQKiwnGpEvvMLpxfPwnQM5XDz8T9Ii26vAX5IyHwyPiIg5ZK8g+CvDS+fgM18Ar/JhDRVK4lWRH3zJrJ2DZdZ094dT3y7BBpe+4vGBm3T6PFfNgr8mh1WAyVBzZwXJeWUyZ3pexFHv148iBwQTvGF/v/Lx10HIoJrGsdTIgv6oxLthPZFjl9aX8ZFG+js9BazKpWEJDj7EfvpgJC65rBG7FVa6OSdPMci8MQmsvYWfqITrsdBNW+KQaCq/MD4sVNhncJ6T9KfoDkd9AAzOcHu0uJ+ToKA+RAlq9z/veu5WFFM6g==
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: 40411f80-992b-4aa4-b65b-08d583f2149f
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Mar 2018 06:10:09.2907 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0501MB2162
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 <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 07 Mar 2018 06:10:12 -0000
Hi Jianfeng
From: Matan Azrad, Wednesday, March 7, 2018 8:01 AM
> Hi Jianfeng
>=20
> 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 <bruce.richardson@intel.com>
> > > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > > > ---
> > > > 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.
> >
> > Ah, yes, you are correct. I will fix that in v2.
> >
> > >
> > > Question:
> > > Does the patch include all the vdevs which allocated private
> > > rte_eth_dev_data?
> >
> > Yes, we are removing all private rte_eth_dev_data. If I missed some
> > device, welcome to point out.
>=20
> net/ring
>=20
What is about next PCI device?
net/cxgbe
> > > 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
> >
> > Yes, related. We now allocate rte_eth_dev_data which can be indexed by
> > all primary/secondary processes.
> >
> > Thanks,
> > Jianfeng