[PATCH v2 net 0/3] ionic: minor code fixes

[PATCH v2 net 0/3] ionic: minor code fixes

[Shannon Nelson]

These are a couple of code fixes for the ionic driver.

Brett Creeley (1):
  ionic: Fix netdev notifier unregister on failure

Shannon Nelson (2):
  ionic: no double destroy workqueue
  ionic: use ee->offset when returning sprom data

v2: dropped nb_work removal from first patch

v1: https://lore.kernel.org/netdev/20241210174828.69525-1-shannon.nelson@amd.com/

 drivers/net/ethernet/pensando/ionic/ionic_dev.c     | 5 ++++-
 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 4 ++--
 drivers/net/ethernet/pensando/ionic/ionic_lif.c     | 4 ++--
 3 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH v2 net 1/3] ionic: Fix netdev notifier unregister on failure

[Shannon Nelson]

From: Brett Creeley <brett.creeley@amd.com>

If register_netdev() fails, then the driver leaks the netdev notifier.
Fix this by calling ionic_lif_unregister() on register_netdev()
failure. This will also call ionic_lif_unregister_phc() if it has
already been registered.

Fixes: 30b87ab4c0b3 ("ionic: remove lif list concept")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 40496587b2b3..3d3f936779f7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -3869,8 +3869,8 @@ int ionic_lif_register(struct ionic_lif *lif)
 	/* only register LIF0 for now */
 	err = register_netdev(lif->netdev);
 	if (err) {
-		dev_err(lif->ionic->dev, "Cannot register net device, aborting\n");
-		ionic_lif_unregister_phc(lif);
+		dev_err(lif->ionic->dev, "Cannot register net device: %d, aborting\n", err);
+		ionic_lif_unregister(lif);
 		return err;
 	}
 
-- 
2.17.1

[PATCH v2 net 2/3] ionic: no double destroy workqueue

[Shannon Nelson]

There are some FW error handling paths that can cause us to
try to destroy the workqueue more than once, so let's be sure
we're checking for that.

The case where this popped up was in an AER event where the
handlers got called in such a way that ionic_reset_prepare()
and thus ionic_dev_teardown() got called twice in a row.
The second time through the workqueue was already destroyed,
and destroy_workqueue() choked on the bad wq pointer.

We didn't hit this in AER handler testing before because at
that time we weren't using a private workqueue.  Later we
replaced the use of the system workqueue with our own private
workqueue but hadn't rerun the AER handler testing since then.

Fixes: 9e25450da700 ("ionic: add private workqueue per-device")
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_dev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index 9e42d599840d..57edcde9e6f8 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -277,7 +277,10 @@ void ionic_dev_teardown(struct ionic *ionic)
 	idev->phy_cmb_pages = 0;
 	idev->cmb_npages = 0;
 
-	destroy_workqueue(ionic->wq);
+	if (ionic->wq) {
+		destroy_workqueue(ionic->wq);
+		ionic->wq = NULL;
+	}
 	mutex_destroy(&idev->cmb_inuse_lock);
 }
 
-- 
2.17.1

[PATCH v2 net 3/3] ionic: use ee->offset when returning sprom data

[Shannon Nelson]

Some calls into ionic_get_module_eeprom() don't use a single
full buffer size, but instead multiple calls with an offset.
Teach our driver to use the offset correctly so we can
respond appropriately to the caller.

Fixes: 4d03e00a2140 ("ionic: Add initial ethtool support")
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index dda22fa4448c..9b7f78b6cdb1 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -961,8 +961,8 @@ static int ionic_get_module_eeprom(struct net_device *netdev,
 	len = min_t(u32, sizeof(xcvr->sprom), ee->len);
 
 	do {
-		memcpy(data, xcvr->sprom, len);
-		memcpy(tbuf, xcvr->sprom, len);
+		memcpy(data, &xcvr->sprom[ee->offset], len);
+		memcpy(tbuf, &xcvr->sprom[ee->offset], len);
 
 		/* Let's make sure we got a consistent copy */
 		if (!memcmp(data, tbuf, len))
-- 
2.17.1

Re: [PATCH v2 net 0/3] ionic: minor code fixes

[Jacob Keller]

On 12/12/2024 1:31 PM, Shannon Nelson wrote:
> These are a couple of code fixes for the ionic driver.
> 
> Brett Creeley (1):
>   ionic: Fix netdev notifier unregister on failure
> 
> Shannon Nelson (2):
>   ionic: no double destroy workqueue
>   ionic: use ee->offset when returning sprom data
> 
> v2: dropped nb_work removal from first patch
> 

Thanks for splitting that out. Everything still looks good to me.

Re: [PATCH v2 net 0/3] ionic: minor code fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 12 Dec 2024 13:31:54 -0800 you wrote:
> These are a couple of code fixes for the ionic driver.
> 
> Brett Creeley (1):
>   ionic: Fix netdev notifier unregister on failure
> 
> Shannon Nelson (2):
>   ionic: no double destroy workqueue
>   ionic: use ee->offset when returning sprom data
> 
> [...]

Here is the summary with links:
  - [v2,net,1/3] ionic: Fix netdev notifier unregister on failure
    https://git.kernel.org/netdev/net/c/9590d32e090e
  - [v2,net,2/3] ionic: no double destroy workqueue
    https://git.kernel.org/netdev/net/c/746e6ae2e202
  - [v2,net,3/3] ionic: use ee->offset when returning sprom data
    https://git.kernel.org/netdev/net/c/b096d62ba132

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html