[PATCH v3 net-next 0/8] ionic updates

[PATCH v3 net-next 0/8] ionic updates

[Shannon Nelson]

This is a set of small updates for the Pensando ionic driver, some
from internal work, some a result of mailing list discussions.

v3 - changed __attribute__(packed)) to __packed
v2 - removed print from ionic_init_module()

Shannon Nelson (8):
  ionic: keep ionic dev on lif init fail
  ionic: remove pragma packed
  ionic: improve irq numa locality
  ionic: clean up bitflag usage
  ionic: support ethtool rxhash disable
  ionic: print pci bus lane info
  ionic: add support for device id 0x1004
  ionic: drop ethtool driver version

 drivers/net/ethernet/pensando/ionic/ionic.h   |  2 +-
 .../ethernet/pensando/ionic/ionic_bus_pci.c   | 10 +++++
 .../ethernet/pensando/ionic/ionic_ethtool.c   | 25 +++++------
 .../net/ethernet/pensando/ionic/ionic_if.h    | 38 ++++++++--------
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 43 ++++++++++++-------
 .../net/ethernet/pensando/ionic/ionic_lif.h   | 15 +++----
 .../net/ethernet/pensando/ionic/ionic_main.c  |  6 +--
 .../net/ethernet/pensando/ionic/ionic_stats.c | 20 ++++-----
 .../net/ethernet/pensando/ionic/ionic_txrx.c  |  4 +-
 9 files changed, 86 insertions(+), 77 deletions(-)

-- 
2.17.1

[PATCH v3 net-next 1/8] ionic: keep ionic dev on lif init fail

[Shannon Nelson]

If the basic ionic interface works but the lif creation fails,
don't fail the probe.  This will allow us to use the driver to
help inspect the hw/fw/pci interface for debugging purposes.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 8 ++++++++
 drivers/net/ethernet/pensando/ionic/ionic_lif.c     | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 448d7b23b2f7..554bafac1147 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -37,6 +37,9 @@ int ionic_bus_alloc_irq_vectors(struct ionic *ionic, unsigned int nintrs)
 
 void ionic_bus_free_irq_vectors(struct ionic *ionic)
 {
+	if (!ionic->nintrs)
+		return;
+
 	pci_free_irq_vectors(ionic->pdev);
 }
 
@@ -346,6 +349,11 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	ionic_reset(ionic);
 err_out_teardown:
 	ionic_dev_teardown(ionic);
+	/* Don't fail the probe for these errors, keep
+	 * the hw interface around for inspection
+	 */
+	return 0;
+
 err_out_unmap_bars:
 	ionic_unmap_bars(ionic);
 	pci_release_regions(pdev);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 191271f6260d..1b7e18fe83db 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -2408,6 +2408,9 @@ void ionic_lifs_unregister(struct ionic *ionic)
 	 * current model, so don't bother searching the
 	 * ionic->lif for candidates to unregister
 	 */
+	if (!ionic->master_lif)
+		return;
+
 	cancel_work_sync(&ionic->master_lif->deferred.work);
 	cancel_work_sync(&ionic->master_lif->tx_timeout_work);
 	if (ionic->master_lif->netdev->reg_state == NETREG_REGISTERED)
-- 
2.17.1

[PATCH v3 net-next 2/8] ionic: remove pragma packed

[Shannon Nelson]

Replace the misguided "#pragma packed" with tags on each
struct/union definition that actually needs it.  This is safer
and more efficient on the various compilers and architectures.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_if.h    | 38 +++++++++----------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_if.h b/drivers/net/ethernet/pensando/ionic/ionic_if.h
index ce07c2931a72..e5e98067fba4 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_if.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_if.h
@@ -4,8 +4,6 @@
 #ifndef _IONIC_IF_H_
 #define _IONIC_IF_H_
 
-#pragma pack(push, 1)
-
 #define IONIC_DEV_INFO_SIGNATURE		0x44455649      /* 'DEVI' */
 #define IONIC_DEV_INFO_VERSION			1
 #define IONIC_IFNAMSIZ				16
@@ -366,7 +364,7 @@ union ionic_lif_config {
 		u8     rsvd2[2];
 		__le64 features;
 		__le32 queue_count[IONIC_QTYPE_MAX];
-	};
+	} __packed;
 	__le32 words[64];
 };
 
@@ -417,7 +415,7 @@ union ionic_lif_identity {
 			__le32 max_frame_size;
 			u8 rsvd2[106];
 			union ionic_lif_config config;
-		} eth;
+		} __packed eth;
 
 		struct {
 			u8 version;
@@ -439,8 +437,8 @@ union ionic_lif_identity {
 			struct ionic_lif_logical_qtype rq_qtype;
 			struct ionic_lif_logical_qtype cq_qtype;
 			struct ionic_lif_logical_qtype eq_qtype;
-		} rdma;
-	};
+		} __packed rdma;
+	} __packed;
 	__le32 words[512];
 };
 
@@ -526,7 +524,7 @@ struct ionic_q_init_cmd {
 	__le64 sg_ring_base;
 	__le32 eq_index;
 	u8     rsvd2[16];
-};
+} __packed;
 
 /**
  * struct ionic_q_init_comp - Queue init command completion
@@ -1095,7 +1093,7 @@ struct ionic_port_status {
 	u8     status;
 	u8     rsvd[51];
 	struct ionic_xcvr_status  xcvr;
-};
+} __packed;
 
 /**
  * struct ionic_port_identify_cmd - Port identify command
@@ -1251,7 +1249,7 @@ struct ionic_port_getattr_comp {
 		u8      pause_type;
 		u8      loopback_mode;
 		u8      rsvd2[11];
-	};
+	} __packed;
 	u8     color;
 };
 
@@ -1319,7 +1317,7 @@ struct ionic_dev_setattr_cmd {
 		char    name[IONIC_IFNAMSIZ];
 		__le64  features;
 		u8      rsvd2[60];
-	};
+	} __packed;
 };
 
 /**
@@ -1334,7 +1332,7 @@ struct ionic_dev_setattr_comp {
 	union {
 		__le64  features;
 		u8      rsvd2[11];
-	};
+	} __packed;
 	u8     color;
 };
 
@@ -1361,7 +1359,7 @@ struct ionic_dev_getattr_comp {
 	union {
 		__le64  features;
 		u8      rsvd2[11];
-	};
+	} __packed;
 	u8     color;
 };
 
@@ -1426,7 +1424,7 @@ struct ionic_lif_setattr_cmd {
 		} rss;
 		u8	stats_ctl;
 		u8      rsvd[60];
-	};
+	} __packed;
 };
 
 /**
@@ -1444,7 +1442,7 @@ struct ionic_lif_setattr_comp {
 	union {
 		__le64  features;
 		u8      rsvd2[11];
-	};
+	} __packed;
 	u8     color;
 };
 
@@ -1483,7 +1481,7 @@ struct ionic_lif_getattr_comp {
 		u8      mac[6];
 		__le64  features;
 		u8      rsvd2[11];
-	};
+	} __packed;
 	u8     color;
 };
 
@@ -1688,7 +1686,7 @@ struct ionic_vf_setattr_cmd {
 		u8     linkstate;
 		__le64 stats_pa;
 		u8     pad[60];
-	};
+	} __packed;
 };
 
 struct ionic_vf_setattr_comp {
@@ -1726,7 +1724,7 @@ struct ionic_vf_getattr_comp {
 		u8     linkstate;
 		__le64 stats_pa;
 		u8     pad[11];
-	};
+	} __packed;
 	u8     color;
 };
 
@@ -2471,7 +2469,7 @@ union ionic_dev_cmd_regs {
 		union ionic_dev_cmd_comp    comp;
 		u8                    rsvd[48];
 		u32                   data[478];
-	};
+	} __packed;
 	u32 words[512];
 };
 
@@ -2484,7 +2482,7 @@ union ionic_dev_regs {
 	struct {
 		union ionic_dev_info_regs info;
 		union ionic_dev_cmd_regs  devcmd;
-	};
+	} __packed;
 	__le32 words[1024];
 };
 
@@ -2574,6 +2572,4 @@ struct ionic_identity {
 	union ionic_qos_identity qos;
 };
 
-#pragma pack(pop)
-
 #endif /* _IONIC_IF_H_ */
