[PATCH v2 net-next 0/5] ionic bits and bytes

[PATCH v2 net-next 0/5] ionic bits and bytes

[Shannon Nelson]

These are a few little updates to the ionic driver while we are in between
other feature work.  While these are mostly Fixes, they are almost all low
priority and needn't be promoted to net.  The one higher need is patch 1,
but it is fixing something that hasn't made it out of net-next yet.

Shannon Nelson (5):
  ionic: stop devlink warn on mgmt device
  ionic: deinit rss only if selected
  ionic: remove adminq napi instance
  ionic: return error for unknown xcvr type
  ionic: add decode for IONIC_RC_ENOSUPP

v2: add Fixes tags to patches 1-4, and a little
    description for patch 5

 .../net/ethernet/pensando/ionic/ionic_devlink.c    |  9 +++++++--
 .../net/ethernet/pensando/ionic/ionic_ethtool.c    | 14 +++++++++++++-
 drivers/net/ethernet/pensando/ionic/ionic_lif.c    |  4 +++-
 drivers/net/ethernet/pensando/ionic/ionic_main.c   |  3 +++
 4 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH v2 net-next 1/5] ionic: stop devlink warn on mgmt device

[Shannon Nelson]

If we don't set a port type, the devlink code will eventually
print a WARN in the kernel log.  Because the mgmt device is
not really a useful port, don't register it as a devlink port.

Fixes: b3f064e9746d ("ionic: add support for device id 0x1004")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_devlink.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index ed14164468a1..273c889faaad 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
@@ -77,12 +77,16 @@ int ionic_devlink_register(struct ionic *ionic)
 		return err;
 	}
 
+	/* don't register the mgmt_nic as a port */
+	if (ionic->is_mgmt_nic)
+		return 0;
+
 	devlink_port_attrs_set(&ionic->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
 			       0, false, 0, NULL, 0);
 	err = devlink_port_register(dl, &ionic->dl_port, 0);
 	if (err)
 		dev_err(ionic->dev, "devlink_port_register failed: %d\n", err);
-	else if (!ionic->is_mgmt_nic)
+	else
 		devlink_port_type_eth_set(&ionic->dl_port,
 					  ionic->master_lif->netdev);
 
@@ -93,6 +97,7 @@ void ionic_devlink_unregister(struct ionic *ionic)
 {
 	struct devlink *dl = priv_to_devlink(ionic);
 
-	devlink_port_unregister(&ionic->dl_port);
+	if (ionic->dl_port.registered)
+		devlink_port_unregister(&ionic->dl_port);
 	devlink_unregister(dl);
 }
-- 
2.17.1

[PATCH v2 net-next 2/5] ionic: deinit rss only if selected

[Shannon Nelson]

Don't bother de-initing RSS if it wasn't selected.

Fixes: aa3198819bea ("ionic: Add RSS support")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index b903016193df..19fd7cc36f28 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -2066,7 +2066,8 @@ static void ionic_lif_deinit(struct ionic_lif *lif)
 	clear_bit(IONIC_LIF_F_INITED, lif->state);
 
 	ionic_rx_filters_deinit(lif);
-	ionic_lif_rss_deinit(lif);
+	if (lif->netdev->features & NETIF_F_RXHASH)
+		ionic_lif_rss_deinit(lif);
 
 	napi_disable(&lif->adminqcq->napi);
 	ionic_lif_qcq_deinit(lif, lif->notifyqcq);
-- 
2.17.1

[PATCH v2 net-next 3/5] ionic: remove adminq napi instance

[Shannon Nelson]

Remove the adminq's napi struct when tearing down
the adminq.

Fixes: 1d062b7b6f64 ("ionic: Add basic adminq support")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 19fd7cc36f28..12e3823b0bc1 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -2070,6 +2070,7 @@ static void ionic_lif_deinit(struct ionic_lif *lif)
 		ionic_lif_rss_deinit(lif);
 
 	napi_disable(&lif->adminqcq->napi);
+	netif_napi_del(&lif->adminqcq->napi);
 	ionic_lif_qcq_deinit(lif, lif->notifyqcq);
 	ionic_lif_qcq_deinit(lif, lif->adminqcq);
 
-- 
2.17.1

[PATCH v2 net-next 4/5] ionic: return error for unknown xcvr type

[Shannon Nelson]

If we don't recognize the transceiver type, return an error
so that ethtool doesn't try dumping bogus eeprom contents.

Fixes: 4d03e00a2140 ("ionic: Add initial ethtool support")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_ethtool.c    | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index a233716eac29..3f92f301a020 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -694,7 +694,7 @@ static int ionic_get_module_info(struct net_device *netdev,
 	default:
 		netdev_info(netdev, "unknown xcvr type 0x%02x\n",
 			    xcvr->sprom[0]);
-		break;
+		return -EINVAL;
 	}
 
 	return 0;
