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

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(-)

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 :(

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