-- 
2.17.1

[PATCH v3 net-next 3/8] ionic: improve irq numa locality

[Shannon Nelson]

Spreading the interrupts across the CPU cores is good for load
balancing, but not necessarily as good when using a CPU/core
that is not part of the NUMA local CPU.  If it can be localized,
the kernel's cpumask_local_spread() service will pick a core
that is on the node close to the PCI device.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 1b7e18fe83db..7df49b433ae5 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -424,8 +424,9 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
 		ionic_intr_mask_assert(idev->intr_ctrl, new->intr.index,
 				       IONIC_INTR_MASK_SET);
 
-		new->intr.cpu = new->intr.index % num_online_cpus();
-		if (cpu_online(new->intr.cpu))
+		new->intr.cpu = cpumask_local_spread(new->intr.index,
+						     dev_to_node(dev));
+		if (new->intr.cpu != -1)
 			cpumask_set_cpu(new->intr.cpu,
 					&new->intr.affinity_mask);
 	} else {
-- 
2.17.1

[PATCH v3 net-next 4/8] ionic: clean up bitflag usage

[Shannon Nelson]

Remove the unused flags field and and fix the bitflag names
to include the _F_ flag hint.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../ethernet/pensando/ionic/ionic_ethtool.c   | 24 ++++++++-----------
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 24 +++++++++----------
 .../net/ethernet/pensando/ionic/ionic_lif.h   | 15 ++++++------
 .../net/ethernet/pensando/ionic/ionic_stats.c | 20 ++++++++--------
 .../net/ethernet/pensando/ionic/ionic_txrx.c  |  4 ++--
 5 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index f778fff034f5..acd53e27d1ec 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -462,7 +462,7 @@ static int ionic_set_coalesce(struct net_device *netdev,
 	if (coal != lif->rx_coalesce_hw) {
 		lif->rx_coalesce_hw = coal;
 
-		if (test_bit(IONIC_LIF_UP, lif->state)) {
+		if (test_bit(IONIC_LIF_F_UP, lif->state)) {
 			for (i = 0; i < lif->nxqs; i++) {
 				qcq = lif->rxqcqs[i].qcq;
 				ionic_intr_coal_init(lif->ionic->idev.intr_ctrl,
@@ -509,11 +509,11 @@ static int ionic_set_ringparam(struct net_device *netdev,
 	    ring->rx_pending == lif->nrxq_descs)
 		return 0;
 
-	err = ionic_wait_for_bit(lif, IONIC_LIF_QUEUE_RESET);
+	err = ionic_wait_for_bit(lif, IONIC_LIF_F_QUEUE_RESET);
 	if (err)
 		return err;
 
-	running = test_bit(IONIC_LIF_UP, lif->state);
+	running = test_bit(IONIC_LIF_F_UP, lif->state);
 	if (running)
 		ionic_stop(netdev);
 
@@ -522,7 +522,7 @@ static int ionic_set_ringparam(struct net_device *netdev,
 
 	if (running)
 		ionic_open(netdev);
-	clear_bit(IONIC_LIF_QUEUE_RESET, lif->state);
+	clear_bit(IONIC_LIF_F_QUEUE_RESET, lif->state);
 
 	return 0;
 }
@@ -553,11 +553,11 @@ static int ionic_set_channels(struct net_device *netdev,
 	if (ch->combined_count == lif->nxqs)
 		return 0;
 
-	err = ionic_wait_for_bit(lif, IONIC_LIF_QUEUE_RESET);
+	err = ionic_wait_for_bit(lif, IONIC_LIF_F_QUEUE_RESET);
 	if (err)
 		return err;
 
-	running = test_bit(IONIC_LIF_UP, lif->state);
+	running = test_bit(IONIC_LIF_F_UP, lif->state);
 	if (running)
 		ionic_stop(netdev);
 
@@ -565,7 +565,7 @@ static int ionic_set_channels(struct net_device *netdev,
 
 	if (running)
 		ionic_open(netdev);
-	clear_bit(IONIC_LIF_QUEUE_RESET, lif->state);
+	clear_bit(IONIC_LIF_F_QUEUE_RESET, lif->state);
 
 	return 0;
 }
@@ -575,7 +575,7 @@ static u32 ionic_get_priv_flags(struct net_device *netdev)
 	struct ionic_lif *lif = netdev_priv(netdev);
 	u32 priv_flags = 0;
 
-	if (test_bit(IONIC_LIF_SW_DEBUG_STATS, lif->state))
+	if (test_bit(IONIC_LIF_F_SW_DEBUG_STATS, lif->state))
 		priv_flags |= PRIV_F_SW_DBG_STATS;
 
 	return priv_flags;
@@ -584,14 +584,10 @@ static u32 ionic_get_priv_flags(struct net_device *netdev)
 static int ionic_set_priv_flags(struct net_device *netdev, u32 priv_flags)
 {
 	struct ionic_lif *lif = netdev_priv(netdev);
-	u32 flags = lif->flags;
 
-	clear_bit(IONIC_LIF_SW_DEBUG_STATS, lif->state);
+	clear_bit(IONIC_LIF_F_SW_DEBUG_STATS, lif->state);
 	if (priv_flags & PRIV_F_SW_DBG_STATS)
-		set_bit(IONIC_LIF_SW_DEBUG_STATS, lif->state);
-
-	if (flags != lif->flags)
-		lif->flags = flags;
+		set_bit(IONIC_LIF_F_SW_DEBUG_STATS, lif->state);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 7df49b433ae5..d1567e477b1f 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -84,7 +84,7 @@ static void ionic_link_status_check(struct ionic_lif *lif)
 		netdev_info(netdev, "Link up - %d Gbps\n",
 			    le32_to_cpu(lif->info->status.link_speed) / 1000);
 
-		if (test_bit(IONIC_LIF_UP, lif->state)) {
+		if (test_bit(IONIC_LIF_F_UP, lif->state)) {
 			netif_tx_wake_all_queues(lif->netdev);
 			netif_carrier_on(netdev);
 		}
@@ -93,12 +93,12 @@ static void ionic_link_status_check(struct ionic_lif *lif)
 
 		/* carrier off first to avoid watchdog timeout */
 		netif_carrier_off(netdev);
-		if (test_bit(IONIC_LIF_UP, lif->state))
+		if (test_bit(IONIC_LIF_F_UP, lif->state))
 			netif_tx_stop_all_queues(netdev);
 	}
 
 link_out:
-	clear_bit(IONIC_LIF_LINK_CHECK_REQUESTED, lif->state);
+	clear_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state);
 }
 
 static void ionic_link_status_check_request(struct ionic_lif *lif)
@@ -106,7 +106,7 @@ static void ionic_link_status_check_request(struct ionic_lif *lif)
 	struct ionic_deferred_work *work;
 
 	/* we only need one request outstanding at a time */
-	if (test_and_set_bit(IONIC_LIF_LINK_CHECK_REQUESTED, lif->state))
+	if (test_and_set_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state))
 		return;
 
 	if (in_interrupt()) {
@@ -1579,7 +1579,7 @@ int ionic_open(struct net_device *netdev)
 	netif_set_real_num_tx_queues(netdev, lif->nxqs);
 	netif_set_real_num_rx_queues(netdev, lif->nxqs);
 
-	set_bit(IONIC_LIF_UP, lif->state);
+	set_bit(IONIC_LIF_F_UP, lif->state);
 
 	ionic_link_status_check_request(lif);
 	if (netif_carrier_ok(netdev))
@@ -1599,13 +1599,13 @@ int ionic_stop(struct net_device *netdev)
 	struct ionic_lif *lif = netdev_priv(netdev);
 	int err = 0;
 
-	if (!test_bit(IONIC_LIF_UP, lif->state)) {
+	if (!test_bit(IONIC_LIF_F_UP, lif->state)) {
 		dev_dbg(lif->ionic->dev, "%s: %s state=DOWN\n",
 			__func__, lif->name);
 		return 0;
 	}
 	dev_dbg(lif->ionic->dev, "%s: %s state=UP\n", __func__, lif->name);
-	clear_bit(IONIC_LIF_UP, lif->state);
+	clear_bit(IONIC_LIF_F_UP, lif->state);
 
 	/* carrier off before disabling queues to avoid watchdog timeout */
 	netif_carrier_off(netdev);
@@ -1872,7 +1872,7 @@ int ionic_reset_queues(struct ionic_lif *lif)
 	/* Put off the next watchdog timeout */
 	netif_trans_update(lif->netdev);
 
-	err = ionic_wait_for_bit(lif, IONIC_LIF_QUEUE_RESET);
+	err = ionic_wait_for_bit(lif, IONIC_LIF_F_QUEUE_RESET);
 	if (err)
 		return err;
 
@@ -1882,7 +1882,7 @@ int ionic_reset_queues(struct ionic_lif *lif)
 	if (!err && running)
 		ionic_open(lif->netdev);
 
-	clear_bit(IONIC_LIF_QUEUE_RESET, lif->state);
+	clear_bit(IONIC_LIF_F_QUEUE_RESET, lif->state);
 
 	return err;
 }
@@ -2049,10 +2049,10 @@ void ionic_lifs_free(struct ionic *ionic)
 
 static void ionic_lif_deinit(struct ionic_lif *lif)
 {
-	if (!test_bit(IONIC_LIF_INITED, lif->state))
+	if (!test_bit(IONIC_LIF_F_INITED, lif->state))
 		return;
 
-	clear_bit(IONIC_LIF_INITED, lif->state);
+	clear_bit(IONIC_LIF_F_INITED, lif->state);
 
 	ionic_rx_filters_deinit(lif);
 	ionic_lif_rss_deinit(lif);
@@ -2288,7 +2288,7 @@ static int ionic_lif_init(struct ionic_lif *lif)
 
 	lif->rx_copybreak = IONIC_RX_COPYBREAK_DEFAULT;
 
-	set_bit(IONIC_LIF_INITED, lif->state);
+	set_bit(IONIC_LIF_F_INITED, lif->state);
 
 	INIT_WORK(&lif->tx_timeout_work, ionic_tx_timeout_work);
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index 9c5a7dd45f9d..7c0c6fef8c0b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -121,14 +121,14 @@ struct ionic_lif_sw_stats {
 };
 
 enum ionic_lif_state_flags {
-	IONIC_LIF_INITED,
-	IONIC_LIF_SW_DEBUG_STATS,
-	IONIC_LIF_UP,
-	IONIC_LIF_LINK_CHECK_REQUESTED,
-	IONIC_LIF_QUEUE_RESET,
+	IONIC_LIF_F_INITED,
+	IONIC_LIF_F_SW_DEBUG_STATS,
+	IONIC_LIF_F_UP,
+	IONIC_LIF_F_LINK_CHECK_REQUESTED,
+	IONIC_LIF_F_QUEUE_RESET,
 
 	/* leave this as last */
-	IONIC_LIF_STATE_SIZE
+	IONIC_LIF_F_STATE_SIZE
 };
 
 #define IONIC_LIF_NAME_MAX_SZ		32
@@ -136,7 +136,7 @@ struct ionic_lif {
 	char name[IONIC_LIF_NAME_MAX_SZ];
 	struct list_head list;
 	struct net_device *netdev;
-	DECLARE_BITMAP(state, IONIC_LIF_STATE_SIZE);
+	DECLARE_BITMAP(state, IONIC_LIF_F_STATE_SIZE);
 	struct ionic *ionic;
 	bool registered;
 	unsigned int index;
@@ -179,7 +179,6 @@ struct ionic_lif {
 	u32 rx_coalesce_usecs;		/* what the user asked for */
 	u32 rx_coalesce_hw;		/* what the hw is using */
 
-	u32 flags;
 	struct work_struct tx_timeout_work;
 };
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_stats.c b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
index a1e9796a660a..8f2a8fb029f1 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_stats.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
@@ -118,8 +118,8 @@ static u64 ionic_sw_stats_get_count(struct ionic_lif *lif)
 	/* rx stats */
 	total += MAX_Q(lif) * IONIC_NUM_RX_STATS;
 
-	if (test_bit(IONIC_LIF_UP, lif->state) &&
-	    test_bit(IONIC_LIF_SW_DEBUG_STATS, lif->state)) {
+	if (test_bit(IONIC_LIF_F_UP, lif->state) &&
+	    test_bit(IONIC_LIF_F_SW_DEBUG_STATS, lif->state)) {
 		/* tx debug stats */
 		total += MAX_Q(lif) * (IONIC_NUM_DBG_CQ_STATS +
 				      IONIC_NUM_TX_Q_STATS +
@@ -151,8 +151,8 @@ static void ionic_sw_stats_get_strings(struct ionic_lif *lif, u8 **buf)
 			*buf += ETH_GSTRING_LEN;
 		}
 
-		if (test_bit(IONIC_LIF_UP, lif->state) &&
-		    test_bit(IONIC_LIF_SW_DEBUG_STATS, lif->state)) {
+		if (test_bit(IONIC_LIF_F_UP, lif->state) &&
+		    test_bit(IONIC_LIF_F_SW_DEBUG_STATS, lif->state)) {
 			for (i = 0; i < IONIC_NUM_TX_Q_STATS; i++) {
 				snprintf(*buf, ETH_GSTRING_LEN,
 					 "txq_%d_%s",
@@ -190,8 +190,8 @@ static void ionic_sw_stats_get_strings(struct ionic_lif *lif, u8 **buf)
 			*buf += ETH_GSTRING_LEN;
 		}
 
-		if (test_bit(IONIC_LIF_UP, lif->state) &&
-		    test_bit(IONIC_LIF_SW_DEBUG_STATS, lif->state)) {
+		if (test_bit(IONIC_LIF_F_UP, lif->state) &&
+		    test_bit(IONIC_LIF_F_SW_DEBUG_STATS, lif->state)) {
 			for (i = 0; i < IONIC_NUM_DBG_CQ_STATS; i++) {
 				snprintf(*buf, ETH_GSTRING_LEN,
 					 "rxq_%d_cq_%s",
@@ -247,8 +247,8 @@ static void ionic_sw_stats_get_values(struct ionic_lif *lif, u64 **buf)
 			(*buf)++;
 		}
 
-		if (test_bit(IONIC_LIF_UP, lif->state) &&
-		    test_bit(IONIC_LIF_SW_DEBUG_STATS, lif->state)) {
+		if (test_bit(IONIC_LIF_F_UP, lif->state) &&
+		    test_bit(IONIC_LIF_F_SW_DEBUG_STATS, lif->state)) {
 			txqcq = lif_to_txqcq(lif, q_num);
 			for (i = 0; i < IONIC_NUM_TX_Q_STATS; i++) {
 				**buf = IONIC_READ_STAT64(&txqcq->q,
@@ -281,8 +281,8 @@ static void ionic_sw_stats_get_values(struct ionic_lif *lif, u64 **buf)
 			(*buf)++;
 		}
 
-		if (test_bit(IONIC_LIF_UP, lif->state) &&
-		    test_bit(IONIC_LIF_SW_DEBUG_STATS, lif->state)) {
+		if (test_bit(IONIC_LIF_F_UP, lif->state) &&
+		    test_bit(IONIC_LIF_F_SW_DEBUG_STATS, lif->state)) {
 			rxqcq = lif_to_rxqcq(lif, q_num);
 			for (i = 0; i < IONIC_NUM_DBG_CQ_STATS; i++) {
 				**buf = IONIC_READ_STAT64(&rxqcq->cq,
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index e452f4242ba0..a268cead9f50 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -158,7 +158,7 @@ static void ionic_rx_clean(struct ionic_queue *q, struct ionic_desc_info *desc_i
 	}
 
 	/* no packet processing while resetting */
-	if (unlikely(test_bit(IONIC_LIF_QUEUE_RESET, q->lif->state))) {
+	if (unlikely(test_bit(IONIC_LIF_F_QUEUE_RESET, q->lif->state))) {
 		stats->dropped++;
 		return;
 	}
@@ -1026,7 +1026,7 @@ netdev_tx_t ionic_start_xmit(struct sk_buff *skb, struct net_device *netdev)
 	int ndescs;
 	int err;
 
-	if (unlikely(!test_bit(IONIC_LIF_UP, lif->state))) {
+	if (unlikely(!test_bit(IONIC_LIF_F_UP, lif->state))) {
 		dev_kfree_skb(skb);
 		return NETDEV_TX_OK;
 	}
-- 
2.17.1

[PATCH v3 net-next 5/8] ionic: support ethtool rxhash disable

[Shannon Nelson]

We can disable rxhashing by setting rss_types to 0.  The user
can toggle this with "ethtool -K <ethX> rxhash off|on",
which calls into the .ndo_set_features callback with the
NETIF_F_RXHASH feature bit set or cleared.  This patch adds
a check for that bit and updates the FW if necessary.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index d1567e477b1f..4b953f9e9084 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1094,6 +1094,7 @@ static int ionic_set_nic_features(struct ionic_lif *lif,
 	u64 vlan_flags = IONIC_ETH_HW_VLAN_TX_TAG |
 			 IONIC_ETH_HW_VLAN_RX_STRIP |
 			 IONIC_ETH_HW_VLAN_RX_FILTER;
+	u64 old_hw_features;
 	int err;
 
 	ctx.cmd.lif_setattr.features = ionic_netdev_features_to_nic(features);
@@ -1101,9 +1102,13 @@ static int ionic_set_nic_features(struct ionic_lif *lif,
 	if (err)
 		return err;
 
+	old_hw_features = lif->hw_features;
 	lif->hw_features = le64_to_cpu(ctx.cmd.lif_setattr.features &
 				       ctx.comp.lif_setattr.features);
 
+	if ((old_hw_features ^ lif->hw_features) & IONIC_ETH_HW_RX_HASH)
+		ionic_lif_rss_config(lif, lif->rss_types, NULL, NULL);
+
 	if ((vlan_flags & features) &&
 	    !(vlan_flags & le64_to_cpu(ctx.comp.lif_setattr.features)))
 		dev_info_once(lif->ionic->dev, "NIC is not supporting vlan offload, likely in SmartNIC mode\n");
@@ -1357,13 +1362,15 @@ int ionic_lif_rss_config(struct ionic_lif *lif, const u16 types,
 		.cmd.lif_setattr = {
 			.opcode = IONIC_CMD_LIF_SETATTR,
 			.attr = IONIC_LIF_ATTR_RSS,
-			.rss.types = cpu_to_le16(types),
 			.rss.addr = cpu_to_le64(lif->rss_ind_tbl_pa),
 		},
 	};
 	unsigned int i, tbl_sz;
 
-	lif->rss_types = types;
+	if (lif->hw_features & IONIC_ETH_HW_RX_HASH) {
+		lif->rss_types = types;
+		ctx.cmd.lif_setattr.rss.types = cpu_to_le16(types);
+	}
 
 	if (key)
 		memcpy(lif->rss_hash_key, key, IONIC_RSS_HASH_KEY_SIZE);
-- 
2.17.1

[PATCH v3 net-next 6/8] ionic: print pci bus lane info

[Shannon Nelson]

Print the PCI bus lane information so people can keep an
eye out for poor configurations that might not support
the advertised speed.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 554bafac1147..59b0091146e6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -248,6 +248,7 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 
 	pci_set_master(pdev);
+	pcie_print_link_status(pdev);
 
 	err = ionic_map_bars(ionic);
 	if (err)
-- 
2.17.1

[PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Shannon Nelson]

Add support for an additional device id.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic.h         | 1 +
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
index bb106a32f416..c8ff33da243a 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic.h
@@ -18,6 +18,7 @@ struct ionic_lif;
 
 #define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF	0x1002
 #define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_VF	0x1003
+#define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT	0x1004
 
 #define DEVCMD_TIMEOUT  10
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 59b0091146e6..3dc985cae391 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -15,6 +15,7 @@
 static const struct pci_device_id ionic_id_table[] = {
 	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF) },
 	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_VF) },
+	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT) },
 	{ 0, }	/* end of table */
 };
 MODULE_DEVICE_TABLE(pci, ionic_id_table);
-- 
2.17.1

[PATCH v3 net-next 8/8] ionic: drop ethtool driver version

[Shannon Nelson]

Use the default kernel version in ethtool drv_info output
and drop the module version.

Cc: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic.h         | 1 -
 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 -
 drivers/net/ethernet/pensando/ionic/ionic_main.c    | 6 ++----
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
index c8ff33da243a..1c720759fd80 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic.h
@@ -12,7 +12,6 @@ struct ionic_lif;
 
 #define IONIC_DRV_NAME		"ionic"
 #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
-#define IONIC_DRV_VERSION	"0.20.0-k"
 
 #define PCI_VENDOR_ID_PENSANDO			0x1dd8
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index acd53e27d1ec..bea9b78e0189 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev,
 	struct ionic *ionic = lif->ionic;
 
 	strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver));
-	strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version));
 	strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version,
 		sizeof(drvinfo->fw_version));
 	strlcpy(drvinfo->bus_info, ionic_bus_info(ionic),
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index a8e3fb73b465..e4a76e66f542 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -6,6 +6,7 @@
 #include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/utsname.h>
+#include <linux/vermagic.h>
 
 #include "ionic.h"
 #include "ionic_bus.h"
@@ -15,7 +16,6 @@
 MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION);
 MODULE_AUTHOR("Pensando Systems, Inc");
 MODULE_LICENSE("GPL");
-MODULE_VERSION(IONIC_DRV_VERSION);
 
 static const char *ionic_error_to_str(enum ionic_status_code code)
 {
@@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic)
 	memset(ident, 0, sizeof(*ident));
 
 	ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX);
-	strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION,
+	strncpy(ident->drv.driver_ver_str, UTS_RELEASE,
 		sizeof(ident->drv.driver_ver_str) - 1);
 
 	mutex_lock(&ionic->dev_cmd_lock);
@@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic)
 
 static int __init ionic_init_module(void)
 {
-	pr_info("%s %s, ver %s\n",
-		IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION);
 	ionic_debugfs_create();
 	return ionic_bus_register_driver();
 }
-- 
2.17.1

Re: [PATCH v3 net-next 8/8] ionic: drop ethtool driver version

[Leon Romanovsky]

On Wed, Mar 04, 2020 at 09:23:19PM -0800, Shannon Nelson wrote:
> Use the default kernel version in ethtool drv_info output
> and drop the module version.
>
> Cc: Leon Romanovsky <leonro@mellanox.com>
> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> ---
>  drivers/net/ethernet/pensando/ionic/ionic.h         | 1 -
>  drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 -
>  drivers/net/ethernet/pensando/ionic/ionic_main.c    | 6 ++----
>  3 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
> index c8ff33da243a..1c720759fd80 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
> @@ -12,7 +12,6 @@ struct ionic_lif;
>
>  #define IONIC_DRV_NAME		"ionic"
>  #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
> -#define IONIC_DRV_VERSION	"0.20.0-k"
>
>  #define PCI_VENDOR_ID_PENSANDO			0x1dd8
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index acd53e27d1ec..bea9b78e0189 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev,
>  	struct ionic *ionic = lif->ionic;
>
>  	strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver));
> -	strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version));
>  	strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version,
>  		sizeof(drvinfo->fw_version));
>  	strlcpy(drvinfo->bus_info, ionic_bus_info(ionic),
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> index a8e3fb73b465..e4a76e66f542 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> @@ -6,6 +6,7 @@
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
>  #include <linux/utsname.h>
> +#include <linux/vermagic.h>
>
>  #include "ionic.h"
>  #include "ionic_bus.h"
> @@ -15,7 +16,6 @@
>  MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION);
>  MODULE_AUTHOR("Pensando Systems, Inc");
>  MODULE_LICENSE("GPL");
> -MODULE_VERSION(IONIC_DRV_VERSION);
>
>  static const char *ionic_error_to_str(enum ionic_status_code code)
>  {
> @@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic)
>  	memset(ident, 0, sizeof(*ident));
>
>  	ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX);
> -	strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION,
> +	strncpy(ident->drv.driver_ver_str, UTS_RELEASE,
>  		sizeof(ident->drv.driver_ver_str) - 1);

i see that you responded to my question about usage of this string [1]
and I can't say anything about netdev policy on that, but in other
subsystems, the idea that driver has duplicated debug functionalities
to the general kernel code is not welcomed.

[1] https://lore.kernel.org/netdev/e0cbc84c-7860-abf2-a622-4035be1479dc@pensando.io

>
>  	mutex_lock(&ionic->dev_cmd_lock);
> @@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic)
>
>  static int __init ionic_init_module(void)
>  {
> -	pr_info("%s %s, ver %s\n",
> -		IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION);
>  	ionic_debugfs_create();
>  	return ionic_bus_register_driver();
>  }
> --
> 2.17.1
>

Re: [PATCH v3 net-next 8/8] ionic: drop ethtool driver version

[Shannon Nelson]

On 3/4/20 10:10 PM, Leon Romanovsky wrote:
> On Wed, Mar 04, 2020 at 09:23:19PM -0800, Shannon Nelson wrote:
>> Use the default kernel version in ethtool drv_info output
>> and drop the module version.
>>
>> Cc: Leon Romanovsky <leonro@mellanox.com>
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>> ---
>>   drivers/net/ethernet/pensando/ionic/ionic.h         | 1 -
>>   drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 -
>>   drivers/net/ethernet/pensando/ionic/ionic_main.c    | 6 ++----
>>   3 files changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
>> index c8ff33da243a..1c720759fd80 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
>> @@ -12,7 +12,6 @@ struct ionic_lif;
>>
>>   #define IONIC_DRV_NAME		"ionic"
>>   #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
>> -#define IONIC_DRV_VERSION	"0.20.0-k"
>>
>>   #define PCI_VENDOR_ID_PENSANDO			0x1dd8
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> index acd53e27d1ec..bea9b78e0189 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> @@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev,
>>   	struct ionic *ionic = lif->ionic;
>>
>>   	strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver));
>> -	strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version));
>>   	strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version,
>>   		sizeof(drvinfo->fw_version));
>>   	strlcpy(drvinfo->bus_info, ionic_bus_info(ionic),
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
>> index a8e3fb73b465..e4a76e66f542 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/module.h>
>>   #include <linux/netdevice.h>
>>   #include <linux/utsname.h>
>> +#include <linux/vermagic.h>
>>
>>   #include "ionic.h"
>>   #include "ionic_bus.h"
>> @@ -15,7 +16,6 @@
>>   MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION);
>>   MODULE_AUTHOR("Pensando Systems, Inc");
>>   MODULE_LICENSE("GPL");
>> -MODULE_VERSION(IONIC_DRV_VERSION);
>>
>>   static const char *ionic_error_to_str(enum ionic_status_code code)
>>   {
>> @@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic)
>>   	memset(ident, 0, sizeof(*ident));
>>
>>   	ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX);
>> -	strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION,
>> +	strncpy(ident->drv.driver_ver_str, UTS_RELEASE,
>>   		sizeof(ident->drv.driver_ver_str) - 1);
> i see that you responded to my question about usage of this string [1]
> and I can't say anything about netdev policy on that, but in other
> subsystems, the idea that driver has duplicated debug functionalities
> to the general kernel code is not welcomed.
>
> [1] https://lore.kernel.org/netdev/e0cbc84c-7860-abf2-a622-4035be1479dc@pensando.io

This DSC (Distributed Services Card) is more than a simple NIC, and in 
several use cases is intended to be managed centrally and installed in 
hosts that can be handed out to customers as bare-metal machines to 
which the datacenter personnel cannot access.  The device can be 
accessed through a separate management network port, similar to an ilom 
or cimc other similar host management gizmo. Getting a little 
information about the driver into the card's logfiles allows for a 
little better debugging context from the management side without having 
access to the host.

Yes, we want to keep functionality duplication to a minimum, but I think 
this is a different case.  We also want to keep customer information 
leakage to a minimum, which is why we were using only the individual 
driver version info before it was replaced with the kernel version.  I'd 
like to keep at least some bit of driver context information available 
to those on the management side of this PCI device.

sln

>>   	mutex_lock(&ionic->dev_cmd_lock);
>> @@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic)
>>
>>   static int __init ionic_init_module(void)
>>   {
>> -	pr_info("%s %s, ver %s\n",
>> -		IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION);
>>   	ionic_debugfs_create();
>>   	return ionic_bus_register_driver();
>>   }
>> --
>> 2.17.1
>>

Re: [PATCH v3 net-next 8/8] ionic: drop ethtool driver version

[Leon Romanovsky]

On Wed, Mar 04, 2020 at 11:20:11PM -0800, Shannon Nelson wrote:
> On 3/4/20 10:10 PM, Leon Romanovsky wrote:
> > On Wed, Mar 04, 2020 at 09:23:19PM -0800, Shannon Nelson wrote:
> > > Use the default kernel version in ethtool drv_info output
> > > and drop the module version.
> > >
> > > Cc: Leon Romanovsky <leonro@mellanox.com>
> > > Signed-off-by: Shannon Nelson <snelson@pensando.io>
> > > ---
> > >   drivers/net/ethernet/pensando/ionic/ionic.h         | 1 -
> > >   drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 -
> > >   drivers/net/ethernet/pensando/ionic/ionic_main.c    | 6 ++----
> > >   3 files changed, 2 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
> > > index c8ff33da243a..1c720759fd80 100644
> > > --- a/drivers/net/ethernet/pensando/ionic/ionic.h
> > > +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
> > > @@ -12,7 +12,6 @@ struct ionic_lif;
> > >
> > >   #define IONIC_DRV_NAME		"ionic"
> > >   #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
> > > -#define IONIC_DRV_VERSION	"0.20.0-k"
> > >
> > >   #define PCI_VENDOR_ID_PENSANDO			0x1dd8
> > >
> > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> > > index acd53e27d1ec..bea9b78e0189 100644
> > > --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> > > +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> > > @@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev,
> > >   	struct ionic *ionic = lif->ionic;
> > >
> > >   	strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver));
> > > -	strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version));
> > >   	strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version,
> > >   		sizeof(drvinfo->fw_version));
> > >   	strlcpy(drvinfo->bus_info, ionic_bus_info(ionic),
> > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> > > index a8e3fb73b465..e4a76e66f542 100644
> > > --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
> > > +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> > > @@ -6,6 +6,7 @@
> > >   #include <linux/module.h>
> > >   #include <linux/netdevice.h>
> > >   #include <linux/utsname.h>
> > > +#include <linux/vermagic.h>
> > >
> > >   #include "ionic.h"
> > >   #include "ionic_bus.h"
> > > @@ -15,7 +16,6 @@
> > >   MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION);
> > >   MODULE_AUTHOR("Pensando Systems, Inc");
> > >   MODULE_LICENSE("GPL");
> > > -MODULE_VERSION(IONIC_DRV_VERSION);
> > >
> > >   static const char *ionic_error_to_str(enum ionic_status_code code)
> > >   {
> > > @@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic)
> > >   	memset(ident, 0, sizeof(*ident));
> > >
> > >   	ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX);
> > > -	strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION,
> > > +	strncpy(ident->drv.driver_ver_str, UTS_RELEASE,
> > >   		sizeof(ident->drv.driver_ver_str) - 1);
> > i see that you responded to my question about usage of this string [1]
> > and I can't say anything about netdev policy on that, but in other
> > subsystems, the idea that driver has duplicated debug functionalities
> > to the general kernel code is not welcomed.
> >
> > [1] https://lore.kernel.org/netdev/e0cbc84c-7860-abf2-a622-4035be1479dc@pensando.io
>
> This DSC (Distributed Services Card) is more than a simple NIC, and in
> several use cases is intended to be managed centrally and installed in hosts
> that can be handed out to customers as bare-metal machines to which the
> datacenter personnel cannot access.  The device can be accessed through a
> separate management network port, similar to an ilom or cimc other similar
> host management gizmo. Getting a little information about the driver into
> the card's logfiles allows for a little better debugging context from the
> management side without having access to the host.
>
> Yes, we want to keep functionality duplication to a minimum, but I think
> this is a different case.  We also want to keep customer information leakage
> to a minimum, which is why we were using only the individual driver version
> info before it was replaced with the kernel version.  I'd like to keep at
> least some bit of driver context information available to those on the
> management side of this PCI device.

