From 29da686dff757968fbf5a9bb5246070ddf602664 Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:20 +0200 Subject: IB/ipoib: When given an invalid UD MTU, give debug msg In datagram mode, the IB UD (Unreliable Datagram) transport is used so the MTU of the interface is equal to the IB L2 MTU minus the IPoIB encapsulation header. Any request to change the MTU value above the maximum range will change the MTU to the max allowed, but will not show any warning message. An ipoib_warn is issued in such cases, letting the user know that even though the value is legal, it can't be currently applied. Signed-off-by: Feras Daoud Signed-off-by: Noa Osherovich Signed-off-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_main.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 3ce0765a05ab..b2a75d85bc23 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -229,6 +229,10 @@ static int ipoib_change_mtu(struct net_device *dev, int new_mtu) priv->admin_mtu = new_mtu; + if (priv->mcast_mtu < priv->admin_mtu) + ipoib_dbg(priv, "MTU must be smaller than the underlying " + "link layer MTU - 4 (%u)\n", priv->mcast_mtu); + dev->mtu = min(priv->mcast_mtu, priv->admin_mtu); return 0; -- cgit v1.2.3-59-g8ed1b From 80b5b35aba62232521b31440f0a3cf6caa033849 Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:21 +0200 Subject: IB/ipoib: Set device connection mode only when needed When changing the connection mode, the ipoib_set_mode function did not check if the previous connection mode equals to the new one. This commit adds the required check and return 0 if the new mode equals to the previous one. Fixes: 839fcaba355a ("IPoIB: Connected mode experimental support") Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Reviewed-by: Alex Vesker Reviewed-by: Yuval Shaia Signed-off-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_main.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index b2a75d85bc23..cbd06a882a60 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -474,6 +474,13 @@ int ipoib_set_mode(struct net_device *dev, const char *buf) { struct ipoib_dev_priv *priv = netdev_priv(dev); + if ((test_bit(IPOIB_FLAG_ADMIN_CM, &priv->flags) && + !strcmp(buf, "connected\n")) || + (!test_bit(IPOIB_FLAG_ADMIN_CM, &priv->flags) && + !strcmp(buf, "datagram\n"))) { + return 0; + } + /* flush paths if we switch modes so that connections are restarted */ if (IPOIB_CM_SUPPORTED(dev->dev_addr) && !strcmp(buf, "connected\n")) { set_bit(IPOIB_FLAG_ADMIN_CM, &priv->flags); -- cgit v1.2.3-59-g8ed1b From 1c3098cdb05207e740715857df7b0998e372f527 Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:22 +0200 Subject: IB/ipoib: Fix deadlock over vlan_mutex This patch fixes Deadlock while executing ipoib_vlan_delete. The function takes the vlan_rwsem semaphore and calls unregister_netdevice. The later function calls ipoib_mcast_stop_thread that cause workqueue flush. When the queue has one of the ipoib_ib_dev_flush_xxx events, a deadlock occur because these events also tries to catch the same vlan_rwsem semaphore. To fix, unregister_netdevice should be called after releasing the semaphore. Fixes: cbbe1efa4972 ("IPoIB: Fix deadlock between ipoib_open() and child interface create") Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Reviewed-by: Alex Vesker Signed-off-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c index fd811115af49..96821838c352 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c @@ -196,7 +196,6 @@ int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey) list_for_each_entry_safe(priv, tpriv, &ppriv->child_intfs, list) { if (priv->pkey == pkey && priv->child_type == IPOIB_LEGACY_CHILD) { - unregister_netdevice(priv->dev); list_del(&priv->list); dev = priv->dev; break; @@ -204,6 +203,11 @@ int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey) } up_write(&ppriv->vlan_rwsem); + if (dev) { + ipoib_dbg(ppriv, "delete child vlan %s\n", dev->name); + unregister_netdevice(dev); + } + rtnl_unlock(); if (dev) { -- cgit v1.2.3-59-g8ed1b From 0a0007f28304cb9fc87809c86abb80ec71317f20 Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:23 +0200 Subject: IB/ipoib: Fix deadlock between rmmod and set_mode When calling set_mode from sys/fs, the call flow locks the sys/fs lock first and then tries to lock rtnl_lock (when calling ipoib_set_mod). On the other hand, the rmmod call flow takes the rtnl_lock first (when calling unregister_netdev) and then tries to take the sys/fs lock. Deadlock a->b, b->a. The problem starts when ipoib_set_mod frees it's rtnl_lck and tries to get it after that. set_mod: [] ? check_preempt_curr+0x6d/0x90 [] __mutex_lock_slowpath+0x13e/0x180 [] ? __rtnl_unlock+0x15/0x20 [] mutex_lock+0x2b/0x50 [] rtnl_lock+0x15/0x20 [] ipoib_set_mode+0x97/0x160 [ib_ipoib] [] set_mode+0x3b/0x80 [ib_ipoib] [] dev_attr_store+0x20/0x30 [] sysfs_write_file+0xe5/0x170 [] vfs_write+0xb8/0x1a0 [] sys_write+0x51/0x90 [] system_call_fastpath+0x16/0x1b rmmod: [] ? put_dec+0x10c/0x110 [] ? number+0x2ee/0x320 [] schedule_timeout+0x215/0x2e0 [] ? vsnprintf+0x484/0x5f0 [] ? string+0x40/0x100 [] wait_for_common+0x123/0x180 [] ? default_wake_function+0x0/0x20 [] ? ifind_fast+0x5e/0xb0 [] wait_for_completion+0x1d/0x20 [] sysfs_addrm_finish+0x228/0x270 [] sysfs_remove_dir+0xa3/0xf0 [] kobject_del+0x16/0x40 [] device_del+0x184/0x1e0 [] netdev_unregister_kobject+0xab/0xc0 [] rollback_registered+0xae/0x130 [] unregister_netdevice+0x22/0x70 [] unregister_netdev+0x1e/0x30 [] ipoib_remove_one+0xe0/0x120 [ib_ipoib] [] ib_unregister_device+0x4f/0x100 [ib_core] [] mlx4_ib_remove+0x41/0x180 [mlx4_ib] [] mlx4_remove_device+0x71/0x90 [mlx4_core] Fixes: 862096a8bbf8 ("IB/ipoib: Add more rtnl_link_ops callbacks") Cc: # v3.6+ Cc: Or Gerlitz Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Signed-off-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 12 +++++++----- drivers/infiniband/ulp/ipoib/ipoib_main.c | 6 ++---- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index 096c4f6fbd65..1c7a9a16efc7 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -1507,12 +1507,14 @@ static ssize_t set_mode(struct device *d, struct device_attribute *attr, ret = ipoib_set_mode(dev, buf); - rtnl_unlock(); - - if (!ret) - return count; + /* The assumption is that the function ipoib_set_mode returned + * with the rtnl held by it, if not the value -EBUSY returned, + * then no need to rtnl_unlock + */ + if (ret != -EBUSY) + rtnl_unlock(); - return ret; + return (!ret || ret == -EBUSY) ? count : ret; } static DEVICE_ATTR(mode, S_IWUSR | S_IRUGO, show_mode, set_mode); diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index cbd06a882a60..045f844d7f4a 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -492,8 +492,7 @@ int ipoib_set_mode(struct net_device *dev, const char *buf) priv->tx_wr.wr.send_flags &= ~IB_SEND_IP_CSUM; ipoib_flush_paths(dev); - rtnl_lock(); - return 0; + return (!rtnl_trylock()) ? -EBUSY : 0; } if (!strcmp(buf, "datagram\n")) { @@ -502,8 +501,7 @@ int ipoib_set_mode(struct net_device *dev, const char *buf) dev_set_mtu(dev, min(priv->mcast_mtu, dev->mtu)); rtnl_unlock(); ipoib_flush_paths(dev); - rtnl_lock(); - return 0; + return (!rtnl_trylock()) ? -EBUSY : 0; } return -EINVAL; -- cgit v1.2.3-59-g8ed1b From 89a3987ab7a923c047c6dec008e60ad6f41fac22 Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:24 +0200 Subject: IB/ipoib: rtnl_unlock can not come after free_netdev The ipoib_vlan_add function calls rtnl_unlock after free_netdev, rtnl_unlock not only releases the lock, but also calls netdev_run_todo. The latter function browses the net_todo_list array and completes the unregistration of all its net_device instances. If we call free_netdev before rtnl_unlock, then netdev_run_todo call over the freed device causes panic. To fix, move rtnl_unlock call before free_netdev call. Fixes: 9baa0b036410 ("IB/ipoib: Add rtnl_link_ops support") Cc: Or Gerlitz Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Reviewed-by: Yuval Shaia Signed-off-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c index 96821838c352..d9dab4a109f7 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c @@ -168,11 +168,11 @@ int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey) out: up_write(&ppriv->vlan_rwsem); + rtnl_unlock(); + if (result) free_netdev(priv->dev); - rtnl_unlock(); - return result; } -- cgit v1.2.3-59-g8ed1b From d32b9a81d7c9bf111536b547a60b50c8dd7fccd1 Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:25 +0200 Subject: IB/ipoib: Add detailed error message to dev_queue_xmit call Add a detailed return code to dev_queue_xmit function when calling to requeue packet via __skb_dequeue. Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Reviewed-by: Yuval Shaia Signed-off-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 7 ++++--- drivers/infiniband/ulp/ipoib/ipoib_main.c | 8 +++++--- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 6 ++++-- 3 files changed, 13 insertions(+), 8 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index 1c7a9a16efc7..a720d2ddddc1 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -1015,9 +1015,10 @@ static int ipoib_cm_rep_handler(struct ib_cm_id *cm_id, struct ib_cm_event *even while ((skb = __skb_dequeue(&skqueue))) { skb->dev = p->dev; - if (dev_queue_xmit(skb)) - ipoib_warn(priv, "dev_queue_xmit failed " - "to requeue packet\n"); + ret = dev_queue_xmit(skb); + if (ret) + ipoib_warn(priv, "%s:dev_queue_xmit failed to re-queue packet, ret:%d\n", + __func__, ret); } ret = ib_send_cm_rtu(cm_id, NULL, 0); diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 045f844d7f4a..1086858f1cbf 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -843,10 +843,12 @@ static void path_rec_completion(int status, ipoib_put_ah(old_ah); while ((skb = __skb_dequeue(&skqueue))) { + int ret; skb->dev = dev; - if (dev_queue_xmit(skb)) - ipoib_warn(priv, "dev_queue_xmit failed " - "to requeue packet\n"); + ret = dev_queue_xmit(skb); + if (ret) + ipoib_warn(priv, "%s: dev_queue_xmit failed to re-queue packet, ret:%d\n", + __func__, ret); } } diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index fddff403d5d2..7c6c67bbdab3 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -314,9 +314,11 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast, netif_tx_unlock_bh(dev); skb->dev = dev; - if (dev_queue_xmit(skb)) - ipoib_warn(priv, "dev_queue_xmit failed to requeue packet\n"); + ret = dev_queue_xmit(skb); + if (ret) + ipoib_warn(priv, "%s:dev_queue_xmit failed to re-queue packet, ret:%d\n", + __func__, ret); netif_tx_lock_bh(dev); } netif_tx_unlock_bh(dev); -- cgit v1.2.3-59-g8ed1b From 13ee429a0271b997f57b22ee67d40fc601f0b220 Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:26 +0200 Subject: IB/ipoib: Use debug prints instead of warnings in RNR WC status If a receive request has not been posted to the work queue, the incoming message is rejected and the peer will receive a receiver-not-ready (RNR) error. In IPoIB, IB_WC_RNR_RETRY_EXC_ERR error is part of the life cycle therefore ipoib_cm_handle_tx_wc function will print to debug instead of warnings. Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Signed-off-by: Leon Romanovsky Reviewed-by: Yuval Shaia Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index a720d2ddddc1..c433e72994f5 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -820,9 +820,12 @@ void ipoib_cm_handle_tx_wc(struct net_device *dev, struct ib_wc *wc) wc->status != IB_WC_WR_FLUSH_ERR) { struct ipoib_neigh *neigh; - ipoib_dbg(priv, "failed cm send event " - "(status=%d, wrid=%d vend_err %x)\n", - wc->status, wr_id, wc->vendor_err); + if (wc->status != IB_WC_RNR_RETRY_EXC_ERR) + ipoib_warn(priv, "failed cm send event (status=%d, wrid=%d vend_err %x)\n", + wc->status, wr_id, wc->vendor_err); + else + ipoib_dbg(priv, "failed cm send event (status=%d, wrid=%d vend_err %x)\n", + wc->status, wr_id, wc->vendor_err); spin_lock_irqsave(&priv->lock, flags); neigh = tx->neigh; -- cgit v1.2.3-59-g8ed1b From c586071d1dc8227a7182179b8e50ee92cc43f6d2 Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:27 +0200 Subject: IB/ipoib: Replace list_del of the neigh->list with list_del_init In order to resolve a situation where a few process delete the same list element in sequence and cause panic, list_del is replaced with list_del_init. In this case if the first process that calls list_del releases the lock before acquiring it again, other processes who can acquire the lock will call list_del_init. Fixes: b63b70d87741 ("IPoIB: Use a private hash table for path lookup") Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Reviewed-by: Alex Vesker Signed-off-by: Leon Romanovsky Reviewed-by: Yuval Shaia Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 1086858f1cbf..ee32a2a87e2d 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -1297,7 +1297,7 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv *priv) rcu_dereference_protected(neigh->hnext, lockdep_is_held(&priv->lock))); /* remove from path/mc list */ - list_del(&neigh->list); + list_del_init(&neigh->list); call_rcu(&neigh->rcu, ipoib_neigh_reclaim); } else { np = &neigh->hnext; @@ -1461,7 +1461,7 @@ void ipoib_neigh_free(struct ipoib_neigh *neigh) rcu_dereference_protected(neigh->hnext, lockdep_is_held(&priv->lock))); /* remove from parent list */ - list_del(&neigh->list); + list_del_init(&neigh->list); call_rcu(&neigh->rcu, ipoib_neigh_reclaim); return; } else { @@ -1546,7 +1546,7 @@ void ipoib_del_neighs_by_gid(struct net_device *dev, u8 *gid) rcu_dereference_protected(neigh->hnext, lockdep_is_held(&priv->lock))); /* remove from parent list */ - list_del(&neigh->list); + list_del_init(&neigh->list); call_rcu(&neigh->rcu, ipoib_neigh_reclaim); } else { np = &neigh->hnext; @@ -1588,7 +1588,7 @@ static void ipoib_flush_neighs(struct ipoib_dev_priv *priv) rcu_dereference_protected(neigh->hnext, lockdep_is_held(&priv->lock))); /* remove from path/mc list */ - list_del(&neigh->list); + list_del_init(&neigh->list); call_rcu(&neigh->rcu, ipoib_neigh_reclaim); } } -- cgit v1.2.3-59-g8ed1b From 27d41d29c7f093f6f77843624fbb080c1b4a8b9c Mon Sep 17 00:00:00 2001 From: Feras Daoud Date: Wed, 28 Dec 2016 14:47:28 +0200 Subject: IB/ipoib: Change list_del to list_del_init in the tx object Since ipoib_cm_tx_start function and ipoib_cm_tx_reap function belong to different work queues, they can run in parallel. In this case if ipoib_cm_tx_reap calls list_del and release the lock, ipoib_cm_tx_start may acquire it and call list_del_init on the already deleted object. Changing list_del to list_del_init in ipoib_cm_tx_reap fixes the problem. Fixes: 839fcaba355a ("IPoIB: Connected mode experimental support") Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Reviewed-by: Alex Vesker Signed-off-by: Leon Romanovsky Reviewed-by: Yuval Shaia Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index c433e72994f5..53d69d7d9ad4 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -1392,7 +1392,7 @@ static void ipoib_cm_tx_reap(struct work_struct *work) while (!list_empty(&priv->cm.reap_list)) { p = list_entry(priv->cm.reap_list.next, typeof(*p), list); - list_del(&p->list); + list_del_init(&p->list); spin_unlock_irqrestore(&priv->lock, flags); netif_tx_unlock_bh(dev); ipoib_cm_tx_destroy(p); -- cgit v1.2.3-59-g8ed1b From 506f71d1811aaf2d6a6c0bd132ceba29f5c14f3e Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 30 Dec 2016 14:38:05 +0000 Subject: IB/isert: fix spelling mistake: "teminating" -> "terminating" Trivial fix to spelling mistake in isert_warn message Signed-off-by: Colin Ian King Reviewed-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/isert/ib_isert.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c index 314e95516068..91cbe86b25c8 100644 --- a/drivers/infiniband/ulp/isert/ib_isert.c +++ b/drivers/infiniband/ulp/isert/ib_isert.c @@ -728,7 +728,7 @@ isert_disconnected_handler(struct rdma_cm_id *cma_id, iscsit_cause_connection_reinstatement(isert_conn->conn, 0); break; default: - isert_warn("conn %p teminating in state %d\n", + isert_warn("conn %p terminating in state %d\n", isert_conn, isert_conn->state); } mutex_unlock(&isert_conn->mutex); -- cgit v1.2.3-59-g8ed1b From f7534f45dcbc1a2ecb486b019db0443188e957d6 Mon Sep 17 00:00:00 2001 From: Zhu Yanjun Date: Thu, 5 Jan 2017 03:56:08 -0500 Subject: IB/ipoib: Remove unnecessary returned value check In the function ipoib_set_dev_features, the returned value is always 0. As such, it is not necessary to check the returned value. This is not a bug. It is a trivial problem. Reviewed-by: Guanglei Li Signed-off-by: Zhu Yanjun Reviewed-by: Yuval Shaia Reviewed-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib.h | 2 +- drivers/infiniband/ulp/ipoib/ipoib_main.c | 8 ++------ drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 4 +--- 3 files changed, 4 insertions(+), 10 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h index da12717a3eb7..f5680642960a 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib.h +++ b/drivers/infiniband/ulp/ipoib/ipoib.h @@ -593,7 +593,7 @@ void ipoib_pkey_open(struct ipoib_dev_priv *priv); void ipoib_drain_cq(struct net_device *dev); void ipoib_set_ethtool_ops(struct net_device *dev); -int ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca); +void ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca); #define IPOIB_FLAGS_RC 0x80 #define IPOIB_FLAGS_UC 0x40 diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index ee32a2a87e2d..c8af6377a288 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -1995,7 +1995,7 @@ int ipoib_add_pkey_attr(struct net_device *dev) return device_create_file(&dev->dev, &dev_attr_pkey); } -int ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca) +void ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca) { priv->hca_caps = hca->attrs.device_cap_flags; @@ -2007,8 +2007,6 @@ int ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca) priv->dev->features |= priv->dev->hw_features; } - - return 0; } static struct net_device *ipoib_add_port(const char *format, @@ -2048,9 +2046,7 @@ static struct net_device *ipoib_add_port(const char *format, goto device_init_failed; } - result = ipoib_set_dev_features(priv, hca); - if (result) - goto device_init_failed; + ipoib_set_dev_features(priv, hca); /* * Set the full membership bit, so that we join the right diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c index d9dab4a109f7..deedb6fc1b05 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c @@ -61,9 +61,7 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, priv->parent = ppriv->dev; set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags); - result = ipoib_set_dev_features(priv, ppriv->ca); - if (result) - goto err; + ipoib_set_dev_features(priv, ppriv->ca); priv->pkey = pkey; -- cgit v1.2.3-59-g8ed1b From dfc0e5550664a727a59921db7d9e7a41c21d03bb Mon Sep 17 00:00:00 2001 From: Zhu Yanjun Date: Thu, 12 Jan 2017 02:39:01 -0500 Subject: IB/ipoib: function interface change The ipoib_ib_dev_down/ipoib_ib_dev_stop return zero unconditionally and the callers never check the returned values, change the return type to void and remove the redundant return values. Reviewed-by: Shan Hai Signed-off-by: Zhu Yanjun Reviewed-by: Yuval Shaia Reviewed-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib.h | 4 ++-- drivers/infiniband/ulp/ipoib/ipoib_ib.c | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h index f5680642960a..6dac79454708 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib.h +++ b/drivers/infiniband/ulp/ipoib/ipoib.h @@ -501,8 +501,8 @@ void ipoib_ib_dev_cleanup(struct net_device *dev); int ipoib_ib_dev_open(struct net_device *dev); int ipoib_ib_dev_up(struct net_device *dev); -int ipoib_ib_dev_down(struct net_device *dev); -int ipoib_ib_dev_stop(struct net_device *dev); +void ipoib_ib_dev_down(struct net_device *dev); +void ipoib_ib_dev_stop(struct net_device *dev); void ipoib_pkey_dev_check_presence(struct net_device *dev); int ipoib_dev_init(struct net_device *dev, struct ib_device *ca, int port); diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c index 5038f9d2d753..6f443f725fca 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c @@ -771,7 +771,7 @@ int ipoib_ib_dev_up(struct net_device *dev) return ipoib_mcast_start_thread(dev); } -int ipoib_ib_dev_down(struct net_device *dev) +void ipoib_ib_dev_down(struct net_device *dev) { struct ipoib_dev_priv *priv = netdev_priv(dev); @@ -784,8 +784,6 @@ int ipoib_ib_dev_down(struct net_device *dev) ipoib_mcast_dev_flush(dev); ipoib_flush_paths(dev); - - return 0; } static int recvs_pending(struct net_device *dev) @@ -840,7 +838,7 @@ void ipoib_drain_cq(struct net_device *dev) local_bh_enable(); } -int ipoib_ib_dev_stop(struct net_device *dev) +void ipoib_ib_dev_stop(struct net_device *dev) { struct ipoib_dev_priv *priv = netdev_priv(dev); struct ib_qp_attr qp_attr; @@ -913,8 +911,6 @@ timeout: ipoib_flush_ah(dev); ib_req_notify_cq(priv->recv_cq, IB_CQ_NEXT_COMP); - - return 0; } int ipoib_ib_dev_init(struct net_device *dev, struct ib_device *ca, int port) -- cgit v1.2.3-59-g8ed1b From 5c37077fd025c0fa3aa3ab2e6b607d653a4fc604 Mon Sep 17 00:00:00 2001 From: Zhu Yanjun Date: Wed, 18 Jan 2017 23:16:06 -0500 Subject: IB/ipoib: Remove the unnecessary error check The function ipoib_mcast_start_thread/ipoib_ib_dev_up always return zero. As such, in the function ipoib_open, err_stop will never be reached. So remove this err_stop and change the return type of the function ipoib_mcast_start_thread/ipoib_ib_dev_up to void. Signed-off-by: Zhu Yanjun Reviewed-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib.h | 4 ++-- drivers/infiniband/ulp/ipoib/ipoib_ib.c | 6 +++--- drivers/infiniband/ulp/ipoib/ipoib_main.c | 6 +----- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 4 +--- 4 files changed, 7 insertions(+), 13 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h index 6dac79454708..bed233bf45c3 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib.h +++ b/drivers/infiniband/ulp/ipoib/ipoib.h @@ -500,7 +500,7 @@ void ipoib_pkey_event(struct work_struct *work); void ipoib_ib_dev_cleanup(struct net_device *dev); int ipoib_ib_dev_open(struct net_device *dev); -int ipoib_ib_dev_up(struct net_device *dev); +void ipoib_ib_dev_up(struct net_device *dev); void ipoib_ib_dev_down(struct net_device *dev); void ipoib_ib_dev_stop(struct net_device *dev); void ipoib_pkey_dev_check_presence(struct net_device *dev); @@ -513,7 +513,7 @@ void ipoib_mcast_carrier_on_task(struct work_struct *work); void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb); void ipoib_mcast_restart_task(struct work_struct *work); -int ipoib_mcast_start_thread(struct net_device *dev); +void ipoib_mcast_start_thread(struct net_device *dev); int ipoib_mcast_stop_thread(struct net_device *dev); void ipoib_mcast_dev_down(struct net_device *dev); diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c index 6f443f725fca..12c4f84a6639 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c @@ -755,7 +755,7 @@ void ipoib_pkey_dev_check_presence(struct net_device *dev) set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); } -int ipoib_ib_dev_up(struct net_device *dev) +void ipoib_ib_dev_up(struct net_device *dev) { struct ipoib_dev_priv *priv = netdev_priv(dev); @@ -763,12 +763,12 @@ int ipoib_ib_dev_up(struct net_device *dev) if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) { ipoib_dbg(priv, "PKEY is not assigned.\n"); - return 0; + return; } set_bit(IPOIB_FLAG_OPER_UP, &priv->flags); - return ipoib_mcast_start_thread(dev); + ipoib_mcast_start_thread(dev); } void ipoib_ib_dev_down(struct net_device *dev) diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index c8af6377a288..e3bfa8a99ad2 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -126,8 +126,7 @@ int ipoib_open(struct net_device *dev) goto err_disable; } - if (ipoib_ib_dev_up(dev)) - goto err_stop; + ipoib_ib_dev_up(dev); if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) { struct ipoib_dev_priv *cpriv; @@ -150,9 +149,6 @@ int ipoib_open(struct net_device *dev) return 0; -err_stop: - ipoib_ib_dev_stop(dev); - err_disable: clear_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags); diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index 7c6c67bbdab3..69e146cdc306 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -676,7 +676,7 @@ out: spin_unlock_irq(&priv->lock); } -int ipoib_mcast_start_thread(struct net_device *dev) +void ipoib_mcast_start_thread(struct net_device *dev) { struct ipoib_dev_priv *priv = netdev_priv(dev); unsigned long flags; @@ -686,8 +686,6 @@ int ipoib_mcast_start_thread(struct net_device *dev) spin_lock_irqsave(&priv->lock, flags); __ipoib_mcast_schedule_join_thread(priv, NULL, 0); spin_unlock_irqrestore(&priv->lock, flags); - - return 0; } int ipoib_mcast_stop_thread(struct net_device *dev) -- cgit v1.2.3-59-g8ed1b From 2bce1a6d2209c8c776a9598741f5aa1991689dcb Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 9 Dec 2016 11:00:33 -0800 Subject: IB/srpt: Accept GUIDs as port names Port and ACL information must be configured before an initiator logs in. Make it possible to configure this information before a subnet prefix has been assigned to a port by not only accepting GIDs as target port and initiator port names but by also accepting port GUIDs. Add a 'priv' member to struct se_wwn to allow target drivers to associate their own data with struct se_wwn. Reported-by: Doug Ledford References: http://www.spinics.net/lists/linux-rdma/msg39505.html Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Nicholas Bellinger Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srpt/ib_srpt.c | 139 ++++++++++++++++++++-------------- drivers/infiniband/ulp/srpt/ib_srpt.h | 18 +++-- drivers/target/target_core_tpg.c | 1 + include/target/target_core_base.h | 1 + 4 files changed, 98 insertions(+), 61 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c index d21ba9d857c3..bc5a2d86ae7e 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.c +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c @@ -500,6 +500,7 @@ static int srpt_refresh_port(struct srpt_port *sport) struct ib_mad_reg_req reg_req; struct ib_port_modify port_modify; struct ib_port_attr port_attr; + __be16 *guid; int ret; memset(&port_modify, 0, sizeof(port_modify)); @@ -522,10 +523,17 @@ static int srpt_refresh_port(struct srpt_port *sport) if (ret) goto err_query_port; + sport->port_guid_wwn.priv = sport; + guid = (__be16 *)&sport->gid.global.interface_id; snprintf(sport->port_guid, sizeof(sport->port_guid), - "0x%016llx%016llx", - be64_to_cpu(sport->gid.global.subnet_prefix), - be64_to_cpu(sport->gid.global.interface_id)); + "%04x:%04x:%04x:%04x", + be16_to_cpu(guid[0]), be16_to_cpu(guid[1]), + be16_to_cpu(guid[2]), be16_to_cpu(guid[3])); + sport->port_gid_wwn.priv = sport; + snprintf(sport->port_gid, sizeof(sport->port_gid), + "0x%016llx%016llx", + be64_to_cpu(sport->gid.global.subnet_prefix), + be64_to_cpu(sport->gid.global.interface_id)); if (!sport->mad_agent) { memset(®_req, 0, sizeof(reg_req)); @@ -1838,6 +1846,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id, struct srp_login_rej *rej; struct ib_cm_rep_param *rep_param; struct srpt_rdma_ch *ch, *tmp_ch; + __be16 *guid; u32 it_iu_len; int i, ret = 0; @@ -1983,26 +1992,30 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id, goto destroy_ib; } - /* - * Use the initator port identifier as the session name, when - * checking against se_node_acl->initiatorname[] this can be - * with or without preceeding '0x'. - */ + guid = (__be16 *)¶m->primary_path->sgid.global.interface_id; + snprintf(ch->ini_guid, sizeof(ch->ini_guid), "%04x:%04x:%04x:%04x", + be16_to_cpu(guid[0]), be16_to_cpu(guid[1]), + be16_to_cpu(guid[2]), be16_to_cpu(guid[3])); snprintf(ch->sess_name, sizeof(ch->sess_name), "0x%016llx%016llx", be64_to_cpu(*(__be64 *)ch->i_port_id), be64_to_cpu(*(__be64 *)(ch->i_port_id + 8))); pr_debug("registering session %s\n", ch->sess_name); - ch->sess = target_alloc_session(&sport->port_tpg_1, 0, 0, + if (sport->port_guid_tpg.se_tpg_wwn) + ch->sess = target_alloc_session(&sport->port_guid_tpg, 0, 0, + TARGET_PROT_NORMAL, + ch->ini_guid, ch, NULL); + if (sport->port_gid_tpg.se_tpg_wwn && IS_ERR_OR_NULL(ch->sess)) + ch->sess = target_alloc_session(&sport->port_gid_tpg, 0, 0, TARGET_PROT_NORMAL, ch->sess_name, ch, NULL); /* Retry without leading "0x" */ - if (IS_ERR(ch->sess)) - ch->sess = target_alloc_session(&sport->port_tpg_1, 0, 0, + if (sport->port_gid_tpg.se_tpg_wwn && IS_ERR_OR_NULL(ch->sess)) + ch->sess = target_alloc_session(&sport->port_gid_tpg, 0, 0, TARGET_PROT_NORMAL, ch->sess_name + 2, ch, NULL); - if (IS_ERR(ch->sess)) { + if (IS_ERR_OR_NULL(ch->sess)) { pr_info("Rejected login because no ACL has been configured yet for initiator %s.\n", ch->sess_name); rej->reason = cpu_to_be32((PTR_ERR(ch->sess) == -ENOMEM) ? @@ -2420,7 +2433,7 @@ static int srpt_release_sdev(struct srpt_device *sdev) return 0; } -static struct srpt_port *__srpt_lookup_port(const char *name) +static struct se_wwn *__srpt_lookup_wwn(const char *name) { struct ib_device *dev; struct srpt_device *sdev; @@ -2435,23 +2448,25 @@ static struct srpt_port *__srpt_lookup_port(const char *name) for (i = 0; i < dev->phys_port_cnt; i++) { sport = &sdev->port[i]; - if (!strcmp(sport->port_guid, name)) - return sport; + if (strcmp(sport->port_guid, name) == 0) + return &sport->port_guid_wwn; + if (strcmp(sport->port_gid, name) == 0) + return &sport->port_gid_wwn; } } return NULL; } -static struct srpt_port *srpt_lookup_port(const char *name) +static struct se_wwn *srpt_lookup_wwn(const char *name) { - struct srpt_port *sport; + struct se_wwn *wwn; spin_lock(&srpt_dev_lock); - sport = __srpt_lookup_port(name); + wwn = __srpt_lookup_wwn(name); spin_unlock(&srpt_dev_lock); - return sport; + return wwn; } /** @@ -2643,11 +2658,19 @@ static char *srpt_get_fabric_name(void) return "srpt"; } +static struct srpt_port *srpt_tpg_to_sport(struct se_portal_group *tpg) +{ + return tpg->se_tpg_wwn->priv; +} + static char *srpt_get_fabric_wwn(struct se_portal_group *tpg) { - struct srpt_port *sport = container_of(tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(tpg); - return sport->port_guid; + WARN_ON_ONCE(tpg != &sport->port_guid_tpg && + tpg != &sport->port_gid_tpg); + return tpg == &sport->port_guid_tpg ? sport->port_guid : + sport->port_gid; } static u16 srpt_get_tag(struct se_portal_group *tpg) @@ -2737,6 +2760,19 @@ static int srpt_get_tcm_cmd_state(struct se_cmd *se_cmd) return srpt_get_cmd_state(ioctx); } +static int srpt_parse_guid(u64 *guid, const char *name) +{ + u16 w[4]; + int ret = -EINVAL; + + if (sscanf(name, "%hx:%hx:%hx:%hx", &w[0], &w[1], &w[2], &w[3]) != 4) + goto out; + *guid = get_unaligned_be64(w); + ret = 0; +out: + return ret; +} + /** * srpt_parse_i_port_id() - Parse an initiator port ID. * @name: ASCII representation of a 128-bit initiator port ID. @@ -2772,20 +2808,23 @@ out: */ static int srpt_init_nodeacl(struct se_node_acl *se_nacl, const char *name) { + u64 guid; u8 i_port_id[16]; + int ret; - if (srpt_parse_i_port_id(i_port_id, name) < 0) { + ret = srpt_parse_guid(&guid, name); + if (ret < 0) + ret = srpt_parse_i_port_id(i_port_id, name); + if (ret < 0) pr_err("invalid initiator port ID %s\n", name); - return -EINVAL; - } - return 0; + return ret; } static ssize_t srpt_tpg_attrib_srp_max_rdma_size_show(struct config_item *item, char *page) { struct se_portal_group *se_tpg = attrib_to_tpg(item); - struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(se_tpg); return sprintf(page, "%u\n", sport->port_attrib.srp_max_rdma_size); } @@ -2794,7 +2833,7 @@ static ssize_t srpt_tpg_attrib_srp_max_rdma_size_store(struct config_item *item, const char *page, size_t count) { struct se_portal_group *se_tpg = attrib_to_tpg(item); - struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(se_tpg); unsigned long val; int ret; @@ -2822,7 +2861,7 @@ static ssize_t srpt_tpg_attrib_srp_max_rsp_size_show(struct config_item *item, char *page) { struct se_portal_group *se_tpg = attrib_to_tpg(item); - struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(se_tpg); return sprintf(page, "%u\n", sport->port_attrib.srp_max_rsp_size); } @@ -2831,7 +2870,7 @@ static ssize_t srpt_tpg_attrib_srp_max_rsp_size_store(struct config_item *item, const char *page, size_t count) { struct se_portal_group *se_tpg = attrib_to_tpg(item); - struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(se_tpg); unsigned long val; int ret; @@ -2859,7 +2898,7 @@ static ssize_t srpt_tpg_attrib_srp_sq_size_show(struct config_item *item, char *page) { struct se_portal_group *se_tpg = attrib_to_tpg(item); - struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(se_tpg); return sprintf(page, "%u\n", sport->port_attrib.srp_sq_size); } @@ -2868,7 +2907,7 @@ static ssize_t srpt_tpg_attrib_srp_sq_size_store(struct config_item *item, const char *page, size_t count) { struct se_portal_group *se_tpg = attrib_to_tpg(item); - struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(se_tpg); unsigned long val; int ret; @@ -2906,7 +2945,7 @@ static struct configfs_attribute *srpt_tpg_attrib_attrs[] = { static ssize_t srpt_tpg_enable_show(struct config_item *item, char *page) { struct se_portal_group *se_tpg = to_tpg(item); - struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(se_tpg); return snprintf(page, PAGE_SIZE, "%d\n", (sport->enabled) ? 1: 0); } @@ -2915,7 +2954,7 @@ static ssize_t srpt_tpg_enable_store(struct config_item *item, const char *page, size_t count) { struct se_portal_group *se_tpg = to_tpg(item); - struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(se_tpg); struct srpt_device *sdev = sport->sdev; struct srpt_rdma_ch *ch; unsigned long tmp; @@ -2967,15 +3006,19 @@ static struct se_portal_group *srpt_make_tpg(struct se_wwn *wwn, struct config_group *group, const char *name) { - struct srpt_port *sport = container_of(wwn, struct srpt_port, port_wwn); + struct srpt_port *sport = wwn->priv; + static struct se_portal_group *tpg; int res; - /* Initialize sport->port_wwn and sport->port_tpg_1 */ - res = core_tpg_register(&sport->port_wwn, &sport->port_tpg_1, SCSI_PROTOCOL_SRP); + WARN_ON_ONCE(wwn != &sport->port_guid_wwn && + wwn != &sport->port_gid_wwn); + tpg = wwn == &sport->port_guid_wwn ? &sport->port_guid_tpg : + &sport->port_gid_tpg; + res = core_tpg_register(wwn, tpg, SCSI_PROTOCOL_SRP); if (res) return ERR_PTR(res); - return &sport->port_tpg_1; + return tpg; } /** @@ -2984,11 +3027,10 @@ static struct se_portal_group *srpt_make_tpg(struct se_wwn *wwn, */ static void srpt_drop_tpg(struct se_portal_group *tpg) { - struct srpt_port *sport = container_of(tpg, - struct srpt_port, port_tpg_1); + struct srpt_port *sport = srpt_tpg_to_sport(tpg); sport->enabled = false; - core_tpg_deregister(&sport->port_tpg_1); + core_tpg_deregister(tpg); } /** @@ -2999,19 +3041,7 @@ static struct se_wwn *srpt_make_tport(struct target_fabric_configfs *tf, struct config_group *group, const char *name) { - struct srpt_port *sport; - int ret; - - sport = srpt_lookup_port(name); - pr_debug("make_tport(%s)\n", name); - ret = -EINVAL; - if (!sport) - goto err; - - return &sport->port_wwn; - -err: - return ERR_PTR(ret); + return srpt_lookup_wwn(name) ? : ERR_PTR(-EINVAL); } /** @@ -3020,9 +3050,6 @@ err: */ static void srpt_drop_tport(struct se_wwn *wwn) { - struct srpt_port *sport = container_of(wwn, struct srpt_port, port_wwn); - - pr_debug("drop_tport(%s\n", config_item_name(&sport->port_wwn.wwn_group.cg_item)); } static ssize_t srpt_wwn_version_show(struct config_item *item, char *buf) diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h index 581878782854..cc1183851af5 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.h +++ b/drivers/infiniband/ulp/srpt/ib_srpt.h @@ -258,6 +258,7 @@ enum rdma_ch_state { * against concurrent modification by the cm_id spinlock. * @sess: Session information associated with this SRP channel. * @sess_name: Session name. + * @ini_guid: Initiator port GUID. * @release_work: Allows scheduling of srpt_release_channel(). * @release_done: Enables waiting for srpt_release_channel() completion. */ @@ -284,6 +285,7 @@ struct srpt_rdma_ch { struct list_head cmd_wait_list; struct se_session *sess; u8 sess_name[36]; + u8 ini_guid[24]; struct work_struct release_work; struct completion *release_done; }; @@ -306,28 +308,34 @@ struct srpt_port_attrib { * @mad_agent: per-port management datagram processing information. * @enabled: Whether or not this target port is enabled. * @port_guid: ASCII representation of Port GUID + * @port_gid: ASCII representation of Port GID * @port: one-based port number. * @sm_lid: cached value of the port's sm_lid. * @lid: cached value of the port's lid. * @gid: cached value of the port's gid. * @port_acl_lock spinlock for port_acl_list: * @work: work structure for refreshing the aforementioned cached values. - * @port_tpg_1 Target portal group = 1 data. - * @port_wwn: Target core WWN data. + * @port_guid_tpg: TPG associated with target port GUID. + * @port_guid_wwn: WWN associated with target port GUID. + * @port_gid_tpg: TPG associated with target port GID. + * @port_gid_wwn: WWN associated with target port GID. * @port_acl_list: Head of the list with all node ACLs for this port. */ struct srpt_port { struct srpt_device *sdev; struct ib_mad_agent *mad_agent; bool enabled; - u8 port_guid[64]; + u8 port_guid[24]; + u8 port_gid[64]; u8 port; u16 sm_lid; u16 lid; union ib_gid gid; struct work_struct work; - struct se_portal_group port_tpg_1; - struct se_wwn port_wwn; + struct se_portal_group port_guid_tpg; + struct se_wwn port_guid_wwn; + struct se_portal_group port_gid_tpg; + struct se_wwn port_gid_wwn; struct srpt_port_attrib port_attrib; }; diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c index d99752c6cd60..4a8b180c478b 100644 --- a/drivers/target/target_core_tpg.c +++ b/drivers/target/target_core_tpg.c @@ -448,6 +448,7 @@ static void core_tpg_lun_ref_release(struct percpu_ref *ref) complete(&lun->lun_ref_comp); } +/* Does not change se_wwn->priv. */ int core_tpg_register( struct se_wwn *se_wwn, struct se_portal_group *se_tpg, diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 29e6858bb164..541b52bc811c 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -909,6 +909,7 @@ static inline struct se_portal_group *param_to_tpg(struct config_item *item) struct se_wwn { struct target_fabric_configfs *wwn_tf; + void *priv; struct config_group wwn_group; struct config_group fabric_stat_group; }; -- cgit v1.2.3-59-g8ed1b From 2b0841766a898aba84630fb723989a77a9d3b4e6 Mon Sep 17 00:00:00 2001 From: Erez Shitrit Date: Wed, 1 Feb 2017 19:10:05 +0200 Subject: IB/IPoIB: Add destination address when re-queue packet When sending packet to destination that was not resolved yet via path query, the driver keeps the skb and tries to re-send it again when the path is resolved. But when re-sending via dev_queue_xmit the kernel doesn't call to dev_hard_header, so IPoIB needs to keep 20 bytes in the skb and to put the destination address inside them. In that way the dev_start_xmit will have the correct destination, and the driver won't take the destination from the skb->data, while nothing exists there, which causes to packet be be dropped. The test flow is: 1. Run the SM on remote node, 2. Restart the driver. 4. Ping some destination, 3. Observe that first ICMP request will be dropped. Fixes: fc791b633515 ("IB/ipoib: move back IB LL address into the hard header") Cc: # v4.8+ Signed-off-by: Erez Shitrit Signed-off-by: Noa Osherovich Signed-off-by: Leon Romanovsky Tested-by: Yuval Shaia Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_main.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index e3bfa8a99ad2..259c59f67394 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -721,6 +721,14 @@ int ipoib_check_sm_sendonly_fullmember_support(struct ipoib_dev_priv *priv) return ret; } +static void push_pseudo_header(struct sk_buff *skb, const char *daddr) +{ + struct ipoib_pseudo_header *phdr; + + phdr = (struct ipoib_pseudo_header *)skb_push(skb, sizeof(*phdr)); + memcpy(phdr->hwaddr, daddr, INFINIBAND_ALEN); +} + void ipoib_flush_paths(struct net_device *dev) { struct ipoib_dev_priv *priv = netdev_priv(dev); @@ -947,8 +955,7 @@ static void neigh_add_path(struct sk_buff *skb, u8 *daddr, } if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) { - /* put pseudoheader back on for next time */ - skb_push(skb, IPOIB_PSEUDO_LEN); + push_pseudo_header(skb, neigh->daddr); __skb_queue_tail(&neigh->queue, skb); } else { ipoib_warn(priv, "queue length limit %d. Packet drop.\n", @@ -966,10 +973,12 @@ static void neigh_add_path(struct sk_buff *skb, u8 *daddr, if (!path->query && path_rec_start(dev, path)) goto err_path; - if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) + if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) { + push_pseudo_header(skb, neigh->daddr); __skb_queue_tail(&neigh->queue, skb); - else + } else { goto err_drop; + } } spin_unlock_irqrestore(&priv->lock, flags); @@ -1005,8 +1014,7 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev, } if (path) { if (skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) { - /* put pseudoheader back on for next time */ - skb_push(skb, IPOIB_PSEUDO_LEN); + push_pseudo_header(skb, phdr->hwaddr); __skb_queue_tail(&path->queue, skb); } else { ++dev->stats.tx_dropped; @@ -1038,8 +1046,7 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev, return; } else if ((path->query || !path_rec_start(dev, path)) && skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) { - /* put pseudoheader back on for next time */ - skb_push(skb, IPOIB_PSEUDO_LEN); + push_pseudo_header(skb, phdr->hwaddr); __skb_queue_tail(&path->queue, skb); } else { ++dev->stats.tx_dropped; @@ -1120,8 +1127,7 @@ send_using_neigh: } if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) { - /* put pseudoheader back on for next time */ - skb_push(skb, sizeof(*phdr)); + push_pseudo_header(skb, phdr->hwaddr); spin_lock_irqsave(&priv->lock, flags); __skb_queue_tail(&neigh->queue, skb); spin_unlock_irqrestore(&priv->lock, flags); @@ -1153,7 +1159,6 @@ static int ipoib_hard_header(struct sk_buff *skb, unsigned short type, const void *daddr, const void *saddr, unsigned len) { - struct ipoib_pseudo_header *phdr; struct ipoib_header *header; header = (struct ipoib_header *) skb_push(skb, sizeof *header); @@ -1166,8 +1171,7 @@ static int ipoib_hard_header(struct sk_buff *skb, * destination address into skb hard header so we can figure out where * to send the packet later. */ - phdr = (struct ipoib_pseudo_header *) skb_push(skb, sizeof(*phdr)); - memcpy(phdr->hwaddr, daddr, INFINIBAND_ALEN); + push_pseudo_header(skb, daddr); return IPOIB_HARD_LEN; } -- cgit v1.2.3-59-g8ed1b From 32f8e839edebe3310dd077439ce0e9a96e0c2352 Mon Sep 17 00:00:00 2001 From: Max Gurtovoy Date: Thu, 2 Feb 2017 01:55:18 +0200 Subject: IB/iser: Protect completion context active_qps update As iser connections can share completion contexts, we need to protect the active_qps update each time we set it. Signed-off-by: Max Gurtovoy Acked-by: Sagi Grimberg Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/iser/iser_verbs.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c index 6a9d1cb548ee..30b622f2ab73 100644 --- a/drivers/infiniband/ulp/iser/iser_verbs.c +++ b/drivers/infiniband/ulp/iser/iser_verbs.c @@ -597,7 +597,9 @@ static void iser_free_ib_conn_res(struct iser_conn *iser_conn, iser_conn, ib_conn->cma_id, ib_conn->qp); if (ib_conn->qp != NULL) { + mutex_lock(&ig.connlist_mutex); ib_conn->comp->active_qps--; + mutex_unlock(&ig.connlist_mutex); rdma_destroy_qp(ib_conn->cma_id); ib_conn->qp = NULL; } -- cgit v1.2.3-59-g8ed1b From c5e8f57b0b560b9cc9ea9166975858d023f81030 Mon Sep 17 00:00:00 2001 From: Zhu Yanjun Date: Fri, 10 Feb 2017 01:48:42 -0500 Subject: IB/ipoib: remove the unnecessary memory free In the function ipoib_cm_nonsrq_init_rx, the memory is not allocated successfully. It is not necessary to free it. CC: Joe Jin CC: Junxiao Bi Signed-off-by: Zhu Yanjun Reviewed-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index 53d69d7d9ad4..eb8cafe80115 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -363,7 +363,7 @@ static int ipoib_cm_nonsrq_init_rx(struct net_device *dev, struct ib_cm_id *cm_i t = kmalloc(sizeof *t, GFP_KERNEL); if (!t) { ret = -ENOMEM; - goto err_free; + goto err_free_1; } ipoib_cm_init_rx_wr(dev, &t->wr, t->sge); @@ -410,6 +410,8 @@ err_count: err_free: kfree(t); + +err_free_1: ipoib_cm_free_rx_ring(dev, rx->rx_ring); return ret; -- cgit v1.2.3-59-g8ed1b From 23536dfaa3ef5b4e61272ea65ed6c1b15db022ec Mon Sep 17 00:00:00 2001 From: Zhu Yanjun Date: Tue, 14 Feb 2017 03:43:12 -0500 Subject: IB/ipoib: Remove redudant label There are 2 labels to mark the same statements. Replace the 2 labels with one versatile labe. Cc: Joe Jin Cc: Junxiao Bi Signed-off-by: Zhu Yanjun Reviewed-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index eb8cafe80115..a6d6c617b597 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -1157,13 +1157,13 @@ static int ipoib_cm_tx_init(struct ipoib_cm_tx *p, u32 qpn, ret = ipoib_cm_modify_tx_init(p->dev, p->id, p->qp); if (ret) { ipoib_warn(priv, "failed to modify tx qp to rtr: %d\n", ret); - goto err_modify; + goto err_modify_send; } ret = ipoib_cm_send_req(p->dev, p->id, p->qp, qpn, pathrec); if (ret) { ipoib_warn(priv, "failed to send cm req: %d\n", ret); - goto err_send_cm; + goto err_modify_send; } ipoib_dbg(priv, "Request connection 0x%x for gid %pI6 qpn 0x%x\n", @@ -1171,8 +1171,7 @@ static int ipoib_cm_tx_init(struct ipoib_cm_tx *p, u32 qpn, return 0; -err_send_cm: -err_modify: +err_modify_send: ib_destroy_cm_id(p->id); err_id: p->id = NULL; -- cgit v1.2.3-59-g8ed1b From d6c58dc40fec35ff6cdb350b53bce0fcf9143709 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 14 Feb 2017 10:56:29 -0800 Subject: IB/SRP: Avoid using IB_MR_TYPE_SG_GAPS Tests have shown that the following error message is reported when using SG-GAPS registration with an mlx5 adapter: scsi host1: ib_srp: failed RECV status WR flushed (5) for CQE ffff880bd4270eb0 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 0f007806 2500002a ad9fafd1 scsi host1: ib_srp: reconnect succeeded mlx5_0:dump_cqe:262:(pid 7369): dump error cqe 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 0f007806 25000032 00105dd0 scsi host1: ib_srp: failed FAST REG status memory management operation error (6) for CQE ffff880b92860138 Hence avoid using SG-GAPS memory registrations. Additionally, always configure the blk_queue_virt_boundary() to avoid to trigger a mapping failure when using adapters that support SG-GAPS (e.g. mlx5). Fixes: commit ad8e66b4a801 ("IB/srp: fix mr allocation when the device supports sg gaps") Fixes: commit 509c5f33f4f6 ("IB/srp: Prevent mapping failures") Reported-by: Laurence Oberman Signed-off-by: Bart Van Assche Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Leon Romanovsky Cc: Mark Bloch Cc: Yuval Shaia Cc: # 4.7+ Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 79bf48477ddb..6e4467150049 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -371,7 +371,6 @@ static struct srp_fr_pool *srp_create_fr_pool(struct ib_device *device, struct srp_fr_desc *d; struct ib_mr *mr; int i, ret = -EINVAL; - enum ib_mr_type mr_type; if (pool_size <= 0) goto err; @@ -385,13 +384,9 @@ static struct srp_fr_pool *srp_create_fr_pool(struct ib_device *device, spin_lock_init(&pool->lock); INIT_LIST_HEAD(&pool->free_list); - if (device->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG) - mr_type = IB_MR_TYPE_SG_GAPS; - else - mr_type = IB_MR_TYPE_MEM_REG; - for (i = 0, d = &pool->desc[0]; i < pool->size; i++, d++) { - mr = ib_alloc_mr(pd, mr_type, max_page_list_len); + mr = ib_alloc_mr(pd, IB_MR_TYPE_MEM_REG, + max_page_list_len); if (IS_ERR(mr)) { ret = PTR_ERR(mr); if (ret == -ENOMEM) @@ -2664,9 +2659,8 @@ static int srp_slave_alloc(struct scsi_device *sdev) struct Scsi_Host *shost = sdev->host; struct srp_target_port *target = host_to_target(shost); struct srp_device *srp_dev = target->srp_host->srp_dev; - struct ib_device *ibdev = srp_dev->dev; - if (!(ibdev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG)) + if (true) blk_queue_virt_boundary(sdev->request_queue, ~srp_dev->mr_page_mask); -- cgit v1.2.3-59-g8ed1b From 6cb72bc1b40bb2c1750ee7a5ebade93bed49a5fb Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 14 Feb 2017 10:56:30 -0800 Subject: IB/srp: Avoid that duplicate responses trigger a kernel bug After srp_process_rsp() returns there is a short time during which the scsi_host_find_tag() call will return a pointer to the SCSI command that is being completed. If during that time a duplicate response is received, avoid that the following call stack appears: BUG: unable to handle kernel NULL pointer dereference at (null) IP: srp_recv_done+0x450/0x6b0 [ib_srp] Oops: 0000 [#1] SMP CPU: 10 PID: 0 Comm: swapper/10 Not tainted 4.10.0-rc7-dbg+ #1 Call Trace: __ib_process_cq+0x4b/0xd0 [ib_core] ib_poll_handler+0x1d/0x70 [ib_core] irq_poll_softirq+0xba/0x120 __do_softirq+0xba/0x4c0 irq_exit+0xbe/0xd0 smp_apic_timer_interrupt+0x38/0x50 apic_timer_interrupt+0x90/0xa0 RIP: srp_recv_done+0x450/0x6b0 [ib_srp] RSP: ffff88046f483e20 Signed-off-by: Bart Van Assche Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Laurence Oberman Cc: Steve Feeley Cc: Reviewed-by: Leon Romanovsky Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 6e4467150049..a49289f600b1 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1892,9 +1892,11 @@ static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp) complete(&ch->tsk_mgmt_done); } else { scmnd = scsi_host_find_tag(target->scsi_host, rsp->tag); - if (scmnd) { + if (scmnd && scmnd->host_scribble) { req = (void *)scmnd->host_scribble; scmnd = srp_claim_req(ch, req, NULL, scmnd); + } else { + scmnd = NULL; } if (!scmnd) { shost_printk(KERN_ERR, target->scsi_host, -- cgit v1.2.3-59-g8ed1b From 0a6fdbdeb1c25e31763c1fb333fa2723a7d2aba6 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 14 Feb 2017 10:56:31 -0800 Subject: IB/srp: Fix race conditions related to task management Avoid that srp_process_rsp() overwrites the status information in ch if the SRP target response timed out and processing of another task management function has already started. Avoid that issuing multiple task management functions concurrently triggers list corruption. This patch prevents that the following stack trace appears in the system log: WARNING: CPU: 8 PID: 9269 at lib/list_debug.c:52 __list_del_entry_valid+0xbc/0xc0 list_del corruption. prev->next should be ffffc90004bb7b00, but was ffff8804052ecc68 CPU: 8 PID: 9269 Comm: sg_reset Tainted: G W 4.10.0-rc7-dbg+ #3 Call Trace: dump_stack+0x68/0x93 __warn+0xc6/0xe0 warn_slowpath_fmt+0x4a/0x50 __list_del_entry_valid+0xbc/0xc0 wait_for_completion_timeout+0x12e/0x170 srp_send_tsk_mgmt+0x1ef/0x2d0 [ib_srp] srp_reset_device+0x5b/0x110 [ib_srp] scsi_ioctl_reset+0x1c7/0x290 scsi_ioctl+0x12a/0x420 sd_ioctl+0x9d/0x100 blkdev_ioctl+0x51e/0x9f0 block_ioctl+0x38/0x40 do_vfs_ioctl+0x8f/0x700 SyS_ioctl+0x3c/0x70 entry_SYSCALL_64_fastpath+0x18/0xad Signed-off-by: Bart Van Assche Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Laurence Oberman Cc: Steve Feeley Cc: Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 45 ++++++++++++++++++++++++------------- drivers/infiniband/ulp/srp/ib_srp.h | 1 + 2 files changed, 30 insertions(+), 16 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index a49289f600b1..d9b57f5958b5 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1884,12 +1884,17 @@ static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp) if (unlikely(rsp->tag & SRP_TAG_TSK_MGMT)) { spin_lock_irqsave(&ch->lock, flags); ch->req_lim += be32_to_cpu(rsp->req_lim_delta); + if (rsp->tag == ch->tsk_mgmt_tag) { + ch->tsk_mgmt_status = -1; + if (be32_to_cpu(rsp->resp_data_len) >= 4) + ch->tsk_mgmt_status = rsp->data[3]; + complete(&ch->tsk_mgmt_done); + } else { + shost_printk(KERN_ERR, target->scsi_host, + "Received tsk mgmt response too late for tag %#llx\n", + rsp->tag); + } spin_unlock_irqrestore(&ch->lock, flags); - - ch->tsk_mgmt_status = -1; - if (be32_to_cpu(rsp->resp_data_len) >= 4) - ch->tsk_mgmt_status = rsp->data[3]; - complete(&ch->tsk_mgmt_done); } else { scmnd = scsi_host_find_tag(target->scsi_host, rsp->tag); if (scmnd && scmnd->host_scribble) { @@ -2528,19 +2533,18 @@ srp_change_queue_depth(struct scsi_device *sdev, int qdepth) } static int srp_send_tsk_mgmt(struct srp_rdma_ch *ch, u64 req_tag, u64 lun, - u8 func) + u8 func, u8 *status) { struct srp_target_port *target = ch->target; struct srp_rport *rport = target->rport; struct ib_device *dev = target->srp_host->srp_dev->dev; struct srp_iu *iu; struct srp_tsk_mgmt *tsk_mgmt; + int res; if (!ch->connected || target->qp_in_error) return -1; - init_completion(&ch->tsk_mgmt_done); - /* * Lock the rport mutex to avoid that srp_create_ch_ib() is * invoked while a task management function is being sent. @@ -2563,10 +2567,16 @@ static int srp_send_tsk_mgmt(struct srp_rdma_ch *ch, u64 req_tag, u64 lun, tsk_mgmt->opcode = SRP_TSK_MGMT; int_to_scsilun(lun, &tsk_mgmt->lun); - tsk_mgmt->tag = req_tag | SRP_TAG_TSK_MGMT; tsk_mgmt->tsk_mgmt_func = func; tsk_mgmt->task_tag = req_tag; + spin_lock_irq(&ch->lock); + ch->tsk_mgmt_tag = (ch->tsk_mgmt_tag + 1) | SRP_TAG_TSK_MGMT; + tsk_mgmt->tag = ch->tsk_mgmt_tag; + spin_unlock_irq(&ch->lock); + + init_completion(&ch->tsk_mgmt_done); + ib_dma_sync_single_for_device(dev, iu->dma, sizeof *tsk_mgmt, DMA_TO_DEVICE); if (srp_post_send(ch, iu, sizeof(*tsk_mgmt))) { @@ -2575,13 +2585,15 @@ static int srp_send_tsk_mgmt(struct srp_rdma_ch *ch, u64 req_tag, u64 lun, return -1; } + res = wait_for_completion_timeout(&ch->tsk_mgmt_done, + msecs_to_jiffies(SRP_ABORT_TIMEOUT_MS)); + if (res > 0 && status) + *status = ch->tsk_mgmt_status; mutex_unlock(&rport->mutex); - if (!wait_for_completion_timeout(&ch->tsk_mgmt_done, - msecs_to_jiffies(SRP_ABORT_TIMEOUT_MS))) - return -1; + WARN_ON_ONCE(res < 0); - return 0; + return res > 0 ? 0 : -1; } static int srp_abort(struct scsi_cmnd *scmnd) @@ -2607,7 +2619,7 @@ static int srp_abort(struct scsi_cmnd *scmnd) shost_printk(KERN_ERR, target->scsi_host, "Sending SRP abort for tag %#x\n", tag); if (srp_send_tsk_mgmt(ch, tag, scmnd->device->lun, - SRP_TSK_ABORT_TASK) == 0) + SRP_TSK_ABORT_TASK, NULL) == 0) ret = SUCCESS; else if (target->rport->state == SRP_RPORT_LOST) ret = FAST_IO_FAIL; @@ -2625,14 +2637,15 @@ static int srp_reset_device(struct scsi_cmnd *scmnd) struct srp_target_port *target = host_to_target(scmnd->device->host); struct srp_rdma_ch *ch; int i; + u8 status; shost_printk(KERN_ERR, target->scsi_host, "SRP reset_device called\n"); ch = &target->ch[0]; if (srp_send_tsk_mgmt(ch, SRP_TAG_NO_REQ, scmnd->device->lun, - SRP_TSK_LUN_RESET)) + SRP_TSK_LUN_RESET, &status)) return FAILED; - if (ch->tsk_mgmt_status) + if (status) return FAILED; for (i = 0; i < target->ch_count; i++) { diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h index 21c69695f9d4..32ed40db3ca2 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.h +++ b/drivers/infiniband/ulp/srp/ib_srp.h @@ -163,6 +163,7 @@ struct srp_rdma_ch { int max_ti_iu_len; int comp_vector; + u64 tsk_mgmt_tag; struct completion tsk_mgmt_done; u8 tsk_mgmt_status; bool connected; -- cgit v1.2.3-59-g8ed1b From 93c76dbb749eb0c791c2680f8fdaeae51240fdf5 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 14 Feb 2017 10:56:32 -0800 Subject: IB/srp: Document locking conventions Use lockdep_assert_held() statements to verify at run-time whether the proper locks are held. Signed-off-by: Bart Van Assche Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Laurence Oberman Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index d9b57f5958b5..862ee2ff5bb9 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -40,6 +40,7 @@ #include #include #include +#include #include #include @@ -1799,6 +1800,8 @@ static struct srp_iu *__srp_get_tx_iu(struct srp_rdma_ch *ch, s32 rsv = (iu_type == SRP_IU_TSK_MGMT) ? 0 : SRP_TSK_MGMT_SQ_SIZE; struct srp_iu *iu; + lockdep_assert_held(&ch->lock); + ib_process_cq_direct(ch->send_cq, -1); if (list_empty(&ch->free_tx)) @@ -1829,6 +1832,8 @@ static void srp_send_done(struct ib_cq *cq, struct ib_wc *wc) return; } + lockdep_assert_held(&ch->lock); + list_add(&iu->list, &ch->free_tx); } -- cgit v1.2.3-59-g8ed1b From a7139ca80f7cd79e034a5c5f9c242bfb0471545c Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 14 Feb 2017 10:56:33 -0800 Subject: IB/srp: Make a diagnostic message more informative Report the destination port GID if connecting fails. Signed-off-by: Bart Van Assche Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Laurence Oberman Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 862ee2ff5bb9..33dd0920c248 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -3435,9 +3435,10 @@ static ssize_t srp_create_target(struct device *dev, ret = srp_connect_ch(ch, multich); if (ret) { shost_printk(KERN_ERR, target->scsi_host, - PFX "Connection %d/%d failed\n", + PFX "Connection %d/%d to %pI6 failed\n", ch_start + cpu_idx, - target->ch_count); + target->ch_count, + ch->target->orig_dgid.raw); if (node_idx == 0 && cpu_idx == 0) { goto err_disconnect; } else { -- cgit v1.2.3-59-g8ed1b From b02c15360b17dde352fbf4003e20dabcd3ae9157 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 14 Feb 2017 10:56:34 -0800 Subject: IB/srp: Improve an error path Avoid that the following message is printed if login fails: scsi host0: ib_srp: Sending CM DREQ failed Signed-off-by: Bart Van Assche Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Laurence Oberman Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 33dd0920c248..e8225cc8b938 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -3440,7 +3440,7 @@ static ssize_t srp_create_target(struct device *dev, target->ch_count, ch->target->orig_dgid.raw); if (node_idx == 0 && cpu_idx == 0) { - goto err_disconnect; + goto free_ch; } else { srp_free_ch_ib(target, ch); srp_free_req_data(target, ch); @@ -3487,6 +3487,7 @@ put: err_disconnect: srp_disconnect_target(target); +free_ch: for (i = 0; i < target->ch_count; i++) { ch = &target->ch[i]; srp_free_ch_ib(target, ch); -- cgit v1.2.3-59-g8ed1b From 9294000d6d895ad609f3cc4aff98c9c6175b466f Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 14 Feb 2017 10:56:36 -0800 Subject: IB/srp: Drain the send queue before destroying a QP A quote from the IB spec: However, if the Consumer does not wait for the Affiliated Asynchronous Last WQE Reached Event, then WQE and Data Segment leakage may occur. Therefore, it is good programming practice to tear down a QP that is associated with an SRQ by using the following process: * Put the QP in the Error State; * wait for the Affiliated Asynchronous Last WQE Reached Event; * either: * drain the CQ by invoking the Poll CQ verb and either wait for CQ to be empty or the number of Poll CQ operations has exceeded CQ capacity size; or * post another WR that completes on the same CQ and wait for this WR to return as a WC; * and then invoke a Destroy QP or Reset QP. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Laurence Oberman Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'drivers/infiniband/ulp') diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index e8225cc8b938..c7d97097d55c 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -466,9 +466,13 @@ static struct srp_fr_pool *srp_alloc_fr_pool(struct srp_target_port *target) * completion handler can access the queue pair while it is * being destroyed. */ -static void srp_destroy_qp(struct ib_qp *qp) +static void srp_destroy_qp(struct srp_rdma_ch *ch, struct ib_qp *qp) { - ib_drain_rq(qp); + spin_lock_irq(&ch->lock); + ib_process_cq_direct(ch->send_cq, -1); + spin_unlock_irq(&ch->lock); + + ib_drain_qp(qp); ib_destroy_qp(qp); } @@ -542,7 +546,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch) } if (ch->qp) - srp_destroy_qp(ch->qp); + srp_destroy_qp(ch, ch->qp); if (ch->recv_cq) ib_free_cq(ch->recv_cq); if (ch->send_cq) @@ -566,7 +570,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch) return 0; err_qp: - srp_destroy_qp(qp); + srp_destroy_qp(ch, qp); err_send_cq: ib_free_cq(send_cq); @@ -609,7 +613,7 @@ static void srp_free_ch_ib(struct srp_target_port *target, ib_destroy_fmr_pool(ch->fmr_pool); } - srp_destroy_qp(ch->qp); + srp_destroy_qp(ch, ch->qp); ib_free_cq(ch->send_cq); ib_free_cq(ch->recv_cq); @@ -1822,6 +1826,11 @@ static struct srp_iu *__srp_get_tx_iu(struct srp_rdma_ch *ch, return iu; } +/* + * Note: if this function is called from inside ib_drain_sq() then it will + * be called without ch->lock being held. If ib_drain_sq() dequeues a WQE + * with status IB_WC_SUCCESS then that's a bug. + */ static void srp_send_done(struct ib_cq *cq, struct ib_wc *wc) { struct srp_iu *iu = container_of(wc->wr_cqe, struct srp_iu, cqe); -- cgit v1.2.3-59-g8ed1b