From 19b1a294b0b3f4e8080584cd560fc058f12123fb Mon Sep 17 00:00:00 2001 From: Erez Alfasi Date: Mon, 25 Feb 2019 08:52:30 +0200 Subject: RDMA: Use __packed annotation instead of __attribute__ ((packed)) "__attribute__" set of macros has been standardized, have became more potentially portable and consistent code back in v2.6.21 by commit 82ddcb040 ("[PATCH] extend the set of "__attribute__" shortcut macros"). Moreover, nowadays checkpatch.pl warns about using __attribute__((packed)) instead of __packed. This patch converts all the "__attribute__ ((packed))" annotations to "__packed" within the RDMA subsystem. Signed-off-by: Erez Alfasi Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe --- drivers/infiniband/ulp/iser/iscsi_iser.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h index a7aeaa0c6fbc..36d525110fd2 100644 --- a/drivers/infiniband/ulp/iser/iscsi_iser.h +++ b/drivers/infiniband/ulp/iser/iscsi_iser.h @@ -311,7 +311,7 @@ struct iser_login_desc { u64 rsp_dma; struct ib_sge sge; struct ib_cqe cqe; -} __attribute__((packed)); +} __packed; struct iser_conn; struct ib_conn; -- cgit v1.2.2 From 1a2e158327c957baef21a50f3a7f6e6942cbca1e Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Wed, 6 Mar 2019 23:08:45 +0100 Subject: drivers: infiniband: Fix whitespace in kconfig Adjust the kconfig whitespace in bnxt_re/iser to match the kernel standard. Signed-off-by: Enrico Weigelt, metux IT consult Signed-off-by: Jason Gunthorpe --- drivers/infiniband/ulp/iser/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/iser/Kconfig b/drivers/infiniband/ulp/iser/Kconfig index d00af71a2cfc..299268f261ee 100644 --- a/drivers/infiniband/ulp/iser/Kconfig +++ b/drivers/infiniband/ulp/iser/Kconfig @@ -4,8 +4,8 @@ config INFINIBAND_ISER select SCSI_ISCSI_ATTRS ---help--- Support for the iSCSI Extensions for RDMA (iSER) Protocol - over InfiniBand. This allows you to access storage devices - that speak iSCSI over iSER over InfiniBand. + over InfiniBand. This allows you to access storage devices + that speak iSCSI over iSER over InfiniBand. The iSER protocol is defined by IETF. See -- cgit v1.2.2 From 9513ea4f67280a17365f5adfa31fac7d344150c6 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Sat, 16 Mar 2019 23:05:12 +0000 Subject: IB/iser: remove uninitialized variable len The variable len is not being inintialized and the uninitialized value is being returned. However, this return path is never reached because the default case in the switch statement returns -ENOSYS. Clean up the code by replacing the return -ENOSYS with a break for the default case and returning -ENOSYS at the end of the function. This allows len to be removed. Also remove redundant break that follows a return statement. Signed-off-by: Colin Ian King Reviewed-by: Max Gurtovoy Reviewed-by: Sagi Grimberg Signed-off-by: Jason Gunthorpe --- drivers/infiniband/ulp/iser/iscsi_iser.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c index 8c707accd148..9c185a8dabd3 100644 --- a/drivers/infiniband/ulp/iser/iscsi_iser.c +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c @@ -763,7 +763,6 @@ static int iscsi_iser_get_ep_param(struct iscsi_endpoint *ep, enum iscsi_param param, char *buf) { struct iser_conn *iser_conn = ep->dd_data; - int len; switch (param) { case ISCSI_PARAM_CONN_PORT: @@ -774,12 +773,10 @@ static int iscsi_iser_get_ep_param(struct iscsi_endpoint *ep, return iscsi_conn_get_addr_param((struct sockaddr_storage *) &iser_conn->ib_conn.cma_id->route.addr.dst_addr, param, buf); - break; default: - return -ENOSYS; + break; } - - return len; + return -ENOSYS; } /** -- cgit v1.2.2 From fae7a699a92577e383c82ae42918ec257cf3bba9 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 20 Feb 2019 16:21:01 -0800 Subject: opa_vnic: Convert vport_idr to XArray Signed-off-by: Matthew Wilcox Signed-off-by: Jason Gunthorpe --- drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 60 ++++++++++--------------- 1 file changed, 23 insertions(+), 37 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c index 560e4f2d466e..76cd09410d9a 100644 --- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c +++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c @@ -51,6 +51,7 @@ */ #include +#include #include #include #include @@ -97,7 +98,7 @@ const char opa_vnic_driver_version[] = DRV_VERSION; * @class_port_info: Class port info information. * @tid: Transaction id * @port_num: OPA port number - * @vport_idr: vnic ports idr + * @vports: vnic ports * @event_handler: ib event handler * @lock: adapter interface lock */ @@ -107,7 +108,7 @@ struct opa_vnic_vema_port { struct opa_class_port_info class_port_info; u64 tid; u8 port_num; - struct idr vport_idr; + struct xarray vports; struct ib_event_handler event_handler; /* Lock to query/update network adapter */ @@ -148,7 +149,7 @@ vema_get_vport_adapter(struct opa_vnic_vema_mad *recvd_mad, { u8 vport_num = vema_get_vport_num(recvd_mad); - return idr_find(&port->vport_idr, vport_num); + return xa_load(&port->vports, vport_num); } /** @@ -207,8 +208,7 @@ static struct opa_vnic_adapter *vema_add_vport(struct opa_vnic_vema_port *port, int rc; adapter->cport = cport; - rc = idr_alloc(&port->vport_idr, adapter, vport_num, - vport_num + 1, GFP_NOWAIT); + rc = xa_insert(&port->vports, vport_num, adapter, GFP_KERNEL); if (rc < 0) { opa_vnic_rem_netdev(adapter); adapter = ERR_PTR(rc); @@ -853,36 +853,14 @@ err_exit: v_err("Aborting trap\n"); } -static int vema_rem_vport(int id, void *p, void *data) -{ - struct opa_vnic_adapter *adapter = p; - - opa_vnic_rem_netdev(adapter); - return 0; -} - -static int vema_enable_vport(int id, void *p, void *data) -{ - struct opa_vnic_adapter *adapter = p; - - netif_carrier_on(adapter->netdev); - return 0; -} - -static int vema_disable_vport(int id, void *p, void *data) -{ - struct opa_vnic_adapter *adapter = p; - - netif_carrier_off(adapter->netdev); - return 0; -} - static void opa_vnic_event(struct ib_event_handler *handler, struct ib_event *record) { struct opa_vnic_vema_port *port = container_of(handler, struct opa_vnic_vema_port, event_handler); struct opa_vnic_ctrl_port *cport = port->cport; + struct opa_vnic_adapter *adapter; + unsigned long index; if (record->element.port_num != port->port_num) return; @@ -891,10 +869,16 @@ static void opa_vnic_event(struct ib_event_handler *handler, record->event, dev_name(&record->device->dev), record->element.port_num); - if (record->event == IB_EVENT_PORT_ERR) - idr_for_each(&port->vport_idr, vema_disable_vport, NULL); - if (record->event == IB_EVENT_PORT_ACTIVE) - idr_for_each(&port->vport_idr, vema_enable_vport, NULL); + if (record->event != IB_EVENT_PORT_ERR || + record->event != IB_EVENT_PORT_ACTIVE) + return; + + xa_for_each(&port->vports, index, adapter) { + if (record->event == IB_EVENT_PORT_ACTIVE) + netif_carrier_on(adapter->netdev); + else + netif_carrier_off(adapter->netdev); + } } /** @@ -905,6 +889,8 @@ static void opa_vnic_event(struct ib_event_handler *handler, */ static void vema_unregister(struct opa_vnic_ctrl_port *cport) { + struct opa_vnic_adapter *adapter; + unsigned long index; int i; for (i = 1; i <= cport->num_ports; i++) { @@ -915,13 +901,14 @@ static void vema_unregister(struct opa_vnic_ctrl_port *cport) /* Lock ensures no MAD is being processed */ mutex_lock(&port->lock); - idr_for_each(&port->vport_idr, vema_rem_vport, NULL); + xa_for_each(&port->vports, index, adapter) + opa_vnic_rem_netdev(adapter); mutex_unlock(&port->lock); ib_unregister_mad_agent(port->mad_agent); port->mad_agent = NULL; mutex_destroy(&port->lock); - idr_destroy(&port->vport_idr); + xa_destroy(&port->vports); ib_unregister_event_handler(&port->event_handler); } } @@ -958,7 +945,7 @@ static int vema_register(struct opa_vnic_ctrl_port *cport) cport->ibdev, opa_vnic_event); ib_register_event_handler(&port->event_handler); - idr_init(&port->vport_idr); + xa_init(&port->vports); mutex_init(&port->lock); port->mad_agent = ib_register_mad_agent(cport->ibdev, i, IB_QPT_GSI, ®_req, @@ -969,7 +956,6 @@ static int vema_register(struct opa_vnic_ctrl_port *cport) ret = PTR_ERR(port->mad_agent); port->mad_agent = NULL; mutex_destroy(&port->lock); - idr_destroy(&port->vport_idr); vema_unregister(cport); return ret; } -- cgit v1.2.2 From 4d2e11d42fe4117c24e79a012904cf0fa7fdcfe3 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 4 Apr 2019 11:04:39 +0100 Subject: opa_vnic: fix check on record->event, incorrect operator used The check on record->event is always true because the wrong operator is being used, used && instead of || Addresses-Coverity: ("Constant expression result") Fixes: fae7a699a925 ("opa_vnic: Convert vport_idr to XArray") Signed-off-by: Colin Ian King Acked-by: Dennis Dalessandro Reviewed-by: Mukesh Ojha Signed-off-by: Jason Gunthorpe --- drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c index 76cd09410d9a..be5befd92d16 100644 --- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c +++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c @@ -869,7 +869,7 @@ static void opa_vnic_event(struct ib_event_handler *handler, record->event, dev_name(&record->device->dev), record->element.port_num); - if (record->event != IB_EVENT_PORT_ERR || + if (record->event != IB_EVENT_PORT_ERR && record->event != IB_EVENT_PORT_ACTIVE) return; -- cgit v1.2.2 From ba7d8117f3cca8eb70d579fde3f9ec8cd6a28f39 Mon Sep 17 00:00:00 2001 From: Dennis Dalessandro Date: Thu, 11 Apr 2019 07:22:35 -0700 Subject: IB/core, ipoib: Do not overreact to SM LID change event When IPoIB receives an SM LID change event, it reacts by flushing its path record cache and rejoining multicast groups. This is the same behavior it performs when it receives a reregistration event. This behavior is unnecessary as an SM may have database backup or synchronization mechanisms which permit the SM location or LID to change without loss of multicast membership and without impact to path records. Both opensm and the OPA FM issue reregistration events if a new SM is started (or restarted with a new config) or an SM event occurs which results in loss of multicast membership records by the SM (such as opensm failover) or the SM encounters new nodes with Active ports (such as after joining 2 fabrics by connecting switches via ISLs). Hence this event can be depended on as the trigger for IPoIB cache and multicast flushing. It appears that some drivers, such as qib, and hfi1 issue the IB_EVENT_SM_CHANGE but other drivers such as mlx4 and mlx5 do not. Empirical testing on Mellanox EDR using ibv_asyncwatch has confirmed that Mellanox EDR HCAs do not generate SM change events and that opensm does generate reregistration. An SM LID change event is generated by the mentioned drivers to reflect that sm_lid and/or sm_sl in the local port info has changed. The intent of this event is to permit applications and ULPs which have a local copy of this information (or an address handle using it) to update their information. The intent is that the reregistration event (caused by the SM via a bit in Set(PortInfo)) be used to inform nodes that they need to rejoin multicast groups, resubscribe for notices and potentially update path records. When an SM migrates or fails over, a SM LID change event can occur. In response IPoIB discards path records and multicast membership and loses connectivity until these records are restored via SA requests. In very large fabrics, it may take minutes for the SM to be ready and for the SA responses to be supplied. This can result in undesirable and unnecessary IPoIB connectivity impacts. It also can result in an unnecessary storm of SA queries from all nodes in a cluster potentially followed by yet another storm if the SM issues the reregistration request. The fact the Mellanox HCAs do not even generate this event, is further evidence that on modern IB fabrics there will be no ill side effects from the proposed changes below to reduce the reaction by 3 kernel components to this event. So these changes should be benign for Mellanox IB fabrics and will benefit OPA fabrics while also making ib_core and ULP behavor "correct" as intended by the IBTA spec and kernel RDMA event APIs. Address these issues by removing IB_EVENT_SM_CHANGE handling from ipoib. IPoIB does not locally store sm_lid nor sm_sl, so it does not need to do anything on SM LID change. IPoIB makes use of other ib_core components to issue SA requests for it and those components correctly track SM LID and SM LID changes. Also in ib_core multicast handling, remove the test for IB_EVENT_SM_CHANGE. This code is moving all multicast groups to the error state, which will trigger rejoins. This code is used by IPoIB as well as the connection manager and other clients of multicast groups. This kernel module centralizes group membership status and joins since a node can only join a given group once but multiple ULPs or applications may want to join the same group. It makes use of the sa_query.c component in ib_core, which correctly trackes SM LID and SL. This component does not track SM LID nor SL itself and hence need not react to their changes. Similarly in the ib_core cache code remove the handling for the IB_EVENT_SM_CHANGE. In this function. The ib_cache_update function which is ultimately called is updating local copies of the pkey table, gid table and lmc. It does not update nor retain sm_lid nor sm_sl. As such it does not need to be called on an SM LID change. It technically also does not need to be called on a reregistration. The LID_CHANGE, PKEY_CHANGE, GID_CHANGE and port state change events (PORT_ERR, PORT_ACTICE) should be sufficient triggers. It is worth noting that the alternative of simply having the hfi1 and qib drivers not generate the SM LID change event was explored. While this would duplicate what Mellanox drivers do now, it is not the correct behavior and removes the ability for an SM to migrate without requiring reregistration. Since both opensm and OPA SM have mechanisms to backup or synchronize registration information, it is desirable to let them perform SM migrations (with LID or SL changes) without requiring reregistration when they deem it appropriate. Suggested-by: Todd Rimmer Tested-by: Michael Brooks Reviewed-by: Mike Marciniszyn Reviewed-by: Todd Rimmer Signed-off-by: Dennis Dalessandro Signed-off-by: Jason Gunthorpe --- drivers/infiniband/ulp/ipoib/ipoib_verbs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c index 1e88213459f2..ba09068f6200 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c @@ -279,8 +279,7 @@ void ipoib_event(struct ib_event_handler *handler, ipoib_dbg(priv, "Event %d on device %s port %d\n", record->event, dev_name(&record->device->dev), record->element.port_num); - if (record->event == IB_EVENT_SM_CHANGE || - record->event == IB_EVENT_CLIENT_REREGISTER) { + if (record->event == IB_EVENT_CLIENT_REREGISTER) { queue_work(ipoib_workqueue, &priv->flush_light); } else if (record->event == IB_EVENT_PORT_ERR || record->event == IB_EVENT_PORT_ACTIVE || -- cgit v1.2.2 From b79656ed44c6865e17bcd93472ec39488bcc4984 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Mon, 6 May 2019 14:23:04 +0300 Subject: RDMA/ipoib: Allow user space differentiate between valid dev_port Systemd triggers the following warning during IPoIB device load: mlx5_core 0000:00:0c.0 ib0: "systemd-udevd" wants to know my dev_id. Should it look at dev_port instead? See Documentation/ABI/testing/sysfs-class-net for more info. This is caused due to user space attempt to differentiate old systems without dev_port and new systems with dev_port. In case dev_port will be zero, the systemd will try to read dev_id instead. There is no need to print a warning in such case, because it is valid situation and it is needed to ensure systemd compatibility with old kernels. Link: https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c#L358 Cc: # 4.19 Fixes: f6350da41dc7 ("IB/ipoib: Log sysfs 'dev_id' accesses from userspace") Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe --- drivers/infiniband/ulp/ipoib/ipoib_main.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 48eda16db1a7..9b5e11d3fb85 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -2402,7 +2402,18 @@ static ssize_t dev_id_show(struct device *dev, { struct net_device *ndev = to_net_dev(dev); - if (ndev->dev_id == ndev->dev_port) + /* + * ndev->dev_port will be equal to 0 in old kernel prior to commit + * 9b8b2a323008 ("IB/ipoib: Use dev_port to expose network interface + * port numbers") Zero was chosen as special case for user space + * applications to fallback and query dev_id to check if it has + * different value or not. + * + * Don't print warning in such scenario. + * + * https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c#L358 + */ + if (ndev->dev_port && ndev->dev_id == ndev->dev_port) netdev_info_once(ndev, "\"%s\" wants to know my dev_id. Should it look at dev_port instead? See Documentation/ABI/testing/sysfs-class-net for more info.\n", current->comm); -- cgit v1.2.2