From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id B508EA0550;
	Thu,  8 Sep 2022 01:38:27 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 5D5D740143;
	Thu,  8 Sep 2022 01:38:27 +0200 (CEST)
Received: from na01-obe.outbound.protection.outlook.com
 (mail-eastus2azon11021025.outbound.protection.outlook.com [52.101.57.25])
 by mails.dpdk.org (Postfix) with ESMTP id 74910400D6
 for <dev@dpdk.org>; Thu,  8 Sep 2022 01:38:25 +0200 (CEST)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=R6H2VPuDnVjKnuGockGh6dWR51hL6nnmjn1KE9T3V0CqEk3Xgz0n5MyvfJRIGx4Yh5RfUEDqVRChiGdHNnjhcN5vnWm/thp12akETuYzsCfm1YmVylNGP4MpGdU7gm3Ap5ofZ2++cYw6eJCrqGtnhnQY9RqNWbHQBKyddTHWuv5hbBNFD0I2GKVNtxQVnxo2veEqn0hV4wbsjwe4T4Vu1O9xNksHDiOMRiq8zWGyv56uUtePPLKikTG9HeHlH2RbcRNMjYiAXGsul1Edxw+/5Q3wjZIh8Uid/hkq77Gvuofw3wucSsS+DtR8T7t43CGxjrTjd/jih8V1FGa/Wy84dA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; 
 s=arcselector9901;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;
 bh=OH9rwlZms8eA1UK11LUXeYLFSiPnmyzpqeBMWpf8FjU=;
 b=YkXoKvKgkD3siO3tG/3aQ/+ibKYzS5XgEqls91GpAa6ow5J5DUyN9CSen2dpS4AjAuJwPpQma8TbCYbuYA793DC6LRVcrNL/HP9s1YVrBTWlsGkuMk75u251am7yLjBEmctg/S1DKhUvVcd0PdeYhbsn3h8ofiwj43HiVcxPYn4tGQO5Xr4Or72HB/GqlPnE7dGVbUuZT8weSHgWWvJwFG5bIf6LdXbbS0GuqbdazY+3C/+j+ITzJTRxnHWpduccz7ZnhP38MghshrlvkyExZMDsy5mjWciRY4NtHpYh5pnEKO7PgDoll5pakfGbcZdX+Wm5+5RmLZqZyPGzn1nBmA==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
 smtp.mailfrom=microsoft.com; dmarc=pass action=none
 header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;
 s=selector2;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=OH9rwlZms8eA1UK11LUXeYLFSiPnmyzpqeBMWpf8FjU=;
 b=J9nwBBToiA4At9g0j4kfPwtWzzajtt83P9xOyN5gaX1Z3/gJoMkDtuSvQ0hA4pKE9zSc/ZVpHANb35Xi1XD2Ou4H1qxNh6YlBjbMB6J8Gl2roJ5kAU4pAIKtrEfxQgnsKx5vPFNN+Wbd8OQcqFpZVRiFR0GxI+RWELL5Oeb8sUo=
Received: from PH7PR21MB3263.namprd21.prod.outlook.com (2603:10b6:510:1db::16)
 by PH7PR21MB3070.namprd21.prod.outlook.com (2603:10b6:510:1e2::19)
 with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5632.2; Wed, 7 Sep
 2022 23:38:23 +0000
Received: from PH7PR21MB3263.namprd21.prod.outlook.com
 ([fe80::c42c:5004:23c1:bcac]) by PH7PR21MB3263.namprd21.prod.outlook.com
 ([fe80::c42c:5004:23c1:bcac%8]) with mapi id 15.20.5632.002; Wed, 7 Sep 2022
 23:38:22 +0000
From: Long Li <longli@microsoft.com>
To: Ferruh Yigit <ferruh.yigit@xilinx.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, Ajay Sharma <sharmaajay@microsoft.com>,
 Stephen Hemminger <sthemmin@microsoft.com>
Subject: RE: [Patch v4 01/17] net/mana: add basic driver, build environment
 and doc
Thread-Topic: [Patch v4 01/17] net/mana: add basic driver, build environment
 and doc
