[Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Nicholas Krause]

This fixes error handling in the function fm10k_setup_tc to properly
check if the call to the function fm10k_open has failed by returning
a error and if so return immediately to the caller of the function
fm10k_setup_tc to properly signal this non recoverable failure.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 99228bf..5a4e702 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1146,6 +1146,7 @@ static struct rtnl_link_stats64 *fm10k_get_stats64(struct net_device *netdev,
 int fm10k_setup_tc(struct net_device *dev, u8 tc)
 {
 	struct fm10k_intfc *interface = netdev_priv(dev);
+	int err;
 
 	/* Currently only the PF supports priority classes */
 	if (tc && (interface->hw.mac.type != fm10k_mac_pf))
@@ -1175,7 +1176,9 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc)
 	fm10k_mbx_request_irq(interface);
 
 	if (netif_running(dev))
-		fm10k_open(dev);
+		err = fm10k_open(dev);
+	if (err)
+		return err;
 
 	/* flag to indicate SWPRI has yet to be updated */
 	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
-- 
2.1.4

[Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Jeff Kirsher]

On Tue, 2015-10-20 at 17:05 -0400, Nicholas Krause wrote:
> This fixes error handling in the function fm10k_setup_tc to properly
> check if the call to the function fm10k_open has failed by returning
> a error and if so return immediately to the caller of the function
> fm10k_setup_tc to properly signal this non recoverable failure.
> 
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Is this a version 2 of your original patch you sent out with the same
title and patch description?  It is difficult to understand the reason
for the change without a sufficient changelog (i.e. why the new version
of the patch).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151020/088641b5/attachment.asc>

Re: [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On Tue, 2015-10-20 at 17:05 -0400, Nicholas Krause wrote:
> This fixes error handling in the function fm10k_setup_tc to properly
> check if the call to the function fm10k_open has failed by returning
> a error and if so return immediately to the caller of the function
> fm10k_setup_tc to properly signal this non recoverable failure.
> 
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Is this a version 2 of your original patch you sent out with the same
title and patch description?  It is difficult to understand the reason
for the change without a sufficient changelog (i.e. why the new version
of the patch).

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Rustad, Mark D]

Nicholas Krause <xerofoify@gmail.com> wrote:

> This fixes error handling in the function fm10k_setup_tc to properly
> check if the call to the function fm10k_open has failed by returning
> a error and if so return immediately to the caller of the function
> fm10k_setup_tc to properly signal this non recoverable failure.
> 
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
> drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> index 99228bf..5a4e702 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> @@ -1146,6 +1146,7 @@ static struct rtnl_link_stats64 *fm10k_get_stats64(struct net_device *netdev,
> int fm10k_setup_tc(struct net_device *dev, u8 tc)
> {
> 	struct fm10k_intfc *interface = netdev_priv(dev);
> +	int err;
> 
> 	/* Currently only the PF supports priority classes */
> 	if (tc && (interface->hw.mac.type != fm10k_mac_pf))
> @@ -1175,7 +1176,9 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc)
> 	fm10k_mbx_request_irq(interface);
> 
> 	if (netif_running(dev))
> -		fm10k_open(dev);
> +		err = fm10k_open(dev);
> +	if (err)
> +		return err;
> 
> 	/* flag to indicate SWPRI has yet to be updated */
> 	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;

NAK. This will reference an uninitialized err variable when netif_running returns false. The compiler should complain about this, suggesting that it wasn't compiled.

--
Mark Rustad, Networking Division, Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151020/15318154/attachment-0001.asc>

Re: [Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Rustad, Mark D]

[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]

Nicholas Krause <xerofoify@gmail.com> wrote:

> This fixes error handling in the function fm10k_setup_tc to properly
> check if the call to the function fm10k_open has failed by returning
> a error and if so return immediately to the caller of the function
> fm10k_setup_tc to properly signal this non recoverable failure.
> 
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
> drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> index 99228bf..5a4e702 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> @@ -1146,6 +1146,7 @@ static struct rtnl_link_stats64 *fm10k_get_stats64(struct net_device *netdev,
> int fm10k_setup_tc(struct net_device *dev, u8 tc)
> {
> 	struct fm10k_intfc *interface = netdev_priv(dev);
> +	int err;
> 
> 	/* Currently only the PF supports priority classes */
> 	if (tc && (interface->hw.mac.type != fm10k_mac_pf))
> @@ -1175,7 +1176,9 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc)
> 	fm10k_mbx_request_irq(interface);
> 
> 	if (netif_running(dev))
> -		fm10k_open(dev);
> +		err = fm10k_open(dev);
> +	if (err)
> +		return err;
> 
> 	/* flag to indicate SWPRI has yet to be updated */
> 	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;

NAK. This will reference an uninitialized err variable when netif_running returns false. The compiler should complain about this, suggesting that it wasn't compiled.

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

[Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Nicholas Krause]

On October 20, 2015 7:11:51 PM EDT, "Rustad, Mark D" <mark.d.rustad@intel.com> wrote:
>Nicholas Krause <xerofoify@gmail.com> wrote:
>
>> This fixes error handling in the function fm10k_setup_tc to properly
>> check if the call to the function fm10k_open has failed by returning
>> a error and if so return immediately to the caller of the function
>> fm10k_setup_tc to properly signal this non recoverable failure.
>> 
>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
>> ---
>> drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>> index 99228bf..5a4e702 100644
>> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>> @@ -1146,6 +1146,7 @@ static struct rtnl_link_stats64
>*fm10k_get_stats64(struct net_device *netdev,
>> int fm10k_setup_tc(struct net_device *dev, u8 tc)
>> {
>> 	struct fm10k_intfc *interface = netdev_priv(dev);
>> +	int err;
>> 
>> 	/* Currently only the PF supports priority classes */
>> 	if (tc && (interface->hw.mac.type != fm10k_mac_pf))
>> @@ -1175,7 +1176,9 @@ int fm10k_setup_tc(struct net_device *dev, u8
>tc)
>> 	fm10k_mbx_request_irq(interface);
>> 
>> 	if (netif_running(dev))
>> -		fm10k_open(dev);
>> +		err = fm10k_open(dev);
>> +	if (err)
>> +		return err;
>> 
>> 	/* flag to indicate SWPRI has yet to be updated */
>> 	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
>
>NAK. This will reference an uninitialized err variable when
>netif_running returns false. The compiler should complain about this,
>suggesting that it wasn't compiled.
>
>--
I compiled it but my compiler builds it fine.  However this is a valid complaint that I don't mind fixing if there are no other complaints about the patch content. 
Nick 



>Mark Rustad, Networking Division, Intel Corporation

[Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Nicholas Krause
Sent: Tuesday, October 20, 2015 2:05 PM
To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
Cc: linux-kernel at vger.kernel.org; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org
Subject: [Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

This fixes error handling in the function fm10k_setup_tc to properly check if the call to the function fm10k_open has failed by returning a error and if so return immediately to the caller of the function fm10k_setup_tc to properly signal this non recoverable failure.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---

Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>

RE: [Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Nicholas Krause
Sent: Tuesday, October 20, 2015 2:05 PM
To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
Cc: linux-kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
Subject: [Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

This fixes error handling in the function fm10k_setup_tc to properly check if the call to the function fm10k_open has failed by returning a error and if so return immediately to the caller of the function fm10k_setup_tc to properly signal this non recoverable failure.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---

Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>

[Intel-wired-lan] [PATCH] fm10k:Fix error handling in the function fm10k_setup_tc

[Nicholas Krause]

This fixes error handling in the function fm10k_setup_tc to
properly check if the calls to the functions fm10k_mbx_request
or fm10k_init_queueing_scheme fail by returning a error code and
if so return the error code returned by these functions immediately
to the call of fm10k_setup_tc to signal a failure has occurred when
called this particular function.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 99228bf..c5469af 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1146,7 +1146,8 @@ static struct rtnl_link_stats64 *fm10k_get_stats64(struct net_device *netdev,
 int fm10k_setup_tc(struct net_device *dev, u8 tc)
 {
 	struct fm10k_intfc *interface = netdev_priv(dev);
-
+	int err;
+
 	/* Currently only the PF supports priority classes */
 	if (tc && (interface->hw.mac.type != fm10k_mac_pf))
 		return -EINVAL;
@@ -1170,9 +1171,13 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc)
 	netdev_reset_tc(dev);
 	netdev_set_num_tc(dev, tc);
 
-	fm10k_init_queueing_scheme(interface);
+	err = fm10k_init_queueing_scheme(interface);
+	if (err)
+		return err;
 
-	fm10k_mbx_request_irq(interface);
+	err = fm10k_mbx_request_irq(interface);
+	if (err)
+		return err;
 
 	if (netif_running(dev))
 		fm10k_open(dev);
-- 
2.1.4