From ecba616e041e64840d14e294b089ca355614b7fb Mon Sep 17 00:00:00 2001 From: Thomas Falcon Date: Mon, 26 Feb 2018 18:10:55 -0600 Subject: [PATCH 1/5] ibmvnic: Fix TX descriptor tracking again Sorry, the previous change introduced a race condition between transmit completion processing and tracking TX descriptors. If a completion is received before the number of descriptors is logged, the number of descriptors will be add but not removed. After enough times, this could halt the transmit queue forever. Log the number of descriptors used by a transmit before sending. I stress tested the fix on two different systems running over the weekend without any issues. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 5a86a916492c..32ee202de13a 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1482,6 +1482,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev) if ((*hdrs >> 7) & 1) { build_hdr_descs_arr(tx_buff, &num_entries, *hdrs); tx_crq.v1.n_crq_elem = num_entries; + tx_buff->num_entries = num_entries; tx_buff->indir_arr[0] = tx_crq; tx_buff->indir_dma = dma_map_single(dev, tx_buff->indir_arr, sizeof(tx_buff->indir_arr), @@ -1500,6 +1501,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev) (u64)tx_buff->indir_dma, (u64)num_entries); } else { + tx_buff->num_entries = num_entries; lpar_rc = send_subcrq(adapter, handle_array[queue_num], &tx_crq); } @@ -1536,7 +1538,6 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev) netif_stop_subqueue(netdev, queue_num); } - tx_buff->num_entries = num_entries; tx_packets++; tx_bytes += skb->len; txq->trans_start = jiffies; From 53cc7721fdf12e649994cfb7d8f562acb0e4510b Mon Sep 17 00:00:00 2001 From: Thomas Falcon Date: Mon, 26 Feb 2018 18:10:56 -0600 Subject: [PATCH 2/5] ibmvnic: Allocate statistics buffers during probe Currently, buffers holding individual queue statistics are allocated when the device is opened. If an ibmvnic interface is hotplugged or initialized but never opened, an attempt to get statistics with ethtool will result in a kernel panic. Since the driver allocates a constant number, the maximum supported queues, of buffers, these can be allocated during device probe and freed when the device is hot-unplugged or the module is removed. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 32ee202de13a..1a0f67b60003 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -845,8 +845,6 @@ static void release_resources(struct ibmvnic_adapter *adapter) release_tx_pools(adapter); release_rx_pools(adapter); - release_stats_token(adapter); - release_stats_buffers(adapter); release_error_buffers(adapter); release_napi(adapter); release_login_rsp_buffer(adapter); @@ -974,14 +972,6 @@ static int init_resources(struct ibmvnic_adapter *adapter) if (rc) return rc; - rc = init_stats_buffers(adapter); - if (rc) - return rc; - - rc = init_stats_token(adapter); - if (rc) - return rc; - adapter->vpd = kzalloc(sizeof(*adapter->vpd), GFP_KERNEL); if (!adapter->vpd) return -ENOMEM; @@ -4431,6 +4421,14 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter) release_crq_queue(adapter); } + rc = init_stats_buffers(adapter); + if (rc) + return rc; + + rc = init_stats_token(adapter); + if (rc) + return rc; + return rc; } @@ -4538,6 +4536,9 @@ static int ibmvnic_remove(struct vio_dev *dev) release_sub_crqs(adapter, 1); release_crq_queue(adapter); + release_stats_token(adapter); + release_stats_buffers(adapter); + adapter->state = VNIC_REMOVED; mutex_unlock(&adapter->reset_lock); From 637f81d164a5347ee3325f106f815c7969c032b5 Mon Sep 17 00:00:00 2001 From: Thomas Falcon Date: Mon, 26 Feb 2018 18:10:57 -0600 Subject: [PATCH 3/5] ibmvnic: Harden TX/RX pool cleaning If the driver releases resources after a failed reset or some other error, the driver might attempt to clean up and free memory that isn't there anymore. Include some additional checks that RX/TX queues along with their associated structures are still there before cleaning. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 1a0f67b60003..b907c0a607e6 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1081,6 +1081,7 @@ static int ibmvnic_open(struct net_device *netdev) static void clean_rx_pools(struct ibmvnic_adapter *adapter) { struct ibmvnic_rx_pool *rx_pool; + struct ibmvnic_rx_buff *rx_buff; u64 rx_entries; int rx_scrqs; int i, j; @@ -1094,14 +1095,15 @@ static void clean_rx_pools(struct ibmvnic_adapter *adapter) /* Free any remaining skbs in the rx buffer pools */ for (i = 0; i < rx_scrqs; i++) { rx_pool = &adapter->rx_pool[i]; - if (!rx_pool) + if (!rx_pool || !rx_pool->rx_buff) continue; netdev_dbg(adapter->netdev, "Cleaning rx_pool[%d]\n", i); for (j = 0; j < rx_entries; j++) { - if (rx_pool->rx_buff[j].skb) { - dev_kfree_skb_any(rx_pool->rx_buff[j].skb); - rx_pool->rx_buff[j].skb = NULL; + rx_buff = &rx_pool->rx_buff[j]; + if (rx_buff && rx_buff->skb) { + dev_kfree_skb_any(rx_buff->skb); + rx_buff->skb = NULL; } } } @@ -1110,6 +1112,7 @@ static void clean_rx_pools(struct ibmvnic_adapter *adapter) static void clean_tx_pools(struct ibmvnic_adapter *adapter) { struct ibmvnic_tx_pool *tx_pool; + struct ibmvnic_tx_buff *tx_buff; u64 tx_entries; int tx_scrqs; int i, j; @@ -1123,14 +1126,15 @@ static void clean_tx_pools(struct ibmvnic_adapter *adapter) /* Free any remaining skbs in the tx buffer pools */ for (i = 0; i < tx_scrqs; i++) { tx_pool = &adapter->tx_pool[i]; - if (!tx_pool) + if (!tx_pool && !tx_pool->tx_buff) continue; netdev_dbg(adapter->netdev, "Cleaning tx_pool[%d]\n", i); for (j = 0; j < tx_entries; j++) { - if (tx_pool->tx_buff[j].skb) { - dev_kfree_skb_any(tx_pool->tx_buff[j].skb); - tx_pool->tx_buff[j].skb = NULL; + tx_buff = &tx_pool->tx_buff[j]; + if (tx_buff && tx_buff->skb) { + dev_kfree_skb_any(tx_buff->skb); + tx_buff->skb = NULL; } } } From 0aecb13ce3abe1372c7306c7d1b86ff748296abb Mon Sep 17 00:00:00 2001 From: Thomas Falcon Date: Mon, 26 Feb 2018 18:10:58 -0600 Subject: [PATCH 4/5] ibmvnic: Report queue stops and restarts as debug output It's not necessary to report each time a queue is stopped and restarted as an informational message. Change that to be a debug message so that it can be observed if needed but not printed by default. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index b907c0a607e6..53b2c91fa786 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1528,7 +1528,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev) if (atomic_add_return(num_entries, &tx_scrq->used) >= adapter->req_tx_entries_per_subcrq) { - netdev_info(netdev, "Stopping queue %d\n", queue_num); + netdev_dbg(netdev, "Stopping queue %d\n", queue_num); netif_stop_subqueue(netdev, queue_num); } @@ -2541,8 +2541,8 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter *adapter, __netif_subqueue_stopped(adapter->netdev, scrq->pool_index)) { netif_wake_subqueue(adapter->netdev, scrq->pool_index); - netdev_info(adapter->netdev, "Started queue %d\n", - scrq->pool_index); + netdev_dbg(adapter->netdev, "Started queue %d\n", + scrq->pool_index); } } From 20a8ab744ff799ccedd35aba0d3139782f341bed Mon Sep 17 00:00:00 2001 From: Thomas Falcon Date: Mon, 26 Feb 2018 18:10:59 -0600 Subject: [PATCH 5/5] ibmvnic: Do not attempt to login if RX or TX queues are not allocated If a device reset fails for some reason, TX and RX queue resources could be released. If a user attempts to open the device in this scenario, it may result in a kernel panic as the driver tries to access this memory. To fix this, include a check before device login that TX/RX queues are still there before enabling the device. In addition, return a value that can be checked in case of any errors to avoid waiting for a completion that will never come. Signed-off-by: Thomas Falcon Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 53b2c91fa786..765407179fdd 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -111,7 +111,7 @@ static int ibmvnic_poll(struct napi_struct *napi, int data); static void send_map_query(struct ibmvnic_adapter *adapter); static void send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8); static void send_request_unmap(struct ibmvnic_adapter *, u8); -static void send_login(struct ibmvnic_adapter *adapter); +static int send_login(struct ibmvnic_adapter *adapter); static void send_cap_queries(struct ibmvnic_adapter *adapter); static int init_sub_crqs(struct ibmvnic_adapter *); static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter); @@ -809,8 +809,11 @@ static int ibmvnic_login(struct net_device *netdev) } reinit_completion(&adapter->init_done); - send_login(adapter); - if (!wait_for_completion_timeout(&adapter->init_done, + rc = send_login(adapter); + if (rc) { + dev_err(dev, "Unable to attempt device login\n"); + return rc; + } else if (!wait_for_completion_timeout(&adapter->init_done, timeout)) { dev_err(dev, "Login timeout\n"); return -1; @@ -3074,7 +3077,7 @@ static void vnic_add_client_data(struct ibmvnic_adapter *adapter, strncpy(&vlcd->name, adapter->netdev->name, len); } -static void send_login(struct ibmvnic_adapter *adapter) +static int send_login(struct ibmvnic_adapter *adapter) { struct ibmvnic_login_rsp_buffer *login_rsp_buffer; struct ibmvnic_login_buffer *login_buffer; @@ -3090,6 +3093,12 @@ static void send_login(struct ibmvnic_adapter *adapter) struct vnic_login_client_data *vlcd; int i; + if (!adapter->tx_scrq || !adapter->rx_scrq) { + netdev_err(adapter->netdev, + "RX or TX queues are not allocated, device login failed\n"); + return -1; + } + release_login_rsp_buffer(adapter); client_data_len = vnic_client_data_len(adapter); @@ -3187,7 +3196,7 @@ static void send_login(struct ibmvnic_adapter *adapter) crq.login.len = cpu_to_be32(buffer_size); ibmvnic_send_crq(adapter, &crq); - return; + return 0; buf_rsp_map_failed: kfree(login_rsp_buffer); @@ -3196,7 +3205,7 @@ static void send_login(struct ibmvnic_adapter *adapter) buf_map_failed: kfree(login_buffer); buf_alloc_failed: - return; + return -1; } static void send_request_map(struct ibmvnic_adapter *adapter, dma_addr_t addr,