diff mbox series

[net] nfp: don't depend on eth_tbl being available

Message ID 20180425182108.31363-1-jakub.kicinski@netronome.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series [net] nfp: don't depend on eth_tbl being available | expand

Commit Message

Jakub Kicinski April 25, 2018, 6:21 p.m. UTC
For very very old generation of the management FW Ethernet port
information table may theoretically not be available.  This in
turn will cause the nfp_port structures to not be allocated.

Make sure we don't crash the kernel when there is no eth_tbl:

RIP: 0010:nfp_net_pci_probe+0xf2/0xb40 [nfp]
...
Call Trace:
  nfp_pci_probe+0x6de/0xab0 [nfp]
  local_pci_probe+0x47/0xa0
  work_for_cpu_fn+0x1a/0x30
  process_one_work+0x1de/0x3e0

Found while working with broken/development version of management FW.

Fixes: a5950182c00e ("nfp: map mac_stats and vf_cfg BARs")
Fixes: 93da7d9660ee ("nfp: provide nfp_port to of nfp_net_get_mac_addr()")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/main.c  |  2 +-
 drivers/net/ethernet/netronome/nfp/nfp_app_nic.c  |  2 +-
 drivers/net/ethernet/netronome/nfp/nfp_main.h     |  4 ++-
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 31 +++++++++++++----------
 4 files changed, 23 insertions(+), 16 deletions(-)

Comments

David Miller April 27, 2018, 3:15 p.m. UTC | #1
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed, 25 Apr 2018 11:21:08 -0700

> For very very old generation of the management FW Ethernet port
> information table may theoretically not be available.  This in
> turn will cause the nfp_port structures to not be allocated.
> 
> Make sure we don't crash the kernel when there is no eth_tbl:
> 
> RIP: 0010:nfp_net_pci_probe+0xf2/0xb40 [nfp]
> ...
> Call Trace:
>   nfp_pci_probe+0x6de/0xab0 [nfp]
>   local_pci_probe+0x47/0xa0
>   work_for_cpu_fn+0x1a/0x30
>   process_one_work+0x1de/0x3e0
> 
> Found while working with broken/development version of management FW.
> 
> Fixes: a5950182c00e ("nfp: map mac_stats and vf_cfg BARs")
> Fixes: 93da7d9660ee ("nfp: provide nfp_port to of nfp_net_get_mac_addr()")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>

Applied, thanks Jakub.

Do you want this queued up for -stable?  It seems borderline, at best, to me.
Jakub Kicinski April 27, 2018, 5:38 p.m. UTC | #2
On Fri, 27 Apr 2018 11:15:56 -0400 (EDT), David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Wed, 25 Apr 2018 11:21:08 -0700
> 
> > For very very old generation of the management FW Ethernet port
> > information table may theoretically not be available.  This in
> > turn will cause the nfp_port structures to not be allocated.
> > 
> > Make sure we don't crash the kernel when there is no eth_tbl:
> > 
> > RIP: 0010:nfp_net_pci_probe+0xf2/0xb40 [nfp]
> > ...
> > Call Trace:
> >   nfp_pci_probe+0x6de/0xab0 [nfp]
> >   local_pci_probe+0x47/0xa0
> >   work_for_cpu_fn+0x1a/0x30
> >   process_one_work+0x1de/0x3e0
> > 
> > Found while working with broken/development version of management FW.
> > 
> > Fixes: a5950182c00e ("nfp: map mac_stats and vf_cfg BARs")
> > Fixes: 93da7d9660ee ("nfp: provide nfp_port to of nfp_net_get_mac_addr()")
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>  
> 
> Applied, thanks Jakub.

Thank you!

> Do you want this queued up for -stable?  It seems borderline, at best, to me.

Yes, I think we don't need stable for now.  This should never happen
(tm) in production and there has been some churn around this code.  I
don't think it's worth a backport.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c
index ad02592a82b7..a997e34bcec2 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
@@ -360,7 +360,7 @@  nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv)
 		}
 
 		SET_NETDEV_DEV(repr, &priv->nn->pdev->dev);