Again, I'm not netdev person and say nothing about if it is ok or not.
However, this string is completely unreliable, especially if user
changes your driver in his bare-metal machine.

Thanks

>
> sln
>
> > >   	mutex_lock(&ionic->dev_cmd_lock);
> > > @@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic)
> > >
> > >   static int __init ionic_init_module(void)
> > >   {
> > > -	pr_info("%s %s, ver %s\n",
> > > -		IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION);
> > >   	ionic_debugfs_create();
> > >   	return ionic_bus_register_driver();
> > >   }
> > > --
> > > 2.17.1
> > >
>

Re: [PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Jakub Kicinski]

On Wed,  4 Mar 2020 21:23:18 -0800 Shannon Nelson wrote:
> Add support for an additional device id.
> 
> Signed-off-by: Shannon Nelson <snelson@pensando.io>

I have thought about this for a while and I wanted to ask you to say 
a bit more about the use of the management device.

Obviously this is not just "additional device id" in the traditional
sense where device IDs differentiate HW SKUs or revisions. This is the
same exact hardware, just a different local feature (as proven by the
fact that you make 0 functional changes).

In the past we (I?) rejected such extensions upstream from Netronome and
Cavium, because there were no clear use cases which can't be solved by
extending standard kernel APIs. Do you have any?

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
> index bb106a32f416..c8ff33da243a 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
> @@ -18,6 +18,7 @@ struct ionic_lif;
>  
>  #define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF	0x1002
>  #define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_VF	0x1003
> +#define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT	0x1004
>  
>  #define DEVCMD_TIMEOUT  10
>  
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> index 59b0091146e6..3dc985cae391 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> @@ -15,6 +15,7 @@
>  static const struct pci_device_id ionic_id_table[] = {
>  	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF) },
>  	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_VF) },
> +	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT) },
>  	{ 0, }	/* end of table */
>  };
>  MODULE_DEVICE_TABLE(pci, ionic_id_table);