Thread-Index: AQHYkyVjWhtjitYA3kCNG+6cd3FSMa27SneAgBmhBgA=
Date: Wed, 7 Sep 2022 23:38:22 +0000
Message-ID: <PH7PR21MB3263F7BAE13D299213949BF5CE419@PH7PR21MB3263.namprd21.prod.outlook.com>
References: <1657324171-31369-1-git-send-email-longli@linuxonhyperv.com>
 <1657324171-31369-2-git-send-email-longli@linuxonhyperv.com>
 <859e95d9-2483-b017-6daa-0852317b4a72@xilinx.com>
In-Reply-To: <859e95d9-2483-b017-6daa-0852317b4a72@xilinx.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
msip_labels: 
authentication-results: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=microsoft.com;
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: PH7PR21MB3263:EE_|PH7PR21MB3070:EE_
x-ms-office365-filtering-correlation-id: f091ac2e-a604-4f56-c895-08da912a0d6e
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: QjSlGRaiDiUOqSjtPEJrjPJHl9njzVt8L1x+kR+Ntmp911bB0cENoMgwrA1BuTaibAIrpAsjOrZ0mWw2TSxlE9GVhNeVpq2vYMaE00y5SWn3TW5uvo/YusjyDouLTzaiV60cYnigv+ea7vLY1Mi38abxrCq8l2X3ghPn3CwKywmQ2QWLjH2vCa7US7h/h+1OfvkCDN9w6Sipx9qTpYaZGiE+CgvOMhGByvG9UI3iotuPN/Uv/gRjCDaCPGgWd4B4Tm/l26RH41Dep3n+UH7DZ7xjSbJNEgXscz4ChIlkayf73fE8l7hpXwLIoIGXbyOO3ofXvlQepfEwKotXDvLZSOEFbJM6A5cq1R/PTTatGCxQrVXgF1AIjLOj5JvZCPpJIqvLT8hDiNJhuvkmnLqPEINnGkCa+CsOGR+aIcSdyy1gH6lzEpEllIgiB98mgyLfmRtZT6i/IkgYpgA3AuX7FrGb0bp7ZGbSC/LFOMc+A9/U3IDlUXdmXN+9Y6+c04oRL5yDLKuC03/+h9Jqk10xDPatQk7o2NACvDuklyIHnhewLSyUV3oOaLzHorx9ne1+AVq4IKTy2iwewopz8U4+MhYul9dK2vnEtGoaVdPOrrmfO5E28JV+aTzTjJfTDX0FVjtF9jpnmNkWcerswHIZH3yC02AyT28Bg+z35L7Y3zmkHHewIWyQ4689apSX1oBKiIYuDcnS4EVKDaahnGYy2XUdndrBn2zJ1wJeQFCPMRIvR9ZLi5q3gu4/6gC2+IluPEMIZuBG1dWaHyKJiFq2CYOkoUMUoOjALQ/SXzgkNrn09kww1XylkeDE3gNXUzLzYkcKNdk+qkVSu/6X+qs/tA==
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
 IPV:NLI; SFV:NSPM; H:PH7PR21MB3263.namprd21.prod.outlook.com; PTR:; CAT:NONE;
 SFS:(13230016)(4636009)(39860400002)(376002)(366004)(396003)(136003)(346002)(451199009)(186003)(66446008)(38100700002)(38070700005)(107886003)(122000001)(66946007)(4326008)(10290500003)(8676002)(64756008)(316002)(54906003)(71200400001)(83380400001)(6916009)(76116006)(66476007)(966005)(26005)(2906002)(5660300002)(53546011)(8990500004)(9686003)(33656002)(66556008)(41300700001)(8936002)(52536014)(86362001)(82950400001)(7696005)(55016003)(30864003)(478600001)(82960400001)(6506007);
 DIR:OUT; SFP:1102; 
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?sCEeOg5phQhUPjIT2DQ0+HBJuEf1iGAMf5FnLjPnOl9AN1GMP8U34Z5IUKcW?=
 =?us-ascii?Q?QeTHk/T2HTmaCEAd9sVTxxnvSKswYH0qHtO8LRiYuZECYzsedQcEFaN0zLTM?=
 =?us-ascii?Q?hw/bgEG7IgGZlSTbXs5MO4/MsW++DuXFYdGth1TIxIDRBgJW6ydbkMa/861K?=
 =?us-ascii?Q?6yFYKbIiloF8wQzK9uBuFAY4qOXVlnWWn3kDNx4GnxsWVhYle+nqcF3X3BqW?=
 =?us-ascii?Q?4fD5JCliXJrfn09YyDCnmsEeE8wAzdsuddH5VHFSIrX36Rt8YxXWDao0wv7U?=
 =?us-ascii?Q?M/966HtZGnZ5qEBXpVHbShjjF7dHHV2wEd4/r5Dry+iR7cUHklBUPB4WiVAW?=
 =?us-ascii?Q?myPJCyhCN9Zos/nYfZI1K04r66pL1sQTzZmM3DY1xrNFKEPLxTaTlp8sHwx0?=
 =?us-ascii?Q?wN48APNSWzh6TRkbyXhsgLLmmPK6J0Y6z01imWYuBC6xYkxwbA4uvOuuYs5O?=
 =?us-ascii?Q?aDsXUTCBlBB21ELEsCrEtSAuqFKr6F1cVJOSwGw3Q3Kga6/D4CgmvWf5sV/C?=
 =?us-ascii?Q?ReB2TT1D5feHUZOXknjNe5MC9MNLkDW+nQQWBGYxigZks7eYtWQvn0i+nSYK?=
 =?us-ascii?Q?DO1M8YWvFHNC8OeKsmcdo4Zlu7yVKzTDmRSXImFkWKJJoPpK2+bFE/oie0+a?=
 =?us-ascii?Q?tB/lAmI3DwRbRgwLbhpiXKrTzxHYf33qacOoa2QwD+9SrXDnRiD7pZLmwFTS?=
 =?us-ascii?Q?Wz04Ogb4kXVSB5x1DEhBpQqlVRfmZEf919eX3lPhMZXoMJDNd9f5Slv5OY9C?=
 =?us-ascii?Q?uw+P0j5vpaEcuOPrzBmCjI401JT7FfNMjlpwd/B4mOPbDAKB46ldfzqd/ZWJ?=
 =?us-ascii?Q?6EYjX1FESaSX/YUv1gAGrHZNhchuCImsvksTJdhG+n4xuYMf+U7gFIc27qOA?=
 =?us-ascii?Q?EvsLLFAfW/aOlREXoFfvTM5yRIOslzbChe82IVO+9BEJoeEezQCLYX6xubzp?=
 =?us-ascii?Q?n+3zAJiL+xVk1lsRV0Z5b0PepStPctflHhzhMo1yptDr1AXzz81OWhXmOOJn?=
 =?us-ascii?Q?Jxjb7vOHuFULDRJKoehGSwZ1exuKAza2JFbzGwxXuYd6/85OfkKuQ1vzE3lC?=
 =?us-ascii?Q?zOQ2Uf/ysW3iWjXLpUl0mmU84KCzn8xXop9EkHEh5Oq9iQge/X3gu+yL14rG?=
 =?us-ascii?Q?1YQTOXMMWFZl34ud/SJBfTpmtyJrLkA+kfxkShnh0+/vFmF/MeWJWPUsY81E?=
 =?us-ascii?Q?xxNffoXgCpXIF750eWpbAkZO5v+JwZof3gQRa1qf2pMbQa4F9KdS6R/MXjbK?=
 =?us-ascii?Q?hoBicOLiJJvmR/ufCLjWoQN5QlhO4X0YFeW2AXfSjQD20oonGz6mxGiZuUUb?=
 =?us-ascii?Q?2hnB5NT1Slsyjin7jvDOm4GTTXCaulq1nrINMQBZGUGdTZ8GpQMDmPU5mnTB?=
 =?us-ascii?Q?hVYjj9LTMhYBi5dINlbtp8aQu420HaraFvmPHLekmV21PjY4KdkSce10YQYG?=
 =?us-ascii?Q?77gCJHhwyDddU3A141MDBr4SXgJLpi1lYtkexFnrxQ2BOZFMaFgaML+uN1Aj?=
 =?us-ascii?Q?5SKxO9Kt8NXBmPlgACx5X3kdmAjWdVq4qAkIibO24PHFTeuLtk3lZ4F+YO1v?=
 =?us-ascii?Q?S5d5JpyzSwrvv4oBzf+h1oVGFYahWDDy/XE478PR?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: microsoft.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR21MB3070
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
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>
Errors-To: dev-bounces@dpdk.org

