[PATCH net 0/7] ionic: small driver fixes

[PATCH net 0/7] ionic: small driver fixes

[Shannon Nelson]

This is a collection of small fixes for the ionic driver,
mostly code cleanup items.

Brett Creeley (5):
  ionic: Use cached VF attributes
  ionic: Don't check null when calling vfree()
  ionic: Make the check for Tx HW timestamping more obvious
  ionic: Fix dim work handling in split interrupt mode
  ionic: Re-arrange ionic_intr_info struct for cache perf

Shannon Nelson (2):
  ionic: fix snprintf format length warning
  ionic: set ionic ptr before setting up ethtool ops

 drivers/net/ethernet/pensando/ionic/ionic.h   |   2 -
 .../net/ethernet/pensando/ionic/ionic_dev.c   |  40 ------
 .../net/ethernet/pensando/ionic/ionic_dev.h   |   9 +-
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 123 ++++--------------
 .../net/ethernet/pensando/ionic/ionic_lif.h   |   5 +
 .../net/ethernet/pensando/ionic/ionic_main.c  |  22 ----
 .../net/ethernet/pensando/ionic/ionic_txrx.c  |  10 +-
 7 files changed, 37 insertions(+), 174 deletions(-)

-- 
2.17.1

[PATCH net 1/7] ionic: fix snprintf format length warning

[Shannon Nelson]

Our friendly kernel test robot has reminded us that with a new
check we have a warning about a potential string truncation.
In this case it really doesn't hurt anything, but it is worth
addressing especially since there really is no reason to reserve
so many bytes for our queue names.  It seems that cutting the
queue name buffer length in half stops the complaint.

Fixes: c06107cabea3 ("ionic: more ionic name tweaks")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202311300201.lO8v7mKU-lkp@intel.com/
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_dev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 1dbc3cb50b1d..9b5463040075 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -223,7 +223,7 @@ struct ionic_desc_info {
 	void *cb_arg;
 };
 
