__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: 7d7195a0 ("ibmvnic: Do not process device remove during device reset")
Signed-off-by: NSukadev Bhattiprolu <sukadev@linux.ibm.com>
Reviewed-by: NDany Madden <drt@linux.ibm.com>
Link: https://lore.kernel.org/r/20210224050229.1155468-1-sukadev@linux.ibm.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

Signed-off-by: NJakub Kicinski <kuba@kernel.org>

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: 6954a9e4 ("ibmvnic: Flush existing work items before device removal")
Signed-off-by: NSukadev Bhattiprolu <sukadev@linux.ibm.com>
Cc:Uwe Kleine-König <uwe@kleine-koenig.org>
Cc:Saeed Mahameed <saeed@kernel.org>
Reviewed-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

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: ed651a10 ("ibmvnic: Updated reset handling")
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

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: 032c5e82 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Acked-by: NThomas Falcon <tlfalcon@linux.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

The CRQ and subCRQ descriptors are DMA mapped, so dma_wmb(),
though weaker, is good enough to protect the data structures.
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Acked-by: NThomas Falcon <tlfalcon@linux.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

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: NLijun Pan <ljp@linux.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

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: 01d9bd79 ("ibmvnic: Reorganize device close")
Signed-off-by: NSukadev Bhattiprolu <sukadev@linux.ibm.com>
Reported-by: NAbdul Haleem <abdhalee@in.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

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: NLijun Pan <lijunp213@gmail.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

stats_lock is no longer used. So remove it.
Signed-off-by: NLijun Pan <lijunp213@gmail.com>
Reviewed-by: NSaeed Mahameed <saeedm@nvidia.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

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: NLijun Pan <lijunp213@gmail.com>
Reviewed-by: NSaeed Mahameed <saeedm@nvidia.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

Fix the following checkpatch warning:
WARNING: Avoid multiple line dereference
Signed-off-by: NLijun Pan <lijunp213@gmail.com>
Reviewed-by: NSaeed Mahameed <saeedm@nvidia.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

Fix the following checkpatch warning:
WARNING: braces {} are not necessary for single statement blocks
Signed-off-by: NLijun Pan <lijunp213@gmail.com>
Reviewed-by: NSaeed Mahameed <saeedm@nvidia.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

Fix the following checkpatch warning:
WARNING: networking block comments don't use an empty /* line, use /* Comment...
Signed-off-by: NLijun Pan <lijunp213@gmail.com>
Reviewed-by: NSaeed Mahameed <saeedm@nvidia.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

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: NLijun Pan <lijunp213@gmail.com>
Reviewed-by: NSaeed Mahameed <saeedm@nvidia.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

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: 1d850493 ("powerpc/vnic: Extend "failover pending" window")
Signed-off-by: NSukadev Bhattiprolu <sukadev@linux.ibm.com>
Tested-by: NCristobal Forno <cforno12@linux.ibm.com>
Link: https://lore.kernel.org/r/20210203050802.680772-1-sukadev@linux.ibm.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

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: NLijun Pan <ljp@linux.ibm.com>
Acked-by: NDwip Banerjee <dnbanerg@us.ibm.com>
Acked-by: NThomas Falcon <tlfalcon@linux.ibm.com>
Reviewed-by: NBrian King <brking@linux.vnet.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

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: NLijun Pan <ljp@linux.ibm.com>
Acked-by: NThomas Falcon <tlfalcon@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

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/#m48f5befd96bc9842ece2a3ad14f4c27747206a53Reported-by: NUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Fixes: 7d7195a0 ("ibmvnic: Do not process device remove during device reset")
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Link: https://lore.kernel.org/r/20210129043402.95744-1-ljp@linux.ibm.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

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: 032c5e82 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Link: https://lore.kernel.org/r/20210128013442.88319-1-ljp@linux.ibm.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

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: NLee Jones <lee.jones@linaro.org>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

