From: Dexuan Cui <decui@microsoft.com>
To: Haiyang Zhang <haiyangz@microsoft.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"kuba@kernel.org" <kuba@kernel.org>,
"gustavoars@kernel.org" <gustavoars@kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: KY Srinivasan <kys@microsoft.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"wei.liu@kernel.org" <wei.liu@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
Shachar Raindel <shacharr@microsoft.com>,
Paul Rosswurm <paulros@microsoft.com>,
"olaf@aepfle.de" <olaf@aepfle.de>, vkuznets <vkuznets@redhat.com>
Subject: RE: [PATCH net-next 4/4] net: mana: Support hibernation and kexec
Date: Mon, 1 Nov 2021 07:03:19 +0000 [thread overview]
Message-ID: <BYAPR21MB1270A16AA0BF1A302BABCB3FBF8A9@BYAPR21MB1270.namprd21.prod.outlook.com> (raw)
In-Reply-To: <BN8PR21MB1284785C320EFE09C75286B6CA889@BN8PR21MB1284.namprd21.prod.outlook.com>
> From: Haiyang Zhang <haiyangz@microsoft.com>
> Sent: Saturday, October 30, 2021 8:55 AM
> >
> > @@ -1844,44 +1845,70 @@ int mana_probe(struct gdma_dev *gd)
> > if (err)
> > return err;
> >
> > - ac = kzalloc(sizeof(*ac), GFP_KERNEL);
> > - if (!ac)
> > - return -ENOMEM;
> > + if (!resuming) {
> > + ac = kzalloc(sizeof(*ac), GFP_KERNEL);
> > + if (!ac)
> > + return -ENOMEM;
> >
> > - ac->gdma_dev = gd;
> > - ac->num_ports = 1;
> > - gd->driver_data = ac;
> > + ac->gdma_dev = gd;
> > + gd->driver_data = ac;
> > + }
> >
> > err = mana_create_eq(ac);
> > if (err)
> > goto out;
> >
> > err = mana_query_device_cfg(ac, MANA_MAJOR_VERSION,
> > MANA_MINOR_VERSION,
> > - MANA_MICRO_VERSION, &ac->num_ports);
> > + MANA_MICRO_VERSION, &num_ports);
> > if (err)
> > goto out;
> >
> > + if (!resuming) {
> > + ac->num_ports = num_ports;
> > + } else {
> > + if (ac->num_ports != num_ports) {
> > + dev_err(dev, "The number of vPorts changed: %d->%d\n",
> > + ac->num_ports, num_ports);
> > + err = -EPROTO;
> > + goto out;
> > + }
> > + }
> > +
> > + if (ac->num_ports == 0)
> > + dev_err(dev, "Failed to detect any vPort\n");
> > +
> > if (ac->num_ports > MAX_PORTS_IN_MANA_DEV)
> > ac->num_ports = MAX_PORTS_IN_MANA_DEV;
> >
> > - for (i = 0; i < ac->num_ports; i++) {
> > - err = mana_probe_port(ac, i, &ac->ports[i]);
> > - if (err)
> > - break;
> > + if (!resuming) {
> > + for (i = 0; i < ac->num_ports; i++) {
> > + err = mana_probe_port(ac, i, &ac->ports[i]);
> > + if (err)
> > + break;
> > + }
> > + } else {
> > + for (i = 0; i < ac->num_ports; i++) {
> > + rtnl_lock();
> > + err = mana_attach(ac->ports[i]);
> > + rtnl_unlock();
> > + if (err)
> > + break;
> > + }
> > }
> > out:
> > if (err)
> > - mana_remove(gd);
> > + mana_remove(gd, false);
>
> The "goto out" can happen in both resuming true/false cases,
> should the error handling path deal with the two cases
> differently?
Let me make the below change in v2. Please let me know
if any further change is needed:
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -1850,8 +1850,10 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
if (!resuming) {
ac = kzalloc(sizeof(*ac), GFP_KERNEL);
- if (!ac)
- return -ENOMEM;
+ if (!ac) {
+ err = -ENOMEM;
+ goto out;
+ }
ac->gdma_dev = gd;
gd->driver_data = ac;
@@ -1872,8 +1874,8 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
if (ac->num_ports != num_ports) {
dev_err(dev, "The number of vPorts changed: %d->%d\n",
ac->num_ports, num_ports);
- err = -EPROTO;
- goto out;
+ /* It's unsafe to proceed. */
+ return -EPROTO;
}
}
@@ -1886,22 +1888,36 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
if (!resuming) {
for (i = 0; i < ac->num_ports; i++) {
err = mana_probe_port(ac, i, &ac->ports[i]);
- if (err)
- break;
+ if (err) {
+ dev_err(dev, "Failed to probe vPort %u: %d\n",
+ i, err);
+ goto out;
+ }
}
} else {
for (i = 0; i < ac->num_ports; i++) {
rtnl_lock();
err = mana_attach(ac->ports[i]);
rtnl_unlock();
- if (err)
- break;
+
+ if (err) {
+ netdev_err(ac->ports[i],
+ "Failed to resume vPort %u: %d\n",
+ i, err);
+ return err;
+ }
}
}
+
+ return 0;
out:
- if (err)
- mana_remove(gd, false);
+ /* In the resuming path, it's safer to leave the device in the failed
+ * state than try to invoke mana_detach().
+ */
+ if (resuming)
+ return err;
+ mana_remove(gd, false);
return err;
}
@@ -1919,7 +1935,7 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
if (!ndev) {
if (i == 0)
dev_err(dev, "No net device to remove\n");
- goto out;
+ break;
}
/* All cleanup actions should stay after rtnl_lock(), otherwise
For your easy reviewing, the new code of the function in v2 will be:
int mana_probe(struct gdma_dev *gd, bool resuming)
{
struct gdma_context *gc = gd->gdma_context;
struct mana_context *ac = gd->driver_data;
struct device *dev = gc->dev;
u16 num_ports = 0;
int err;
int i;
dev_info(dev,
"Microsoft Azure Network Adapter protocol version: %d.%d.%d\n",
MANA_MAJOR_VERSION, MANA_MINOR_VERSION, MANA_MICRO_VERSION);
err = mana_gd_register_device(gd);
if (err)
return err;
if (!resuming) {
ac = kzalloc(sizeof(*ac), GFP_KERNEL);
if (!ac) {
err = -ENOMEM;
goto out;
}
ac->gdma_dev = gd;
gd->driver_data = ac;
}
err = mana_create_eq(ac);
if (err)
goto out;
err = mana_query_device_cfg(ac, MANA_MAJOR_VERSION, MANA_MINOR_VERSION,
MANA_MICRO_VERSION, &num_ports);
if (err)
goto out;
if (!resuming) {
ac->num_ports = num_ports;
} else {
if (ac->num_ports != num_ports) {
dev_err(dev, "The number of vPorts changed: %d->%d\n",
ac->num_ports, num_ports);
/* It's unsafe to proceed. */
return -EPROTO;
}
}
if (ac->num_ports == 0)
dev_err(dev, "Failed to detect any vPort\n");
if (ac->num_ports > MAX_PORTS_IN_MANA_DEV)
ac->num_ports = MAX_PORTS_IN_MANA_DEV;
if (!resuming) {
for (i = 0; i < ac->num_ports; i++) {
err = mana_probe_port(ac, i, &ac->ports[i]);
if (err) {
dev_err(dev, "Failed to probe vPort %u: %d\n",
i, err);
goto out;
}
}
} else {
for (i = 0; i < ac->num_ports; i++) {
rtnl_lock();
err = mana_attach(ac->ports[i]);
rtnl_unlock();
if (err) {
netdev_err(ac->ports[i],
"Failed to resume vPort %u: %d\n",
i, err);
return err;
}
}
}
return 0;
out:
/* In the resuming path, it's safer to leave the device in the failed
* state than try to invoke mana_detach().
*/
if (resuming)
return err;
mana_remove(gd, false);
return err;
}
next prev parent reply other threads:[~2021-11-01 7:03 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-30 0:54 [PATCH net-next 0/4] net: mana: some misc patches Dexuan Cui
2021-10-30 0:54 ` [PATCH net-next 1/4] net: mana: Fix the netdev_err()'s vPort argument in mana_init_port() Dexuan Cui
2021-10-30 15:30 ` Haiyang Zhang
2021-10-30 20:07 ` Stephen Hemminger
2021-11-01 5:59 ` Dexuan Cui
2021-10-30 0:54 ` [PATCH net-next 2/4] net: mana: Report OS info to the PF driver Dexuan Cui
2021-10-30 15:35 ` Haiyang Zhang
2021-11-01 6:07 ` Dexuan Cui
2021-10-30 0:54 ` [PATCH net-next 3/4] net: mana: Improve the HWC error handling Dexuan Cui
2021-10-30 15:36 ` Haiyang Zhang
2021-10-30 0:54 ` [PATCH net-next 4/4] net: mana: Support hibernation and kexec Dexuan Cui
2021-10-30 15:54 ` Haiyang Zhang
2021-11-01 7:03 ` Dexuan Cui [this message]
2021-11-01 15:12 ` Haiyang Zhang
2021-11-01 16:41 ` Dexuan Cui
2021-11-01 13:30 ` [PATCH net-next 0/4] net: mana: some misc patches patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BYAPR21MB1270A16AA0BF1A302BABCB3FBF8A9@BYAPR21MB1270.namprd21.prod.outlook.com \
--to=decui@microsoft.com \
--cc=davem@davemloft.net \
--cc=gustavoars@kernel.org \
--cc=haiyangz@microsoft.com \
--cc=kuba@kernel.org \
--cc=kys@microsoft.com \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olaf@aepfle.de \
--cc=paulros@microsoft.com \
--cc=shacharr@microsoft.com \
--cc=stephen@networkplumber.org \
--cc=vkuznets@redhat.com \
--cc=wei.liu@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).