-#define IONIC_QUEUE_NAME_MAX_SZ		32
+#define IONIC_QUEUE_NAME_MAX_SZ		16
 
 struct ionic_queue {
 	struct device *dev;
-- 
2.17.1

[PATCH net 2/7] ionic: Use cached VF attributes

[Shannon Nelson]

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

Each time a VF attribute is set via iproute a call to get the VF
configuration is also made. This is currently problematic because for
each VF configuration call there are multiple commands sent to the
device. Unfortunately, this doesn't scale well. Fix this by reporting
the cached VF attributes.

The original change to query the device for getting the VF attributes
was made to remain consistent with device set VF attributes. However,
after further investigation there is no need to query the device.

Fixes: f16f5be31009 ("ionic: Query FW when getting VF info via ndo_get_vf_config")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic.h   |  2 -
 .../net/ethernet/pensando/ionic/ionic_dev.c   | 40 --------
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  3 +-
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 93 ++-----------------
 .../net/ethernet/pensando/ionic/ionic_main.c  | 22 -----
 5 files changed, 11 insertions(+), 149 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
index 2453a40f6ee8..9ffef2e06885 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic.h
@@ -91,6 +91,4 @@ int ionic_port_identify(struct ionic *ionic);
 int ionic_port_init(struct ionic *ionic);
 int ionic_port_reset(struct ionic *ionic);
 
-const char *ionic_vf_attr_to_str(enum ionic_vf_attr attr);
-
 #endif /* _IONIC_H_ */
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index c06576f43916..bb9245d933e4 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -469,46 +469,6 @@ int ionic_set_vf_config(struct ionic *ionic, int vf,
 	return err;
 }
 
-int ionic_dev_cmd_vf_getattr(struct ionic *ionic, int vf, u8 attr,
-			     struct ionic_vf_getattr_comp *comp)
-{
-	union ionic_dev_cmd cmd = {
-		.vf_getattr.opcode = IONIC_CMD_VF_GETATTR,
-		.vf_getattr.attr = attr,
-		.vf_getattr.vf_index = cpu_to_le16(vf),
-	};
-	int err;
-
-	if (vf >= ionic->num_vfs)
-		return -EINVAL;
-
-	switch (attr) {
-	case IONIC_VF_ATTR_SPOOFCHK:
-	case IONIC_VF_ATTR_TRUST:
-	case IONIC_VF_ATTR_LINKSTATE:
-	case IONIC_VF_ATTR_MAC:
-	case IONIC_VF_ATTR_VLAN:
-	case IONIC_VF_ATTR_RATE:
-		break;
-	case IONIC_VF_ATTR_STATSADDR:
-	default:
-		return -EINVAL;
-	}
-
-	mutex_lock(&ionic->dev_cmd_lock);
-	ionic_dev_cmd_go(&ionic->idev, &cmd);
-	err = ionic_dev_cmd_wait_nomsg(ionic, DEVCMD_TIMEOUT);
-	memcpy_fromio(comp, &ionic->idev.dev_cmd_regs->comp.vf_getattr,
-		      sizeof(*comp));
-	mutex_unlock(&ionic->dev_cmd_lock);
-
-	if (err && comp->status != IONIC_RC_ENOSUPP)
-		ionic_dev_cmd_dev_err_print(ionic, cmd.vf_getattr.opcode,
-					    comp->status, err);
-
-	return err;
-}
-
 void ionic_vf_start(struct ionic *ionic)
 {
 	union ionic_dev_cmd cmd = {
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 9b5463040075..745a3292be92 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -341,8 +341,7 @@ void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type);
 
 int ionic_set_vf_config(struct ionic *ionic, int vf,
 			struct ionic_vf_setattr_cmd *vfc);
-int ionic_dev_cmd_vf_getattr(struct ionic *ionic, int vf, u8 attr,
-			     struct ionic_vf_getattr_comp *comp);
+
 void ionic_dev_cmd_queue_identify(struct ionic_dev *idev,
 				  u16 lif_type, u8 qtype, u8 qver);
 void ionic_vf_start(struct ionic *ionic);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index edc14730ce88..afb77e2d04c5 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -2332,82 +2332,11 @@ static int ionic_eth_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd
 	}
 }
 
