From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id D2AEFA00E6 for ; Tue, 16 Apr 2019 07:43:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 050EC397D; Tue, 16 Apr 2019 07:43:24 +0200 (CEST) Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50056.outbound.protection.outlook.com [40.107.5.56]) by dpdk.org (Postfix) with ESMTP id 0799F3772 for ; Tue, 16 Apr 2019 07:43:23 +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=M7aWeA+udlOzph0CIFnau2x7yatAtkuYMl6agmpwhmI=; b=uY0oYqV7WZTmDyEb9fRF0IrzIY6EJKta0ka0K6PRhGf/o4lCE9IWw3Q7KAoh34mW00sd40irlumGXyf9e9l1XgfW0hH/PJWk17Z6j9MemAs66MgXPUwaTu6ESGuMFCKxoOnAOHVGfUCpVTailP7jqSAsAyR8cO3Taew6Sk/1u+A= Received: from AM0PR0502MB3795.eurprd05.prod.outlook.com (52.133.45.150) by AM0PR0502MB4050.eurprd05.prod.outlook.com (52.133.37.26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1792.19; Tue, 16 Apr 2019 05:43:21 +0000 Received: from AM0PR0502MB3795.eurprd05.prod.outlook.com ([fe80::4192:b468:41e1:c323]) by AM0PR0502MB3795.eurprd05.prod.outlook.com ([fe80::4192:b468:41e1:c323%4]) with mapi id 15.20.1792.020; Tue, 16 Apr 2019 05:43:21 +0000 From: Shahaf Shuler To: Slava Ovsiienko , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 1/1] net/mlx5: add support for PF representor Thread-Index: AQHU8Uc1OLN+yNXlCUWNTkvjYSC346Y7QNuwgAGzHoCAAVeOoA== Date: Tue, 16 Apr 2019 05:43:21 +0000 Message-ID: References: <1555084107-24692-1-git-send-email-viacheslavo@mellanox.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=shahafs@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 103c5020-4f5c-4f78-5832-08d6c22e6fcd x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600140)(711020)(4605104)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7193020); SRVR:AM0PR0502MB4050; x-ms-traffictypediagnostic: AM0PR0502MB4050: x-microsoft-antispam-prvs: x-forefront-prvs: 000947967F x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(366004)(39860400002)(376002)(396003)(346002)(189003)(199004)(13464003)(76176011)(6436002)(316002)(99286004)(7696005)(71200400001)(14454004)(102836004)(53546011)(2501003)(110136005)(6506007)(105586002)(106356001)(229853002)(97736004)(33656002)(66066001)(6116002)(476003)(11346002)(26005)(71190400001)(478600001)(9686003)(86362001)(53936002)(52536014)(6246003)(81156014)(14444005)(256004)(25786009)(305945005)(5024004)(74316002)(446003)(81166006)(68736007)(3846002)(486006)(186003)(5660300002)(8676002)(8936002)(55016002)(2906002)(7736002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR0502MB4050; H:AM0PR0502MB3795.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-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: 3p/ghplzZGeHIopCE9KK9uzFPL5hcYn1bKf6ttb5SvDQ7skzMpimi/8OosXpTMGT+EDROfGhJbg+FgRPXLwXmEHMOA+a8uJqwBCrYfkmRaAQ5XDSwrpoSP7/rpBMiufhyl0nrMpNSmvAEF/sJS2HfPBfmlJkf9mT4jySHxaZAHRzYNA1skKtOHjxBaZTkBrxvA4SIQTqGg3X+nGIWpLnAYdB957A5d715jRBp+M0YSmZvJBD9PTPblYS8kK6H/fOqrTeEvAl/7lhOYHKQz5OTPQAeCpVYMb3Ui+tCoHxSPggdifthjHG1iAYUTgw++YP3aX9CV25Lpxus6mX6NG6p4BslA1Jh2x5nFWSdN2pPi9T9erJqbDT8RxWe9Vx5LBNtzOX0Ysq+hNTSNichzJcEvwjDAbXa8MQqK3FvN8pyTY= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 103c5020-4f5c-4f78-5832-08d6c22e6fcd X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Apr 2019 05:43:21.7815 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR0502MB4050 Subject: Re: [dpdk-dev] [PATCH 1/1] net/mlx5: add support for PF representor 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190416054321.DwnAKuJsh6pRnEa5B_AOdBHGnRXnpFbUQ8HO5Pm5igA@z> Monday, April 15, 2019 12:12 PM, Slava Ovsiienko: > Subject: RE: [dpdk-dev] [PATCH 1/1] net/mlx5: add support for PF > representor >=20 > Hi, Shahaf >=20 > > -----Original Message----- > > From: Shahaf Shuler > > Sent: Sunday, April 14, 2019 10:43 > > To: Slava Ovsiienko ; dev@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH 1/1] net/mlx5: add support for PF > > representor > > > > Hi Slava, > > > > Friday, April 12, 2019 6:48 PM, Viacheslav Ovsiienko: > > > Subject: [dpdk-dev] [PATCH 1/1] net/mlx5: add support for PF > > > representor > > > > > > On BlueField platform we have the new entity - PF representor. > > > This one represents the PCI PF attached to external host on the side > > > of > > ARM. > > > The traffic sent by the external host to the NIC via PF will be seem > > > by ARM on this PF representor. > > > > > > This patch extends port recognizing capability on the base of > > > physical port name. The following naming formats are supported: > > > > > > - missing physical port name (no sysfs/netlink key) at all, > > > this is old style (before kernel 5.0) format, master assumed > > > - 1 (decimal digits) - old style (before kernel 5.0) format, > > > exists only for representors, master does not have physical > > > port name at all (see above) > > > - p0 ("p" followed by decimal digits), new style (kernel version > > > is 5.0 or higher, Mellanox OFED 4.6 or higher) name format > > > for uplink representor, plays the role of master > > > - pf0vf0 ("pf" followed by PF index concatenated with "vf" > > > followed by VF index), new style (kernel version is 5.0 > > > or higher, Mellanox OFED 4.6 or higher) name format for > > > VF representor. If index of VF is "-1" it is a special case > > > of host PF representor, this representor must be indexed in > > > devargs as 65535, for example representor=3D[0-3,65535] will > > > allow representors for VF0, VF1, VF2, VF3 and host PF. > > > Note: do not specify representor=3D[0-65535] it causes devargs > > > processing error, because number of ports (rte_eth_dev) is > > > limited. > > > > > > > The above is a bit complex to understand and in fact we have 2 modes: > > 1. legacy - phys_port_name are numbers. Master doesn't have > > phys_port_name 2. modern - phys_port_name are strings. > > uplink representor is p%d > > representors are (including PF representor) pf%dvf%d. the vf index for > > the PF representor is 65535. >=20 > OK, I'll try to update the commit message to make it more clear. > > > > > Applications should distinguish representors and master devices > > > exclusively by device flag RTE_ETH_DEV_REPRESENTOR and do not rely > > > on switch port_id (mlx5 PMD deduces ones from representor_id) values > > > returned by > > > dev_infos_get() API. > > > > > > > Please also reference the kernel commit which introduce the name > change. > OK. >=20 > > > > > Signed-off-by: Viacheslav Ovsiienko > > > --- > > > drivers/net/mlx5/mlx5.h | 11 ++++++- > > > drivers/net/mlx5/mlx5_ethdev.c | 68 > +++++++++++++++++++++++++++---- > > > ----------- > > > drivers/net/mlx5/mlx5_nl.c | 42 +++++++++++++++++--------- > > > 3 files changed, 82 insertions(+), 39 deletions(-) > > > > > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index > > > 8eb1019..81c02ce 100644 > > > --- a/drivers/net/mlx5/mlx5.h > > > +++ b/drivers/net/mlx5/mlx5.h > > > @@ -80,11 +80,20 @@ struct mlx5_mp_param { > > > /** Key string for IPC. */ > > > #define MLX5_MP_NAME "net_mlx5_mp" > > > > > > +/* Recognized Infiniband device physical port name types. */ enum > > > +mlx5_phys_port_name_type { > > > + MLX5_PHYS_PORT_NAME_TYPE_UNKNOWN =3D 0, /* Unrecognized. > > > */ > > > + MLX5_PHYS_PORT_NAME_TYPE_LEGACY, /* before kernel ver < 5.0 > > > */ > > > + MLX5_PHYS_PORT_NAME_TYPE_UPLINK, /* p0, kernel ver >=3D 5.0 */ > > > + MLX5_PHYS_PORT_NAME_TYPE_PFVF, /* pf0vf0, kernel ver >=3D 5.0 > > > */ }; > > > + > > > /** Switch information returned by mlx5_nl_switch_info(). */ > > > struct mlx5_switch_info { > > > uint32_t master:1; /**< Master device. */ > > > uint32_t representor:1; /**< Representor device. */ > > > - uint32_t port_name_new:1; /**< Rep. port name is in new format. > > > */ > > > + enum mlx5_phys_port_name_type name_type; /** < Port name > > > type. */ > > > + int32_t pf_num; /**< PF number (valid for pfxvfx format only). */ > > > int32_t port_name; /**< Representor port name. */ > > > uint64_t switch_id; /**< Switch identifier. */ }; diff --git > > > a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c > > > index 3992918..371989f 100644 > > > --- a/drivers/net/mlx5/mlx5_ethdev.c > > > +++ b/drivers/net/mlx5/mlx5_ethdev.c > > > @@ -1395,12 +1395,11 @@ int mlx5_fw_version_get(struct rte_eth_dev > > > *dev, char *fw_ver, size_t fw_size) > > > struct mlx5_switch_info data =3D { > > > .master =3D 0, > > > .representor =3D 0, > > > - .port_name_new =3D 0, > > > + .name_type =3D MLX5_PHYS_PORT_NAME_TYPE_UNKNOWN, > > > .port_name =3D 0, > > > .switch_id =3D 0, > > > }; > > > DIR *dir; > > > - bool port_name_set =3D false; > > > bool port_switch_id_set =3D false; > > > bool device_dir =3D false; > > > char c; > > > @@ -1423,8 +1422,7 @@ int mlx5_fw_version_get(struct rte_eth_dev > > *dev, > > > char *fw_ver, size_t fw_size) > > > ret =3D fscanf(file, "%s", port_name); > > > fclose(file); > > > if (ret =3D=3D 1) > > > - port_name_set =3D > > > mlx5_translate_port_name(port_name, > > > - &data); > > > + mlx5_translate_port_name(port_name, &data); > > > } > > > file =3D fopen(phys_switch_id, "rb"); > > > if (file =3D=3D NULL) { > > > @@ -1440,8 +1438,15 @@ int mlx5_fw_version_get(struct rte_eth_dev > > > *dev, char *fw_ver, size_t fw_size) > > > closedir(dir); > > > device_dir =3D true; > > > } > > > - data.master =3D port_switch_id_set && (!port_name_set || > > > device_dir); > > > - data.representor =3D port_switch_id_set && port_name_set && > > > !device_dir; > > > + if (port_switch_id_set) { > > > + data.master =3D > > > + device_dir || > > > + data.name_type =3D=3D > > > MLX5_PHYS_PORT_NAME_TYPE_UNKNOWN || > > > + data.name_type =3D=3D > > > MLX5_PHYS_PORT_NAME_TYPE_UPLINK; > > > + data.representor =3D !device_dir && > > > + (data.name_type =3D=3D > > > MLX5_PHYS_PORT_NAME_TYPE_LEGACY || > > > + data.name_type =3D=3D > > > MLX5_PHYS_PORT_NAME_TYPE_PFVF); > > > > > > Why we need to split the logic of the master/representor detection > > between the mlx5_translate_port_name and the caller function? >=20 > We have two stages: > - gathering the port attributes (name, vf_num, switchdev_id, presence of > device directory, etc.) in callbacks > - analyzing the parameters on gathering completion. We need all the > parameters to make a reliable master/representor recognition. >=20 > Translate routine is called on gathering stage and just recognizes the na= me > format, extracts useful values (pf/vf num) and stores the results in comp= act > form. It is easier than storing the entire port name. >=20 > > > > The way I envision it is mlx5_tranlate_port_name receives the > > phys_port_name and maybe more metadata and return the port > This "metadata" may be not gathered yet at the moment of port name > processing. >=20 > > classification (master/representor) and the representor/pf number. > > No need for data.master =3D some_logic(translate_port_name_info). > > > > Inside the translate function I would expect to have 2 smaller function= : > > 1. to handle the modern format (strings) 2. to handle the legacy > > format > > (integers) >=20 > Actually this patch is also some kind of preparation for coming subfuctio= ns. > If we have set of mutual exclusive formats the switch/case seems to be th= e > most native way to treat these ones. I'd prefer to keep switch/case. > Comments are clear and it is easy to understand where legacy/new format i= s > treated. And it is easy to extend for coming subfunctions. >=20 > I think we should isolate master/representor recognition logic into separ= ate > routine, "translate" routine is for parameter gathering stage, "recognize= " for > analyzing. If you insist to split it then we should have 2 separate routines, unlike t= he current approach where switch/case are spread in different places.