| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 30 Oct 2020 11:56:48 -0700
From: drt <drt@...ux.vnet.ibm.com>
To: Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
Cc: netdev@...r.kernel.org, Lijun Pan <ljp@...ux.ibm.com>,
Brian King <brking@...ux.vnet.ibm.com>
Subject: Re: [PATCH v3 1/1] powerpc/vnic: Extend "failover pending" window
On 2020-10-30 10:07, Sukadev Bhattiprolu wrote:
> Commit 5a18e1e0c193b introduced the 'failover_pending' state to track
> the "failover pending window" - where we wait for the partner to become
> ready (after a transport event) before actually attempting to failover.
> i.e window is between following two events:
>
> a. we get a transport event due to a FAILOVER
>
> b. later, we get CRQ_INITIALIZED indicating the partner is
> ready at which point we schedule a FAILOVER reset.
>
> and ->failover_pending is true during this window.
>
> If during this window, we attempt to open (or close) a device, we
> pretend
> that the operation succeded and let the FAILOVER reset path complete
> the
> operation.
>
> This is fine, except if the transport event ("a" above) occurs during
> the
> open and after open has already checked whether a failover is pending.
> If
> that happens, we fail the open, which can cause the boot scripts to
> leave
> the interface down requiring administrator to manually bring up the
> device.
>
> This fix "extends" the failover pending window till we are _actually_
> ready to perform the failover reset (i.e until after we get the RTNL
> lock). Since open() holds the RTNL lock, we can be sure that we either
> finish the open or if the open() fails due to the failover pending
> window,
> we can again pretend that open is done and let the failover complete
> it.
>
> We could try and block the open until failover is completed but a) that
> could still timeout the application and b) Existing code "pretends"
> that
> failover occurred "just after" open succeeded, so marks the open
> successful
> and lets the failover complete the open. So, mark the open successful
> even
> if the transport event occurs before we actually start the open.
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
> ---
> Changelog [v3]:
> [Lijun Pan]: Add a few more notes to patch description.
>
> Changelog [v2]:
> [Brian King] Ensure we clear failover_pending during hard reset
> ---
Thanks, Suka!
Acked-by: Dany Madden <drt@...ux.ibm.com>
> drivers/net/ethernet/ibm/ibmvnic.c | 36 ++++++++++++++++++++++++++----
> 1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 1b702a43a5d0..2a0f6f6820db 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1197,18 +1197,27 @@ static int ibmvnic_open(struct net_device
> *netdev)
> if (adapter->state != VNIC_CLOSED) {
> rc = ibmvnic_login(netdev);
> if (rc)
> - return rc;
> + goto out;
>
> rc = init_resources(adapter);
> if (rc) {
> netdev_err(netdev, "failed to initialize resources\n");
> release_resources(adapter);
> - return rc;
> + goto out;
> }
> }
>
> 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 (rc && adapter->failover_pending) {
> + adapter->state = VNIC_OPEN;
> + rc = 0;
> + }
> return rc;
> }
>
> @@ -1931,6 +1940,13 @@ static int do_reset(struct ibmvnic_adapter
> *adapter,
> rwi->reset_reason);
>
> rtnl_lock();
> + /*
> + * 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.
> + */
> + if (rwi->reset_reason == VNIC_RESET_FAILOVER)
> + adapter->failover_pending = false;
>
> netif_carrier_off(netdev);
> adapter->reset_reason = rwi->reset_reason;
> @@ -2211,6 +2227,13 @@ static void __ibmvnic_reset(struct work_struct
> *work)
> /* CHANGE_PARAM requestor holds rtnl_lock */
> rc = do_change_param_reset(adapter, rwi, reset_state);
> } else if (adapter->force_reset_recovery) {
> + /*
> + * Since we are doing a hard reset now, clear the
> + * failover_pending flag so we don't ignore any
> + * future MOBILITY or other resets.
> + */
> + adapter->failover_pending = false;
> +
> /* Transport event occurred during previous reset */
> if (adapter->wait_for_reset) {
> /* Previous was CHANGE_PARAM; caller locked */
> @@ -2275,9 +2298,15 @@ static int ibmvnic_reset(struct ibmvnic_adapter
> *adapter,
> unsigned long flags;
> int ret;
>
> + /*
> + * 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.
> + */
> if (adapter->state == VNIC_REMOVING ||
> adapter->state == VNIC_REMOVED ||
> - adapter->failover_pending) {
> + (adapter->failover_pending && reason != VNIC_RESET_FAILOVER)) {
> ret = EBUSY;
> netdev_dbg(netdev, "Adapter removing or pending failover, skipping
> reset\n");
> goto err;
> @@ -4653,7 +4682,6 @@ static void ibmvnic_handle_crq(union ibmvnic_crq
> *crq,
> case IBMVNIC_CRQ_INIT:
> dev_info(dev, "Partner initialized\n");
> adapter->from_passive_init = true;
> - adapter->failover_pending = false;
> if (!completion_done(&adapter->init_done)) {
> complete(&adapter->init_done);
> adapter->init_done_rc = -EIO;
Powered by blists - more mailing lists