-static int ionic_get_fw_vf_config(struct ionic *ionic, int vf, struct ionic_vf *vfdata)
-{
-	struct ionic_vf_getattr_comp comp = { 0 };
-	int err;
-	u8 attr;
-
-	attr = IONIC_VF_ATTR_VLAN;
-	err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp);
-	if (err && comp.status != IONIC_RC_ENOSUPP)
-		goto err_out;
-	if (!err)
-		vfdata->vlanid = comp.vlanid;
-
-	attr = IONIC_VF_ATTR_SPOOFCHK;
-	err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp);
-	if (err && comp.status != IONIC_RC_ENOSUPP)
-		goto err_out;
-	if (!err)
-		vfdata->spoofchk = comp.spoofchk;
-
-	attr = IONIC_VF_ATTR_LINKSTATE;
-	err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp);
-	if (err && comp.status != IONIC_RC_ENOSUPP)
-		goto err_out;
-	if (!err) {
-		switch (comp.linkstate) {
-		case IONIC_VF_LINK_STATUS_UP:
-			vfdata->linkstate = IFLA_VF_LINK_STATE_ENABLE;
-			break;
-		case IONIC_VF_LINK_STATUS_DOWN:
-			vfdata->linkstate = IFLA_VF_LINK_STATE_DISABLE;
-			break;
-		case IONIC_VF_LINK_STATUS_AUTO:
-			vfdata->linkstate = IFLA_VF_LINK_STATE_AUTO;
-			break;
-		default:
-			dev_warn(ionic->dev, "Unexpected link state %u\n", comp.linkstate);
-			break;
-		}
-	}
-
-	attr = IONIC_VF_ATTR_RATE;
-	err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp);
-	if (err && comp.status != IONIC_RC_ENOSUPP)
-		goto err_out;
-	if (!err)
-		vfdata->maxrate = comp.maxrate;
-
-	attr = IONIC_VF_ATTR_TRUST;
-	err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp);
-	if (err && comp.status != IONIC_RC_ENOSUPP)
-		goto err_out;
-	if (!err)
-		vfdata->trusted = comp.trust;
-
-	attr = IONIC_VF_ATTR_MAC;
-	err = ionic_dev_cmd_vf_getattr(ionic, vf, attr, &comp);
-	if (err && comp.status != IONIC_RC_ENOSUPP)
-		goto err_out;
-	if (!err)
-		ether_addr_copy(vfdata->macaddr, comp.macaddr);
-
-err_out:
-	if (err)
-		dev_err(ionic->dev, "Failed to get %s for VF %d\n",
-			ionic_vf_attr_to_str(attr), vf);
-
-	return err;
-}
-
 static int ionic_get_vf_config(struct net_device *netdev,
 			       int vf, struct ifla_vf_info *ivf)
 {
 	struct ionic_lif *lif = netdev_priv(netdev);
 	struct ionic *ionic = lif->ionic;
-	struct ionic_vf vfdata = { 0 };
 	int ret = 0;
 
 	if (!netif_device_present(netdev))
@@ -2418,18 +2347,16 @@ static int ionic_get_vf_config(struct net_device *netdev,
 	if (vf >= pci_num_vf(ionic->pdev) || !ionic->vfs) {
 		ret = -EINVAL;
 	} else {
-		ivf->vf = vf;
-		ivf->qos = 0;
-
-		ret = ionic_get_fw_vf_config(ionic, vf, &vfdata);
-		if (!ret) {
-			ivf->vlan         = le16_to_cpu(vfdata.vlanid);
-			ivf->spoofchk     = vfdata.spoofchk;
-			ivf->linkstate    = vfdata.linkstate;
-			ivf->max_tx_rate  = le32_to_cpu(vfdata.maxrate);
-			ivf->trusted      = vfdata.trusted;
-			ether_addr_copy(ivf->mac, vfdata.macaddr);
-		}
+		struct ionic_vf *vfdata = &ionic->vfs[vf];
+
+		ivf->vf		  = vf;
+		ivf->qos	  = 0;
+		ivf->vlan         = le16_to_cpu(vfdata->vlanid);
+		ivf->spoofchk     = vfdata->spoofchk;
+		ivf->linkstate    = vfdata->linkstate;
+		ivf->max_tx_rate  = le32_to_cpu(vfdata->maxrate);
+		ivf->trusted      = vfdata->trusted;
+		ether_addr_copy(ivf->mac, vfdata->macaddr);
 	}
 
 	up_read(&ionic->vf_op_lock);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 835577392178..8d15f9203bd5 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -188,28 +188,6 @@ static const char *ionic_opcode_to_str(enum ionic_cmd_opcode opcode)
 	}
 }
 