-		nfp_net_get_mac_addr(app->pf, port);
+		nfp_net_get_mac_addr(app->pf, repr, port);
 
 		cmsg_port_id = nfp_flower_cmsg_phys_port(phys_port);
 		err = nfp_repr_init(app, repr,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app_nic.c b/drivers/net/ethernet/netronome/nfp/nfp_app_nic.c
index 2a2f2fbc8850..b9618c37403f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app_nic.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app_nic.c
@@ -69,7 +69,7 @@  int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
 	if (err)
 		return err < 0 ? err : 0;
 
-	nfp_net_get_mac_addr(app->pf, nn->port);
+	nfp_net_get_mac_addr(app->pf, nn->dp.netdev, nn->port);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h b/drivers/net/ethernet/netronome/nfp/nfp_main.h
index add46e28212b..42211083b51f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
@@ -171,7 +171,9 @@  void nfp_net_pci_remove(struct nfp_pf *pf);
 int nfp_hwmon_register(struct nfp_pf *pf);
 void nfp_hwmon_unregister(struct nfp_pf *pf);
 
-void nfp_net_get_mac_addr(struct nfp_pf *pf, struct nfp_port *port);
+void
+nfp_net_get_mac_addr(struct nfp_pf *pf, struct net_device *netdev,
+		     struct nfp_port *port);
 
 bool nfp_ctrl_tx(struct nfp_net *nn, struct sk_buff *skb);
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 15fa47f622aa..45cd2092e498 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -67,23 +67,26 @@ 
 /**
  * nfp_net_get_mac_addr() - Get the MAC address.
  * @pf:       NFP PF handle
+ * @netdev:   net_device to set MAC address on
  * @port:     NFP port structure
  *
  * First try to get the MAC address from NSP ETH table. If that
  * fails generate a random address.
  */
-void nfp_net_get_mac_addr(struct nfp_pf *pf, struct nfp_port *port)
+void
+nfp_net_get_mac_addr(struct nfp_pf *pf, struct net_device *netdev,
+		     struct nfp_port *port)
 {
 	struct nfp_eth_table_port *eth_port;
 
 	eth_port = __nfp_port_get_eth_port(port);
 	if (!eth_port) {
-		eth_hw_addr_random(port->netdev);
+		eth_hw_addr_random(netdev);
 		return;
 	}
 
-	ether_addr_copy(port->netdev->dev_addr, eth_port->mac_addr);
-	ether_addr_copy(port->netdev->perm_addr, eth_port->mac_addr);
+	ether_addr_copy(netdev->dev_addr, eth_port->mac_addr);
+	ether_addr_copy(netdev->perm_addr, eth_port->mac_addr);
 }
 
 static struct nfp_eth_table_port *
@@ -511,16 +514,18 @@  static int nfp_net_pci_map_mem(struct nfp_pf *pf)
 		return PTR_ERR(mem);
 	}
 
-	min_size =  NFP_MAC_STATS_SIZE * (pf->eth_tbl->max_index + 1);
-	pf->mac_stats_mem = nfp_rtsym_map(pf->rtbl, "_mac_stats",
-					  "net.macstats", min_size,
-					  &pf->mac_stats_bar);
-	if (IS_ERR(pf->mac_stats_mem)) {
-		if (PTR_ERR(pf->mac_stats_mem) != -ENOENT) {
-			err = PTR_ERR(pf->mac_stats_mem);
-			goto err_unmap_ctrl;
+	if (pf->eth_tbl) {
+		min_size =  NFP_MAC_STATS_SIZE * (pf->eth_tbl->max_index + 1);
+		pf->mac_stats_mem = nfp_rtsym_map(pf->rtbl, "_mac_stats",
+						  "net.macstats", min_size,
+						  &pf->mac_stats_bar);
+		if (IS_ERR(pf->mac_stats_mem)) {
+			if (PTR_ERR(pf->mac_stats_mem) != -ENOENT) {
+				err = PTR_ERR(pf->mac_stats_mem);
+				goto err_unmap_ctrl;
+			}
+			pf->mac_stats_mem = NULL;
 		}
-		pf->mac_stats_mem = NULL;
 	}
 
 	pf->vf_cfg_mem = nfp_net_pf_map_rtsym(pf, "net.vfcfg",