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 E534DA05D3 for ; Sun, 24 Mar 2019 10:07:19 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 13EB91B802; Sun, 24 Mar 2019 10:07:19 +0100 (CET) Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20083.outbound.protection.outlook.com [40.107.2.83]) by dpdk.org (Postfix) with ESMTP id B9DE31B7F3 for ; Sun, 24 Mar 2019 10:07:16 +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:X-MS-Exchange-SenderADCheck; bh=Bs5GYstjxsbGgDDK/h1/Lqs3SZxSlOX3lnTql9WtzhY=; b=J82lwP0CeDY8XqDgBpBSu74h6oHXZ/rfYt4s2aKmkL3OOThlwe3POtZDaw14Nm8vf+K8PU/biYu8Ine0Pus0MrUl0/lHdKYT14cyIKgDx/3zMFHHOz8SyJVs5sBIPBSq66Xx61eX9IchW66wBJGZW0PDpGtcB5uljFRAIC3pjKw= Received: from AM0PR0502MB3795.eurprd05.prod.outlook.com (52.133.45.150) by AM0PR0502MB3683.eurprd05.prod.outlook.com (52.133.50.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1730.17; Sun, 24 Mar 2019 09:07:15 +0000 Received: from AM0PR0502MB3795.eurprd05.prod.outlook.com ([fe80::84f3:7e92:7a51:1003]) by AM0PR0502MB3795.eurprd05.prod.outlook.com ([fe80::84f3:7e92:7a51:1003%2]) with mapi id 15.20.1730.019; Sun, 24 Mar 2019 09:07:14 +0000 From: Shahaf Shuler To: Slava Ovsiienko , "dev@dpdk.org" Thread-Topic: [PATCH 12/14] net/mlx5: update install/uninstall int handler routines Thread-Index: AQHU373ZnaYVRFjDE06NvmRmNxlBvqYV+PRQgAAk1oCABGOXsA== Date: Sun, 24 Mar 2019 09:07:14 +0000 Message-ID: References: <1551376985-11096-1-git-send-email-viacheslavo@mellanox.com> <1553155888-27498-1-git-send-email-viacheslavo@mellanox.com> <1553155888-27498-13-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: x-originating-ip: [31.154.10.105] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: e0e2812a-5e8f-42a4-c928-08d6b0381bc1 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600127)(711020)(4605104)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:AM0PR0502MB3683; x-ms-traffictypediagnostic: AM0PR0502MB3683: authentication-results: spf=none (sender IP is ) smtp.mailfrom=shahafs@mellanox.com; x-microsoft-antispam-prvs: x-forefront-prvs: 09860C2161 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(39850400004)(366004)(396003)(346002)(376002)(189003)(199004)(25786009)(446003)(476003)(71190400001)(11346002)(5660300002)(33656002)(74316002)(66066001)(486006)(6116002)(3846002)(7736002)(2501003)(316002)(8936002)(81166006)(81156014)(68736007)(97736004)(8676002)(99286004)(76176011)(26005)(2906002)(186003)(102836004)(6506007)(110136005)(93886005)(6246003)(7696005)(305945005)(256004)(14444005)(5024004)(53936002)(478600001)(15650500001)(86362001)(14454004)(71200400001)(9686003)(55016002)(6436002)(105586002)(106356001)(229853002)(52536014); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR0502MB3683; 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: JYS7biBINuZ0n+GbyDujfcRYquYakeJRW6FrdMzhJer5mhAYUf4Hj/gqSeTCPqquqEGXPb1nAeabyVf5NoxfmrxxzvG9zXeFR4mAmmXLYJg415whF6jDOkb2Da+9IqjPJo9Viw+kzkUY6BqHCTiPO09gjqA5yTtBEytYaizMfu8M33os3GAAzAfW8GqO1UGPqJHgUf4Jnqm3yzvvdXOt3CHNqjHofYCv8T8KR7OkUpv3nkksAtYRe4do+20Gde2exbiBhUOVnEFD5LQyS91nOCAMBKxWcpHoeWEnP7ln7HDY0tltAusxm0LWMjLoS8RT5nJbspMQaXb3ZhbSbLr1n14qWKgnJjgpvqW1gp+I0uJuXLFxrma4GNuDt5UteLS2GlvQ+riOOINJDmlUJL3gTwy0TMOr9BAEdWf2IJNZaA0= 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: e0e2812a-5e8f-42a4-c928-08d6b0381bc1 X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2019 09:07:14.5953 (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: AM0PR0502MB3683 Subject: Re: [dpdk-dev] [PATCH 12/14] net/mlx5: update install/uninstall int handler routines 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: <20190324090714.YvyFMnjoI2XR8PV7qMB5MjKkwWkPTUxeo0TXkIfwdgY@z> Thursday, March 21, 2019 4:02 PM, Slava Ovsiienko: > To: Shahaf Shuler ; dev@dpdk.org > Subject: RE: [PATCH 12/14] net/mlx5: update install/uninstall int handler > routines > > > > Thursday, March 21, 2019 10:11 AM, Viacheslav Ovsiienko: > > > Subject: [PATCH 12/14] net/mlx5: update install/uninstall int > > > handler routines > > > > > > We are implementing the support for multport Infiniband device withj > > > representors attached to these multiple ports. Asynchronous device > > > event notifications (link status change, removal event, etc.) should > > > be shared between ports. We are going to implement shared event > > > handler and this patch introduces appropriate device structure > > > changes and updated event handler install and uninstall routines. > > > > > > Signed-off-by: Viacheslav Ovsiienko [...] > > > > > > assert(spawn); > > > /* Search for IB context by device name. */ @@ -212,6 +213,9 @@ > > > struct mlx5_dev_spawn_data { > > > sizeof(sh->ibdev_name)); > > > strncpy(sh->ibdev_path, sh->ctx->device->ibdev_path, > > > sizeof(sh->ibdev_path)); > > > + pthread_mutex_init(&sh->intr_mutex, NULL); > > > + for (i =3D 0; i < sh->max_port; i++) > > > + sh->port[i].port_id =3D RTE_MAX_ETHPORTS; > > > > Why you need struct here? You port array is not just of uint32_t type? >=20 > For the case if we would like to add some other per-port data accessible = only > from shared context. For example - in interrupt handler we have only one > parameter - the shared context, and we should deduce eth_dev for the > some device (not DPDK port_id) port >=20 > Actually it is uint_32_t array for now, but it is easily extandable, for = example, > we could add per-port context for interrupt handler. OK, then you need to doc it as such ("per port context for interrupt").=20 >=20 > > [...] > > > + assert(priv->ibv_port <=3D sh->max_port); > > > + assert(dev->data->port_id < RTE_MAX_ETHPORTS); > > > + if (sh->port[priv->ibv_port - 1].port_id < RTE_MAX_ETHPORTS) { > > > > I don't understand why need an array to understand handler is already > > exists. > > Why not the refcnt? >=20 > Array is needed to deduce the eth_dev from the device port number. > Here is interrupt handler flow: > - entry > - for() > - get_event() > - get device port (note, this is IB port index, not DPDK port id) from ev= ent > - check in the array whether the handler is installed for this port > (array member is less than RTE_MAX_ETHPORTS) > - get DPDK port_id from array() >=20 > Array member just indicates whether the handler for given IB port is > installed. Reference counter is used for rte_intr_callback_register/ > rte_intr_callback_unregister calls. > rte_intr_callback_register() is called when the first handler for the por= t is > being installed. > rte_intr_callback_unregister() is called when the lastt handler for the p= ort is > being gone away. OK, it will be much clear to have all the handler patches in a single patch= .=20 >=20 > > > > > + /* The handler is already installed for this port. */ > > > + assert(sh->intr_cnt++); > > > > Asserts are compiled only in debug mode. You should not put any logic > > (++) into them. >=20 > Yes, it is a bug, there should no be "++" at all. Thanks. >=20 > > > > > + goto exit; > > > + } > > > + sh->port[priv->ibv_port - 1].port_id =3D (uint32_t)dev->data->port_= id; > > > + if (sh->intr_cnt) { > > > + sh->intr_cnt++; > > > + goto exit; > > > + } > > > + /* No shared handler installed. */ > > > + assert(sh->ctx->async_fd > 0); > > > + flags =3D fcntl(sh->ctx->async_fd, F_GETFL); > > > + ret =3D fcntl(sh->ctx->async_fd, F_SETFL, flags | O_NONBLOCK); > > > + if (ret) { > > > + DRV_LOG(INFO, "failed to change file descriptor" > > > + " async event queue"); > > > + /* Indicate there will be no interrupts. */ > > > + dev->data->dev_conf.intr_conf.lsc =3D 0; > > > + dev->data->dev_conf.intr_conf.rmv =3D 0; > > > + sh->port[priv->ibv_port - 1].port_id =3D RTE_MAX_ETHPORTS; > > > + goto exit; > > > + } > > > + sh->intr_handle.fd =3D sh->ctx->async_fd; > > > + sh->intr_handle.type =3D RTE_INTR_HANDLE_EXT; > > > + rte_intr_callback_register(&sh->intr_handle, > > > + mlx5_dev_interrupt_handler, sh); > > > + sh->intr_cnt++; > > > +exit: > > > + pthread_mutex_unlock(&sh->intr_mutex); > > > +} > > > + > > > +/** > > > * Uninstall interrupt handler. > > > * > > > * @param dev > > > @@ -1119,15 +1209,10 @@ int mlx5_fw_version_get(struct rte_eth_dev > > > *dev, char *fw_ver, size_t fw_size) { > > > struct mlx5_priv *priv =3D dev->data->dev_private; > > > > > > - if (dev->data->dev_conf.intr_conf.lsc || > > > - dev->data->dev_conf.intr_conf.rmv) > > > - rte_intr_callback_unregister(&priv->intr_handle, > > > - mlx5_dev_interrupt_handler, > > > dev); > > > + mlx5_dev_shared_handler_uninstall(dev); > > > if (priv->primary_socket) > > > rte_intr_callback_unregister(&priv->intr_handle_socket, > > > mlx5_dev_handler_socket, dev); > > > - priv->intr_handle.fd =3D 0; > > > - priv->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > > > priv->intr_handle_socket.fd =3D 0; > > > priv->intr_handle_socket.type =3D RTE_INTR_HANDLE_UNKNOWN; } > > @@ > > > -1142,28 +1227,9 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev, > > > char *fw_ver, size_t fw_size) > > > mlx5_dev_interrupt_handler_install(struct rte_eth_dev *dev) { > > > struct mlx5_priv *priv =3D dev->data->dev_private; > > > - struct ibv_context *ctx =3D priv->sh->ctx; > > > int ret; > > > - int flags; > > > > > > - assert(ctx->async_fd > 0); > > > - flags =3D fcntl(ctx->async_fd, F_GETFL); > > > - ret =3D fcntl(ctx->async_fd, F_SETFL, flags | O_NONBLOCK); > > > - if (ret) { > > > - DRV_LOG(INFO, > > > - "port %u failed to change file descriptor async event" > > > - " queue", > > > - dev->data->port_id); > > > - dev->data->dev_conf.intr_conf.lsc =3D 0; > > > - dev->data->dev_conf.intr_conf.rmv =3D 0; > > > - } > > > - if (dev->data->dev_conf.intr_conf.lsc || > > > - dev->data->dev_conf.intr_conf.rmv) { > > > - priv->intr_handle.fd =3D ctx->async_fd; > > > - priv->intr_handle.type =3D RTE_INTR_HANDLE_EXT; > > > - rte_intr_callback_register(&priv->intr_handle, > > > - mlx5_dev_interrupt_handler, dev); > > > - } > > > + mlx5_dev_shared_handler_install(dev); > > > ret =3D mlx5_socket_init(dev); > > > if (ret) > > > DRV_LOG(ERR, "port %u cannot initialise socket: %s", > > > -- > > > 1.8.3.1