Re: [PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Shannon Nelson]

On 3/5/20 2:03 PM, Jakub Kicinski wrote:
> On Wed,  4 Mar 2020 21:23:18 -0800 Shannon Nelson wrote:
>> Add support for an additional device id.
>>
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> I have thought about this for a while and I wanted to ask you to say
> a bit more about the use of the management device.
>
> Obviously this is not just "additional device id" in the traditional
> sense where device IDs differentiate HW SKUs or revisions. This is the
> same exact hardware, just a different local feature (as proven by the
> fact that you make 0 functional changes).
>
> In the past we (I?) rejected such extensions upstream from Netronome and
> Cavium, because there were no clear use cases which can't be solved by
> extending standard kernel APIs. Do you have any?

Do you by chance have any references handy to such past discussions?  
I'd be interested in reading them to see what similarities and 
differences we have.

The network device we present is only a portion of the DSC's functions.  
The device configuration and management for the various services is 
handled in userspace programs on the OS running inside the device.  
These are accessed through a secured REST API, typically through the 
external management ethernet port.  In addition to our centralized 
management user interface, we have a command line tool for managing the 
device configuration using that same REST interface.

In some configurations we make it possible to open a network connection 
into the device through the host PCI, just as if you were to connect 
through the external mgmt port.  This is the PCI deviceid that 
corresponds to that port, and allows use of the command line tool on the 
host.

