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 21DE2A00E6 for ; Sun, 14 Apr 2019 09:42:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DF521325F; Sun, 14 Apr 2019 09:42:52 +0200 (CEST) Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-eopbgr150042.outbound.protection.outlook.com [40.107.15.42]) by dpdk.org (Postfix) with ESMTP id CED4C29AC for ; Sun, 14 Apr 2019 09:42:51 +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=bkoS4IjHDuepPqYQHgfsIDwHVIm6TOHyPPUhMHqCFrs=; b=ybhTYjfw840AQGFBvJJvJGV+6C937YDABJzp57orlFh8Ab3gS6xz+ZVkDWU3/5E/8XkmcypAk+d9vkWb9qB8GLJnC0avhe+P+z+godA+TqZ5F7W1XcatBRPHDRKaIM4wXdMgTWJ86wZhMWUyUUTL7sH4FKpy18zAD9q+yYiZP00= Received: from AM0PR0502MB3795.eurprd05.prod.outlook.com (52.133.45.150) by AM0PR0502MB3988.eurprd05.prod.outlook.com (52.133.34.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1792.17; Sun, 14 Apr 2019 07:42:50 +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.018; Sun, 14 Apr 2019 07:42:50 +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+yNXlCUWNTkvjYSC346Y7QNuw Date: Sun, 14 Apr 2019 07:42:49 +0000 Message-ID: References: <1555084107-24692-1-git-send-email-viacheslavo@mellanox.com> In-Reply-To: <1555084107-24692-1-git-send-email-viacheslavo@mellanox.com> 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: [31.154.10.105] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 986186e5-de56-4f2c-95d6-08d6c0accb86 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600139)(711020)(4605104)(4618075)(2017052603328)(7193020); SRVR:AM0PR0502MB3988; x-ms-traffictypediagnostic: AM0PR0502MB3988: x-microsoft-antispam-prvs: x-forefront-prvs: 00073DB75F x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(396003)(346002)(366004)(39850400004)(376002)(136003)(199004)(189003)(14454004)(105586002)(102836004)(7736002)(52536014)(81166006)(186003)(6506007)(81156014)(6246003)(66066001)(478600001)(2501003)(229853002)(74316002)(86362001)(106356001)(68736007)(256004)(33656002)(110136005)(2906002)(8936002)(99286004)(71200400001)(71190400001)(5660300002)(76176011)(6116002)(3846002)(5024004)(305945005)(6436002)(7696005)(316002)(9686003)(55016002)(26005)(8676002)(30864003)(97736004)(476003)(53936002)(486006)(11346002)(446003)(25786009); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR0502MB3988; H:AM0PR0502MB3795.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX: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: JxTep06aBvwJEoHemhzaq6kD69KisOu2BmuByljlqwJpIwxAdkuqwG9FJCMdmz+DJKDY9OyLOufPo7HWU35M9qvqs8ooryEdGipiqQRDMrxdMOSFBEjAZuP8Plt93qsmnCM3/YGl//3KJlAdqCk9nO2e3JhDsYWtzpP4qmnju44bYPXKpAG1hn5pdorngKEQhegmE+Ip7iT876svkeBX0U4t/M82AKpLH0d0yF/JW9guIQP95+zPNgVpZ0NrrzA3UMUa0OnXeoHaPy8wURG3PafgNcdaW+XIV8cOZYIGf43yezDO/p27sb5KEUqLJ+40WNug0aGVbo8Yi8LhuWlrXZeVnwaYzJ1DsVrH4YIfm0wN5rRhKiUxyYscf3TU0TH9AC6MJfYw6DgL2yOJ3s5w3vEgMQxbKf0HHZ2OE9KSvig= 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: 986186e5-de56-4f2c-95d6-08d6c0accb86 X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Apr 2019 07:42:50.0296 (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: AM0PR0502MB3988 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: <20190414074249.2CcgOzHNFeMxIOSXosy2R9Zc4BsyBnZ71pBLK3cN-to@z> Hi Slava, Friday, April 12, 2019 6:48 PM, Viacheslav Ovsiienko: > Subject: [dpdk-dev] [PATCH 1/1] net/mlx5: add support for PF representor >=20 > On BlueField platform we have the new entity - PF representor. > This one represents the PCI PF attached to external host on the side of A= RM. > The traffic sent by the external host to the NIC via PF will be seem by A= RM on > this PF representor. >=20 > This patch extends port recognizing capability on the base of physical po= rt > name. The following naming formats are supported: >=20 > - 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. >=20 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.=20 uplink representor is p%d representors are (including PF representor) pf%dvf%d. the vf index for the = PF representor is 65535.=20 > Applications should distinguish representors and master devices exclusive= ly > 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. >=20 Please also reference the kernel commit which introduce the name change.=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(-) >=20 > 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" >=20 > +/* 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? The way I envision it is mlx5_tranlate_port_name receives the phys_port_nam= e and maybe more metadata and return the port classification (master/repres= entor) and the representor/pf number.=20 No need for data.master =3D some_logic(translate_port_name_info).=20 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 > + } > *info =3D data; > assert(!(data.master && data.representor)); > if (data.master && data.representor) { @@ -1459,10 +1464,11 @@ int > mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t > fw_size) > * @param[in] port_name_in > * String representing the port name. > * @param[out] port_info_out > - * Port information, including port name as a number. > + * Port information, including port name as a number and port name > + * type if recognized > * > * @return > - * true on success, false otherwise. > + * true on success (if name format recognized), false otherwise. > */ > bool > mlx5_translate_port_name(const char *port_name_in, @@ -1470,25 > +1476,39 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev, char > *fw_ver, size_t fw_size) { > char pf_c1, pf_c2, vf_c1, vf_c2; > char *end; > - int32_t pf_num; > - bool port_name_set =3D false; > + int sc_items; >=20 > /* > * Check for port-name as a string of the form pf0vf0 > - * (support kernel ver >=3D 5.0) > + * (support kernel ver >=3D 5.0 or OFED ver >=3D 4.6). > */ > - port_name_set =3D (sscanf(port_name_in, "%c%c%d%c%c%d", > &pf_c1, &pf_c2, > - &pf_num, &vf_c1, &vf_c2, > - &port_info_out->port_name) =3D=3D 6); > - if (port_name_set) { > - port_info_out->port_name_new =3D 1; > - } else { > - /* Check for port-name as a number (support kernel ver < > 5.0 */ > - errno =3D 0; > - port_info_out->port_name =3D strtol(port_name_in, &end, 0); > - if (!errno && > - (size_t)(end - port_name_in) =3D=3D strlen(port_name_in)) > - port_name_set =3D true; > + sc_items =3D sscanf(port_name_in, "%c%c%d%c%c%d", > + &pf_c1, &pf_c2, &port_info_out->pf_num, > + &vf_c1, &vf_c2, &port_info_out->port_name); > + if (sc_items =3D=3D 6 && > + pf_c1 =3D=3D 'p' && pf_c2 =3D=3D 'f' && > + vf_c1 =3D=3D 'v' && vf_c2 =3D=3D 'f') { > + port_info_out->name_type =3D > MLX5_PHYS_PORT_NAME_TYPE_PFVF; > + return true; > + } > + /* > + * Check for port-name as a string of the form p0 > + * (support kernel ver >=3D 5.0, or OFED ver >=3D 4.6). > + */ > + sc_items =3D sscanf(port_name_in, "%c%d", > + &pf_c1, &port_info_out->port_name); > + if (sc_items =3D=3D 2 && pf_c1 =3D=3D 'p') { > + port_info_out->name_type =3D > MLX5_PHYS_PORT_NAME_TYPE_UPLINK; > + return true; > + } > + /* Check for port-name as a number (support kernel ver < 5.0 */ > + errno =3D 0; > + port_info_out->port_name =3D strtol(port_name_in, &end, 0); > + if (!errno && > + (size_t)(end - port_name_in) =3D=3D strlen(port_name_in)) { > + port_info_out->name_type =3D > MLX5_PHYS_PORT_NAME_TYPE_LEGACY; > + return true; > } > - return port_name_set; > + port_info_out->name_type =3D > MLX5_PHYS_PORT_NAME_TYPE_UNKNOWN; > + return false; > } > diff --git a/drivers/net/mlx5/mlx5_nl.c b/drivers/net/mlx5/mlx5_nl.c inde= x > fd9226b..669de76 100644 > --- a/drivers/net/mlx5/mlx5_nl.c > +++ b/drivers/net/mlx5/mlx5_nl.c > @@ -887,12 +887,11 @@ struct mlx5_nl_ifindex_data { > struct mlx5_switch_info info =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, > }; > size_t off =3D NLMSG_LENGTH(sizeof(struct ifinfomsg)); > - bool port_name_set =3D false; > bool switch_id_set =3D false; > bool num_vf_set =3D false; >=20 > @@ -910,9 +909,7 @@ struct mlx5_nl_ifindex_data { > num_vf_set =3D true; > break; > case IFLA_PHYS_PORT_NAME: > - port_name_set =3D > - mlx5_translate_port_name((char *)payload, > - &info); > + mlx5_translate_port_name((char *)payload, &info); > break; > case IFLA_PHYS_SWITCH_ID: > info.switch_id =3D 0; > @@ -926,16 +923,33 @@ struct mlx5_nl_ifindex_data { > off +=3D RTA_ALIGN(ra->rta_len); > } > if (switch_id_set) { > - if (info.port_name_new) { > - /* New representors naming schema. */ > - if (port_name_set) { > - info.master =3D (info.port_name =3D=3D -1); > - info.representor =3D (info.port_name !=3D -1); > - } > - } else { > + /* We have some E-Switch configuration. */ > + switch (info.name_type) { > + case MLX5_PHYS_PORT_NAME_TYPE_UNKNOWN: > + /* > + * Name is not recognized or not set, > + * it can not be representor, check > + * VF number to see if it is a master. > + */ > + info.master =3D num_vf_set; > + break; > + case MLX5_PHYS_PORT_NAME_TYPE_LEGACY: > /* Legacy representors naming schema. */ > - info.master =3D (!port_name_set || num_vf_set); > - info.representor =3D port_name_set && !num_vf_set; > + info.representor =3D !num_vf_set; > + break; > + case MLX5_PHYS_PORT_NAME_TYPE_UPLINK: > + /* New uplink naming schema. */ > + info.master =3D 1; > + break; > + case MLX5_PHYS_PORT_NAME_TYPE_PFVF: > + /* New representors naming schema. */ > + info.representor =3D 1; > + break; > + } > + if (!info.master && !info.representor) { > + DRV_LOG(INFO, > + "unable to recognize master/representors" > + " on the device in switch domain"); Same comment as above. Would like to avoid this switch case outside of the = translate function . > } > } > assert(!(info.master && info.representor)); > -- > 1.8.3.1