> Subject: Re: [Patch v4 01/17] net/mana: add basic driver, build environme=
nt
> and doc
>=20
> On 7/9/2022 12:49 AM, longli@linuxonhyperv.com wrote:
> > CAUTION: This message has originated from an External Source. Please us=
e
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
> >
> >
> > From: Long Li <longli@microsoft.com>
> >
> > MANA is a PCI device. It uses IB verbs to access hardware through the
> > kernel RDMA layer. This patch introduces build environment and basic
> > device probe functions.
> >
> > Signed-off-by: Long Li <longli@microsoft.com>
> > ---
> > Change log:
> > v2:
> > Fix typos.
> > Make the driver build only on x86-64 and Linux.
> > Remove unused header files.
> > Change port definition to uint16_t or uint8_t (for IB).
> > Use getline() in place of fgets() to read and truncate a line.
> > v3:
> > Add meson build check for required functions from RDMA direct verb
> > header file
> > v4:
> > Remove extra "\n" in logging code.
> > Use "r" in place of "rb" in fopen() to read text files.
> >
>=20
> <...>
>=20
> > --- /dev/null
> > +++ b/doc/guides/nics/mana.rst
> > @@ -0,0 +1,66 @@
> > +..  SPDX-License-Identifier: BSD-3-Clause
> > +    Copyright 2022 Microsoft Corporation
> > +
> > +MANA poll mode driver library
> > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D
> > +
> > +The MANA poll mode driver library (**librte_net_mana**) implements
> > +support for Microsoft Azure Network Adapter VF in SR-IOV context.
> > +
>=20
> Can you please provide any link to an official product description? As a
> reference point for anybody interested more with the product details.