Commit b27507bb ("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: NLijun Pan <ljp@linux.ibm.com>
Link: https://lore.kernel.org/r/20210106213514.76027-1-ljp@linux.ibm.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

The error is due to dereference a null pointer in function
reset_one_sub_crq_queue():

if (!scrq) {
    netdev_dbg(adapter->netdev,
               "Invalid scrq reset. irq (%d) or msgs(%p).\n",
		scrq->irq, scrq->msgs);
		return -EINVAL;
}

If the expression is true, scrq must be a null pointer and cannot
dereference.

Fixes: 9281cf2d ("ibmvnic: avoid memset null scrq msgs")
Signed-off-by: NYANG LI <abaci-bugfix@linux.alibaba.com>
Reported-by: NAbaci <abaci@linux.alibaba.com>
Acked-by: NLijun Pan <ljp@linux.ibm.com>
Link: https://lore.kernel.org/r/1609312994-121032-1-git-send-email-abaci-bugfix@linux.alibaba.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

Commit f9c6cea0 ("ibmvnic: Skip fatal error reset after passive init")
says "If the passive
CRQ initialization occurs before the FATAL reset task is processed,
the FATAL error reset task would try to access a CRQ message queue
that was freed, causing an oops. The problem may be most likely to
occur during DLPAR add vNIC with a non-default MTU, because the DLPAR
process will automatically issue a change MTU request.
Fix this by not processing fatal error reset if CRQ is passively
initialized after client-driven CRQ initialization fails."

The original commit skips a specific reset condition, but that does
not fix the problem it claims to fix, and misses a reset condition.
The effective fix is commit 0e435bef ("ibmvnic: fix NULL pointer
dereference in ibmvic_reset_crq") and commit a0faaa27 ("ibmvnic:
fix NULL pointer dereference in reset_sub_crq_queues"). With above
two fixes, there are no more crashes seen as described even without
the original commit, so I would like to revert the original commit.

Fixes: f9c6cea0 ("ibmvnic: Skip fatal error reset after passive init")
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Link: https://lore.kernel.org/r/20201223204904.12677-1-ljp@linux.ibm.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

Commit 34f0f4e3 ("ibmvnic: Fix login buffer memory leaks") frees
login_rsp_buffer in release_resources() and send_login()
because handle_login_rsp() does not free it.
Commit f3ae59c0 ("ibmvnic: store RX and TX subCRQ handle array in
ibmvnic_adapter struct") frees login_rsp_buffer in handle_login_rsp().
It seems unnecessary to free it in release_resources() and send_login().
There are chances that handle_login_rsp returns earlier without freeing
buffers. Double-checking the buffer is harmless since
release_login_buffer and release_login_rsp_buffer will
do nothing if buffer is already freed.

Fixes: f3ae59c0 ("ibmvnic: store RX and TX subCRQ handle array in ibmvnic_adapter struct")
Fixes: 34f0f4e3 ("ibmvnic: Fix login buffer memory leaks")
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Link: https://lore.kernel.org/r/20201219213919.21045-1-ljp@linux.ibm.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

Start to use the lockless version of netdev_notify_peers.
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

We observed that in the error case for batched send_subcrq_indirect() the
driver does not account for the partial success case. This caused Linux to
crash when free_map and pool index are inconsistent.

Driver needs to update the rx pools "available" count when some batched
sends worked but an error was encountered as part of the whole operation.
Also track replenish_add_buff_failure for statistic purposes.

Fixes: 4f0b6812 ("ibmvnic: Introduce batched RX buffer descriptor transmission")
Signed-off-by: NDwip N. Banerjee <dnbanerg@us.ibm.com>
Reviewed-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

We sometimes run into situations where a soft/hard reset of the adapter
takes a long time or fails to complete. Having additional messages that
include important adapter state info will hopefully help understand what
is happening, reduce the guess work and minimize requests to reproduce
problems with debug patches.
Signed-off-by: NSukadev Bhattiprolu <sukadev@linux.ibm.com>
Link: https://lore.kernel.org/r/20201205022235.2414110-1-sukadev@linux.ibm.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>

TX completions received with an error return code are not
being processed properly. When an error code is seen, do not
proceed to the next completion before cleaning up the existing
entry's data structures.

Fixes: 032c5e82 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: NThomas Falcon <tlfalcon@linux.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

Ensure that received Subordinate Command-Response Queue (SCRQ)
entries are properly read in order by the driver. These queues
are used in the ibmvnic device to process RX buffer and TX completion
descriptors. dma_rmb barriers have been added after checking for a
pending descriptor to ensure the correct descriptor entry is checked
and after reading the SCRQ descriptor to ensure the entire
descriptor is read before processing.

Fixes: 032c5e82 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: NThomas Falcon <tlfalcon@linux.ibm.com>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

Reduce the wait time for Command Response Queue response from 30 seconds
to 20 seconds, as recommended by VIOS and Power Hypervisor teams.

Fixes: bd0b6723 ("ibmvnic: Move login and queue negotiation into ibmvnic_open")
Fixes: 53da09e9 ("ibmvnic: Add set_link_state routine for setting adapter link state")
Signed-off-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

Reset timeout is going off right after adapter reset. This patch ensures
that timeout is scheduled if it has been 5 seconds since the last reset.
5 seconds is the default watchdog timeout.

Fixes: ed651a10 ("ibmvnic: Updated reset handling")
Signed-off-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

send_login() does not check for the result of ibmvnic_send_crq() of the
login request. This results in the driver needlessly retrying the login
10 times even when CRQ is no longer active. Check the return code and
give up in case of errors in sending the CRQ.

The only time we want to retry is if we get a PARITALSUCCESS response
from the partner.

Fixes: 032c5e82 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NSukadev Bhattiprolu <sukadev@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

If after ibmvnic sends a LOGIN it gets a FAILOVER, it is possible that
the worker thread will start reset process and free the login response
buffer before it gets a (now stale) LOGIN_RSP. The ibmvnic tasklet will
then try to access the login response buffer and crash.

Have ibmvnic track pending logins and discard any stale login responses.

Fixes: 032c5e82 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: NSukadev Bhattiprolu <sukadev@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

If auto-priority failover is enabled, the backing device needs time
to settle if hard resetting fails for any reason. Add a delay of 60
seconds before retrying the hard-reset.

Fixes: 2770a798 ("ibmvnic: Introduce hard reset recovery")
Signed-off-by: NSukadev Bhattiprolu <sukadev@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

In a failed reset, driver could end up in VNIC_PROBED or VNIC_CLOSED
state and cannot recover in subsequent resets, leaving it offline.
This patch restores the adapter state to reset_state, the original
state when reset was called.

Fixes: b27507bb ("net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run")
Fixes: 2770a798 ("ibmvnic: Introduce hard reset recovery")
Signed-off-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

scrq->msgs could be NULL during device reset, causing Linux to crash.
So, check before memset scrq->msgs.

Fixes: c8b2ad0a ("ibmvnic: Sanitize entire SCRQ buffer on reset")
Signed-off-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

When ibmvnic fails to reset, it breaks out of the reset loop and frees
all of the remaining resets from the workqueue. Doing so prevents the
adapter from recovering if no reset is scheduled after that. Instead,
have the driver continue to process resets on the workqueue.

Remove the no longer need free_all_rwi().

Fixes: ed651a10 ("ibmvnic: Updated reset handling")
Signed-off-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

Inconsistent login with the vnicserver is causing the device to be
removed. This does not give the device a chance to recover from error
state. This patch schedules a FATAL reset instead to bring the adapter
up.

Fixes: 032c5e82 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: NDany Madden <drt@linux.ibm.com>
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>

Based on the discussion with Sukadev Bhattiprolu and Dany Madden,
we believe that checking adapter->resetting bit is preferred
since RESETTING state flag is not as strict as resetting bit.
RESETTING state flag is removed since it is verbose now.

Fixes: 7d7195a0 ("ibmvnic: Do not process device remove during device reset")
Signed-off-by: NLijun Pan <ljp@linux.ibm.com>
Signed-off-by: NJakub Kicinski <kuba@kernel.org>