-const char *ionic_vf_attr_to_str(enum ionic_vf_attr attr)
-{
-	switch (attr) {
-	case IONIC_VF_ATTR_SPOOFCHK:
-		return "IONIC_VF_ATTR_SPOOFCHK";
-	case IONIC_VF_ATTR_TRUST:
-		return "IONIC_VF_ATTR_TRUST";
-	case IONIC_VF_ATTR_LINKSTATE:
-		return "IONIC_VF_ATTR_LINKSTATE";
-	case IONIC_VF_ATTR_MAC:
-		return "IONIC_VF_ATTR_MAC";
-	case IONIC_VF_ATTR_VLAN:
-		return "IONIC_VF_ATTR_VLAN";
-	case IONIC_VF_ATTR_RATE:
-		return "IONIC_VF_ATTR_RATE";
-	case IONIC_VF_ATTR_STATSADDR:
-		return "IONIC_VF_ATTR_STATSADDR";
-	default:
-		return "IONIC_VF_ATTR_UNKNOWN";
-	}
-}
-
 static void ionic_adminq_flush(struct ionic_lif *lif)
 {
 	struct ionic_desc_info *desc_info;
-- 
2.17.1

[PATCH net 3/7] ionic: set ionic ptr before setting up ethtool ops

[Shannon Nelson]

To be sure there are no race conditions in startup, make sure
that the lif->ionic value used in some ethtool callbacks is
defined before setting ethtool ops.

Fixes: 1a58e196467f ("ionic: Add basic lif support")
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index afb77e2d04c5..a5e6b1e2f5ee 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -3054,6 +3054,7 @@ int ionic_lif_alloc(struct ionic *ionic)
 	lif = netdev_priv(netdev);
 	lif->netdev = netdev;
 	ionic->lif = lif;
+	lif->ionic = ionic;
 	netdev->netdev_ops = &ionic_netdev_ops;
 	ionic_ethtool_set_ops(netdev);
 
@@ -3076,7 +3077,6 @@ int ionic_lif_alloc(struct ionic *ionic)
 	lif->neqs = ionic->neqs_per_lif;
 	lif->nxqs = ionic->ntxqs_per_lif;
 
-	lif->ionic = ionic;
 	lif->index = 0;
 
 	if (is_kdump_kernel()) {
-- 
2.17.1

[PATCH net 4/7] ionic: Don't check null when calling vfree()

[Shannon Nelson]

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

vfree() checks for null internally, so there's no need to
check in the caller. So, always vfree() on variables
allocated with valloc(). If the variables are never
alloc'd vfree() is still safe.

Fixes: 116dce0ff047 ("ionic: Use vzalloc for large per-queue related buffers")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index a5e6b1e2f5ee..6842a31fc04b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -424,14 +424,10 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
 
 	ionic_qcq_intr_free(lif, qcq);
 
-	if (qcq->cq.info) {
-		vfree(qcq->cq.info);
-		qcq->cq.info = NULL;
-	}
-	if (qcq->q.info) {
-		vfree(qcq->q.info);
-		qcq->q.info = NULL;
-	}
+	vfree(qcq->cq.info);
+	qcq->cq.info = NULL;
+	vfree(qcq->q.info);
+	qcq->q.info = NULL;
 }
 
 void ionic_qcqs_free(struct ionic_lif *lif)
-- 
2.17.1

[PATCH net 5/7] ionic: Make the check for Tx HW timestamping more obvious

[Shannon Nelson]

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

Currently the checks are:

[1] unlikely(q->features & IONIC_TXQ_F_HWSTAMP)
[2] !unlikely(q->features & IONIC_TXQ_F_HWSTAMP)

[1] is clear enough, but [2] isn't exactly obvious to the
reader because !unlikely reads as likely. However, that's
not what this means.

[2] means that it's unlikely that the q has
IONIC_TXQ_F_HWSTAMP enabled.

Write an inline helper function to hide the unlikely
optimization to make these checks more readable.

Fixes: a8771bfe0554 ("ionic: add and enable tx and rx timestamp handling")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.h  |  5 +++++
 drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 10 +++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index 457c24195ca6..61548b3eea93 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -312,6 +312,11 @@ static inline u32 ionic_coal_usec_to_hw(struct ionic *ionic, u32 usecs)
 	return (usecs * mult) / div;
 }
 
