From 3f5ec374ae3f3d5381622e29319a22c4ecd09413 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Wed, 6 Jan 2021 15:35:14 -0600 Subject: ibmvnic: merge do_change_param_reset into do_reset Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run") introduced do_change_param_reset function to solve the rtnl lock issue. Majority of the code in do_change_param_reset duplicates do_reset. Also, we can handle the rtnl lock issue in do_reset itself. Hence merge do_change_param_reset back into do_reset to clean up the code. Signed-off-by: Lijun Pan Link: https://lore.kernel.org/r/20210106213514.76027-1-ljp@linux.ibm.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 154 +++++++++++-------------------------- 1 file changed, 44 insertions(+), 110 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 9778c83150f1..aed985e08e8a 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1925,92 +1925,6 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) return rc; } -/** - * do_change_param_reset returns zero if we are able to keep processing reset - * events, or non-zero if we hit a fatal error and must halt. - */ -static int do_change_param_reset(struct ibmvnic_adapter *adapter, - struct ibmvnic_rwi *rwi, - u32 reset_state) -{ - struct net_device *netdev = adapter->netdev; - int i, rc; - - netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n", - rwi->reset_reason); - - netif_carrier_off(netdev); - adapter->reset_reason = rwi->reset_reason; - - ibmvnic_cleanup(netdev); - - if (reset_state == VNIC_OPEN) { - rc = __ibmvnic_close(netdev); - if (rc) - goto out; - } - - release_resources(adapter); - release_sub_crqs(adapter, 1); - release_crq_queue(adapter); - - adapter->state = VNIC_PROBED; - - rc = init_crq_queue(adapter); - - if (rc) { - netdev_err(adapter->netdev, - "Couldn't initialize crq. rc=%d\n", rc); - return rc; - } - - rc = ibmvnic_reset_init(adapter, true); - if (rc) { - rc = IBMVNIC_INIT_FAILED; - goto out; - } - - /* If the adapter was in PROBE state prior to the reset, - * exit here. - */ - if (reset_state == VNIC_PROBED) - goto out; - - rc = ibmvnic_login(netdev); - if (rc) { - goto out; - } - - rc = init_resources(adapter); - if (rc) - goto out; - - ibmvnic_disable_irqs(adapter); - - adapter->state = VNIC_CLOSED; - - if (reset_state == VNIC_CLOSED) - return 0; - - rc = __ibmvnic_open(netdev); - if (rc) { - rc = IBMVNIC_OPEN_FAILED; - goto out; - } - - /* refresh device's multicast list */ - ibmvnic_set_multi(netdev); - - /* kick napi */ - for (i = 0; i < adapter->req_rx_queues; i++) - napi_schedule(&adapter->napi[i]); - -out: - if (rc) - adapter->state = reset_state; - return rc; -} - /** * do_reset returns zero if we are able to keep processing reset events, or * non-zero if we hit a fatal error and must halt. @@ -2028,7 +1942,11 @@ static int do_reset(struct ibmvnic_adapter *adapter, adapter->state, adapter->failover_pending, rwi->reset_reason, reset_state); - rtnl_lock(); + adapter->reset_reason = rwi->reset_reason; + /* requestor of VNIC_RESET_CHANGE_PARAM already has the rtnl lock */ + if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM)) + rtnl_lock(); + /* * Now that we have the rtnl lock, clear any pending failover. * This will ensure ibmvnic_open() has either completed or will @@ -2038,7 +1956,6 @@ static int do_reset(struct ibmvnic_adapter *adapter, adapter->failover_pending = false; netif_carrier_off(netdev); - adapter->reset_reason = rwi->reset_reason; old_num_rx_queues = adapter->req_rx_queues; old_num_tx_queues = adapter->req_tx_queues; @@ -2050,25 +1967,37 @@ static int do_reset(struct ibmvnic_adapter *adapter, if (reset_state == VNIC_OPEN && adapter->reset_reason != VNIC_RESET_MOBILITY && adapter->reset_reason != VNIC_RESET_FAILOVER) { - adapter->state = VNIC_CLOSING; + if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) { + rc = __ibmvnic_close(netdev); + if (rc) + goto out; + } else { + adapter->state = VNIC_CLOSING; - /* Release the RTNL lock before link state change and - * re-acquire after the link state change to allow - * linkwatch_event to grab the RTNL lock and run during - * a reset. - */ - rtnl_unlock(); - rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN); - rtnl_lock(); - if (rc) - goto out; + /* Release the RTNL lock before link state change and + * re-acquire after the link state change to allow + * linkwatch_event to grab the RTNL lock and run during + * a reset. + */ + rtnl_unlock(); + rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN); + rtnl_lock(); + if (rc) + goto out; - if (adapter->state != VNIC_CLOSING) { - rc = -1; - goto out; + if (adapter->state != VNIC_CLOSING) { + rc = -1; + goto out; + } + + adapter->state = VNIC_CLOSED; } + } - adapter->state = VNIC_CLOSED; + if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) { + release_resources(adapter); + release_sub_crqs(adapter, 1); + release_crq_queue(adapter); } if (adapter->reset_reason != VNIC_RESET_NON_FATAL) { @@ -2077,7 +2006,9 @@ static int do_reset(struct ibmvnic_adapter *adapter, */ adapter->state = VNIC_PROBED; - if (adapter->reset_reason == VNIC_RESET_MOBILITY) { + if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) { + rc = init_crq_queue(adapter); + } else if (adapter->reset_reason == VNIC_RESET_MOBILITY) { rc = ibmvnic_reenable_crq_queue(adapter); release_sub_crqs(adapter, 1); } else { @@ -2116,7 +2047,11 @@ static int do_reset(struct ibmvnic_adapter *adapter, goto out; } - if (adapter->req_rx_queues != old_num_rx_queues || + if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) { + rc = init_resources(adapter); + if (rc) + goto out; + } else if (adapter->req_rx_queues != old_num_rx_queues || adapter->req_tx_queues != old_num_tx_queues || adapter->req_rx_add_entries_per_subcrq != old_num_rx_slots || @@ -2181,7 +2116,9 @@ out: /* restore the adapter state if reset failed */ if (rc) adapter->state = reset_state; - rtnl_unlock(); + /* requestor of VNIC_RESET_CHANGE_PARAM should still hold the rtnl lock */ + if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM)) + rtnl_unlock(); netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Reset done, rc %d\n", adapter->state, adapter->failover_pending, rc); @@ -2312,10 +2249,7 @@ static void __ibmvnic_reset(struct work_struct *work) } spin_unlock_irqrestore(&adapter->state_lock, flags); - if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) { - /* CHANGE_PARAM requestor holds rtnl_lock */ - rc = do_change_param_reset(adapter, rwi, reset_state); - } else if (adapter->force_reset_recovery) { + if (adapter->force_reset_recovery) { /* * Since we are doing a hard reset now, clear the * failover_pending flag so we don't ignore any -- cgit v1.2.3 From 807086021bf510ba03bd69a80a54e1de4a0fda30 Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Fri, 15 Jan 2021 20:09:03 +0000 Subject: net: ethernet: ibm: ibmvnic: Fix some kernel-doc misdemeanours MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the following W=1 kernel build warning(s): from drivers/net/ethernet/ibm/ibmvnic.c:35: inlined from ‘handle_vpd_rsp’ at drivers/net/ethernet/ibm/ibmvnic.c:4124:3: drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_field' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'skb' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_len' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 'hdr_data' not described in 'build_hdr_data' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_field' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_data' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'len' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'hdr_len' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 'scrq_arr' not described in 'create_hdr_descs' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'txbuff' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'num_entries' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 'hdr_field' not described in 'build_hdr_descs_arr' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'adapter' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'rwi' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 'reset_state' not described in 'do_change_param_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'adapter' not described in 'do_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'rwi' not described in 'do_reset' drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 'reset_state' not described in 'do_reset' Signed-off-by: Lee Jones Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 27 +++++++++++++-------------- drivers/net/xen-netfront.c | 6 +++--- 2 files changed, 16 insertions(+), 17 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index aed985e08e8a..4c4252e68de5 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1384,10 +1384,10 @@ static int ibmvnic_close(struct net_device *netdev) /** * build_hdr_data - creates L2/L3/L4 header data buffer - * @hdr_field - bitfield determining needed headers - * @skb - socket buffer - * @hdr_len - array of header lengths - * @tot_len - total length of data + * @hdr_field: bitfield determining needed headers + * @skb: socket buffer + * @hdr_len: array of header lengths + * @hdr_data: buffer to write the header to * * Reads hdr_field to determine which headers are needed by firmware. * Builds a buffer containing these headers. Saves individual header @@ -1444,11 +1444,11 @@ static int build_hdr_data(u8 hdr_field, struct sk_buff *skb, /** * create_hdr_descs - create header and header extension descriptors - * @hdr_field - bitfield determining needed headers - * @data - buffer containing header data - * @len - length of data buffer - * @hdr_len - array of individual header lengths - * @scrq_arr - descriptor array + * @hdr_field: bitfield determining needed headers + * @hdr_data: buffer containing header data + * @len: length of data buffer + * @hdr_len: array of individual header lengths + * @scrq_arr: descriptor array * * Creates header and, if needed, header extension descriptors and * places them in a descriptor array, scrq_arr @@ -1496,10 +1496,9 @@ static int create_hdr_descs(u8 hdr_field, u8 *hdr_data, int len, int *hdr_len, /** * build_hdr_descs_arr - build a header descriptor array - * @skb - socket buffer - * @num_entries - number of descriptors to be sent - * @subcrq - first TX descriptor - * @hdr_field - bit field determining which headers will be sent + * @txbuff: tx buffer + * @num_entries: number of descriptors to be sent + * @hdr_field: bit field determining which headers will be sent * * This function will build a TX descriptor array with applicable * L2/L3/L4 packet header descriptors to be sent by send_subcrq_indirect. @@ -1925,7 +1924,7 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) return rc; } -/** +/* * do_reset returns zero if we are able to keep processing reset events, or * non-zero if we hit a fatal error and must halt. */ diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index c20b78120bb4..6ef2adbd283a 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1580,7 +1580,7 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev) return ERR_PTR(err); } -/** +/* * Entry point to this code when a new device is created. Allocate the basic * structures and the ring buffers for communication with the backend, and * inform the backend of the appropriate details for those. @@ -1657,7 +1657,7 @@ static void xennet_disconnect_backend(struct netfront_info *info) } } -/** +/* * We are reconnecting to the backend, due to a suspend/resume, or a backend * driver restart. We tear down our netif structure and recreate it, but * leave the device-layer structures intact so that this is transparent to the @@ -2303,7 +2303,7 @@ static int xennet_connect(struct net_device *dev) return 0; } -/** +/* * Callback received when the backend's state changes. */ static void netback_changed(struct xenbus_device *dev, -- cgit v1.2.3 From e41aec79e62fa50f940cf222d1e9577f14e149dc Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Wed, 27 Jan 2021 19:34:42 -0600 Subject: ibmvnic: Ensure that CRQ entry read are correctly ordered Ensure that received Command-Response Queue (CRQ) entries are properly read in order by the driver. dma_rmb barrier has been added before accessing the CRQ descriptor to ensure the entire descriptor is read before processing. Fixes: 032c5e82847a ("Driver for IBM System i/p VNIC protocol") Signed-off-by: Lijun Pan Link: https://lore.kernel.org/r/20210128013442.88319-1-ljp@linux.ibm.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 9778c83150f1..8820c98ea891 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -5084,6 +5084,12 @@ static void ibmvnic_tasklet(struct tasklet_struct *t) while (!done) { /* Pull all the valid messages off the CRQ */ while ((crq = ibmvnic_next_crq(adapter)) != NULL) { + /* This barrier makes sure ibmvnic_next_crq()'s + * crq->generic.first & IBMVNIC_CRQ_CMD_RSP is loaded + * before ibmvnic_handle_crq()'s + * switch(gen_crq->first) and switch(gen_crq->cmd). + */ + dma_rmb(); ibmvnic_handle_crq(crq, adapter); crq->generic.first = 0; } -- cgit v1.2.3 From 5e9eff5dfa460cd1a74b7c1fde4fced7c04383af Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Thu, 28 Jan 2021 22:34:01 -0600 Subject: ibmvnic: device remove has higher precedence over reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Returning -EBUSY in ibmvnic_remove() does not actually hold the removal procedure since driver core doesn't care for the return value (see __device_release_driver() in drivers/base/dd.c calling dev->bus->remove()) though vio_bus_remove (in arch/powerpc/platforms/pseries/vio.c) records the return value and passes it on. [1] During the device removal precedure, checking for resetting bit is dropped so that we can continue executing all the cleanup calls in the rest of the remove function. Otherwise, it can cause latent memory leaks and kernel crashes. [1] https://lore.kernel.org/linuxppc-dev/20210117101242.dpwayq6wdgfdzirl@pengutronix.de/T/#m48f5befd96bc9842ece2a3ad14f4c27747206a53 Reported-by: Uwe Kleine-König Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during device reset") Signed-off-by: Lijun Pan Link: https://lore.kernel.org/r/20210129043402.95744-1-ljp@linux.ibm.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 8820c98ea891..f79034c786c8 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -5444,11 +5444,6 @@ static int ibmvnic_remove(struct vio_dev *dev) unsigned long flags; spin_lock_irqsave(&adapter->state_lock, flags); - if (test_bit(0, &adapter->resetting)) { - spin_unlock_irqrestore(&adapter->state_lock, flags); - return -EBUSY; - } - adapter->state = VNIC_REMOVING; spin_unlock_irqrestore(&adapter->state_lock, flags); -- cgit v1.2.3 From 665ab1eb18d7e8eaa8377fb8bf4924bfeb63bbce Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Fri, 29 Jan 2021 19:19:04 -0600 Subject: ibmvnic: rework to ensure SCRQ entry reads are properly ordered Move the dma_rmb() between pending_scrq() and ibmvnic_next_scrq() into the end of pending_scrq() to save the duplicated code since this dma_rmb will be used 3 times. Signed-off-by: Lijun Pan Acked-by: Thomas Falcon Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index a40af510018a..331ebca2f57a 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2444,12 +2444,6 @@ restart_poll: if (!pending_scrq(adapter, rx_scrq)) break; - /* The queue entry at the current index is peeked at above - * to determine that there is a valid descriptor awaiting - * processing. We want to be sure that the current slot - * holds a valid descriptor before reading its contents. - */ - dma_rmb(); next = ibmvnic_next_scrq(adapter, rx_scrq); rx_buff = (struct ibmvnic_rx_buff *)be64_to_cpu(next-> @@ -3189,13 +3183,6 @@ restart_loop: int total_bytes = 0; int num_packets = 0; - /* The queue entry at the current index is peeked at above - * to determine that there is a valid descriptor awaiting - * processing. We want to be sure that the current slot - * holds a valid descriptor before reading its contents. - */ - dma_rmb(); - next = ibmvnic_next_scrq(adapter, scrq); for (i = 0; i < next->tx_comp.num_comps; i++) { if (next->tx_comp.rcs[i]) @@ -3569,11 +3556,16 @@ static int pending_scrq(struct ibmvnic_adapter *adapter, struct ibmvnic_sub_crq_queue *scrq) { union sub_crq *entry = &scrq->msgs[scrq->cur]; + int rc; - if (entry->generic.first & IBMVNIC_CRQ_CMD_RSP) - return 1; - else - return 0; + rc = !!(entry->generic.first & IBMVNIC_CRQ_CMD_RSP); + + /* Ensure that the SCRQ valid flag is loaded prior to loading the + * contents of the SCRQ descriptor + */ + dma_rmb(); + + return rc; } static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *adapter, @@ -3592,8 +3584,8 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *adapter, } spin_unlock_irqrestore(&scrq->lock, flags); - /* Ensure that the entire buffer descriptor has been - * loaded before reading its contents + /* Ensure that the SCRQ valid flag is loaded prior to loading the + * contents of the SCRQ descriptor */ dma_rmb(); -- cgit v1.2.3 From 2719cb445da5fec698e961abdf75cf9e4d61fba4 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Fri, 29 Jan 2021 19:19:05 -0600 Subject: ibmvnic: remove unnecessary rmb() inside ibmvnic_poll rmb() can be removed since: 1. pending_scrq() has dma_rmb() at the function end; 2. dma_rmb(), though weaker, is enough here. Signed-off-by: Lijun Pan Acked-by: Dwip Banerjee Acked-by: Thomas Falcon Reviewed-by: Brian King Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 331ebca2f57a..0ed169ef1cfc 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2510,7 +2510,6 @@ restart_poll: if (napi_complete_done(napi, frames_processed)) { enable_scrq_irq(adapter, rx_scrq); if (pending_scrq(adapter, rx_scrq)) { - rmb(); if (napi_reschedule(napi)) { disable_scrq_irq(adapter, rx_scrq); goto restart_poll; -- cgit v1.2.3 From ef66a1eace968ff22a35f45e6e8ec36b668b6116 Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu Date: Tue, 2 Feb 2021 21:08:02 -0800 Subject: ibmvnic: Clear failover_pending if unable to schedule Normally we clear the failover_pending flag when processing the reset. But if we are unable to schedule a failover reset we must clear the flag ourselves. We could fail to schedule the reset if we are in PROBING state (eg: when booting via kexec) or because we could not allocate memory. Thanks to Cris Forno for helping isolate the problem and for testing. Fixes: 1d8504937478 ("powerpc/vnic: Extend "failover pending" window") Signed-off-by: Sukadev Bhattiprolu Tested-by: Cristobal Forno Link: https://lore.kernel.org/r/20210203050802.680772-1-sukadev@linux.ibm.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index f79034c786c8..a536fdbf05e1 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -4918,7 +4918,22 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, complete(&adapter->init_done); adapter->init_done_rc = -EIO; } - ibmvnic_reset(adapter, VNIC_RESET_FAILOVER); + rc = ibmvnic_reset(adapter, VNIC_RESET_FAILOVER); + if (rc && rc != -EBUSY) { + /* We were unable to schedule the failover + * reset either because the adapter was still + * probing (eg: during kexec) or we could not + * allocate memory. Clear the failover_pending + * flag since no one else will. We ignore + * EBUSY because it means either FAILOVER reset + * is already scheduled or the adapter is + * being removed. + */ + netdev_err(netdev, + "Error %ld scheduling failover reset\n", + rc); + adapter->failover_pending = false; + } break; case IBMVNIC_CRQ_INIT_COMPLETE: dev_info(dev, "Partner initialization complete\n"); -- cgit v1.2.3 From 429aa36469f95b0e389b512dcbd1065225e9d0a2 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Thu, 11 Feb 2021 00:43:18 -0600 Subject: ibmvnic: prefer 'unsigned long' over 'unsigned long int' Fix the following checkpatch warnings: WARNING: Prefer 'unsigned long' over 'unsigned long int' as the int is unnecessary WARNING: Prefer 'long' over 'long int' as the int is unnecessary Signed-off-by: Lijun Pan Reviewed-by: Saeed Mahameed Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index a1579cd4bfe1..55970f02d9da 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -3654,8 +3654,8 @@ static int ibmvnic_send_crq(struct ibmvnic_adapter *adapter, int rc; netdev_dbg(adapter->netdev, "Sending CRQ: %016lx %016lx\n", - (unsigned long int)cpu_to_be64(u64_crq[0]), - (unsigned long int)cpu_to_be64(u64_crq[1])); + (unsigned long)cpu_to_be64(u64_crq[0]), + (unsigned long)cpu_to_be64(u64_crq[1])); if (!adapter->crq.active && crq->generic.first != IBMVNIC_CRQ_INIT_CMD) { @@ -3884,7 +3884,7 @@ static int send_login(struct ibmvnic_adapter *adapter) netdev_dbg(adapter->netdev, "Login Buffer:\n"); for (i = 0; i < (adapter->login_buf_sz - 1) / 8 + 1; i++) { netdev_dbg(adapter->netdev, "%016lx\n", - ((unsigned long int *)(adapter->login_buf))[i]); + ((unsigned long *)(adapter->login_buf))[i]); } memset(&crq, 0, sizeof(crq)); @@ -4252,7 +4252,7 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter) netdev_dbg(adapter->netdev, "Query IP Offload Buffer:\n"); for (i = 0; i < (sizeof(adapter->ip_offload_buf) - 1) / 8 + 1; i++) netdev_dbg(adapter->netdev, "%016lx\n", - ((unsigned long int *)(buf))[i]); + ((unsigned long *)(buf))[i]); netdev_dbg(adapter->netdev, "ipv4_chksum = %d\n", buf->ipv4_chksum); netdev_dbg(adapter->netdev, "ipv6_chksum = %d\n", buf->ipv6_chksum); @@ -4411,7 +4411,7 @@ static void handle_request_cap_rsp(union ibmvnic_crq *crq, case PARTIALSUCCESS: dev_info(dev, "req=%lld, rsp=%ld in %s queue, retrying.\n", *req_value, - (long int)be64_to_cpu(crq->request_capability_rsp. + (long)be64_to_cpu(crq->request_capability_rsp. number), name); if (be16_to_cpu(crq->request_capability_rsp.capability) == @@ -4482,7 +4482,7 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq, netdev_dbg(adapter->netdev, "Login Response Buffer:\n"); for (i = 0; i < (adapter->login_rsp_buf_sz - 1) / 8 + 1; i++) { netdev_dbg(adapter->netdev, "%016lx\n", - ((unsigned long int *)(adapter->login_rsp_buf))[i]); + ((unsigned long *)(adapter->login_rsp_buf))[i]); } /* Sanity checks */ @@ -4825,8 +4825,8 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, long rc; netdev_dbg(netdev, "Handling CRQ: %016lx %016lx\n", - (unsigned long int)cpu_to_be64(u64_crq[0]), - (unsigned long int)cpu_to_be64(u64_crq[1])); + (unsigned long)cpu_to_be64(u64_crq[0]), + (unsigned long)cpu_to_be64(u64_crq[1])); switch (gen_crq->first) { case IBMVNIC_CRQ_INIT_RSP: switch (gen_crq->cmd) { -- cgit v1.2.3 From bab08bedcdc33a644682197f3c88a6e56c325fcf Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Thu, 11 Feb 2021 00:43:19 -0600 Subject: ibmvnic: fix block comments Fix the following checkpatch warning: WARNING: networking block comments don't use an empty /* line, use /* Comment... Signed-off-by: Lijun Pan Reviewed-by: Saeed Mahameed Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 55970f02d9da..5c6f6a7b2e3f 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1221,8 +1221,7 @@ static int ibmvnic_open(struct net_device *netdev) rc = __ibmvnic_open(netdev); out: - /* - * If open fails due to a pending failover, set device state and + /* If open fails due to a pending failover, set device state and * return. Device operation will be handled by reset routine. */ if (rc && adapter->failover_pending) { @@ -1946,8 +1945,7 @@ static int do_reset(struct ibmvnic_adapter *adapter, if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM)) rtnl_lock(); - /* - * Now that we have the rtnl lock, clear any pending failover. + /* Now that we have the rtnl lock, clear any pending failover. * This will ensure ibmvnic_open() has either completed or will * block until failover is complete. */ @@ -2249,8 +2247,7 @@ static void __ibmvnic_reset(struct work_struct *work) spin_unlock_irqrestore(&adapter->state_lock, flags); if (adapter->force_reset_recovery) { - /* - * Since we are doing a hard reset now, clear the + /* Since we are doing a hard reset now, clear the * failover_pending flag so we don't ignore any * future MOBILITY or other resets. */ @@ -2322,8 +2319,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, unsigned long flags; int ret; - /* - * If failover is pending don't schedule any other reset. + /* If failover is pending don't schedule any other reset. * Instead let the failover complete. If there is already a * a failover reset scheduled, we will detect and drop the * duplicate reset when walking the ->rwi_list below. -- cgit v1.2.3 From f78afaace636dfa89501be99fc5b431d85a0f639 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Thu, 11 Feb 2021 00:43:20 -0600 Subject: ibmvnic: fix braces Fix the following checkpatch warning: WARNING: braces {} are not necessary for single statement blocks Signed-off-by: Lijun Pan Reviewed-by: Saeed Mahameed Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 5c6f6a7b2e3f..6c1fc73d66dc 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2040,9 +2040,8 @@ static int do_reset(struct ibmvnic_adapter *adapter, } rc = ibmvnic_login(netdev); - if (rc) { + if (rc) goto out; - } if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) { rc = init_resources(adapter); -- cgit v1.2.3 From 914789acaaae890b930ebae37bcc6d8d76ea7166 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Thu, 11 Feb 2021 00:43:21 -0600 Subject: ibmvnic: avoid multiple line dereference Fix the following checkpatch warning: WARNING: Avoid multiple line dereference Signed-off-by: Lijun Pan Reviewed-by: Saeed Mahameed Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 6c1fc73d66dc..67b09a53d7ac 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2440,9 +2440,8 @@ restart_poll: if (!pending_scrq(adapter, rx_scrq)) break; next = ibmvnic_next_scrq(adapter, rx_scrq); - rx_buff = - (struct ibmvnic_rx_buff *)be64_to_cpu(next-> - rx_comp.correlator); + rx_buff = (struct ibmvnic_rx_buff *) + be64_to_cpu(next->rx_comp.correlator); /* do error checking */ if (next->rx_comp.rc) { netdev_dbg(netdev, "rx buffer returned with rc %x\n", @@ -3855,15 +3854,15 @@ static int send_login(struct ibmvnic_adapter *adapter) for (i = 0; i < adapter->req_tx_queues; i++) { if (adapter->tx_scrq[i]) { - tx_list_p[i] = cpu_to_be64(adapter->tx_scrq[i]-> - crq_num); + tx_list_p[i] = + cpu_to_be64(adapter->tx_scrq[i]->crq_num); } } for (i = 0; i < adapter->req_rx_queues; i++) { if (adapter->rx_scrq[i]) { - rx_list_p[i] = cpu_to_be64(adapter->rx_scrq[i]-> - crq_num); + rx_list_p[i] = + cpu_to_be64(adapter->rx_scrq[i]->crq_num); } } @@ -4406,8 +4405,8 @@ static void handle_request_cap_rsp(union ibmvnic_crq *crq, case PARTIALSUCCESS: dev_info(dev, "req=%lld, rsp=%ld in %s queue, retrying.\n", *req_value, - (long)be64_to_cpu(crq->request_capability_rsp. - number), name); + (long)be64_to_cpu(crq->request_capability_rsp.number), + name); if (be16_to_cpu(crq->request_capability_rsp.capability) == REQ_MTU) { -- cgit v1.2.3 From 91dc5d2553fbf20e2e8384ac997f278a50c70561 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Thu, 11 Feb 2021 00:43:22 -0600 Subject: ibmvnic: fix miscellaneous checks Fix the following checkpatch checks: CHECK: Macro argument 'off' may be better as '(off)' to avoid precedence issues CHECK: Alignment should match open parenthesis CHECK: multiple assignments should be avoided CHECK: Blank lines aren't necessary before a close brace '}' CHECK: Please use a blank line after function/struct/union/enum declarations CHECK: Unnecessary parentheses around 'rc != H_FUNCTION' Signed-off-by: Lijun Pan Reviewed-by: Saeed Mahameed Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 67b09a53d7ac..0c27a1f9663a 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -115,7 +115,7 @@ struct ibmvnic_stat { #define IBMVNIC_STAT_OFF(stat) (offsetof(struct ibmvnic_adapter, stats) + \ offsetof(struct ibmvnic_statistics, stat)) -#define IBMVNIC_GET_STAT(a, off) (*((u64 *)(((unsigned long)(a)) + off))) +#define IBMVNIC_GET_STAT(a, off) (*((u64 *)(((unsigned long)(a)) + (off)))) static const struct ibmvnic_stat ibmvnic_stats[] = { {"rx_packets", IBMVNIC_STAT_OFF(rx_packets)}, @@ -2069,14 +2069,14 @@ static int do_reset(struct ibmvnic_adapter *adapter, rc = reset_tx_pools(adapter); if (rc) { netdev_dbg(adapter->netdev, "reset tx pools failed (%d)\n", - rc); + rc); goto out; } rc = reset_rx_pools(adapter); if (rc) { netdev_dbg(adapter->netdev, "reset rx pools failed (%d)\n", - rc); + rc); goto out; } } @@ -2333,7 +2333,8 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, if (adapter->state == VNIC_PROBING) { netdev_warn(netdev, "Adapter reset during probe\n"); - ret = adapter->init_done_rc = EAGAIN; + adapter->init_done_rc = EAGAIN; + ret = EAGAIN; goto err; } @@ -2746,7 +2747,6 @@ static int ibmvnic_set_channels(struct net_device *netdev, channels->rx_count, channels->tx_count, adapter->req_rx_queues, adapter->req_tx_queues); return ret; - } static void ibmvnic_get_strings(struct net_device *dev, u32 stringset, u8 *data) @@ -2835,8 +2835,8 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev, return; for (i = 0; i < ARRAY_SIZE(ibmvnic_stats); i++) - data[i] = be64_to_cpu(IBMVNIC_GET_STAT(adapter, - ibmvnic_stats[i].offset)); + data[i] = be64_to_cpu(IBMVNIC_GET_STAT + (adapter, ibmvnic_stats[i].offset)); for (j = 0; j < adapter->req_tx_queues; j++) { data[i] = adapter->tx_stats_buffers[j].packets; @@ -2876,6 +2876,7 @@ static int ibmvnic_set_priv_flags(struct net_device *netdev, u32 flags) return 0; } + static const struct ethtool_ops ibmvnic_ethtool_ops = { .get_drvinfo = ibmvnic_get_drvinfo, .get_msglevel = ibmvnic_get_msglevel, @@ -3145,7 +3146,7 @@ static int enable_scrq_irq(struct ibmvnic_adapter *adapter, /* H_EOI would fail with rc = H_FUNCTION when running * in XIVE mode which is expected, but not an error. */ - if (rc && (rc != H_FUNCTION)) + if (rc && rc != H_FUNCTION) dev_err(dev, "H_EOI FAILED irq 0x%llx. rc=%ld\n", val, rc); } -- cgit v1.2.3 From 4bb9f2e48299d068a704f490c1be4b1fb6d278ce Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Thu, 11 Feb 2021 00:43:24 -0600 Subject: ibmvnic: remove unused spinlock_t stats_lock definition stats_lock is no longer used. So remove it. Signed-off-by: Lijun Pan Reviewed-by: Saeed Mahameed Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 2 -- drivers/net/ethernet/ibm/ibmvnic.h | 1 - 2 files changed, 3 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 0c27a1f9663a..778e56e05cd7 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -5295,8 +5295,6 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) netdev->ethtool_ops = &ibmvnic_ethtool_ops; SET_NETDEV_DEV(netdev, &dev->dev); - spin_lock_init(&adapter->stats_lock); - INIT_WORK(&adapter->ibmvnic_reset, __ibmvnic_reset); INIT_DELAYED_WORK(&adapter->ibmvnic_delayed_reset, __ibmvnic_delayed_reset); diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index 8f73a30a7593..270d1cac86a4 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -987,7 +987,6 @@ struct ibmvnic_adapter { struct ibmvnic_statistics stats; dma_addr_t stats_token; struct completion stats_done; - spinlock_t stats_lock; int replenish_no_mem; int replenish_add_buff_success; int replenish_add_buff_failure; -- cgit v1.2.3 From 8a96c80e2774172c6a091b5d40579fe957228393 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Thu, 11 Feb 2021 00:43:25 -0600 Subject: ibmvnic: prefer strscpy over strlcpy Fix this warning: WARNING: Prefer strscpy over strlcpy - see: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/ Signed-off-by: Lijun Pan Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 778e56e05cd7..1774fbaab146 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2633,9 +2633,9 @@ static void ibmvnic_get_drvinfo(struct net_device *netdev, { struct ibmvnic_adapter *adapter = netdev_priv(netdev); - strlcpy(info->driver, ibmvnic_driver_name, sizeof(info->driver)); - strlcpy(info->version, IBMVNIC_DRIVER_VERSION, sizeof(info->version)); - strlcpy(info->fw_version, adapter->fw_version, + strscpy(info->driver, ibmvnic_driver_name, sizeof(info->driver)); + strscpy(info->version, IBMVNIC_DRIVER_VERSION, sizeof(info->version)); + strscpy(info->fw_version, adapter->fw_version, sizeof(info->fw_version)); } -- cgit v1.2.3 From d4083d3c00f60a09ad82e3bf17ff57fec69c8aa6 Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu Date: Wed, 10 Feb 2021 17:41:43 -0800 Subject: ibmvnic: Set to CLOSED state even on error If set_link_state() fails for any reason, we still cleanup the adapter state and cannot recover from a partial close anyway. So set the adapter to CLOSED state. That way if a new soft/hard reset is processed, the adapter will remain in the CLOSED state until the next ibmvnic_open(). Fixes: 01d9bd792d16 ("ibmvnic: Reorganize device close") Signed-off-by: Sukadev Bhattiprolu Reported-by: Abdul Haleem Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index a536fdbf05e1..621be6d2da97 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1353,10 +1353,8 @@ static int __ibmvnic_close(struct net_device *netdev) adapter->state = VNIC_CLOSING; rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN); - if (rc) - return rc; adapter->state = VNIC_CLOSED; - return 0; + return rc; } static int ibmvnic_close(struct net_device *netdev) -- cgit v1.2.3 From 1c7d45e7b2c29080bf6c8cd0e213cc3cbb62a054 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Fri, 12 Feb 2021 20:36:10 -0600 Subject: ibmvnic: simplify reset_long_term_buff function The only thing reset_long_term_buff() should do is set buffer to zero. After doing that, it is not necessary to send_request_map again to VIOS since it actually does not change the mapping. So, keep memset function and remove all others. Signed-off-by: Lijun Pan Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 46 +++++++------------------------------- 1 file changed, 8 insertions(+), 38 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 1774fbaab146..7a5e589e7223 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -253,40 +253,12 @@ static void free_long_term_buff(struct ibmvnic_adapter *adapter, dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr); } -static int reset_long_term_buff(struct ibmvnic_adapter *adapter, - struct ibmvnic_long_term_buff *ltb) +static int reset_long_term_buff(struct ibmvnic_long_term_buff *ltb) { - struct device *dev = &adapter->vdev->dev; - int rc; + if (!ltb->buff) + return -EINVAL; memset(ltb->buff, 0, ltb->size); - - mutex_lock(&adapter->fw_lock); - adapter->fw_done_rc = 0; - - reinit_completion(&adapter->fw_done); - rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id); - if (rc) { - mutex_unlock(&adapter->fw_lock); - return rc; - } - - rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000); - if (rc) { - dev_info(dev, - "Reset failed, long term map request timed out or aborted\n"); - mutex_unlock(&adapter->fw_lock); - return rc; - } - - if (adapter->fw_done_rc) { - dev_info(dev, - "Reset failed, attempting to free and reallocate buffer\n"); - free_long_term_buff(adapter, ltb); - mutex_unlock(&adapter->fw_lock); - return alloc_long_term_buff(adapter, ltb, ltb->size); - } - mutex_unlock(&adapter->fw_lock); return 0; } @@ -508,8 +480,7 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter) rx_pool->size * rx_pool->buff_size); } else { - rc = reset_long_term_buff(adapter, - &rx_pool->long_term_buff); + rc = reset_long_term_buff(&rx_pool->long_term_buff); } if (rc) @@ -632,12 +603,11 @@ static int init_rx_pools(struct net_device *netdev) return 0; } -static int reset_one_tx_pool(struct ibmvnic_adapter *adapter, - struct ibmvnic_tx_pool *tx_pool) +static int reset_one_tx_pool(struct ibmvnic_tx_pool *tx_pool) { int rc, i; - rc = reset_long_term_buff(adapter, &tx_pool->long_term_buff); + rc = reset_long_term_buff(&tx_pool->long_term_buff); if (rc) return rc; @@ -664,10 +634,10 @@ static int reset_tx_pools(struct ibmvnic_adapter *adapter) tx_scrqs = adapter->num_active_tx_pools; for (i = 0; i < tx_scrqs; i++) { - rc = reset_one_tx_pool(adapter, &adapter->tso_pool[i]); + rc = reset_one_tx_pool(&adapter->tso_pool[i]); if (rc) return rc; - rc = reset_one_tx_pool(adapter, &adapter->tx_pool[i]); + rc = reset_one_tx_pool(&adapter->tx_pool[i]); if (rc) return rc; } -- cgit v1.2.3 From 1a42156f52bd4de0e2442671bc24b7ffc7b01c52 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Fri, 12 Feb 2021 20:36:46 -0600 Subject: ibmvnic: substitute mb() with dma_wmb() for send_*crq* functions The CRQ and subCRQ descriptors are DMA mapped, so dma_wmb(), though weaker, is good enough to protect the data structures. Signed-off-by: Lijun Pan Acked-by: Thomas Falcon Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 7a5e589e7223..927d5f36d308 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -3599,7 +3599,7 @@ static int send_subcrq_indirect(struct ibmvnic_adapter *adapter, int rc; /* Make sure the hypervisor sees the complete request */ - mb(); + dma_wmb(); rc = plpar_hcall_norets(H_SEND_SUB_CRQ_INDIRECT, ua, cpu_to_be64(remote_handle), ioba, num_entries); @@ -3629,7 +3629,7 @@ static int ibmvnic_send_crq(struct ibmvnic_adapter *adapter, } /* Make sure the hypervisor sees the complete request */ - mb(); + dma_wmb(); rc = plpar_hcall_norets(H_SEND_CRQ, ua, cpu_to_be64(u64_crq[0]), -- cgit v1.2.3 From 42557dab78edc8235aba5b441f2eb35f725a0ede Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Fri, 12 Feb 2021 20:48:40 -0600 Subject: ibmvnic: add memory barrier to protect long term buffer dma_rmb() barrier is added to load the long term buffer before copying it to socket buffer; and dma_wmb() barrier is added to update the long term buffer before it being accessed by VIOS (virtual i/o server). Fixes: 032c5e82847a ("Driver for IBM System i/p VNIC protocol") Signed-off-by: Lijun Pan Acked-by: Thomas Falcon Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 621be6d2da97..f6402a20ba32 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1700,6 +1700,9 @@ static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev) skb_copy_from_linear_data(skb, dst, skb->len); } + /* post changes to long_term_buff *dst before VIOS accessing it */ + dma_wmb(); + tx_pool->consumer_index = (tx_pool->consumer_index + 1) % tx_pool->num_buffers; @@ -2539,6 +2542,8 @@ restart_poll: offset = be16_to_cpu(next->rx_comp.off_frame_data); flags = next->rx_comp.flags; skb = rx_buff->skb; + /* load long_term_buff before copying to skb */ + dma_rmb(); skb_copy_to_linear_data(skb, rx_buff->data + offset, length); -- cgit v1.2.3 From 7d3a7b9ea59ddb223aec59b45fa1713c633aaed4 Mon Sep 17 00:00:00 2001 From: Lijun Pan Date: Fri, 12 Feb 2021 20:49:00 -0600 Subject: ibmvnic: skip send_request_unmap for timeout reset Timeout reset will trigger the VIOS to unmap it automatically, similarly as FAILVOER and MOBILITY events. If we unmap it in the linux side, we will see errors like "30000003: Error 4 in REQUEST_UNMAP_RSP". So, don't call send_request_unmap for timeout reset. Fixes: ed651a10875f ("ibmvnic: Updated reset handling") Signed-off-by: Lijun Pan Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index f6402a20ba32..cd201f89ce6c 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -247,8 +247,13 @@ static void free_long_term_buff(struct ibmvnic_adapter *adapter, if (!ltb->buff) return; + /* VIOS automatically unmaps the long term buffer at remote + * end for the following resets: + * FAILOVER, MOBILITY, TIMEOUT. + */ if (adapter->reset_reason != VNIC_RESET_FAILOVER && - adapter->reset_reason != VNIC_RESET_MOBILITY) + adapter->reset_reason != VNIC_RESET_MOBILITY && + adapter->reset_reason != VNIC_RESET_TIMEOUT) send_request_unmap(adapter, ltb->map_id); dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr); } -- cgit v1.2.3 From 4a41c421f3676fdeea91733cf434dcf319c4c351 Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu Date: Fri, 12 Feb 2021 20:42:50 -0800 Subject: ibmvnic: serialize access to work queue on remove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The work queue is used to queue reset requests like CHANGE-PARAM or FAILOVER resets for the worker thread. When the adapter is being removed the adapter state is set to VNIC_REMOVING and the work queue is flushed so no new work is added. However the check for adapter being removed is racy in that the adapter can go into REMOVING state just after we check and we might end up adding work just as it is being flushed (or after). The ->rwi_lock is already being used to serialize queue/dequeue work. Extend its usage ensure there is no race when scheduling/flushing work. Fixes: 6954a9e4192b ("ibmvnic: Flush existing work items before device removal") Signed-off-by: Sukadev Bhattiprolu Cc:Uwe Kleine-König Cc:Saeed Mahameed Reviewed-by: Dany Madden Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 27 ++++++++++++++++++++------- drivers/net/ethernet/ibm/ibmvnic.h | 5 ++++- 2 files changed, 24 insertions(+), 8 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index cd201f89ce6c..13ae7eee7ef5 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2395,6 +2395,8 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, unsigned long flags; int ret; + spin_lock_irqsave(&adapter->rwi_lock, flags); + /* * If failover is pending don't schedule any other reset. * Instead let the failover complete. If there is already a @@ -2415,14 +2417,11 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, goto err; } - spin_lock_irqsave(&adapter->rwi_lock, flags); - list_for_each(entry, &adapter->rwi_list) { tmp = list_entry(entry, struct ibmvnic_rwi, list); if (tmp->reset_reason == reason) { netdev_dbg(netdev, "Skipping matching reset, reason=%d\n", reason); - spin_unlock_irqrestore(&adapter->rwi_lock, flags); ret = EBUSY; goto err; } @@ -2430,8 +2429,6 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, rwi = kzalloc(sizeof(*rwi), GFP_ATOMIC); if (!rwi) { - spin_unlock_irqrestore(&adapter->rwi_lock, flags); - ibmvnic_close(netdev); ret = ENOMEM; goto err; } @@ -2444,12 +2441,17 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, } rwi->reset_reason = reason; list_add_tail(&rwi->list, &adapter->rwi_list); - spin_unlock_irqrestore(&adapter->rwi_lock, flags); netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason); schedule_work(&adapter->ibmvnic_reset); - return 0; + ret = 0; err: + /* ibmvnic_close() below can block, so drop the lock first */ + spin_unlock_irqrestore(&adapter->rwi_lock, flags); + + if (ret == ENOMEM) + ibmvnic_close(netdev); + return -ret; } @@ -5467,7 +5469,18 @@ static int ibmvnic_remove(struct vio_dev *dev) unsigned long flags; spin_lock_irqsave(&adapter->state_lock, flags); + + /* If ibmvnic_reset() is scheduling a reset, wait for it to + * finish. Then, set the state to REMOVING to prevent it from + * scheduling any more work and to have reset functions ignore + * any resets that have already been scheduled. Drop the lock + * after setting state, so __ibmvnic_reset() which is called + * from the flush_work() below, can make progress. + */ + spin_lock_irqsave(&adapter->rwi_lock, flags); adapter->state = VNIC_REMOVING; + spin_unlock_irqrestore(&adapter->rwi_lock, flags); + spin_unlock_irqrestore(&adapter->state_lock, flags); flush_work(&adapter->ibmvnic_reset); diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index 07ced1016aa4..72fea3b1c87d 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -1081,6 +1081,7 @@ struct ibmvnic_adapter { struct tasklet_struct tasklet; enum vnic_state state; enum ibmvnic_reset_reason reset_reason; + /* when taking both state and rwi locks, take state lock first */ spinlock_t rwi_lock; struct list_head rwi_list; struct work_struct ibmvnic_reset; @@ -1097,6 +1098,8 @@ struct ibmvnic_adapter { struct ibmvnic_tunables desired; struct ibmvnic_tunables fallback; - /* Used for serializatin of state field */ + /* Used for serialization of state field. When taking both state + * and rwi locks, take state lock first. + */ spinlock_t state_lock; }; -- cgit v1.2.3 From b646acd5eb48ec49ef90404336d7e8ee502ecd05 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 16 Feb 2021 22:58:44 -0800 Subject: net: re-solve some conflicts after net -> net-next merge Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 5 +++-- drivers/net/ethernet/ibm/ibmvnic.h | 10 +++++---- drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 5 ----- drivers/net/ethernet/realtek/r8169_main.c | 25 ---------------------- .../testing/selftests/net/forwarding/tc_flower.sh | 3 ++- 5 files changed, 11 insertions(+), 37 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 5cf7e5a367f0..1c0e4beb56e7 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2294,8 +2294,9 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, unsigned long flags; int ret; - /* - * If failover is pending don't schedule any other reset. + spin_lock_irqsave(&adapter->rwi_lock, flags); + + /* If failover is pending don't schedule any other reset. * Instead let the failover complete. If there is already a * a failover reset scheduled, we will detect and drop the * duplicate reset when walking the ->rwi_list below. diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index e4dcc63b9710..806aa75a4e86 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -1081,13 +1081,15 @@ struct ibmvnic_adapter { struct tasklet_struct tasklet; enum vnic_state state; - /* Used for serializatin of state field */ + /* Used for serialization of state field. When taking both state + * and rwi locks, take state lock first. + */ spinlock_t state_lock; enum ibmvnic_reset_reason reset_reason; - /* when taking both state and rwi locks, take state lock first */ - spinlock_t rwi_lock; struct list_head rwi_list; - /* Used for serialization of rwi_list */ + /* Used for serialization of rwi_list. When taking both state + * and rwi locks, take state lock first + */ spinlock_t rwi_lock; struct work_struct ibmvnic_reset; struct delayed_work ibmvnic_delayed_reset; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c index 39acbc83682d..ec2fcb2a2977 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c @@ -2123,12 +2123,7 @@ static void mlx5e_build_rq_frags_info(struct mlx5_core_dev *mdev, u32 buf_size = 0; int i; -<<<<<<< HEAD - if (MLX5_IPSEC_DEV(mdev)) -======= -#ifdef CONFIG_MLX5_EN_IPSEC if (mlx5_fpga_is_ipsec_device(mdev)) ->>>>>>> 3af409ca278d4a8d50e91f9f7c4c33b175645cf3 byte_count += MLX5E_METADATA_ETHER_LEN; if (mlx5e_rx_is_linear_skb(params, xsk)) { diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index 9ce98e3d3f9f..cbc30df4e08a 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -2230,31 +2230,6 @@ static void rtl_prepare_power_down(struct rtl8169_private *tp) phy_speed_down(tp->phydev, false); rtl_wol_enable_rx(tp); } - - switch (tp->mac_version) { - case RTL_GIGA_MAC_VER_25 ... RTL_GIGA_MAC_VER_26: - case RTL_GIGA_MAC_VER_29 ... RTL_GIGA_MAC_VER_30: - case RTL_GIGA_MAC_VER_32 ... RTL_GIGA_MAC_VER_33: - case RTL_GIGA_MAC_VER_37: - case RTL_GIGA_MAC_VER_39: - case RTL_GIGA_MAC_VER_43: - case RTL_GIGA_MAC_VER_44: - case RTL_GIGA_MAC_VER_45: - case RTL_GIGA_MAC_VER_46: - case RTL_GIGA_MAC_VER_47: - case RTL_GIGA_MAC_VER_48: - case RTL_GIGA_MAC_VER_50 ... RTL_GIGA_MAC_VER_63: - RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) & ~0x80); - break; - case RTL_GIGA_MAC_VER_40: - case RTL_GIGA_MAC_VER_41: - case RTL_GIGA_MAC_VER_49: - rtl_eri_clear_bits(tp, 0x1a8, 0xfc000000); - RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) & ~0x80); - break; - default: - break; - } } static void rtl_init_rxcfg(struct rtl8169_private *tp) diff --git a/tools/testing/selftests/net/forwarding/tc_flower.sh b/tools/testing/selftests/net/forwarding/tc_flower.sh index 4b58ccae3429..683711f41aa9 100755 --- a/tools/testing/selftests/net/forwarding/tc_flower.sh +++ b/tools/testing/selftests/net/forwarding/tc_flower.sh @@ -3,7 +3,8 @@ ALL_TESTS="match_dst_mac_test match_src_mac_test match_dst_ip_test \ match_src_ip_test match_ip_flags_test match_pcp_test match_vlan_test \ - match_ip_tos_test match_indev_testmatch_ip_ttl_test match_mpls_label_test \ + match_ip_tos_test match_indev_test match_ip_ttl_test + match_mpls_label_test \ match_mpls_tc_test match_mpls_bos_test match_mpls_ttl_test \ match_mpls_lse_test" NUM_NETIFS=2 -- cgit v1.2.3 From 8f1c0fd2c84c8bf738b7139d09d4ea53027f47c3 Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu Date: Tue, 23 Feb 2021 21:02:29 -0800 Subject: ibmvnic: fix a race between open and reset __ibmvnic_reset() currently reads the adapter->state before getting the rtnl and saves that state as the "target state" for the reset. If this read occurs when adapter is in PROBED state, the target state would be PROBED. Just after the target state is saved, and before the actual reset process is started (i.e before rtnl is acquired) if we get an ibmvnic_open() call we would move the adapter to OPEN state. But when the reset is processed (after ibmvnic_open()) drops the rtnl), it will leave the adapter in PROBED state even though we already moved it to OPEN. To fix this, use the RTNL to improve serialization when reading/updating the adapter state. i.e determine the target state of a reset only after getting the RTNL. And if a reset is in progress during an open, simply set the target state of the adapter and let the reset code finish the open (like we currently do if failover is pending). One twist to this serialization is if the adapter state changes when we drop the RTNL to update the link state. Account for this by checking if there was an intervening open and update the target state for the reset accordingly (see new comments in the code). Note that only the reset functions and ibmvnic_open() can set the adapter to OPEN state and this must happen under rtnl. Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during device reset") Signed-off-by: Sukadev Bhattiprolu Reviewed-by: Dany Madden Link: https://lore.kernel.org/r/20210224050229.1155468-1-sukadev@linux.ibm.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/ibm/ibmvnic.c | 63 +++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 8 deletions(-) (limited to 'drivers/net/ethernet/ibm/ibmvnic.c') diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 1c0e4beb56e7..118a4bd3f877 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1172,12 +1172,25 @@ static int ibmvnic_open(struct net_device *netdev) struct ibmvnic_adapter *adapter = netdev_priv(netdev); int rc; - /* If device failover is pending, just set device state and return. - * Device operation will be handled by reset routine. + ASSERT_RTNL(); + + /* If device failover is pending or we are about to reset, just set + * device state and return. Device operation will be handled by reset + * routine. + * + * It should be safe to overwrite the adapter->state here. Since + * we hold the rtnl, either the reset has not actually started or + * the rtnl got dropped during the set_link_state() in do_reset(). + * In the former case, no one else is changing the state (again we + * have the rtnl) and in the latter case, do_reset() will detect and + * honor our setting below. */ - if (adapter->failover_pending) { + if (adapter->failover_pending || (test_bit(0, &adapter->resetting))) { + netdev_dbg(netdev, "[S:%d FOP:%d] Resetting, deferring open\n", + adapter->state, adapter->failover_pending); adapter->state = VNIC_OPEN; - return 0; + rc = 0; + goto out; } if (adapter->state != VNIC_CLOSED) { @@ -1196,10 +1209,12 @@ static int ibmvnic_open(struct net_device *netdev) rc = __ibmvnic_open(netdev); out: - /* If open fails due to a pending failover, set device state and - * return. Device operation will be handled by reset routine. + /* If open failed and there is a pending failover or in-progress reset, + * set device state and return. Device operation will be handled by + * reset routine. See also comments above regarding rtnl. */ - if (rc && adapter->failover_pending) { + if (rc && + (adapter->failover_pending || (test_bit(0, &adapter->resetting)))) { adapter->state = VNIC_OPEN; rc = 0; } @@ -1928,6 +1943,14 @@ static int do_reset(struct ibmvnic_adapter *adapter, if (rwi->reset_reason == VNIC_RESET_FAILOVER) adapter->failover_pending = false; + /* read the state and check (again) after getting rtnl */ + reset_state = adapter->state; + + if (reset_state == VNIC_REMOVING || reset_state == VNIC_REMOVED) { + rc = -EBUSY; + goto out; + } + netif_carrier_off(netdev); old_num_rx_queues = adapter->req_rx_queues; @@ -1958,11 +1981,27 @@ static int do_reset(struct ibmvnic_adapter *adapter, if (rc) goto out; + if (adapter->state == VNIC_OPEN) { + /* When we dropped rtnl, ibmvnic_open() got + * it and noticed that we are resetting and + * set the adapter state to OPEN. Update our + * new "target" state, and resume the reset + * from VNIC_CLOSING state. + */ + netdev_dbg(netdev, + "Open changed state from %d, updating.\n", + reset_state); + reset_state = VNIC_OPEN; + adapter->state = VNIC_CLOSING; + } + if (adapter->state != VNIC_CLOSING) { + /* If someone else changed the adapter state + * when we dropped the rtnl, fail the reset + */ rc = -1; goto out; } - adapter->state = VNIC_CLOSED; } } @@ -2106,6 +2145,14 @@ static int do_hard_reset(struct ibmvnic_adapter *adapter, netdev_dbg(adapter->netdev, "Hard resetting driver (%d)\n", rwi->reset_reason); + /* read the state and check (again) after getting rtnl */ + reset_state = adapter->state; + + if (reset_state == VNIC_REMOVING || reset_state == VNIC_REMOVED) { + rc = -EBUSY; + goto out; + } + netif_carrier_off(netdev); adapter->reset_reason = rwi->reset_reason; -- cgit v1.2.3