The host network driver doesn't have access to the device management 
commands, it only can configure the NIC portion for what it needs for 
passing network packets.

sln



>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
>> index bb106a32f416..c8ff33da243a 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
>> @@ -18,6 +18,7 @@ struct ionic_lif;
>>   
>>   #define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF	0x1002
>>   #define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_VF	0x1003
>> +#define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT	0x1004
>>   
>>   #define DEVCMD_TIMEOUT  10
>>   
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> index 59b0091146e6..3dc985cae391 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> @@ -15,6 +15,7 @@
>>   static const struct pci_device_id ionic_id_table[] = {
>>   	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF) },
>>   	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_VF) },
>> +	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT) },
>>   	{ 0, }	/* end of table */
>>   };
>>   MODULE_DEVICE_TABLE(pci, ionic_id_table);

Re: [PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Jakub Kicinski]

On Thu, 5 Mar 2020 16:41:48 -0800 Shannon Nelson wrote:
> On 3/5/20 2:03 PM, Jakub Kicinski wrote:
> > On Wed,  4 Mar 2020 21:23:18 -0800 Shannon Nelson wrote:  
> >> Add support for an additional device id.
> >>
> >> Signed-off-by: Shannon Nelson <snelson@pensando.io>  
> > I have thought about this for a while and I wanted to ask you to say
> > a bit more about the use of the management device.
> >
> > Obviously this is not just "additional device id" in the traditional
> > sense where device IDs differentiate HW SKUs or revisions. This is the
> > same exact hardware, just a different local feature (as proven by the
> > fact that you make 0 functional changes).
> >
> > In the past we (I?) rejected such extensions upstream from Netronome and
> > Cavium, because there were no clear use cases which can't be solved by
> > extending standard kernel APIs. Do you have any?  
> 
> Do you by chance have any references handy to such past discussions?  
> I'd be interested in reading them to see what similarities and 
> differences we have.