+static inline bool ionic_txq_hwstamp_enabled(struct ionic_queue *q)
+{
+	return unlikely(q->features & IONIC_TXQ_F_HWSTAMP);
+}
+
 void ionic_link_status_check_request(struct ionic_lif *lif, bool can_sleep);
 void ionic_get_stats64(struct net_device *netdev,
 		       struct rtnl_link_stats64 *ns);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index ccc1b1d407e4..54cd96b035d6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -803,7 +803,7 @@ static void ionic_tx_clean(struct ionic_queue *q,
 
 	qi = skb_get_queue_mapping(skb);
 
-	if (unlikely(q->features & IONIC_TXQ_F_HWSTAMP)) {
+	if (ionic_txq_hwstamp_enabled(q)) {
 		if (cq_info) {
 			struct skb_shared_hwtstamps hwts = {};
 			__le64 *cq_desc_hwstamp;
@@ -870,7 +870,7 @@ bool ionic_tx_service(struct ionic_cq *cq, struct ionic_cq_info *cq_info)
 		desc_info->cb_arg = NULL;
 	} while (index != le16_to_cpu(comp->comp_index));
 
-	if (pkts && bytes && !unlikely(q->features & IONIC_TXQ_F_HWSTAMP))
+	if (pkts && bytes && !ionic_txq_hwstamp_enabled(q))
 		netdev_tx_completed_queue(q_to_ndq(q), pkts, bytes);
 
 	return true;
@@ -908,7 +908,7 @@ void ionic_tx_empty(struct ionic_queue *q)
 		desc_info->cb_arg = NULL;
 	}
 
-	if (pkts && bytes && !unlikely(q->features & IONIC_TXQ_F_HWSTAMP))
+	if (pkts && bytes && !ionic_txq_hwstamp_enabled(q))
 		netdev_tx_completed_queue(q_to_ndq(q), pkts, bytes);
 }
 
@@ -986,7 +986,7 @@ static void ionic_tx_tso_post(struct ionic_queue *q,
 
 	if (start) {
 		skb_tx_timestamp(skb);
-		if (!unlikely(q->features & IONIC_TXQ_F_HWSTAMP))
+		if (!ionic_txq_hwstamp_enabled(q))
 			netdev_tx_sent_queue(q_to_ndq(q), skb->len);
 		ionic_txq_post(q, false, ionic_tx_clean, skb);
 	} else {
@@ -1233,7 +1233,7 @@ static int ionic_tx(struct ionic_queue *q, struct sk_buff *skb)
 	stats->pkts++;
 	stats->bytes += skb->len;
 
-	if (!unlikely(q->features & IONIC_TXQ_F_HWSTAMP))
+	if (!ionic_txq_hwstamp_enabled(q))
 		netdev_tx_sent_queue(q_to_ndq(q), skb->len);
 	ionic_txq_post(q, !netdev_xmit_more(), ionic_tx_clean, skb);
 
-- 
2.17.1

[PATCH net 6/7] ionic: Fix dim work handling in split interrupt mode

[Shannon Nelson]

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

Currently ionic_dim_work() is incorrect when in
split interrupt mode. This is because the interrupt
rate is only being changed for the Rx side even for
dim running on Tx. Fix this by using the qcq from
the container_of macro. Also, introduce some local
variables for a bit of cleanup.

Fixes: a6ff85e0a2d9 ("ionic: remove intr coalesce update from napi")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 6842a31fc04b..3bb0cfc40576 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -49,24 +49,24 @@ static void ionic_lif_queue_identify(struct ionic_lif *lif);
 static void ionic_dim_work(struct work_struct *work)
 {
 	struct dim *dim = container_of(work, struct dim, work);
+	struct ionic_intr_info *intr;
 	struct dim_cq_moder cur_moder;
 	struct ionic_qcq *qcq;
+	struct ionic_lif *lif;
 	u32 new_coal;
 
 	cur_moder = net_dim_get_rx_moderation(dim->mode, dim->profile_ix);
 	qcq = container_of(dim, struct ionic_qcq, dim);
-	new_coal = ionic_coal_usec_to_hw(qcq->q.lif->ionic, cur_moder.usec);
+	lif = qcq->q.lif;
+	new_coal = ionic_coal_usec_to_hw(lif->ionic, cur_moder.usec);
 	new_coal = new_coal ? new_coal : 1;
 
-	if (qcq->intr.dim_coal_hw != new_coal) {
-		unsigned int qi = qcq->cq.bound_q->index;
-		struct ionic_lif *lif = qcq->q.lif;
-
-		qcq->intr.dim_coal_hw = new_coal;
+	intr = &qcq->intr;
+	if (intr->dim_coal_hw != new_coal) {
+		intr->dim_coal_hw = new_coal;
 
 		ionic_intr_coal_init(lif->ionic->idev.intr_ctrl,
-				     lif->rxqcqs[qi]->intr.index,
-				     qcq->intr.dim_coal_hw);
+				     intr->index, intr->dim_coal_hw);
 	}
 
 	dim->state = DIM_START_MEASURE;
-- 
2.17.1

[PATCH net 7/7] ionic: Re-arrange ionic_intr_info struct for cache perf

[Shannon Nelson]

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

dim_coal_hw is accessed in the hotpath along with other values
from the first cacheline of ionic_intr_info. So, re-arrange
the structure so the hot path variables are on the first
cacheline.

Before:

struct ionic_intr_info {
	char                       name[32];             /*     0    32 */
	unsigned int               index;                /*    32     4 */
	unsigned int               vector;               /*    36     4 */
	u64                        rearm_count;          /*    40     8 */
	unsigned int               cpu;                  /*    48     4 */

	/* XXX 4 bytes hole, try to pack */

	cpumask_t                  affinity_mask;        /*    56  1024 */
	/* --- cacheline 16 boundary (1024 bytes) was 56 bytes ago --- */
	u32                        dim_coal_hw;          /*  1080     4 */

	/* size: 1088, cachelines: 17, members: 7 */
	/* sum members: 1080, holes: 1, sum holes: 4 */
	/* padding: 4 */
};

After:

struct ionic_intr_info {
	char                       name[32];             /*     0    32 */
	u64                        rearm_count;          /*    32     8 */
	unsigned int               index;                /*    40     4 */
	unsigned int               vector;               /*    44     4 */
	unsigned int               cpu;                  /*    48     4 */
	u32                        dim_coal_hw;          /*    52     4 */
	cpumask_t                  affinity_mask;        /*    56  1024 */

	/* size: 1080, cachelines: 17, members: 7 */
	/* last cacheline: 56 bytes */
};


Fixes: 04a834592bf5 ("ionic: dynamic interrupt moderation")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_dev.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 745a3292be92..c47c059465ae 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -269,12 +269,12 @@ struct ionic_queue {
 
 struct ionic_intr_info {
 	char name[IONIC_INTR_NAME_MAX_SZ];
+	u64 rearm_count;
 	unsigned int index;
 	unsigned int vector;
-	u64 rearm_count;
 	unsigned int cpu;
-	cpumask_t affinity_mask;
 	u32 dim_coal_hw;
+	cpumask_t affinity_mask;
 };
 
 struct ionic_cq {
-- 
2.17.1

Re: [PATCH net 0/7] ionic: small driver fixes

[Florian Fainelli]

On 11/30/2023 4:05 PM, Shannon Nelson wrote:
> This is a collection of small fixes for the ionic driver,
> mostly code cleanup items.
> 
> Brett Creeley (5):
>    ionic: Use cached VF attributes
>    ionic: Don't check null when calling vfree()
>    ionic: Make the check for Tx HW timestamping more obvious
>    ionic: Fix dim work handling in split interrupt mode
>    ionic: Re-arrange ionic_intr_info struct for cache perf
> 
> Shannon Nelson (2):
>    ionic: fix snprintf format length warning
>    ionic: set ionic ptr before setting up ethtool ops

FWIW, for the whole series:

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>

Clear and concise commit messages, nice!
-- 
Florian

Re: [PATCH net 0/7] ionic: small driver fixes

[Nelson, Shannon]

On 11/30/2023 8:19 PM, Florian Fainelli wrote:
> 
> On 11/30/2023 4:05 PM, Shannon Nelson wrote:
>> This is a collection of small fixes for the ionic driver,
>> mostly code cleanup items.
>>
>> Brett Creeley (5):
>>    ionic: Use cached VF attributes
>>    ionic: Don't check null when calling vfree()
>>    ionic: Make the check for Tx HW timestamping more obvious
>>    ionic: Fix dim work handling in split interrupt mode
>>    ionic: Re-arrange ionic_intr_info struct for cache perf
>>
>> Shannon Nelson (2):
>>    ionic: fix snprintf format length warning
>>    ionic: set ionic ptr before setting up ethtool ops
> 
> FWIW, for the whole series:
> 
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> 
> Clear and concise commit messages, nice!

Thanks, Florian
sln


> -- 
> Florian

Re: [PATCH net 0/7] ionic: small driver fixes

[Jakub Kicinski]

On Thu, 30 Nov 2023 16:05:12 -0800 Shannon Nelson wrote:
> This is a collection of small fixes for the ionic driver,
> mostly code cleanup items.

Hm, looks cleanup-y indeed. Majority of this looks like net-next
material, really.

1 - fine for net
2 - perf optimization, we generally follow stable rules, which say:

   Serious issues as reported by a user of a distribution kernel may
   also be considered if they fix a notable performance or
   interactivity issue. As these fixes are not as obvious and have a
   higher risk of a subtle regression they should only be submitted by
   a distribution kernel maintainer and include an addendum linking to
   a bugzilla entry if it exists and additional information on the
   user-visible impact.

I doubt serious "user-visible impact" will be the case here, however.

3 - I don't see how this matters, netdev is not registered, locks are
    not initialized, who's going to access that pointer?
4 - cleanup / nop
5 - cleanup / nop
6 - fine for net
7 - optimization and a minor one at that

I appreciate the diligent Fixes tags but I'm afraid we need to be a bit
more judicious in what we consider a fix.
-- 
pw-bot: cr

Re: [PATCH net 0/7] ionic: small driver fixes

[Nelson, Shannon]

On 12/1/2023 5:42 PM, Jakub Kicinski wrote:
> 
> On Thu, 30 Nov 2023 16:05:12 -0800 Shannon Nelson wrote:
>> This is a collection of small fixes for the ionic driver,
>> mostly code cleanup items.
> 
> Hm, looks cleanup-y indeed. Majority of this looks like net-next
> material, really.
> 
> 1 - fine for net
> 2 - perf optimization, we generally follow stable rules, which say:
> 
>     Serious issues as reported by a user of a distribution kernel may
>     also be considered if they fix a notable performance or
>     interactivity issue. As these fixes are not as obvious and have a
>     higher risk of a subtle regression they should only be submitted by
>     a distribution kernel maintainer and include an addendum linking to
>     a bugzilla entry if it exists and additional information on the
>     user-visible impact.
> 
> I doubt serious "user-visible impact" will be the case here, however.
> 
> 3 - I don't see how this matters, netdev is not registered, locks are
>      not initialized, who's going to access that pointer?
> 4 - cleanup / nop
> 5 - cleanup / nop
> 6 - fine for net
> 7 - optimization and a minor one at that
> 
> I appreciate the diligent Fixes tags but I'm afraid we need to be a bit
> more judicious in what we consider a fix.

Okay, you would prefer most of these as net-next, and you like the Fixes 
tags, but normally if there are Fixes tags they should be in net.  So, 
do you want the Fixes tags removed when these patches are sent to net-next?

sln


> --
> pw-bot: cr

Re: [PATCH net 0/7] ionic: small driver fixes

[Jakub Kicinski]

On Mon, 4 Dec 2023 10:23:40 -0800 Nelson, Shannon wrote:
> Okay, you would prefer most of these as net-next, and you like the Fixes 
> tags, but normally if there are Fixes tags they should be in net.  So, 
> do you want the Fixes tags removed when these patches are sent to net-next?

Yes, that's right. What goes to net-next should have Fixes tag stripped.
If you want to preserve the reference for your own needs, you can say
something like:

This code was introduced in commit ...

Just not a bona fide Fixes tag, please.