Skip to content

Commit 6db541a

Browse files
Nick Childkuba-moo
authored andcommitted
ibmvnic: Ensure login failure recovery is safe from other resets
If a login request fails, the recovery process should be protected against parallel resets. It is a known issue that freeing and registering CRQ's in quick succession can result in a failover CRQ from the VIOS. Processing a failover during login recovery is dangerous for two reasons: 1. This will result in two parallel initialization processes, this can cause serious issues during login. 2. It is possible that the failover CRQ is received but never executed. We get notified of a pending failover through a transport event CRQ. The reset is not performed until a INIT CRQ request is received. Previously, if CRQ init fails during login recovery, then the ibmvnic irq is freed and the login process returned error. If failover_pending is true (a transport event was received), then the ibmvnic device would never be able to process the reset since it cannot receive the CRQ_INIT request due to the irq being freed. This leaved the device in a inoperable state. Therefore, the login failure recovery process must be hardened against these possible issues. Possible failovers (due to quick CRQ free and init) must be avoided and any issues during re-initialization should be dealt with instead of being propagated up the stack. This logic is similar to that of ibmvnic_probe(). Fixes: dff515a ("ibmvnic: Harden device login requests") Signed-off-by: Nick Child <[email protected]> Reviewed-by: Simon Horman <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 23cc5f6 commit 6db541a

File tree

1 file changed

+47
-21
lines changed

1 file changed

+47
-21
lines changed

drivers/net/ethernet/ibm/ibmvnic.c

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ static void ibmvnic_tx_scrq_clean_buffer(struct ibmvnic_adapter *adapter,
116116
static void free_long_term_buff(struct ibmvnic_adapter *adapter,
117117
struct ibmvnic_long_term_buff *ltb);
118118
static void ibmvnic_disable_irqs(struct ibmvnic_adapter *adapter);
119+
static void flush_reset_queue(struct ibmvnic_adapter *adapter);
119120

120121
struct ibmvnic_stat {
121122
char name[ETH_GSTRING_LEN];
@@ -1507,8 +1508,8 @@ static const char *adapter_state_to_string(enum vnic_state state)
15071508

15081509
static int ibmvnic_login(struct net_device *netdev)
15091510
{
1511+
unsigned long flags, timeout = msecs_to_jiffies(20000);
15101512
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
1511-
unsigned long timeout = msecs_to_jiffies(20000);
15121513
int retry_count = 0;
15131514
int retries = 10;
15141515
bool retry;
@@ -1573,6 +1574,7 @@ static int ibmvnic_login(struct net_device *netdev)
15731574
"SCRQ irq initialization failed\n");
15741575
return rc;
15751576
}
1577+
/* Default/timeout error handling, reset and start fresh */
15761578
} else if (adapter->init_done_rc) {
15771579
netdev_warn(netdev, "Adapter login failed, init_done_rc = %d\n",
15781580
adapter->init_done_rc);
@@ -1588,29 +1590,53 @@ static int ibmvnic_login(struct net_device *netdev)
15881590
"Freeing and re-registering CRQs before attempting to login again\n");
15891591
retry = true;
15901592
adapter->init_done_rc = 0;
1591-
retry_count++;
15921593
release_sub_crqs(adapter, true);
1593-
reinit_init_done(adapter);
1594-
release_crq_queue(adapter);
1595-
/* If we don't sleep here then we risk an unnecessary
1596-
* failover event from the VIOS. This is a known VIOS
1597-
* issue caused by a vnic device freeing and registering
1598-
* a CRQ too quickly.
1594+
/* Much of this is similar logic as ibmvnic_probe(),
1595+
* we are essentially re-initializing communication
1596+
* with the server. We really should not run any
1597+
* resets/failovers here because this is already a form
1598+
* of reset and we do not want parallel resets occurring
15991599
*/
1600-
msleep(1500);
1601-
rc = init_crq_queue(adapter);
1602-
if (rc) {
1603-
netdev_err(netdev, "login recovery: init CRQ failed %d\n",
1604-
rc);
1605-
return -EIO;
1606-
}
1600+
do {
1601+
reinit_init_done(adapter);
1602+
/* Clear any failovers we got in the previous
1603+
* pass since we are re-initializing the CRQ
1604+
*/
1605+
adapter->failover_pending = false;
1606+
release_crq_queue(adapter);
1607+
/* If we don't sleep here then we risk an
1608+
* unnecessary failover event from the VIOS.
1609+
* This is a known VIOS issue caused by a vnic
1610+
* device freeing and registering a CRQ too
1611+
* quickly.
1612+
*/
1613+
msleep(1500);
1614+
/* Avoid any resets, since we are currently
1615+
* resetting.
1616+
*/
1617+
spin_lock_irqsave(&adapter->rwi_lock, flags);
1618+
flush_reset_queue(adapter);
1619+
spin_unlock_irqrestore(&adapter->rwi_lock,
1620+
flags);
1621+
1622+
rc = init_crq_queue(adapter);
1623+
if (rc) {
1624+
netdev_err(netdev, "login recovery: init CRQ failed %d\n",
1625+
rc);
1626+
return -EIO;
1627+
}
16071628

1608-
rc = ibmvnic_reset_init(adapter, false);
1609-
if (rc) {
1610-
netdev_err(netdev, "login recovery: Reset init failed %d\n",
1611-
rc);
1612-
return -EIO;
1613-
}
1629+
rc = ibmvnic_reset_init(adapter, false);
1630+
if (rc)
1631+
netdev_err(netdev, "login recovery: Reset init failed %d\n",
1632+
rc);
1633+
/* IBMVNIC_CRQ_INIT will return EAGAIN if it
1634+
* fails, since ibmvnic_reset_init will free
1635+
* irq's in failure, we won't be able to receive
1636+
* new CRQs so we need to keep trying. probe()
1637+
* handles this similarly.
1638+
*/
1639+
} while (rc == -EAGAIN && retry_count++ < retries);
16141640
}
16151641
} while (retry);
16161642

0 commit comments

Comments
 (0)