As of today, we don't have an official page about the product. I will add a=
 link as soon as the public product page is made available.

>=20
>=20
> <..>
>=20
> > +
> > +Netvsc PMD arguments > +--------------------
>=20
> 'Netvsc'? Do you mean 'MANA'?

It's a typo. Will fix this.

> j
> > +
> > +The user can specify below argument in devargs.
> > +
> > +#.  ``mac``:
> > +
> > +    Specify the MAC address for this device. If it is set, the driver
> > +    probes and loads the NIC with a matching mac address. If it is not
> > +    set, the driver probes on all the NICs on the PCI device. The defa=
ult
> > +    value is not set, meaning all the NICs will be probed and loaded.
>=20
>=20
> Code accepts up to 8 mac value, should this be documented?
Yes, I will document this.

>=20
> Also why this devarg is needed?

Because a PCI device may expose multiple NICs (ports), this devarg is used =
to specify which NIC to use. MANA doesn't support creating multiple hardwar=
e flows on the same NIC. The user needs to configure the system in a way th=
at the same NIC is not actively used in the kernel. This NIC can be specifi=
ed and passed to DPDK.=20

>=20
> > diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
> > new file mode 100644
> > index 0000000000..cb59eb6882
> > --- /dev/null
> > +++ b/drivers/net/mana/mana.c
> > @@ -0,0 +1,704 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2022 Microsoft Corporation
> > + */
> > +
> > +#include <unistd.h>
> > +#include <dirent.h>
> > +#include <fcntl.h>
> > +#include <sys/mman.h>
> > +
> > +#include <ethdev_driver.h>
> > +#include <ethdev_pci.h>
> > +#include <rte_kvargs.h>
> > +#include <rte_eal_paging.h>
> > +
> > +#include <infiniband/verbs.h>
> > +#include <infiniband/manadv.h>
> > +
> > +#include <assert.h>
> > +
> > +#include "mana.h"
> > +
> > +/* Shared memory between primary/secondary processes, per driver */
> > +struct mana_shared_data *mana_shared_data;
> > +const struct rte_memzone *mana_shared_mz;
>=20
> If these global variables are not used by other compilation units,
> please try to make them static as much as possible.

Sure, will change mana_shared_mz to static. mana_shared_data will be used i=
n mp.c, so keep it unchanged.

>=20
> > +static const char *MZ_MANA_SHARED_DATA =3D "mana_shared_data";
> > +
> > +struct mana_shared_data mana_local_data;
> > +
>=20
> Can you put some comment to this global variables?

Yes, will add.