Here you go:

https://lore.kernel.org/netdev/20170718115827.7bd737f2@cakuba.netronome.com/

> The network device we present is only a portion of the DSC's functions.  
> The device configuration and management for the various services is 
> handled in userspace programs on the OS running inside the device.  
> These are accessed through a secured REST API, typically through the 
> external management ethernet port.  In addition to our centralized 
> management user interface, we have a command line tool for managing the 
> device configuration using that same REST interface.

We try to encourage vendors to create common interfaces, as you'd
understand that command line tool is raising red flags.

Admittedly most vendors have some form of command line tool which can
poke directly into registers, anyway, but IMHO we should avoid any
precedents of merging driver patches with explicit goal of enabling
such tools.

> In some configurations we make it possible to open a network connection 
> into the device through the host PCI, just as if you were to connect 
> through the external mgmt port.  This is the PCI deviceid that 
> corresponds to that port, and allows use of the command line tool on the 
> host.
> 
> The host network driver doesn't have access to the device management 
> commands, it only can configure the NIC portion for what it needs for 
> passing network packets.

Re: [PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Shannon Nelson]

On 3/5/20 5:18 PM, Jakub Kicinski wrote:
> On Thu, 5 Mar 2020 16:41:48 -0800 Shannon Nelson wrote:
>> On 3/5/20 2:03 PM, Jakub Kicinski wrote:
>>> On Wed,  4 Mar 2020 21:23:18 -0800 Shannon Nelson wrote:
>>>> Add support for an additional device id.
>>>>
>>>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>>> I have thought about this for a while and I wanted to ask you to say
>>> a bit more about the use of the management device.
>>>
>>> Obviously this is not just "additional device id" in the traditional
>>> sense where device IDs differentiate HW SKUs or revisions. This is the
>>> same exact hardware, just a different local feature (as proven by the
>>> fact that you make 0 functional changes).
>>>
>>> In the past we (I?) rejected such extensions upstream from Netronome and
>>> Cavium, because there were no clear use cases which can't be solved by
>>> extending standard kernel APIs. Do you have any?
>> Do you by chance have any references handy to such past discussions?
>> I'd be interested in reading them to see what similarities and
>> differences we have.
> Here you go:
>
> https://lore.kernel.org/netdev/20170718115827.7bd737f2@cakuba.netronome.com/

