[PATCH net-next 0/3] qede: add netpoll and per-queue coalesce support

[PATCH net-next 0/3] qede: add netpoll and per-queue coalesce support

[Bhaskar Upadhaya]

Patch 1: Add net poll controller support to transmit kernel printks
         over UDP.
Patch 2: QLogic card support multiple queues and each queue can be
         configured with respective coalescing parameters, this patch
         add per queue rx-usecs, tx-usecs coalescing parameters.
Patch 3: set default per queue rx-usecs, tx-usecs coalescing parameters.

Bhaskar Upadhaya (3):
  qede: add netpoll support for qede driver
  qede: add per queue coalesce support for qede driver
  qede: set default per queue rx/tx usecs coalescing parameters

 drivers/net/ethernet/qlogic/qede/qede.h       |   5 +
 .../net/ethernet/qlogic/qede/qede_ethtool.c   | 126 +++++++++++++++++-
 drivers/net/ethernet/qlogic/qede/qede_fp.c    |  14 ++
 drivers/net/ethernet/qlogic/qede/qede_main.c  |   7 +
 4 files changed, 150 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH net-next 1/3] qede: add netpoll support for qede driver

[Bhaskar Upadhaya]

Add net poll controller support to transmit kernel printks
over UDP

Signed-off-by: Bhaskar Upadhaya <bupadhaya@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qede/qede.h      |  4 ++++
 drivers/net/ethernet/qlogic/qede/qede_fp.c   | 14 ++++++++++++++
 drivers/net/ethernet/qlogic/qede/qede_main.c |  3 +++
 3 files changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index 3efc5899f656..ac12e5beb596 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -582,6 +582,10 @@ int qede_add_tc_flower_fltr(struct qede_dev *edev, __be16 proto,
 
 void qede_forced_speed_maps_init(void);
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+void qede_poll_controller(struct net_device *dev);
+#endif
+
 #define RX_RING_SIZE_POW	13
 #define RX_RING_SIZE		((u16)BIT(RX_RING_SIZE_POW))
 #define NUM_RX_BDS_MAX		(RX_RING_SIZE - 1)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index a2494bf85007..a626f1f45212 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -1804,3 +1804,17 @@ netdev_features_t qede_features_check(struct sk_buff *skb,
 
 	return features;
 }
