From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 To: Ferruh Yigit CC: "dev@dpdk.org" , Ajay Sharma , Stephen Hemminger 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: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 > > > > 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 > > --- > > 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 > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > + > > +#include > > + > > +#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&data=3D05%7C > 01%7Clongli%40microsoft.com%7Ccd41ac9636a7472eabf408da844f861e%7C7 > 2f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637967774356090060%7CU > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI > 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3DqTHvw4H%2 > FGVn6DVyFZaSQNhDVqBxyGTSPHKDwSCLxarw%3D&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_', 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