Interesting - thanks.

>
>> The network device we present is only a portion of the DSC's functions.
>> The device configuration and management for the various services is
>> handled in userspace programs on the OS running inside the device.
>> These are accessed through a secured REST API, typically through the
>> external management ethernet port.  In addition to our centralized
>> management user interface, we have a command line tool for managing the
>> device configuration using that same REST interface.
> We try to encourage vendors to create common interfaces, as you'd
> understand that command line tool is raising red flags.
>
> Admittedly most vendors have some form of command line tool which can
> poke directly into registers, anyway, but IMHO we should avoid any
> precedents of merging driver patches with explicit goal of enabling
> such tools.

Yes, and if we were just writing registers, that would make sense. When 
I can get to it I do intend to try expand our use of the devlink 
interfaces where it will work for us.

However, this device id does exist on some of the DSC configurations, 
and I'd prefer to explicitly acknowledge its existence in the driver and 
perhaps keep better control over it, whether or not it gets used by our 
3rd party tool, rather than leave it as some obscure port for someone to 
"discover".

sln

>
>> In some configurations we make it possible to open a network connection
>> into the device through the host PCI, just as if you were to connect
>> through the external mgmt port.  This is the PCI deviceid that
>> corresponds to that port, and allows use of the command line tool on the
>> host.
>>
>> The host network driver doesn't have access to the device management
>> commands, it only can configure the NIC portion for what it needs for
>> passing network packets.

