From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10082.outbound.protection.outlook.com [40.107.1.82]) by dpdk.org (Postfix) with ESMTP id E6DD72951 for ; Wed, 17 Jan 2018 09:40:02 +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=GVCSV81AybZ8ZT18QYO5c/rATkd9OxpAZMNJs4mr/Ps=; b=LxB16LDzapCQjf7u5PRSjFO1QxJ309la0R6vX7JXQ7abBXjq3IYomzrWh32TAOS6hmd8hy5RGUheS2ZpAJcbIn0QoqAiGd/zRxzOC4/bhxvnb5yCQtyadXUxDAiunsb+2siqC+YTYgFyrC46gjeWKOXSYAj4tE+D4tSBeS/xJAM= Received: from AM6PR0502MB3797.eurprd05.prod.outlook.com (52.133.21.26) by AM6PR0502MB3862.eurprd05.prod.outlook.com (52.133.21.147) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.407.7; Wed, 17 Jan 2018 08:40:01 +0000 Received: from AM6PR0502MB3797.eurprd05.prod.outlook.com ([fe80::6c28:c6b3:de94:a733]) by AM6PR0502MB3797.eurprd05.prod.outlook.com ([fe80::6c28:c6b3:de94:a733%13]) with mapi id 15.20.0407.012; Wed, 17 Jan 2018 08:40:01 +0000 From: Matan Azrad To: =?iso-8859-1?Q?Ga=EBtan_Rivet?= CC: Ferruh Yigit , Thomas Monjalon , "dev@dpdk.org" , "stephen@networkplumber.org" Thread-Topic: [PATCH v3 3/8] net/failsafe: support probed sub-devices getting Thread-Index: AQHTjrp/VHV3LzKwTE6Nrr2gSXYAm6N2Z8DQgAAqzACAAAzw8IAAGE+AgAABgXCAAFyiAIAAqbkw Date: Wed, 17 Jan 2018 08:40:00 +0000 Message-ID: References: <20171222173846.20731-1-adrien.mazarguil@6wind.com> <1515509253-17834-1-git-send-email-matan@mellanox.com> <1515509253-17834-4-git-send-email-matan@mellanox.com> <20180116110920.vqp3bqjroudsdjm4@bidouze.vm.6wind.com> <20180116144050.ho4k2dp24lgzhtdr@bidouze.vm.6wind.com> <20180116165409.a463ukxbsmtdoc2v@bidouze.vm.6wind.com> <20180116223104.y2iesdbxqkf775xp@bidouze.vm.6wind.com> In-Reply-To: <20180116223104.y2iesdbxqkf775xp@bidouze.vm.6wind.com> Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM6PR0502MB3862; 7:JACMjrZ7QBC7jmvJxU85L/yeRqMyWwATsGIPwYYrMI5bZKr2a0t1yMsKKX6CuUvm29x/2d2o/1Q+Z24Z03jsRA16PivH1dBQIY7nrfwXBoWu2Y65RnUvS9YEXaq5GwvH5vO92fw7X0LZ9wEbVn3T7EGPWI04G3PSugAYr9SeLeQ/1pv7X7wpiRqmJmTMsSnUV0CsdbuzBNXjLGxclEhP5detV5Ua/mku+rkPwKFaKfD1CoUqEn46efUkT9ux4mPh x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: dba1ee61-5efc-4c0b-6c09-08d55d85e5d9 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(3008032)(2017052603307)(7153060)(7193020); SRVR:AM6PR0502MB3862; x-ms-traffictypediagnostic: AM6PR0502MB3862: authentication-results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040470)(2401047)(5005006)(8121501046)(93006095)(93001095)(3231023)(2400043)(944501161)(10201501046)(3002001)(6055026)(6041268)(20161123560045)(20161123558120)(20161123562045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(6072148)(201708071742011); SRVR:AM6PR0502MB3862; BCL:0; PCL:0; RULEID:(100000803101)(100110400095); SRVR:AM6PR0502MB3862; x-forefront-prvs: 0555EC8317 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(39860400002)(376002)(396003)(39380400002)(366004)(189003)(199004)(76104003)(24454002)(229853002)(6506007)(6436002)(86362001)(68736007)(2906002)(5660300001)(74316002)(316002)(99286004)(105586002)(26005)(54906003)(33656002)(6116002)(3846002)(8676002)(305945005)(14454004)(4326008)(81156014)(81166006)(7736002)(25786009)(6246003)(478600001)(97736004)(93886005)(2950100002)(3280700002)(6916009)(76176011)(55016002)(9686003)(8936002)(53936002)(7696005)(3660700001)(59450400001)(106356001)(2900100001)(66066001)(5250100002)(102836004); DIR:OUT; SFP:1101; SCL:1; SRVR:AM6PR0502MB3862; H:AM6PR0502MB3797.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: C2DrPDPqCSSZJRb4Bqr2RGUJy0xc8E4w/caOH9qJ0+hqtyXQDKQqntoaVS7bY63kqbxZHpqrMjhBbdqoiZrZxw== spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: dba1ee61-5efc-4c0b-6c09-08d55d85e5d9 X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Jan 2018 08:40:01.0120 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR0502MB3862 Subject: Re: [dpdk-dev] [PATCH v3 3/8] net/failsafe: support probed sub-devices getting 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, 17 Jan 2018 08:40:03 -0000 Hi Gaetan From: Ga=EBtan Rivet, Wednesday, January 17, 2018 12:31 AM > Hi Matan, >=20 > On Tue, Jan 16, 2018 at 05:20:27PM +0000, Matan Azrad wrote: > > Hi Gaetan > > >=20 > >=20 > > > > > In 18.05, or 18.08 there should be an EAL function that would be > > > > > able to identify a device given a specific ID string (very close > > > > > to an > > > rte_devargs). > > > > > Currently, this API does not exist. > > > > > > > > > > You can hack your way around this for the moment, IF you really, > > > > > really > > > > > want: parse your devargs, get the bus, use the bus->parse() > > > > > function to get a binary device representation, and compare > > > > > bytes per bytes the binary representation given by your devargs > > > > > and by the device- > > > >name. > > > > > > > > > > But this is a hack, and a pretty ugly one at that: you have no > > > > > way of knowing the size taken by this binary representation, so > > > > > you can restrict yourself to the vdev and PCI bus for the moment > > > > > and take the larger of an rte_vdev_driver pointer and an > rte_pci_addr.... > > > > > > > > > > { > > > > > union { > > > > > rte_vdev_driver *drv; > > > > > struct rte_pci_addr pci_addr; > > > > > } bindev1, bindev2; > > > > > memset(&bindev1, 0, sizeof(bindev1)); > > > > > memset(&bindev2, 0, sizeof(bindev2)); > > > > > rte_eal_devargs_parse(device->name, da1); > > > > > rte_eal_devargs_parse(your_devstr, da2); > > > > > RTE_ASSERT(da1->bus =3D=3D rte_bus_find_by_name("pci"= ) || > > > > > da1->bus =3D=3D rte_bus_find_by_name("vdev= ")); > > > > > RTE_ASSERT(da2->bus =3D=3D rte_bus_find_by_name("pci"= ) || > > > > > da2->bus =3D=3D rte_bus_find_by_name("vdev= ")); > > > > > da1->bus->parse(da1->name, &bindev1); > > > > > da1->bus->parse(da2->name, &bindev2); > > > > > if (memcmp(&bindev1, &bindev2, sizeof(bindev1)) =3D= =3D 0) { > > > > > /* found the device */ > > > > > } else { > > > > > /* not found */ > > > > > } > > > > > } > > > > > > > > > > So, really, really ugly. Anyway. > > > > > > > > > Yes, ugly :) Thanks for this update! > > > > Will keep the comparison by device->name. > > > > > > > > > > Well as explained, above, the comparison by device->name only works > > > with whitelisted devices. > > > > > > > > So either implement something broken right now that you will need to > > > update in 18.05, or implement it properly in 18.05 from the get go. > > > > > For the current needs it is enough. > > We can also say that it is the user responsibility to pass to failsafe = the same > names and same args as he passes for EAL(or default EAL names). > > I think I emphasized it in documentation. > > >=20 > Okay, as you wish. Just be aware of this limitation. >=20 > I think this functionality is good and useful, but it needs to be made cl= ean. > The proper function should be available soon, then this implementaion > should be cleaned up. Sure. >=20 > > > > > > > > > > > > > > > > > > + /* Take control of device probed by EAL > > > options. */ > > > > > > > > + DEBUG("Taking control of a probed sub > > > device" > > > > > > > > + " %d named %s", i, da->name); > > > > > > > > > > > > > > In this case, the devargs of the probed device must be > > > > > > > copied within the sub- device definition and removed from > > > > > > > the EAL using the proper rte_devargs API. > > > > > > > > > > > > > > Note that there is no rte_devargs copy function. You can use > > > > > > > rte_devargs_parse instead, "parsing" again the original > > > > > > > devargs into the sub- device one. It is necessary for > > > > > > > complying with internal rte_devargs requirements (da->args > > > > > > > being malloc-ed, at the moment, > > > > > but may evolve). > > > > > > > > > > > > > > The rte_eal_devargs_parse function is not easy enough to use > > > > > > > right now, you will have to build a devargs string (using > > > > > > > snprintf) and > > > submit it. > > > > > > > I proposed a change this release for it but it will not make > > > > > > > it for 18.02, that would have simplified your implementation. > > > > > > > > > > > > > > > > > > > Got you. You right we need to remove the created devargs in > > > > > > fail-safe > > > > > parse level. > > > > > > What do you think about checking it in the parse level and > > > > > > avoid the new > > > > > devargs creation? > > > > > > Also to do the copy in parse level(same method as we are doing > > > > > > in probe > > > > > level)? > > > > > > > > > > > > > > > > Not sure I follow here, but the new rte_devargs is part of the > > > > > sub-device (it is not a pointer, but allocated alongside the > sub_device). > > > > > > > > > > So keep everything here, it is the right place to deal with these= things. > > > > > > > > > But it will prevent the double parsing and also saves the method: > > > > If the device already parsed - copy its devargs and continue. > > > > If the device already probed - copy the device pointer and continue= . > > > > > > > > I think this is the right dealing, no? > > > > Why to deal with parse level in probe level? Just keep all the > > > > parse work to > > > parse level and the probe work to probe level. > > > > > > After re-reading, I think we misunderstood each other. > > > You cannot remove the rte_devargs created during parsing: it is > > > allocated alongside the sub_device structure. > > > > > > You must only remove the rte_devargs allocated by the EAL (using > > > rte_eal_devargs_remove()). > > > > > > > Sure. > > > > > Before removing it, you must copy its content in the local > > > sub_device rte_devargs structure. I only proposed a way to do this > > > copy that would not deal with rte_devargs internals, as it is bound t= o > evolve rather soon. > > > > > Yes. > > > > > Otherwise, no, I do not want to complicate the parsing operations, > > > they are already too complicated and too criticals. Better to keep it= all > here. > > > > I think fs_parse_device function is not complicated and it is the natur= al > place for devargs games. > > For me this is the right place for the copy & remove devargs. > > Are you insisting to put all in fs_bus_init? >=20 > You would have to put fs_ethdev_portid_find in failsafe_args, which is > mixing layers. Sorry but yes, please keep all these changes in this file. >=20 OK, Thanks man! > Thanks, > -- > Ga=EBtan Rivet > 6WIND