| Message ID | 20210129034711.518250-2-sukadev@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,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 | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| 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: 19 this patch: 19 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 25 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 Thu, Jan 28, 2021 at 10:51 PM 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> Isn't this handled in the rtnetlink core based on IFF_UP? if ((old_flags ^ flags) & IFF_UP) { if (old_flags & IFF_UP) __dev_close(dev); else ret = __dev_open(dev, extack); }
Willem de Bruijn [willemdebruijn.kernel@gmail.com] wrote: > On Thu, Jan 28, 2021 at 10:51 PM 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> > > Isn't this handled in the rtnetlink core based on IFF_UP? > > if ((old_flags ^ flags) & IFF_UP) { > if (old_flags & IFF_UP) > __dev_close(dev); > else > ret = __dev_open(dev, extack); > } Good question. During our testing we hit the "adapter already open" debug message a few times. Without this change, the core layer and dirver disagree on the state and the adapter becomes unsuable. I will debug this at the core layer also later this week. We are working on rewriting parts of driver surrounding locking/adapter state and not sure if that will reveal any other cause for this behavior. Sukadev
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index cb7ddfefb03e..84b772921f35 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1219,6 +1219,13 @@ 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); + return 0; + } + if (adapter->state != VNIC_CLOSED) { rc = ibmvnic_login(netdev); if (rc) @@ -1392,6 +1399,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> --- drivers/net/ethernet/ibm/ibmvnic.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)