[PATCH net-next] liquidio: fix kernel panic when NIC firmware is older than 1.7.2

[PATCH net-next] liquidio: fix kernel panic when NIC firmware is older than 1.7.2

[Felix Manlunas]

From: Rick Farrington <ricardo.farrington@cavium.com>

Pre-1.7.2 NIC firmware does not support (and does not respond to) the "get
speed" command which is sent by the 1.7.2 driver during modprobe.  Due to a
bug in older firmware (with respect to unknown commands), this unsupported
command causes a cascade of errors that ends in a kernel panic.

Fix it by making the sending of the "get speed" command conditional on the
firmware version.

Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Acked-by: Derek Chickles <derek.chickles@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
Note: To avoid checkpatch.pl "WARNING: line over 80 characters", the comma
      that separates the arguments in the call to strcmp() was placed one
      line below the usual spot.

 drivers/net/ethernet/cavium/liquidio/lio_main.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 7cb4e75..f83f884 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -3671,7 +3671,16 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
 			OCTEON_CN2350_25GB_SUBSYS_ID ||
 		    octeon_dev->subsystem_id ==
 			OCTEON_CN2360_25GB_SUBSYS_ID) {
-			liquidio_get_speed(lio);
+			/* speed control unsupported in f/w older than 1.7.2 */
+			if (strcmp(octeon_dev->fw_info.liquidio_firmware_version
+			   , "1.7.2") < 0) {
+				dev_info(&octeon_dev->pci_dev->dev,
+					 "speed setting not supported by f/w.");
+				octeon_dev->speed_setting = 25;
+				octeon_dev->no_speed_setting = 1;
+			} else {
+				liquidio_get_speed(lio);
+			}
 
 			if (octeon_dev->speed_setting == 0) {
 				octeon_dev->speed_setting = 25;

Re: [PATCH net-next] liquidio: fix kernel panic when NIC firmware is older than 1.7.2

[Shannon Nelson]

On 6/26/2018 4:58 AM, Felix Manlunas wrote:
> From: Rick Farrington <ricardo.farrington@cavium.com>
> 
> Pre-1.7.2 NIC firmware does not support (and does not respond to) the "get
> speed" command which is sent by the 1.7.2 driver during modprobe.  Due to a
> bug in older firmware (with respect to unknown commands), this unsupported
> command causes a cascade of errors that ends in a kernel panic.
> 
> Fix it by making the sending of the "get speed" command conditional on the
> firmware version.
> 
> Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
> Acked-by: Derek Chickles <derek.chickles@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
> ---
> Note: To avoid checkpatch.pl "WARNING: line over 80 characters", the comma
>        that separates the arguments in the call to strcmp() was placed one
>        line below the usual spot.
> 
>   drivers/net/ethernet/cavium/liquidio/lio_main.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> index 7cb4e75..f83f884 100644
> --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
> +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> @@ -3671,7 +3671,16 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
>   			OCTEON_CN2350_25GB_SUBSYS_ID ||
>   		    octeon_dev->subsystem_id ==
>   			OCTEON_CN2360_25GB_SUBSYS_ID) {
> -			liquidio_get_speed(lio);
> +			/* speed control unsupported in f/w older than 1.7.2 */
> +			if (strcmp(octeon_dev->fw_info.liquidio_firmware_version
> +			   , "1.7.2") < 0) {

Will the liquidio_firmware_version ever end up something like 1.7.10? 
If so, this strcmp() may not do what you want.

sln

> +				dev_info(&octeon_dev->pci_dev->dev,
> +					 "speed setting not supported by f/w.");
> +				octeon_dev->speed_setting = 25;
> +				octeon_dev->no_speed_setting = 1;
> +			} else {
> +				liquidio_get_speed(lio);
> +			}
>   
>   			if (octeon_dev->speed_setting == 0) {
>   				octeon_dev->speed_setting = 25;
> 

Re: [PATCH net-next] liquidio: fix kernel panic when NIC firmware is older than 1.7.2

[Felix Manlunas]

On Tue, Jun 26, 2018 at 09:03:25AM -0700, Shannon Nelson wrote:
> On 6/26/2018 4:58 AM, Felix Manlunas wrote:
> > From: Rick Farrington <ricardo.farrington@cavium.com>
> > 
> > Pre-1.7.2 NIC firmware does not support (and does not respond to) the "get
> > speed" command which is sent by the 1.7.2 driver during modprobe.  Due to a
> > bug in older firmware (with respect to unknown commands), this unsupported
> > command causes a cascade of errors that ends in a kernel panic.
> > 
> > Fix it by making the sending of the "get speed" command conditional on the
> > firmware version.
> > 
> > Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
> > Acked-by: Derek Chickles <derek.chickles@cavium.com>
> > Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
> > ---
> > Note: To avoid checkpatch.pl "WARNING: line over 80 characters", the comma
> >        that separates the arguments in the call to strcmp() was placed one
> >        line below the usual spot.
> > 
> >   drivers/net/ethernet/cavium/liquidio/lio_main.c | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > index 7cb4e75..f83f884 100644
> > --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > @@ -3671,7 +3671,16 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
> >                       OCTEON_CN2350_25GB_SUBSYS_ID ||
> >                   octeon_dev->subsystem_id ==
> >                       OCTEON_CN2360_25GB_SUBSYS_ID) {
> > -                     liquidio_get_speed(lio);
> > +                     /* speed control unsupported in f/w older than 1.7.2 */
> > +                     if (strcmp(octeon_dev->fw_info.liquidio_firmware_version
> > +                        , "1.7.2") < 0) {
> 
> Will the liquidio_firmware_version ever end up something like 1.7.10?
> If so, this strcmp() may not do what you want.
> 
> sln

Yes, it's possible that the liquidio_firmware_version will reach 1.7.10.

We agree that using strcmp() will not give the correct result for that case,
so please disregard this patch.

We will submit a V2 patch.

Felix