>=20
> > +/* Spinlock for mana_shared_data */
> > +static rte_spinlock_t mana_shared_data_lock =3D
> RTE_SPINLOCK_INITIALIZER;
> > +
> > +/* Allocate a buffer on the stack and fill it with a printf format str=
ing. */
> > +#define MKSTR(name, ...) \
> > +       int mkstr_size_##name =3D snprintf(NULL, 0, "" __VA_ARGS__); \
> > +       char name[mkstr_size_##name + 1]; \
> > +       \
> > +       memset(name, 0, mkstr_size_##name + 1); \
> > +       snprintf(name, sizeof(name), "" __VA_ARGS__)
> > +
> > +int mana_logtype_driver;
> > +int mana_logtype_init;
> > +
> > +const struct eth_dev_ops mana_dev_ops =3D {
> > +};
> > +
> > +const struct eth_dev_ops mana_dev_sec_ops =3D {
> > +};
>=20
> It may be better to expand 'sec' to secondary to not confuse with
> security etc...

Will change this.

>=20
> > +
> > +uint16_t
> > +mana_rx_burst_removed(void *dpdk_rxq __rte_unused,
> > +                     struct rte_mbuf **pkts __rte_unused,
> > +                     uint16_t pkts_n __rte_unused)
> > +{
> > +       rte_mb();
> > +       return 0;
> > +}
> > +
> > +uint16_t
> > +mana_tx_burst_removed(void *dpdk_rxq __rte_unused,
> > +                     struct rte_mbuf **pkts __rte_unused,
> > +                     uint16_t pkts_n __rte_unused)
> > +{
> > +       rte_mb();
> > +       return 0;
> > +}
> > +
> > +static const char *mana_init_args[] =3D {
> > +       "mac",
> > +       NULL,
> > +};
> > +
> > +/* Support of parsing up to 8 mac address from EAL command line */
> > +#define MAX_NUM_ADDRESS 8
> > +struct mana_conf {
> > +       struct rte_ether_addr mac_array[MAX_NUM_ADDRESS];
> > +       unsigned int index;
> > +};
> > +
> > +static int mana_arg_parse_callback(const char *key, const char *val,
> > +                                  void *private)
>=20
> Since this is new driver, better to follow the coding convention:
> https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fdoc.
> dpdk.org%2Fguides%2Fcontributing%2Fcoding_style.html&amp;data=3D05%7C
> 01%7Clongli%40microsoft.com%7Ccd41ac9636a7472eabf408da844f861e%7C7
> 2f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637967774356090060%7CU
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=3DqTHvw4H%2
> FGVn6DVyFZaSQNhDVqBxyGTSPHKDwSCLxarw%3D&amp;reserved=3D0
>=20
> Please put return type to another line:
>=20
> static int
> mana_arg_parse_callback(const char *key, const char *val, void *private)

Will fix all the styling issues.

>=20
> > +{
> > +       struct mana_conf *conf =3D (struct mana_conf *)private;
> > +       int ret;
> > +
> > +       DRV_LOG(INFO, "key=3D%s value=3D%s index=3D%d", key, val, conf-=
>index);
> > +
> > +       if (conf->index >=3D MAX_NUM_ADDRESS) {
> > +               DRV_LOG(ERR, "Exceeding max MAC address");
> > +               return 1;
> > +       }
> > +
> > +       ret =3D rte_ether_unformat_addr(val, &conf->mac_array[conf->ind=
ex]);
> > +       if (ret) {
> > +               DRV_LOG(ERR, "Invalid MAC address %s", val);
> > +               return ret;
> > +       }
> > +
> > +       conf->index++;
> > +
> > +       return 0;
> > +}
> > +
>=20
> <...>
>=20
> > +static int get_port_mac(struct ibv_device *device, unsigned int port,
> > +                       struct rte_ether_addr *addr)
> > +{
> > +       FILE *file;
> > +       int ret =3D 0;
> > +       DIR *dir;
> > +       struct dirent *dent;
> > +       unsigned int dev_port;
> > +       char mac[20];
> > +
> > +       MKSTR(path, "%s/device/net", device->ibdev_path);
> > +
> > +       dir =3D opendir(path);
> > +       if (!dir)
> > +               return -ENOENT;
> > +
> > +       while ((dent =3D readdir(dir))) {
> > +               char *name =3D dent->d_name;
> > +
> > +               MKSTR(filepath, "%s/%s/dev_port", path, name);
> > +
> > +               /* Ignore . and .. */
> > +               if ((name[0] =3D=3D '.') &&
> > +                   ((name[1] =3D=3D '\0') ||
> > +                    ((name[1] =3D=3D '.') && (name[2] =3D=3D '\0'))))
> > +                       continue;
> > +
> > +               file =3D fopen(filepath, "r");
> > +               if (!file)
> > +                       continue;
> > +
> > +               ret =3D fscanf(file, "%u", &dev_port);
> > +               fclose(file);
> > +
> > +               if (ret !=3D 1)
> > +                       continue;
> > +
> > +               /* Ethernet ports start at 0, IB port start at 1 */
> > +               if (dev_port =3D=3D port - 1) {
> > +                       MKSTR(filepath, "%s/%s/address", path, name);
>=20
>=20
> 'MKSTR' macro adds two variables related with first argument, 'filepath'
> already used above. Yes there is a new scope but better to not define
> new variables, can you select a new name here?

Will change this.

>=20
> <...>
>=20
> > +
> > +static int mana_pci_probe_mac(struct rte_pci_driver *pci_drv
> __rte_unused,
>=20
> This is a static function, if you don't use 'pci_drv', why not drop it
> from the argument list.

Yes, dropping it.

>=20
> > +                             struct rte_pci_device *pci_dev,
> > +                             struct rte_ether_addr *mac_addr)
> > +{
> > +       struct ibv_device **ibv_list;
> > +       int ibv_idx;
> > +       struct ibv_context *ctx;
> > +       struct ibv_device_attr_ex dev_attr;
> > +       int num_devices;
> > +       int ret =3D 0;
> > +       uint8_t port;
> > +       struct mana_priv *priv =3D NULL;
> > +       struct rte_eth_dev *eth_dev =3D NULL;
> > +       bool found_port;
> > +
> > +       ibv_list =3D ibv_get_device_list(&num_devices);
> > +       for (ibv_idx =3D 0; ibv_idx < num_devices; ibv_idx++) {
> > +               struct ibv_device *ibdev =3D ibv_list[ibv_idx];
> > +               struct rte_pci_addr pci_addr;
> > +
> > +               DRV_LOG(INFO, "Probe device name %s dev_name %s
> ibdev_path %s",
> > +                       ibdev->name, ibdev->dev_name, ibdev->ibdev_path=
);
> > +
> > +               if (mana_ibv_device_to_pci_addr(ibdev, &pci_addr))
> > +                       continue;
> > +
> > +               /* Ignore if this IB device is not this PCI device */
> > +               if (pci_dev->addr.domain !=3D pci_addr.domain ||
> > +                   pci_dev->addr.bus !=3D pci_addr.bus ||
> > +                   pci_dev->addr.devid !=3D pci_addr.devid ||
> > +                   pci_dev->addr.function !=3D pci_addr.function)
> > +                       continue;
> > +
>=20
> As far as I understand, intention of this loop is to find 'ibdev'
> matching this device, code gooes through all "ibv device list" for this,
> I wonder if there is a easy way for doing this, like a sysfs entry to
> help getting this information?
> And how mlx4/5 does this?
>=20
> > +               ctx =3D ibv_open_device(ibdev);
> > +               if (!ctx) {
> > +                       DRV_LOG(ERR, "Failed to open IB device %s",
> > +                               ibdev->name);
> > +                       continue;
> > +               }
> > +
> > +               ret =3D ibv_query_device_ex(ctx, NULL, &dev_attr);
> > +               DRV_LOG(INFO, "dev_attr.orig_attr.phys_port_cnt %u",
> > +                       dev_attr.orig_attr.phys_port_cnt);
> > +               found_port =3D false;
> > +
> > +               for (port =3D 1; port <=3D dev_attr.orig_attr.phys_port=
_cnt;
> > +                    port++) {
> > +                       struct ibv_parent_domain_init_attr attr =3D {};
>=20
> "=3D { 0 };" for portability.
>=20
> <...>
>=20
> > +static int mana_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> > +                         struct rte_pci_device *pci_dev)
> > +{
> > +       struct rte_devargs *args =3D pci_dev->device.devargs;
> > +       struct mana_conf conf =3D {};
>=20
> afaik, this is not part of c spec yet, why not initialize as " =3D {0}".

Will fix this.

>=20
> > +       unsigned int i;
> > +       int ret;
> > +
> > +       if (args && args->args) {
>=20
> You can prefer 'args->drv_str', which is newer name of the args.

Will change this.

>=20
> <...>
>=20
> > +static const struct rte_pci_id mana_pci_id_map[] =3D {
> > +       {
> > +               RTE_PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT,
> > +                              PCI_DEVICE_ID_MICROSOFT_MANA)
> > +       },
>=20
> PCI ID list should be terminated with ".vendor_id =3D 0", otherwise PCI
> bus scan loop may behave unexpectedly.

Thank you!

>=20
> > +};
> > +
> > +static struct rte_pci_driver mana_pci_driver =3D {
> > +       .driver =3D {
> > +               .name =3D "mana_pci",
>=20
> driver names are mostly like 'net_<driver_name>', is there a reason to
> diverge from it?
> Also if you use 'RTE_PMD_REGISTER_PCI' macro, it will be standardised
> anyway.
>=20
> > +       },
> > +       .id_table =3D mana_pci_id_map,
> > +       .probe =3D mana_pci_probe,
> > +       .remove =3D mana_pci_remove,
> > +       .drv_flags =3D RTE_PCI_DRV_INTR_RMV,
> > +};
> > +
> > +RTE_INIT(rte_mana_pmd_init)
> > +{
> > +       rte_pci_register(&mana_pci_driver);
> > +}
> > +
>=20
> Why not using 'RTE_PMD_REGISTER_PCI()' macro instead?

Will fix this.

>=20
> > +RTE_PMD_EXPORT_NAME(net_mana, __COUNTER__);
> > +RTE_PMD_REGISTER_PCI_TABLE(net_mana, mana_pci_id_map);
> > +RTE_PMD_REGISTER_KMOD_DEP(net_mana, "* ib_uverbs & mana_ib");
> > +RTE_LOG_REGISTER_SUFFIX(mana_logtype_init, init, NOTICE);
> > +RTE_LOG_REGISTER_SUFFIX(mana_logtype_driver, driver, NOTICE);
> > diff --git a/drivers/net/mana/mana.h b/drivers/net/mana/mana.h
> > new file mode 100644
> > index 0000000000..e30c030b4e
> > --- /dev/null
> > +++ b/drivers/net/mana/mana.h
> > @@ -0,0 +1,210 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2022 Microsoft Corporation
> > + */
> > +
> > +#ifndef __MANA_H__
> > +#define __MANA_H__
> > +
> > +enum {
> > +       PCI_VENDOR_ID_MICROSOFT =3D 0x1414,
> > +};
> > +
> > +enum {
> > +       PCI_DEVICE_ID_MICROSOFT_MANA =3D 0x00ba,
> > +};
> > +
> > +/* Shared data between primary/secondary processes */
> > +struct mana_shared_data {
> > +       rte_spinlock_t lock;
> > +       int init_done;
> > +       unsigned int primary_cnt;
> > +       unsigned int secondary_cnt;
> > +};
> > +
> > +#define MIN_RX_BUF_SIZE        1024
> > +#define MAX_FRAME_SIZE RTE_ETHER_MAX_LEN
> > +#define BNIC_MAX_MAC_ADDR 1
> > +
>=20
> What 'BNIC_' prefix stands for? If it is related to the PMD, what do you
> think to use 'MANA_' as prefix?
> Same for multiple macros below.

Yes, will change those to MANA_.

>=20
> <...>
>=20
> > +
> > +#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
> > +
> > +const uint32_t *mana_supported_ptypes(struct rte_eth_dev *dev);
> > +
>=20
> This function is not defined in this patch, so can drop declarataion.

Yes, moving it to a later patch.

>=20
> <...>
>=20
> > diff --git a/drivers/net/mana/version.map
> b/drivers/net/mana/version.map
> > new file mode 100644
> > index 0000000000..c2e0723b4c
> > --- /dev/null
> > +++ b/drivers/net/mana/version.map
> > @@ -0,0 +1,3 @@
> > +DPDK_22 {
>=20
> It is 'DPDK_23' now.

Will change this along with other fixes.

Long