Re: [PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Jakub Kicinski]

On Thu, 5 Mar 2020 23:43:44 -0800 Shannon Nelson wrote:
> Yes, and if we were just writing registers, that would make sense. When 
> I can get to it I do intend to try expand our use of the devlink 
> interfaces where it will work for us.

Yes, please do that.

> However, this device id does exist on some of the DSC configurations, 
> and I'd prefer to explicitly acknowledge its existence in the driver and 
> perhaps keep better control over it, whether or not it gets used by our 
> 3rd party tool, rather than leave it as some obscure port for someone to 
> "discover".

I understand, but disagree. Your driver can certainly bind to that
management device but it has to be for the internal use of the kernel.
You shouldn't just expose that FW interface right out to user space as 
a netdev.

Re: [PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Shannon Nelson]

On 3/6/20 10:20 AM, Jakub Kicinski wrote:
> On Thu, 5 Mar 2020 23:43:44 -0800 Shannon Nelson wrote:
>> Yes, and if we were just writing registers, that would make sense. When
>> I can get to it I do intend to try expand our use of the devlink
>> interfaces where it will work for us.
> Yes, please do that.
>
>> However, this device id does exist on some of the DSC configurations,
>> and I'd prefer to explicitly acknowledge its existence in the driver and
>> perhaps keep better control over it, whether or not it gets used by our
>> 3rd party tool, rather than leave it as some obscure port for someone to
>> "discover".
> I understand, but disagree. Your driver can certainly bind to that
> management device but it has to be for the internal use of the kernel.
> You shouldn't just expose that FW interface right out to user space as
> a netdev.
So for now the driver should simply capture and configure the PCI 
device, but stop at that point and not setup a netdev.  This would leave 
the device available for devlink commands.

If that sounds reasonable to you, I'll add it and respin the patchset.

sln

Re: [PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Jakub Kicinski]

On Fri, 6 Mar 2020 12:32:51 -0800 Shannon Nelson wrote:
> >> However, this device id does exist on some of the DSC configurations,
> >> and I'd prefer to explicitly acknowledge its existence in the driver and
> >> perhaps keep better control over it, whether or not it gets used by our
> >> 3rd party tool, rather than leave it as some obscure port for someone to
> >> "discover".  
> > I understand, but disagree. Your driver can certainly bind to that
> > management device but it has to be for the internal use of the kernel.
> > You shouldn't just expose that FW interface right out to user space as
> > a netdev.  
> 
> So for now the driver should simply capture and configure the PCI 
> device, but stop at that point and not setup a netdev.  This would leave 
> the device available for devlink commands.
> 
> If that sounds reasonable to you, I'll add it and respin the patchset.

I presume the driver currently creates a devlink instance per PCI
function? (Given we have no real infrastructure in place to combine
them.) It still feels a little strange to have a devlink instance that
doesn't represent any entity user would care about, but a communication
channel. It'd be better if other functions made use of the
communication channel behind the scene. That said AFAIU driver with just
a devlink instance won't allow passing arbitrary commands, so that would
indeed address my biggest concern.

What operations would that devlink instance expose?

Re: [PATCH v3 net-next 7/8] ionic: add support for device id 0x1004

[Shannon Nelson]

On 3/6/20 1:28 PM, Jakub Kicinski wrote:
> On Fri, 6 Mar 2020 12:32:51 -0800 Shannon Nelson wrote:
>>>> However, this device id does exist on some of the DSC configurations,
>>>> and I'd prefer to explicitly acknowledge its existence in the driver and
>>>> perhaps keep better control over it, whether or not it gets used by our
>>>> 3rd party tool, rather than leave it as some obscure port for someone to
>>>> "discover".
>>> I understand, but disagree. Your driver can certainly bind to that
>>> management device but it has to be for the internal use of the kernel.
>>> You shouldn't just expose that FW interface right out to user space as
>>> a netdev.
>> So for now the driver should simply capture and configure the PCI
>> device, but stop at that point and not setup a netdev.  This would leave
>> the device available for devlink commands.
>>
>> If that sounds reasonable to you, I'll add it and respin the patchset.
> I presume the driver currently creates a devlink instance per PCI
> function? (Given we have no real infrastructure in place to combine
> them.) It still feels a little strange to have a devlink instance that
> doesn't represent any entity user would care about, but a communication
> channel. It'd be better if other functions made use of the
> communication channel behind the scene. That said AFAIU driver with just
> a devlink instance won't allow passing arbitrary commands, so that would
> indeed address my biggest concern.
>
> What operations would that devlink instance expose?

Being as this is still a new idea for us and we aren't up to speed yet 
on what all devlink offers, I don't have a good answer at the moment.  
For now, nothing more than already exposed, which is simple device info.

sln