@@ -714,7 +714,19 @@ static int ionic_get_module_eeprom(struct net_device *netdev,
 	/* The NIC keeps the module prom up-to-date in the DMA space
 	 * so we can simply copy the module bytes into the data buffer.
 	 */
+
 	xcvr = &idev->port_info->status.xcvr;
+	switch (xcvr->sprom[0]) {
+	case 0x03: /* SFP */
+	case 0x0D: /* QSFP */
+	case 0x11: /* QSFP28 */
+		break;
+	default:
+		netdev_info(netdev, "unknown xcvr type 0x%02x\n",
+			    xcvr->sprom[0]);
+		return -EINVAL;
+	}
+
 	len = min_t(u32, sizeof(xcvr->sprom), ee->len);
 
 	do {
-- 
2.17.1

[PATCH v2 net-next 5/5] ionic: add decode for IONIC_RC_ENOSUPP

[Shannon Nelson]

Add decoding for a new firmware error code.

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

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index e4a76e66f542..c5e3d7639f7e 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -58,6 +58,8 @@ static const char *ionic_error_to_str(enum ionic_status_code code)
 		return "IONIC_RC_BAD_ADDR";
 	case IONIC_RC_DEV_CMD:
 		return "IONIC_RC_DEV_CMD";
+	case IONIC_RC_ENOSUPP:
+		return "IONIC_RC_ENOSUPP";
 	case IONIC_RC_ERROR:
 		return "IONIC_RC_ERROR";
 	case IONIC_RC_ERDMA:
@@ -76,6 +78,7 @@ static int ionic_error_to_errno(enum ionic_status_code code)
 	case IONIC_RC_EQTYPE:
 	case IONIC_RC_EQID:
 	case IONIC_RC_EINVAL:
+	case IONIC_RC_ENOSUPP:
 		return -EINVAL;
 	case IONIC_RC_EPERM:
 		return -EPERM;
-- 
2.17.1

Re: [PATCH v2 net-next 4/5] ionic: return error for unknown xcvr type

[Jakub Kicinski]

On Mon, 16 Mar 2020 12:31:33 -0700 Shannon Nelson wrote:
> If we don't recognize the transceiver type, return an error
> so that ethtool doesn't try dumping bogus eeprom contents.
> 
> Fixes: 4d03e00a2140 ("ionic: Add initial ethtool support")
> Signed-off-by: Shannon Nelson <snelson@pensando.io>

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index a233716eac29..3f92f301a020 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -694,7 +694,7 @@ static int ionic_get_module_info(struct net_device *netdev,
>  	default:
>  		netdev_info(netdev, "unknown xcvr type 0x%02x\n",
>  			    xcvr->sprom[0]);
> -		break;
> +		return -EINVAL;
>  	}
>  
>  	return 0;
> @@ -714,7 +714,19 @@ static int ionic_get_module_eeprom(struct net_device *netdev,
>  	/* The NIC keeps the module prom up-to-date in the DMA space
>  	 * so we can simply copy the module bytes into the data buffer.
>  	 */
> +
>  	xcvr = &idev->port_info->status.xcvr;
> +	switch (xcvr->sprom[0]) {
> +	case 0x03: /* SFP */
> +	case 0x0D: /* QSFP */
> +	case 0x11: /* QSFP28 */

Please use defines from sfp.h

> +		break;
> +	default:
> +		netdev_info(netdev, "unknown xcvr type 0x%02x\n",
> +			    xcvr->sprom[0]);
> +		return -EINVAL;

Isn't there _some_ amount of eeprom that we could always return?

> +	}
> +
>  	len = min_t(u32, sizeof(xcvr->sprom), ee->len);
>  
>  	do {

The pluggable module eeprom stuff really calls for some common infra :(

Re: [PATCH v2 net-next 4/5] ionic: return error for unknown xcvr type

[Shannon Nelson]

On 3/16/20 3:01 PM, Jakub Kicinski wrote:
> On Mon, 16 Mar 2020 12:31:33 -0700 Shannon Nelson wrote:
>> If we don't recognize the transceiver type, return an error
>> so that ethtool doesn't try dumping bogus eeprom contents.
>>
>> Fixes: 4d03e00a2140 ("ionic: Add initial ethtool support")
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> index a233716eac29..3f92f301a020 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> @@ -694,7 +694,7 @@ static int ionic_get_module_info(struct net_device *netdev,
>>   	default:
>>   		netdev_info(netdev, "unknown xcvr type 0x%02x\n",
>>   			    xcvr->sprom[0]);
>> -		break;
>> +		return -EINVAL;
>>   	}
>>   
>>   	return 0;
>> @@ -714,7 +714,19 @@ static int ionic_get_module_eeprom(struct net_device *netdev,
>>   	/* The NIC keeps the module prom up-to-date in the DMA space
>>   	 * so we can simply copy the module bytes into the data buffer.
>>   	 */
>> +
>>   	xcvr = &idev->port_info->status.xcvr;
>> +	switch (xcvr->sprom[0]) {
>> +	case 0x03: /* SFP */
>> +	case 0x0D: /* QSFP */
>> +	case 0x11: /* QSFP28 */
> Please use defines from sfp.h

Yep, thanks, it's nice we have those now.

>
>> +		break;
>> +	default:
>> +		netdev_info(netdev, "unknown xcvr type 0x%02x\n",
>> +			    xcvr->sprom[0]);
>> +		return -EINVAL;
> Isn't there _some_ amount of eeprom that we could always return?

It probably would be useful to return the first page (256 bytes) to help 
the reader figure out what's up with the data.

This only gets called it ionic_get_module_info() returns with no error, 
so possibly that function could set type to 0 or -1 and len to 256, more 
or less as default values for getting something printed.

I'll play with that a little.

Thanks,
sln