[PATCH v2] enic: add ethtool get_channel support

Jon Kohler posted 1 patch 1 year, 5 months ago
There is a newer version of this series
.../net/ethernet/cisco/enic/enic_ethtool.c    | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
[PATCH v2] enic: add ethtool get_channel support
Posted by Jon Kohler 1 year, 5 months ago
Add .get_channel to enic_ethtool_ops to enable basic ethtool -l
support to get the current channel configuration.

Note that the driver does not support dynamically changing queue
configuration, so .set_channel is intentionally unused. Instead, users
should use Cisco's hardware management tools (UCSM/IMC) to modify
virtual interface card configuration out of band.

Signed-off-by: Jon Kohler <jon@nutanix.com>
---
v1
- https://lore.kernel.org/netdev/20240618160146.3900470-1-jon@nutanix.com/T/#u
v1 -> v2:
- Addressed comments from Przemek and Jakub
---
 .../net/ethernet/cisco/enic/enic_ethtool.c    | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/net/ethernet/cisco/enic/enic_ethtool.c b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
index 241906697019..54f542238b4e 100644
--- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
+++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
@@ -608,6 +608,32 @@ static int enic_get_ts_info(struct net_device *netdev,
 	return 0;
 }
 
+static void enic_get_channels(struct net_device *netdev,
+			      struct ethtool_channels *channels)
+{
+	struct enic *enic = netdev_priv(netdev);
+
+	switch (vnic_dev_get_intr_mode(enic->vdev)) {
+	case VNIC_DEV_INTR_MODE_MSIX:
+		channels->max_rx = ENIC_RQ_MAX;
+		channels->max_tx = ENIC_WQ_MAX;
+		channels->rx_count = enic->rq_count;
+		channels->tx_count = enic->wq_count;
+		break;
+	case VNIC_DEV_INTR_MODE_MSI:
+		channels->max_rx = 1;
+		channels->max_tx = 1;
+		channels->rx_count = 1;
+		channels->tx_count = 1;
+		break;
+	case VNIC_DEV_INTR_MODE_INTX:
+		channels->max_combined = 1;
+		channels->combined_count = 1;
+	default:
+		break;
+	}
+}
+
 static const struct ethtool_ops enic_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_USE_ADAPTIVE_RX |
@@ -632,6 +658,7 @@ static const struct ethtool_ops enic_ethtool_ops = {
 	.set_rxfh = enic_set_rxfh,
 	.get_link_ksettings = enic_get_ksettings,
 	.get_ts_info = enic_get_ts_info,
+	.get_channels = enic_get_channels,
 };
 
 void enic_set_ethtool_ops(struct net_device *netdev)
-- 
2.43.0
Re: [PATCH v2] enic: add ethtool get_channel support
Posted by Jakub Kicinski 1 year, 5 months ago
On Mon, 24 Jun 2024 11:49:00 -0700 Jon Kohler wrote:
> +	switch (vnic_dev_get_intr_mode(enic->vdev)) {
> +	case VNIC_DEV_INTR_MODE_MSIX:
> +		channels->max_rx = ENIC_RQ_MAX;
> +		channels->max_tx = ENIC_WQ_MAX;
> +		channels->rx_count = enic->rq_count;
> +		channels->tx_count = enic->wq_count;
> +		break;
> +	case VNIC_DEV_INTR_MODE_MSI:
> +		channels->max_rx = 1;
> +		channels->max_tx = 1;
> +		channels->rx_count = 1;
> +		channels->tx_count = 1;
> +		break;
> +	case VNIC_DEV_INTR_MODE_INTX:
> +		channels->max_combined = 1;
> +		channels->combined_count = 1;
> +	default:
> +		break;
> +	}

sorry for not responding properly to your earlier email, but I think
MSI should also be combined. What matters is whether the IRQ serves
just one of {Rx, Tx} or both.

For MSI, I see:

1 . enic_dev_init() does:
	netif_napi_add(netdev, &enic->napi[0], enic_poll);
                                               ^^^^^^^^^

2. enic_request_intr() does
	request_irq(enic->pdev->irq, enic_isr_msi, ...
                                     ^^^^^^^^^^^^

3. enic_isr_msi() does 
	napi_schedule_irqoff(&enic->napi[0]); 
thus matching the NAPI from step #1.

4. enic_poll() calls both enic_wq_service, and enic_rq_service

So it's combined, AFAICT, similar to INTX in the relevant parts.
-- 
pw-bot: cr
Re: [PATCH v2] enic: add ethtool get_channel support
Posted by Jon Kohler 1 year, 5 months ago

> On Jun 25, 2024, at 8:03 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> On Mon, 24 Jun 2024 11:49:00 -0700 Jon Kohler wrote:
>> + switch (vnic_dev_get_intr_mode(enic->vdev)) {
>> + case VNIC_DEV_INTR_MODE_MSIX:
>> + channels->max_rx = ENIC_RQ_MAX;
>> + channels->max_tx = ENIC_WQ_MAX;
>> + channels->rx_count = enic->rq_count;
>> + channels->tx_count = enic->wq_count;
>> + break;
>> + case VNIC_DEV_INTR_MODE_MSI:
>> + channels->max_rx = 1;
>> + channels->max_tx = 1;
>> + channels->rx_count = 1;
>> + channels->tx_count = 1;
>> + break;
>> + case VNIC_DEV_INTR_MODE_INTX:
>> + channels->max_combined = 1;
>> + channels->combined_count = 1;
>> + default:
>> + break;
>> + }
> 
> sorry for not responding properly to your earlier email, but I think
> MSI should also be combined. What matters is whether the IRQ serves
> just one of {Rx, Tx} or both.
> 
> For MSI, I see:
> 
> 1 . enic_dev_init() does:
> netif_napi_add(netdev, &enic->napi[0], enic_poll);
>                                               ^^^^^^^^^
> 
> 2. enic_request_intr() does
> request_irq(enic->pdev->irq, enic_isr_msi, ...
>                                     ^^^^^^^^^^^^
> 
> 3. enic_isr_msi() does 
> napi_schedule_irqoff(&enic->napi[0]); 
> thus matching the NAPI from step #1.
> 
> 4. enic_poll() calls both enic_wq_service, and enic_rq_service
> 
> So it's combined, AFAICT, similar to INTX in the relevant parts.

Ok, thanks for the tip, I appreciate it. I’ll send out a v3 shortly

> -- 
> pw-bot: cr