| Message ID | 20210203050650.680656-2-sukadev@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,1/2] ibmvnic: fix a race between open and reset | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 7 maintainers not CCed: benh@kernel.crashing.org mpe@ellerman.id.au paulus@samba.org nfont@linux.vnet.ibm.com linuxppc-dev@lists.ozlabs.org davem@davemloft.net kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 3 this patch: 3 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 26 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Wed, Feb 3, 2021 at 12:10 AM Sukadev Bhattiprolu <sukadev@linux.ibm.com> wrote: > > If two or more instances of 'ip link set' commands race and first one > already brings the interface up (or down), the subsequent instances > can simply return without redoing the up/down operation. > > Fixes: ed651a10875f ("ibmvnic: Updated reset handling") > Reported-by: Abdul Haleem <abdhalee@in.ibm.com> > Tested-by: Abdul Haleem <abdhalee@in.ibm.com> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com> > > --- > Changelog[v2] For consistency with ibmvnic_open() use "goto out" and return > from end of function. Did you find the code path that triggers this? In v1 we discussed how the usual ip link path should not call the driver twice based on IFF_UP.
Willem de Bruijn [willemdebruijn.kernel@gmail.com] wrote: > On Wed, Feb 3, 2021 at 12:10 AM Sukadev Bhattiprolu > <sukadev@linux.ibm.com> wrote: > > > > If two or more instances of 'ip link set' commands race and first one > > already brings the interface up (or down), the subsequent instances > > can simply return without redoing the up/down operation. > > > > Fixes: ed651a10875f ("ibmvnic: Updated reset handling") > > Reported-by: Abdul Haleem <abdhalee@in.ibm.com> > > Tested-by: Abdul Haleem <abdhalee@in.ibm.com> > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com> > > > > --- > > Changelog[v2] For consistency with ibmvnic_open() use "goto out" and return > > from end of function. > > Did you find the code path that triggers this? Not yet. I need to find time on the system to repro/debug that > > In v1 we discussed how the usual ip link path should not call the > driver twice based on IFF_UP. Yes, we can hold this patch for now if necessary. Hopefully Patch 1/2 is ok. Sukadev
On Wed, 3 Feb 2021 12:56:52 -0800 Sukadev Bhattiprolu wrote: > Willem de Bruijn [willemdebruijn.kernel@gmail.com] wrote: > > On Wed, Feb 3, 2021 at 12:10 AM Sukadev Bhattiprolu > > <sukadev@linux.ibm.com> wrote: > > > > > > If two or more instances of 'ip link set' commands race and first one > > > already brings the interface up (or down), the subsequent instances > > > can simply return without redoing the up/down operation. > > > > > > Fixes: ed651a10875f ("ibmvnic: Updated reset handling") > > > Reported-by: Abdul Haleem <abdhalee@in.ibm.com> > > > Tested-by: Abdul Haleem <abdhalee@in.ibm.com> > > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com> > > > > > > --- > > > Changelog[v2] For consistency with ibmvnic_open() use "goto out" and return > > > from end of function. > > > > Did you find the code path that triggers this? > > Not yet. I need to find time on the system to repro/debug that > > > > In v1 we discussed how the usual ip link path should not call the > > driver twice based on IFF_UP. > > Yes, we can hold this patch for now if necessary. Hopefully Patch 1/2 is > ok. Patch 1 does not apply to net as is. Please rebase and fix up the comments the way I fixed them for "ibmvnic: Clear failover_pending if unable to schedule" Thanks!
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 78d244aeee69..df1b4884b4e8 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1219,6 +1219,14 @@ static int ibmvnic_open(struct net_device *netdev) goto out; } + /* If adapter is already open, we don't have to do anything. */ + if (adapter->state == VNIC_OPEN) { + netdev_dbg(netdev, "[S:%d] adapter already open\n", + adapter->state); + rc = 0; + goto out; + } + if (adapter->state != VNIC_CLOSED) { rc = ibmvnic_login(netdev); if (rc) @@ -1392,6 +1400,12 @@ static int ibmvnic_close(struct net_device *netdev) return 0; } + /* If adapter is already closed, we don't have to do anything. */ + if (adapter->state == VNIC_CLOSED) { + netdev_dbg(netdev, "[S:%d] adapter already closed\n", + adapter->state); + return 0; + } rc = __ibmvnic_close(netdev); ibmvnic_cleanup(netdev);
If two or more instances of 'ip link set' commands race and first one already brings the interface up (or down), the subsequent instances can simply return without redoing the up/down operation. Fixes: ed651a10875f ("ibmvnic: Updated reset handling") Reported-by: Abdul Haleem <abdhalee@in.ibm.com> Tested-by: Abdul Haleem <abdhalee@in.ibm.com> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com> --- Changelog[v2] For consistency with ibmvnic_open() use "goto out" and return from end of function. --- drivers/net/ethernet/ibm/ibmvnic.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)