From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00053.outbound.protection.outlook.com [40.107.0.53]) by dpdk.org (Postfix) with ESMTP id 2B1A31B4CB for ; Thu, 21 Mar 2019 15:01:55 +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=P6r2Jc1WLjYV4lnIH2Tnv8kWDXlAwyf5Ddsf9x6npPk=; b=hPD+XScEXeGyAZjwJs0812T6rj9cL7tfh8TS74KyExvxOJsl7jNXPsQ2+x3buAAT+J7yBMBmAU//ZZ92/0BUpmlCH1/6Zhk80E8dd7GAbAn5K3SdmNUtFMjy/MzoeCA3Wx5CXeIWJQ9ZC1NjWvjxdC8v54TNsTyDjzbw+c9SSMM= Received: from AM4PR05MB3265.eurprd05.prod.outlook.com (10.171.188.154) by AM4PR05MB3428.eurprd05.prod.outlook.com (10.171.186.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1709.14; Thu, 21 Mar 2019 14:01:53 +0000 Received: from AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::11b0:de86:8d93:8b02]) by AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::11b0:de86:8d93:8b02%3]) with mapi id 15.20.1709.015; Thu, 21 Mar 2019 14:01:53 +0000 From: Slava Ovsiienko To: Shahaf Shuler , "dev@dpdk.org" Thread-Topic: [PATCH 12/14] net/mlx5: update install/uninstall int handler routines Thread-Index: AQHU39+7dk02X1WwiUK3qCpZAVTOQaYWGVlA Date: Thu, 21 Mar 2019 14:01:53 +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: authentication-results: spf=none (sender IP is ) smtp.mailfrom=viacheslavo@mellanox.com; x-originating-ip: [95.67.35.250] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: bc5a60ad-d596-4aeb-79f3-08d6ae05c5c4 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020); SRVR:AM4PR05MB3428; x-ms-traffictypediagnostic: AM4PR05MB3428: x-microsoft-antispam-prvs: x-forefront-prvs: 0983EAD6B2 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(366004)(396003)(346002)(39860400002)(376002)(199004)(189003)(13464003)(86362001)(110136005)(97736004)(2906002)(6436002)(71190400001)(7736002)(81166006)(71200400001)(105586002)(68736007)(8676002)(66066001)(486006)(14454004)(81156014)(106356001)(30864003)(11346002)(305945005)(93886005)(316002)(256004)(478600001)(2501003)(5024004)(476003)(99286004)(14444005)(26005)(9686003)(76176011)(53936002)(6506007)(25786009)(5660300002)(55016002)(6246003)(74316002)(33656002)(6116002)(7696005)(186003)(102836004)(15650500001)(52536014)(3846002)(8936002)(229853002)(53546011)(446003); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR05MB3428; H:AM4PR05MB3265.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: VKcdqPGEvqN4EQVwzf/hga32V+JV1bnHui60/f1TnmZW16ZLxfFUthhI/Y240QN8LgBE10/MwzNMVnq9dQJ7TyoxT6xIPlckjkqIvrUjueoZbryDJ85P2HHp8OHJNo0HxFMR/FD13gURdRsWFWQLSERg2Kwvw4f7fJdYC9XlsNhjFSdHBIqoqW10A1QvrctdIY/SGDpyGEWbO2W8qVq3HW0J6cLOrlDzAVAc7VQlej3WEW31KxiFcGqO/4+7mlcju0Wj2/XVMCYPyaM6vuIgRjLCX/xgzODJGZ52ftEwUMq1xAXEZJLR+4nvovIpCFZcj69WPBGYAWBRtw4oTCItmyhn/uXqjj2LtT0X3MeS1+WZMGf2zB04Cm3ptzambZZ8I4/KmhMnMjipa0yP8dH44rAKBVK0p7fF6O9rK+dc5fo= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: bc5a60ad-d596-4aeb-79f3-08d6ae05c5c4 X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Mar 2019 14:01:53.3759 (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: AM4PR05MB3428 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: , X-List-Received-Date: Thu, 21 Mar 2019 14:01:55 -0000 > -----Original Message----- > From: Shahaf Shuler > Sent: Thursday, March 21, 2019 14:15 > To: Slava Ovsiienko ; dev@dpdk.org > Subject: RE: [PATCH 12/14] net/mlx5: update install/uninstall int handler > routines >=20 > 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 > > --- > > drivers/net/mlx5/mlx5.c | 14 ++++- > > drivers/net/mlx5/mlx5.h | 3 +- > > drivers/net/mlx5/mlx5_ethdev.c | 118 > > ++++++++++++++++++++++++++++++++--------- > > 3 files changed, 107 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index > > 312c42b..44b7a87 100644 > > --- a/drivers/net/mlx5/mlx5.c > > +++ b/drivers/net/mlx5/mlx5.c > > @@ -165,6 +165,7 @@ struct mlx5_dev_spawn_data { { > > struct mlx5_ibv_shared *sh; > > int err =3D 0; > > + uint32_t i; > > > > 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; >=20 > Why you need struct here? You port array is not just of uint32_t type? 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 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. =09 >=20 > > if (rte_eal_process_type() =3D=3D RTE_PROC_SECONDARY) { > > /* > > * For secondary process we just open the IB device @@ - > > 276,6 +280,15 @@ struct mlx5_dev_spawn_data { > > assert(!sh->pd); > > } > > LIST_REMOVE(sh, next); > > + /* > > + * Ensure there is no async event handler installed. > > + * Only primary process handles async device events. > > + **/ > > + assert(!sh->intr_cnt); > > + if (sh->intr_cnt) > > + rte_intr_callback_unregister > > + (&sh->intr_handle, mlx5_dev_interrupt_handler, > > sh); > > + pthread_mutex_destroy(&sh->intr_mutex); > > if (sh->pd) > > claim_zero(mlx5_glue->dealloc_pd(sh->pd)); > > if (sh->ctx) > > @@ -283,7 +296,6 @@ struct mlx5_dev_spawn_data { > > rte_free(sh); > > } > > > > - > > /** > > * Prepare shared data between primary and secondary process. > > */ > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index > > d816d24..f23298e 100644 > > --- a/drivers/net/mlx5/mlx5.h > > +++ b/drivers/net/mlx5/mlx5.h > > @@ -216,6 +216,8 @@ struct mlx5_ibv_shared { > > char ibdev_name[IBV_SYSFS_NAME_MAX]; /* IB device name. */ > > char ibdev_path[IBV_SYSFS_PATH_MAX]; /* IB device path for > secondary > > */ > > struct ibv_device_attr_ex device_attr; /* Device properties. */ > > + pthread_mutex_t intr_mutex; /* Interrupt config mutex. */ > > + uint32_t intr_cnt; /* Interrupt handler reference counter. */ > > struct rte_intr_handle intr_handle; /* Interrupt handler for device. > > */ > > struct mlx5_ibv_shared_port port[]; /* per device port data array. > > */ }; @@ -245,7 +247,6 @@ struct mlx5_priv { > > struct mlx5_txq_data *(*txqs)[]; /* TX queues. */ > > struct rte_mempool *mprq_mp; /* Mempool for Multi-Packet RQ. > > */ > > struct rte_eth_rss_conf rss_conf; /* RSS configuration. */ > > - struct rte_intr_handle intr_handle; /* Interrupt handler. */ > > unsigned int (*reta_idx)[]; /* RETA index table. */ > > unsigned int reta_idx_n; /* RETA index size. */ > > struct mlx5_drop drop_queue; /* Flow drop queues. */ diff --git > > a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c > > index > > 1b2173b..8358cd2 100644 > > --- a/drivers/net/mlx5/mlx5_ethdev.c > > +++ b/drivers/net/mlx5/mlx5_ethdev.c > > @@ -1109,6 +1109,96 @@ int mlx5_fw_version_get(struct rte_eth_dev > > *dev, char *fw_ver, size_t fw_size) } > > > > /** > > + * Uninstall shared asynchronous device events handler. > > + * This function is implemeted to support event sharing > > + * between multiple ports of single IB device. > > + * > > + * @param dev > > + * Pointer to Ethernet device. > > + */ > > +static void > > +mlx5_dev_shared_handler_uninstall(struct rte_eth_dev *dev) { > > + struct mlx5_priv *priv =3D dev->data->dev_private; > > + struct mlx5_ibv_shared *sh =3D priv->sh; > > + > > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > > + return; > > + pthread_mutex_lock(&sh->intr_mutex); > > + assert(priv->ibv_port); > > + 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 >=3D RTE_MAX_ETHPORTS) > > + goto exit; > > + assert(sh->port[priv->ibv_port - 1].port_id =3D=3D > > + (uint32_t)dev->data->port_id); > > + assert(sh->intr_cnt); > > + sh->port[priv->ibv_port - 1].port_id =3D RTE_MAX_ETHPORTS; > > + if (!sh->intr_cnt || --sh->intr_cnt) > > + goto exit; > > + rte_intr_callback_unregister(&sh->intr_handle, > > + mlx5_dev_interrupt_handler, sh); > > + sh->intr_handle.fd =3D 0; > > + sh->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > > +exit: > > + pthread_mutex_unlock(&sh->intr_mutex); > > +} > > + > > +/** > > + * Install shared asyncronous device events handler. > > + * This function is implemeted to support event sharing > > + * between multiple ports of single IB device. > > + * > > + * @param dev > > + * Pointer to Ethernet device. > > + */ > > +static void > > +mlx5_dev_shared_handler_install(struct rte_eth_dev *dev) { > > + struct mlx5_priv *priv =3D dev->data->dev_private; > > + struct mlx5_ibv_shared *sh =3D priv->sh; > > + int ret; > > + int flags; > > + > > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > > + return; > > + pthread_mutex_lock(&sh->intr_mutex); > > + assert(priv->ibv_port); > > + 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) { >=20 > I don't understand why need an array to understand handler is already > exists. > Why not the refcnt? 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 even= t - check in the array whether the handler is installed for this port=20 (array member is less than RTE_MAX_ETHPORTS) - get DPDK port_id from array() 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.=20 rte_intr_callback_register() is called when the first handler for the port = is being installed. rte_intr_callback_unregister() is called when the lastt handler for the por= t is being gone away. >=20 > > + /* The handler is already installed for this port. */ > > + assert(sh->intr_cnt++); >=20 > Asserts are compiled only in debug mode. You should not put any logic (++= ) > into them. Yes, it is a bug, there should no be "++" at all. Thanks.=20 >=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 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 913C3A00E6 for ; Thu, 21 Mar 2019 15:01:56 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 55CD31B4DC; Thu, 21 Mar 2019 15:01:56 +0100 (CET) Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00053.outbound.protection.outlook.com [40.107.0.53]) by dpdk.org (Postfix) with ESMTP id 2B1A31B4CB for ; Thu, 21 Mar 2019 15:01:55 +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=P6r2Jc1WLjYV4lnIH2Tnv8kWDXlAwyf5Ddsf9x6npPk=; b=hPD+XScEXeGyAZjwJs0812T6rj9cL7tfh8TS74KyExvxOJsl7jNXPsQ2+x3buAAT+J7yBMBmAU//ZZ92/0BUpmlCH1/6Zhk80E8dd7GAbAn5K3SdmNUtFMjy/MzoeCA3Wx5CXeIWJQ9ZC1NjWvjxdC8v54TNsTyDjzbw+c9SSMM= Received: from AM4PR05MB3265.eurprd05.prod.outlook.com (10.171.188.154) by AM4PR05MB3428.eurprd05.prod.outlook.com (10.171.186.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1709.14; Thu, 21 Mar 2019 14:01:53 +0000 Received: from AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::11b0:de86:8d93:8b02]) by AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::11b0:de86:8d93:8b02%3]) with mapi id 15.20.1709.015; Thu, 21 Mar 2019 14:01:53 +0000 From: Slava Ovsiienko To: Shahaf Shuler , "dev@dpdk.org" Thread-Topic: [PATCH 12/14] net/mlx5: update install/uninstall int handler routines Thread-Index: AQHU39+7dk02X1WwiUK3qCpZAVTOQaYWGVlA Date: Thu, 21 Mar 2019 14:01:53 +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: authentication-results: spf=none (sender IP is ) smtp.mailfrom=viacheslavo@mellanox.com; x-originating-ip: [95.67.35.250] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: bc5a60ad-d596-4aeb-79f3-08d6ae05c5c4 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020); SRVR:AM4PR05MB3428; x-ms-traffictypediagnostic: AM4PR05MB3428: x-microsoft-antispam-prvs: x-forefront-prvs: 0983EAD6B2 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(366004)(396003)(346002)(39860400002)(376002)(199004)(189003)(13464003)(86362001)(110136005)(97736004)(2906002)(6436002)(71190400001)(7736002)(81166006)(71200400001)(105586002)(68736007)(8676002)(66066001)(486006)(14454004)(81156014)(106356001)(30864003)(11346002)(305945005)(93886005)(316002)(256004)(478600001)(2501003)(5024004)(476003)(99286004)(14444005)(26005)(9686003)(76176011)(53936002)(6506007)(25786009)(5660300002)(55016002)(6246003)(74316002)(33656002)(6116002)(7696005)(186003)(102836004)(15650500001)(52536014)(3846002)(8936002)(229853002)(53546011)(446003); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR05MB3428; H:AM4PR05MB3265.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: VKcdqPGEvqN4EQVwzf/hga32V+JV1bnHui60/f1TnmZW16ZLxfFUthhI/Y240QN8LgBE10/MwzNMVnq9dQJ7TyoxT6xIPlckjkqIvrUjueoZbryDJ85P2HHp8OHJNo0HxFMR/FD13gURdRsWFWQLSERg2Kwvw4f7fJdYC9XlsNhjFSdHBIqoqW10A1QvrctdIY/SGDpyGEWbO2W8qVq3HW0J6cLOrlDzAVAc7VQlej3WEW31KxiFcGqO/4+7mlcju0Wj2/XVMCYPyaM6vuIgRjLCX/xgzODJGZ52ftEwUMq1xAXEZJLR+4nvovIpCFZcj69WPBGYAWBRtw4oTCItmyhn/uXqjj2LtT0X3MeS1+WZMGf2zB04Cm3ptzambZZ8I4/KmhMnMjipa0yP8dH44rAKBVK0p7fF6O9rK+dc5fo= 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: bc5a60ad-d596-4aeb-79f3-08d6ae05c5c4 X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Mar 2019 14:01:53.3759 (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: AM4PR05MB3428 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: <20190321140153.rPv9OvCaLgqVbyGmljHaUw3b8waeU8Y1CdIzczIIEf8@z> > -----Original Message----- > From: Shahaf Shuler > Sent: Thursday, March 21, 2019 14:15 > To: Slava Ovsiienko ; dev@dpdk.org > Subject: RE: [PATCH 12/14] net/mlx5: update install/uninstall int handler > routines >=20 > 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 > > --- > > drivers/net/mlx5/mlx5.c | 14 ++++- > > drivers/net/mlx5/mlx5.h | 3 +- > > drivers/net/mlx5/mlx5_ethdev.c | 118 > > ++++++++++++++++++++++++++++++++--------- > > 3 files changed, 107 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index > > 312c42b..44b7a87 100644 > > --- a/drivers/net/mlx5/mlx5.c > > +++ b/drivers/net/mlx5/mlx5.c > > @@ -165,6 +165,7 @@ struct mlx5_dev_spawn_data { { > > struct mlx5_ibv_shared *sh; > > int err =3D 0; > > + uint32_t i; > > > > 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; >=20 > Why you need struct here? You port array is not just of uint32_t type? 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 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. =09 >=20 > > if (rte_eal_process_type() =3D=3D RTE_PROC_SECONDARY) { > > /* > > * For secondary process we just open the IB device @@ - > > 276,6 +280,15 @@ struct mlx5_dev_spawn_data { > > assert(!sh->pd); > > } > > LIST_REMOVE(sh, next); > > + /* > > + * Ensure there is no async event handler installed. > > + * Only primary process handles async device events. > > + **/ > > + assert(!sh->intr_cnt); > > + if (sh->intr_cnt) > > + rte_intr_callback_unregister > > + (&sh->intr_handle, mlx5_dev_interrupt_handler, > > sh); > > + pthread_mutex_destroy(&sh->intr_mutex); > > if (sh->pd) > > claim_zero(mlx5_glue->dealloc_pd(sh->pd)); > > if (sh->ctx) > > @@ -283,7 +296,6 @@ struct mlx5_dev_spawn_data { > > rte_free(sh); > > } > > > > - > > /** > > * Prepare shared data between primary and secondary process. > > */ > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index > > d816d24..f23298e 100644 > > --- a/drivers/net/mlx5/mlx5.h > > +++ b/drivers/net/mlx5/mlx5.h > > @@ -216,6 +216,8 @@ struct mlx5_ibv_shared { > > char ibdev_name[IBV_SYSFS_NAME_MAX]; /* IB device name. */ > > char ibdev_path[IBV_SYSFS_PATH_MAX]; /* IB device path for > secondary > > */ > > struct ibv_device_attr_ex device_attr; /* Device properties. */ > > + pthread_mutex_t intr_mutex; /* Interrupt config mutex. */ > > + uint32_t intr_cnt; /* Interrupt handler reference counter. */ > > struct rte_intr_handle intr_handle; /* Interrupt handler for device. > > */ > > struct mlx5_ibv_shared_port port[]; /* per device port data array. > > */ }; @@ -245,7 +247,6 @@ struct mlx5_priv { > > struct mlx5_txq_data *(*txqs)[]; /* TX queues. */ > > struct rte_mempool *mprq_mp; /* Mempool for Multi-Packet RQ. > > */ > > struct rte_eth_rss_conf rss_conf; /* RSS configuration. */ > > - struct rte_intr_handle intr_handle; /* Interrupt handler. */ > > unsigned int (*reta_idx)[]; /* RETA index table. */ > > unsigned int reta_idx_n; /* RETA index size. */ > > struct mlx5_drop drop_queue; /* Flow drop queues. */ diff --git > > a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c > > index > > 1b2173b..8358cd2 100644 > > --- a/drivers/net/mlx5/mlx5_ethdev.c > > +++ b/drivers/net/mlx5/mlx5_ethdev.c > > @@ -1109,6 +1109,96 @@ int mlx5_fw_version_get(struct rte_eth_dev > > *dev, char *fw_ver, size_t fw_size) } > > > > /** > > + * Uninstall shared asynchronous device events handler. > > + * This function is implemeted to support event sharing > > + * between multiple ports of single IB device. > > + * > > + * @param dev > > + * Pointer to Ethernet device. > > + */ > > +static void > > +mlx5_dev_shared_handler_uninstall(struct rte_eth_dev *dev) { > > + struct mlx5_priv *priv =3D dev->data->dev_private; > > + struct mlx5_ibv_shared *sh =3D priv->sh; > > + > > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > > + return; > > + pthread_mutex_lock(&sh->intr_mutex); > > + assert(priv->ibv_port); > > + 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 >=3D RTE_MAX_ETHPORTS) > > + goto exit; > > + assert(sh->port[priv->ibv_port - 1].port_id =3D=3D > > + (uint32_t)dev->data->port_id); > > + assert(sh->intr_cnt); > > + sh->port[priv->ibv_port - 1].port_id =3D RTE_MAX_ETHPORTS; > > + if (!sh->intr_cnt || --sh->intr_cnt) > > + goto exit; > > + rte_intr_callback_unregister(&sh->intr_handle, > > + mlx5_dev_interrupt_handler, sh); > > + sh->intr_handle.fd =3D 0; > > + sh->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > > +exit: > > + pthread_mutex_unlock(&sh->intr_mutex); > > +} > > + > > +/** > > + * Install shared asyncronous device events handler. > > + * This function is implemeted to support event sharing > > + * between multiple ports of single IB device. > > + * > > + * @param dev > > + * Pointer to Ethernet device. > > + */ > > +static void > > +mlx5_dev_shared_handler_install(struct rte_eth_dev *dev) { > > + struct mlx5_priv *priv =3D dev->data->dev_private; > > + struct mlx5_ibv_shared *sh =3D priv->sh; > > + int ret; > > + int flags; > > + > > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > > + return; > > + pthread_mutex_lock(&sh->intr_mutex); > > + assert(priv->ibv_port); > > + 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) { >=20 > I don't understand why need an array to understand handler is already > exists. > Why not the refcnt? 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 even= t - check in the array whether the handler is installed for this port=20 (array member is less than RTE_MAX_ETHPORTS) - get DPDK port_id from array() 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.=20 rte_intr_callback_register() is called when the first handler for the port = is being installed. rte_intr_callback_unregister() is called when the lastt handler for the por= t is being gone away. >=20 > > + /* The handler is already installed for this port. */ > > + assert(sh->intr_cnt++); >=20 > Asserts are compiled only in debug mode. You should not put any logic (++= ) > into them. Yes, it is a bug, there should no be "++" at all. Thanks.=20 >=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