From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <matan@mellanox.com>
Received: from EUR02-AM5-obe.outbound.protection.outlook.com
(mail-eopbgr00042.outbound.protection.outlook.com [40.107.0.42])
by dpdk.org (Postfix) with ESMTP id B937F1BDDE
for <dev@dpdk.org>; Tue, 26 Jun 2018 18:54:36 +0200 (CEST)
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=VTKUPKtphIJt6zRUA1fEVQiw92eUr9VUEDL1x8bopRk=;
b=xDWAHS622du1qt+MxsAJkR25CV/4dBEpgutlWOkBFC4XPJLeSSSe1efwx3Xt95ZfO5Ck41V3N578ejUi4tguf0pI4pKDZ6X4uR6oMPkJscH2SoXECuxz0fc2oYBMdPosWECclvfENehduKr3dRv3oBGPpoyO6sCZkZmw3tcMdrE=
Received: from VI1PR0501MB2608.eurprd05.prod.outlook.com (10.168.137.20) by
VI1PR0501MB2111.eurprd05.prod.outlook.com (10.167.196.15) with Microsoft SMTP
Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
15.20.863.19; Tue, 26 Jun 2018 16:54:33 +0000
Received: from VI1PR0501MB2608.eurprd05.prod.outlook.com
([fe80::9dd0:9bdb:fd59:b615]) by VI1PR0501MB2608.eurprd05.prod.outlook.com
([fe80::9dd0:9bdb:fd59:b615%7]) with mapi id 15.20.0884.024; Tue, 26 Jun 2018
16:54:33 +0000
From: Matan Azrad <matan@mellanox.com>
To: "Zhang, Qi Z" <qi.z.zhang@intel.com>, Thomas Monjalon
<thomas@monjalon.net>, "Burakov, Anatoly" <anatoly.burakov@intel.com>
CC: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "dev@dpdk.org"
<dev@dpdk.org>, "Richardson, Bruce" <bruce.richardson@intel.com>, "Yigit,
Ferruh" <ferruh.yigit@intel.com>, "Shelton, Benjamin H"
<benjamin.h.shelton@intel.com>, "Vangati, Narender"
<narender.vangati@intel.com>
Thread-Topic: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to release port
in local process
Thread-Index: AQHUDRyBOOZkWOgLrEmS0dpaVlkVNqRygFFggAAIlQCAADW0kA==
Date: Tue, 26 Jun 2018 16:54:33 +0000
Message-ID: <VI1PR0501MB260879DD114286F860ACCFCAD2490@VI1PR0501MB2608.eurprd05.prod.outlook.com>
References: <20180607123849.14439-1-qi.z.zhang@intel.com>
<20180626070832.3055-1-qi.z.zhang@intel.com>
<20180626070832.3055-4-qi.z.zhang@intel.com>
<VI1PR0501MB26085CAE828E1500BA75EC55D2490@VI1PR0501MB2608.eurprd05.prod.outlook.com>
<039ED4275CED7440929022BC67E706115323E812@SHSMSX103.ccr.corp.intel.com>
In-Reply-To: <039ED4275CED7440929022BC67E706115323E812@SHSMSX103.ccr.corp.intel.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: [85.64.136.190]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; VI1PR0501MB2111;
7:Msk72eLySAAs0jwMekVmOg8XVUM8PDyvgcmZ96S1d9ErmXAz6ehIgjClGGSbwk8JS8cGCr1r296uXJIPtXX1yTRcYPiQ/aGqBpJpRfPQrfyMynDOGlsJF35126Q2Jlp5hMsucUWytkvlx40e0cWHnW6FQEnfjMrq3fM9/c7EkMnHGve3frAW3pGbO0WpddStiz1xEx1zFngcIaxpErVF/1z6yMVU/MJtmyTDLXWf7jNkQFJe0C6QBskeUEAUYQNi
x-ms-exchange-antispam-srfa-diagnostics: SOS;
x-ms-office365-filtering-correlation-id: b50e2305-1c50-469e-841e-08d5db857e36
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: UriScan:; BCL:0; PCL:0;
RULEID:(7020095)(4652020)(8989117)(4534165)(4627221)(201703031133081)(201702281549075)(8990107)(5600026)(711020)(48565401081)(2017052603328)(7153060)(7193020);
SRVR:VI1PR0501MB2111;
x-ms-traffictypediagnostic: VI1PR0501MB2111:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <VI1PR0501MB2111D49857FF1438DA47158BD2490@VI1PR0501MB2111.eurprd05.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(60795455431006)(228905959029699);
x-ms-exchange-senderadcheck: 1
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0;
RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3231254)(944501410)(52105095)(93006095)(93001095)(10201501046)(3002001)(6055026)(149027)(150027)(6041310)(20161123558120)(20161123560045)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(6072148)(201708071742011)(7699016);
SRVR:VI1PR0501MB2111; BCL:0; PCL:0; RULEID:; SRVR:VI1PR0501MB2111;
x-forefront-prvs: 071518EF63
x-forefront-antispam-report: SFV:NSPM;
SFS:(10009020)(366004)(346002)(39860400002)(376002)(136003)(396003)(13464003)(189003)(199004)(229853002)(478600001)(76176011)(86362001)(26005)(186003)(2906002)(5250100002)(66066001)(99286004)(110136005)(6436002)(14444005)(5024004)(54906003)(3846002)(316002)(53546011)(6506007)(102836004)(7696005)(33656002)(6116002)(476003)(4326008)(25786009)(9686003)(11346002)(486006)(53936002)(6246003)(14454004)(93886005)(55016002)(81156014)(105586002)(81166006)(106356001)(68736007)(7736002)(446003)(305945005)(8936002)(74316002)(5660300001)(97736004)(256004)(2900100001);
DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR0501MB2111;
H:VI1PR0501MB2608.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en;
PTR:InfoNoRecords; MX:1; A:1;
received-spf: None (protection.outlook.com: mellanox.com does not designate
permitted sender hosts)
x-microsoft-antispam-message-info: K+91kSfsBgMeI0hVHn0Xn5y9lznVGuKnmjunxVJayanWamVAuPsrfPVJUw9F3SUtRvrsn3zs3djniabiE2CyI/JUNqyqsCJuD2q23miZ9EnDqUlBAIhHUAwouv/E8STC4W24cGx+dDGd5NAD8ZGrzQsk4ym2rgAu/NkldTNCuaxKBhRrMI2ox+MotZLQWk5svpssYgoTc27QVojsGSr/RKFUN71B44MWQnUH9kXIOmh0BneIlaS9JfT+RVaLuNMuNFmAZ/smSQqc3LmGVfM2JNqxun2h6owR+7VGrZiZPis4GPrrvIPEBwhMX9fHhlH4LimLrWc34sttmK3MEHGPpwZTUOkG2kBJ5UH+i05OBYg=
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: b50e2305-1c50-469e-841e-08d5db857e36
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Jun 2018 16:54:33.6022 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0501MB2111
Subject: Re: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to release
port in local process
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://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 26 Jun 2018 16:54:37 -0000
Hi Zhang
From: Zhang, Qi Z=20
> Sent: Tuesday, June 26, 2018 4:30 PM
> To: Matan Azrad <matan@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Shelton, Benjamin H
> <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to release =
port
> in local process
>=20
>=20
>=20
> > -----Original Message-----
> > From: Zhang, Qi Z
> > Sent: Tuesday, June 26, 2018 9:29 PM
> > To: 'Matan Azrad' <matan@mellanox.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Burakov, Anatoly <anatoly.burakov@intel.com>
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> > Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; Shelton, Benjamin H
> > <benjamin.h.shelton@intel.com>; Vangati, Narender
> > <narender.vangati@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to
> > release port in local process
> >
> > Hi Matan:
> >
> > > -----Original Message-----
> > > From: Matan Azrad [mailto:matan@mellanox.com]
> > > Sent: Tuesday, June 26, 2018 7:50 PM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Burakov, Anatoly <anatoly.burakov@intel.com>
> > > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> > > dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; Yigit,
> > > Ferruh <ferruh.yigit@intel.com>; Shelton, Benjamin H
> > > <benjamin.h.shelton@intel.com>; Vangati, Narender
> > > <narender.vangati@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to
> > > release port in local process
> > >
> > > Hi Qi
> > >
> > > Please see comments\questions..
> > >
> > > From: Qi Zhang
> > > > Add driver API rte_eth_release_port_private to support the
> > > > requirement that an ethdev only be released on secondary process,
> > > > so only local state be set to unused , share data will not be
> > > > reset so primary process can
> > > still use it.
> > > >
> > > > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > ---
> > > >
> > > > lib/librte_ethdev/rte_ethdev.c | 24 +++++++++++++++++++++--=
-
> > > > lib/librte_ethdev/rte_ethdev_driver.h | 13 +++++++++++++
> > > > 2 files changed, 34 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/lib/librte_ethdev/rte_ethdev.c
> > > > b/lib/librte_ethdev/rte_ethdev.c index a9977df97..205b2ee33 100644
> > > > --- a/lib/librte_ethdev/rte_ethdev.c
> > > > +++ b/lib/librte_ethdev/rte_ethdev.c
> > > > @@ -359,6 +359,23 @@ rte_eth_dev_attach_secondary(const char
> > > *name) }
> > > >
> > > > int
> > > > +rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev) {
> > > > + if (eth_dev =3D=3D NULL)
> > > > + return -EINVAL;
> > > > +
> > > > + _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_DESTROY,
> > > > NULL);
> > > > +
> > > > + rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
> > >
> > > I don't think you need the lock here because there is not shared
> > > data to
> > reset.
> >
> > OK, will remove this.
> >
> > >
> > > > + eth_dev->state =3D RTE_ETH_DEV_UNUSED;
> > > > +
> > > > + rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +int
> > > > rte_eth_dev_release_port(struct rte_eth_dev *eth_dev) {
> > > > if (eth_dev =3D=3D NULL)
> > > > @@ -370,9 +387,10 @@ rte_eth_dev_release_port(struct rte_eth_dev
> > > > *eth_dev)
> > > >
> > > > rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
> > > >
> > > > - eth_dev->state =3D RTE_ETH_DEV_UNUSED;
> > > > -
> > > > - memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data));
> > > > + if (eth_dev->state !=3D RTE_ETH_DEV_UNUSED) {
> > >
> > > Can you explain why not to release the shared data in case of
> > > locally unused port?
> >
> > Now, we have rte_eth_dev_release_port be called in rte_eth_dev_detach,
> > its redundant for some driver, because they already will release port
> > in
> > dev->remove, but I'm not sure if it is true for all drivers.
rte_eth_dev_release_port() like detach port should be called only by the po=
rt owner, by default is the application, so it should not be called by the =
PMD, maybe need to fix PMDs which does it.
> >
> > On secondary process, when detach a device, it is possible that
> > rte_eth_dev_release_port be called after
> > rte_eth_dev_release_port_local , so it will reset share data which is
> > not expected, since the primary process will fail on detach it because
> rte_eth_dev_allocated will return NULL.
> >
> > There could be other way, but current implementation help to reuse
> > exist code., I can't add more comment on this change in v5
>=20
> I can add more comment
This solution is suffering from much more complexity,
The port id should be the same in all the processes, so you can pass it by =
the mp channel and not use rte_eth_dev_allocated.
I see it like this:
The initiator process asks from all the other processes to detach a port by=
port id, all the other processes using rte_eth_dev_release_port_local and =
sending ack back to the initiator,
Then, the initiator using the non-local release to release the port.
=20
Am I missing something?
> >
> > Regards
> > Qi
> >
> > >
> > > > + eth_dev->state =3D RTE_ETH_DEV_UNUSED;
> > > > + memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data));
> > > > + }
> > > >
> > > > rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
> > > >
> > > > diff --git a/lib/librte_ethdev/rte_ethdev_driver.h
> > > > b/lib/librte_ethdev/rte_ethdev_driver.h
> > > > index c9c825e3f..49c27223d 100644
> > > > --- a/lib/librte_ethdev/rte_ethdev_driver.h
> > > > +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> > > > @@ -70,6 +70,19 @@ int rte_eth_dev_release_port(struct rte_eth_dev
> > > > *eth_dev);
> > > >
> > > > /**
> > > > * @internal
> > > > + * Release the specified ethdev port in local process, only set
> > > > +to ethdev
> > > > + * state to unused, but not reset share data since it assume
> > > > +other process
> > > > + * is still using it, typically it is called by secondary process.
> > > > + *
> > > > + * @param eth_dev
> > > > + * The *eth_dev* pointer is the address of the *rte_eth_dev* struc=
ture.
> > > > + * @return
> > > > + * - 0 on success, negative on error
> > > > + */
> > > > +int rte_eth_dev_release_port_private(struct rte_eth_dev
> > > > +*eth_dev);
> > > > +
> > > > +/**
> > > > + * @internal
> > > > * Release device queues and clear its configuration to force the =
user
> > > > * application to reconfigure it. It is for internal use only.
> > > > *
> > > > --
> > > > 2.13.6