+
+#ifdef CONFIG_NET_POLL_CONTROLLER
+/* This is used by netconsole to send skbs without having to re-enable
+ * interrupts.It's not called while the normal interrupt routine is executing.
+ */
+void qede_poll_controller(struct net_device *dev)
+{
+	struct qede_dev *edev = netdev_priv(dev);
+	int i;
+
+	for_each_queue(i)
+		napi_schedule(&edev->fp_array[i].napi);
+}
+#endif
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 05e3a3b60269..2ff6c49de745 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -644,6 +644,9 @@ static const struct net_device_ops qede_netdev_ops = {
 	.ndo_set_rx_mode	= qede_set_rx_mode,
 	.ndo_set_mac_address	= qede_set_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	.ndo_poll_controller = qede_poll_controller,
+#endif
 	.ndo_change_mtu		= qede_change_mtu,
 	.ndo_do_ioctl		= qede_ioctl,
 	.ndo_tx_timeout		= qede_tx_timeout,
-- 
2.17.1

[PATCH net-next 2/3] qede: add per queue coalesce support for qede driver

[Bhaskar Upadhaya]

per queue coalescing allows better and more finegrained control
over interrupt rates.

Signed-off-by: Bhaskar Upadhaya <bupadhaya@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 .../net/ethernet/qlogic/qede/qede_ethtool.c   | 123 ++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index bedbb85a179a..522736968496 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -2105,6 +2105,125 @@ static int qede_get_dump_data(struct net_device *dev,
 	return rc;
 }
 
+static int qede_set_per_coalesce(struct net_device *dev,
+				 u32 queue,
+				 struct ethtool_coalesce *coal)
+{
+	struct qede_dev *edev = netdev_priv(dev);
+	struct qede_fastpath *fp;
+	u16 rxc, txc;
+	int rc = 0;
+
+	if (coal->rx_coalesce_usecs > QED_COALESCE_MAX ||
+	    coal->tx_coalesce_usecs > QED_COALESCE_MAX) {
+		DP_INFO(edev,
+			"Can't support requested %s coalesce value [max supported value %d]\n",
+			coal->rx_coalesce_usecs > QED_COALESCE_MAX ? "rx"
+								   : "tx",
+			QED_COALESCE_MAX);
+		return -EINVAL;
+	}
+
+	rxc = (u16)coal->rx_coalesce_usecs;
+	txc = (u16)coal->tx_coalesce_usecs;
+
+	__qede_lock(edev);
+	if (queue >= edev->num_queues) {
+		DP_INFO(edev, "Invalid queue\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (edev->state != QEDE_STATE_OPEN) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+	fp = &edev->fp_array[queue];
+
+	if (edev->fp_array[queue].type & QEDE_FASTPATH_RX) {
+		rc = edev->ops->common->set_coalesce(edev->cdev,
+						     rxc, 0,
+						     fp->rxq->handle);
+		if (rc) {
+			DP_INFO(edev,
+				"Set RX coalesce error, rc = %d\n", rc);
+			goto out;
+		}
+	}
+
+	if (edev->fp_array[queue].type & QEDE_FASTPATH_TX) {
+		rc = edev->ops->common->set_coalesce(edev->cdev,
+						     0, txc,
+						     fp->txq->handle);
+		if (rc) {
+			DP_INFO(edev,
+				"Set TX coalesce error, rc = %d\n", rc);
+			goto out;
+		}
+	}
+out:
+	__qede_unlock(edev);
+
+	return rc;
+}
+
+static int qede_get_per_coalesce(struct net_device *dev,
+				 u32 queue,
+				 struct ethtool_coalesce *coal)
+{
+	void *rx_handle = NULL, *tx_handle = NULL;
+	struct qede_dev *edev = netdev_priv(dev);
+	u16 rx_coal, tx_coal, rc = 0;
+	struct qede_fastpath *fp;
+
+	rx_coal = QED_DEFAULT_RX_USECS;
+	tx_coal = QED_DEFAULT_TX_USECS;
+
+	memset(coal, 0, sizeof(struct ethtool_coalesce));
+
+	__qede_lock(edev);
+	if (queue >= edev->num_queues) {
+		DP_INFO(edev, "Invalid queue\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (edev->state != QEDE_STATE_OPEN) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+	fp = &edev->fp_array[queue];
+
+	if (fp->type & QEDE_FASTPATH_RX)
+		rx_handle = fp->rxq->handle;
+
+	rc = edev->ops->get_coalesce(edev->cdev, &rx_coal,
+				     rx_handle);
+	if (rc) {
+		DP_INFO(edev, "Read Rx coalesce error\n");
+		goto out;
+	}
+
+	fp = &edev->fp_array[queue];
+	if (fp->type & QEDE_FASTPATH_TX)
+		tx_handle = fp->txq->handle;
+
+	rc = edev->ops->get_coalesce(edev->cdev, &tx_coal,
+				      tx_handle);
+	if (rc)
+		DP_INFO(edev, "Read Tx coalesce error\n");
+
+out:
+	__qede_unlock(edev);
+
+	coal->rx_coalesce_usecs = rx_coal;
+	coal->tx_coalesce_usecs = tx_coal;
+
+	return rc;
+}
+
 static const struct ethtool_ops qede_ethtool_ops = {
 	.supported_coalesce_params	= ETHTOOL_COALESCE_USECS,
 	.get_link_ksettings		= qede_get_link_ksettings,
@@ -2148,6 +2267,8 @@ static const struct ethtool_ops qede_ethtool_ops = {
 	.set_fecparam			= qede_set_fecparam,
 	.get_tunable			= qede_get_tunable,
 	.set_tunable			= qede_set_tunable,
+	.get_per_queue_coalesce		= qede_get_per_coalesce,
+	.set_per_queue_coalesce		= qede_set_per_coalesce,
 	.flash_device			= qede_flash_device,
 	.get_dump_flag			= qede_get_dump_flag,
 	.get_dump_data			= qede_get_dump_data,
@@ -2177,6 +2298,8 @@ static const struct ethtool_ops qede_vf_ethtool_ops = {
 	.set_rxfh			= qede_set_rxfh,
 	.get_channels			= qede_get_channels,
 	.set_channels			= qede_set_channels,
+	.get_per_queue_coalesce		= qede_get_per_coalesce,
+	.set_per_queue_coalesce		= qede_set_per_coalesce,
 	.get_tunable			= qede_get_tunable,
 	.set_tunable			= qede_set_tunable,
 };
-- 
2.17.1

[PATCH net-next 3/3] qede: set default per queue rx/tx usecs coalescing parameters

[Bhaskar Upadhaya]

Here we do the initialization of coalescing values on load.
Although the default device values are the same - explicit
config is better visible.

Signed-off-by: Bhaskar Upadhaya <bupadhaya@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qede/qede.h         | 1 +
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 3 +--
 drivers/net/ethernet/qlogic/qede/qede_main.c    | 4 ++++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index ac12e5beb596..5270c9226b8b 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -581,6 +581,7 @@ int qede_add_tc_flower_fltr(struct qede_dev *edev, __be16 proto,
 			    struct flow_cls_offload *f);
 
 void qede_forced_speed_maps_init(void);
+int qede_set_coalesce(struct net_device *dev, struct ethtool_coalesce *coal);
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
 void qede_poll_controller(struct net_device *dev);
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 522736968496..e094a6ef299c 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -819,8 +819,7 @@ static int qede_get_coalesce(struct net_device *dev,
 	return rc;
 }
 
-static int qede_set_coalesce(struct net_device *dev,
-			     struct ethtool_coalesce *coal)
+int qede_set_coalesce(struct net_device *dev, struct ethtool_coalesce *coal)
 {
 	struct qede_dev *edev = netdev_priv(dev);
 	struct qede_fastpath *fp;
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 2ff6c49de745..3eb821f55ccb 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2337,6 +2337,7 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode,
 		     bool is_locked)
 {
 	struct qed_link_params link_params;
+	struct ethtool_coalesce coal = {};
 	u8 num_tc;
 	int rc;
 
@@ -2399,6 +2400,9 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode,
 
 	edev->state = QEDE_STATE_OPEN;
 
+	coal.rx_coalesce_usecs = QED_DEFAULT_RX_USECS;
+	coal.tx_coalesce_usecs = QED_DEFAULT_TX_USECS;
+	qede_set_coalesce(edev->ndev, &coal);
 	DP_INFO(edev, "Ending successfully qede load\n");
 
 	goto out;
-- 
2.17.1

Re: [PATCH net-next 1/3] qede: add netpoll support for qede driver

[Jakub Kicinski]

On Fri, 15 Jan 2021 01:06:08 -0800 Bhaskar Upadhaya wrote:
> Add net poll controller support to transmit kernel printks
> over UDP

Why do you need this patch? Couple years back netpoll was taught 
how to pull NAPIs by itself, and all you do is schedule NAPIs.

All the driver should do is to make sure that when napi is called 
with budget of 0 it only processes Tx completions, not Rx traffic.

Re: [PATCH net-next 3/3] qede: set default per queue rx/tx usecs coalescing parameters

[Jakub Kicinski]

On Fri, 15 Jan 2021 01:06:10 -0800 Bhaskar Upadhaya wrote:
> Here we do the initialization of coalescing values on load.
> Although the default device values are the same - explicit
> config is better visible.

Can you also make the driver store the settings across ifdown / ifup
and allow the settings to be checked while the interface is down, 
while at it?

Re: [EXT] Re: [PATCH net-next 1/3] qede: add netpoll support for qede driver

[Igor Russkikh]

> On Fri, 15 Jan 2021 01:06:08 -0800 Bhaskar Upadhaya wrote:
>> Add net poll controller support to transmit kernel printks
>> over UDP
> 
> Why do you need this patch? Couple years back netpoll was taught 
> how to pull NAPIs by itself, and all you do is schedule NAPIs.
> 
> All the driver should do is to make sure that when napi is called 
> with budget of 0 it only processes Tx completions, not Rx traffic.

Hi Jakub,

Thanks for the hint, we were not aware of that.

I see our driver may not handle zero budget accordingly. Will check.

But then, all this means .ndo_poll_controller is basically deprecated?

Regards,
   Igor

Re: [EXT] Re: [PATCH net-next 3/3] qede: set default per queue rx/tx usecs coalescing parameters

[Igor Russkikh]

>> Here we do the initialization of coalescing values on load.
>> Although the default device values are the same - explicit
>> config is better visible.
> 
> Can you also make the driver store the settings across ifdown / ifup
> and allow the settings to be checked while the interface is down, 
> while at it?

Good suggestion, we'll implement that.

Thanks,
   Igor

Re: [EXT] Re: [PATCH net-next 1/3] qede: add netpoll support for qede driver

[Jakub Kicinski]

On Sun, 17 Jan 2021 17:35:30 +0100 Igor Russkikh wrote:
> > On Fri, 15 Jan 2021 01:06:08 -0800 Bhaskar Upadhaya wrote:  
> >> Add net poll controller support to transmit kernel printks
> >> over UDP  
> > 
> > Why do you need this patch? Couple years back netpoll was taught 
> > how to pull NAPIs by itself, and all you do is schedule NAPIs.
> > 
> > All the driver should do is to make sure that when napi is called 
> > with budget of 0 it only processes Tx completions, not Rx traffic.  
> 
> Hi Jakub,
> 
> Thanks for the hint, we were not aware of that.
> 
> I see our driver may not handle zero budget accordingly. Will check.
> 
> But then, all this means .ndo_poll_controller is basically deprecated?

It's still needed for special devices, off the top of my head for
example bonding uses it to poll its members. But for normal NIC
drivers, yes, it's pretty much deprecated.