* [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant
@ 2019-04-08 14:48 Michael Zhivich
2019-04-08 14:48 ` [PATCH v2 1/3] ethtool: avoid signed-unsigned comparison in ethtool_validate_speed() Michael Zhivich
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Michael Zhivich @ 2019-04-08 14:48 UTC (permalink / raw)
To: davem
Cc: siva.kallam, prashant, mchan, shshaikh, manishc, netdev, Michael Zhivich
This patch series addresses 2 related issues:
1. ethtool_validate_speed() triggers a "signed-unsigned comparison"
warning due to type difference of SPEED_UNKNOWN constant (int)
and argument to ethtool_validate_speed (__u32).
2. some drivers use u16 storage for SPEED_UNKNOWN constant,
resulting in value truncation and thus failure to test against
SPEED_UNKNOWN correctly.
This revised series addresses several feedback comments:
- split up the patch in to series
- do not unnecessarily change drivers that use "int" storage
for speed values
Michael Zhivich (3):
ethtool: avoid signed-unsigned comparison in ethtool_validate_speed()
broadcom: tg3: fix use of SPEED_UNKNOWN ethtool constant
qlogic: qlcnic: fix use of SPEED_UNKNOWN ethtool constant
drivers/net/ethernet/broadcom/tg3.c | 8 ++++----
drivers/net/ethernet/broadcom/tg3.h | 4 ++--
drivers/net/ethernet/qlogic/qlcnic/qlcnic.h | 2 +-
include/uapi/linux/ethtool.h | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] ethtool: avoid signed-unsigned comparison in ethtool_validate_speed()
2019-04-08 14:48 [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Michael Zhivich
@ 2019-04-08 14:48 ` Michael Zhivich
2019-04-08 14:48 ` [PATCH v2 2/3] broadcom: tg3: fix use of SPEED_UNKNOWN ethtool constant Michael Zhivich
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Michael Zhivich @ 2019-04-08 14:48 UTC (permalink / raw)
To: davem
Cc: siva.kallam, prashant, mchan, shshaikh, manishc, netdev, Michael Zhivich
When building C++ userspace code that includes ethtool.h
with "-Werror -Wall", g++ complains about signed-unsigned comparison in
ethtool_validate_speed() due to definition of SPEED_UNKNOWN as -1.
Explicitly cast SPEED_UNKNOWN to __u32 to match type of
ethtool_validate_speed() argument.
Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
---
include/uapi/linux/ethtool.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 3652b239..d473e5e 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1591,7 +1591,7 @@ enum ethtool_link_mode_bit_indices {
static inline int ethtool_validate_speed(__u32 speed)
{
- return speed <= INT_MAX || speed == SPEED_UNKNOWN;
+ return speed <= INT_MAX || speed == (__u32)SPEED_UNKNOWN;
}
/* Duplex, half or full. */
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] broadcom: tg3: fix use of SPEED_UNKNOWN ethtool constant
2019-04-08 14:48 [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Michael Zhivich
2019-04-08 14:48 ` [PATCH v2 1/3] ethtool: avoid signed-unsigned comparison in ethtool_validate_speed() Michael Zhivich
@ 2019-04-08 14:48 ` Michael Zhivich
2019-04-08 17:52 ` Andrew Lunn
2019-04-08 14:48 ` [PATCH v2 3/3] qlogic: qlcnic: " Michael Zhivich
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Michael Zhivich @ 2019-04-08 14:48 UTC (permalink / raw)
To: davem
Cc: siva.kallam, prashant, mchan, shshaikh, manishc, netdev, Michael Zhivich
tg3 driver uses u16 to store SPEED_UKNOWN ethtool constant,
which is defined as -1, resulting in value truncation and
thus incorrect test results against SPEED_UNKNOWN.
For example, the following test will print "False":
u16 speed = SPEED_UNKNOWN;
if (speed == SPEED_UNKNOWN)
printf("True");
else
printf("False");
Change storage of speed to use u32 to avoid this issue.
Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
---
drivers/net/ethernet/broadcom/tg3.c | 8 ++++----
drivers/net/ethernet/broadcom/tg3.h | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 328373e..060a6f3 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -4283,7 +4283,7 @@ static void tg3_power_down(struct tg3 *tp)
pci_set_power_state(tp->pdev, PCI_D3hot);
}
-static void tg3_aux_stat_to_speed_duplex(struct tg3 *tp, u32 val, u16 *speed, u8 *duplex)
+static void tg3_aux_stat_to_speed_duplex(struct tg3 *tp, u32 val, u32 *speed, u8 *duplex)
{
switch (val & MII_TG3_AUX_STAT_SPDMASK) {
case MII_TG3_AUX_STAT_10HALF:
@@ -4787,7 +4787,7 @@ static int tg3_setup_copper_phy(struct tg3 *tp, bool force_reset)
bool current_link_up;
u32 bmsr, val;
u32 lcl_adv, rmt_adv;
- u16 current_speed;
+ u32 current_speed;
u8 current_duplex;
int i, err;
@@ -5719,7 +5719,7 @@ static bool tg3_setup_fiber_by_hand(struct tg3 *tp, u32 mac_status)
static int tg3_setup_fiber_phy(struct tg3 *tp, bool force_reset)
{
u32 orig_pause_cfg;
- u16 orig_active_speed;
+ u32 orig_active_speed;
u8 orig_active_duplex;
u32 mac_status;
bool current_link_up;
@@ -5823,7 +5823,7 @@ static int tg3_setup_fiber_mii_phy(struct tg3 *tp, bool force_reset)
{
int err = 0;
u32 bmsr, bmcr;
- u16 current_speed = SPEED_UNKNOWN;
+ u32 current_speed = SPEED_UNKNOWN;
u8 current_duplex = DUPLEX_UNKNOWN;
bool current_link_up = false;
u32 local_adv, remote_adv, sgsr;
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index a772a33..6953d05 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2873,7 +2873,7 @@ struct tg3_tx_ring_info {
struct tg3_link_config {
/* Describes what we're trying to get. */
u32 advertising;
- u16 speed;
+ u32 speed;
u8 duplex;
u8 autoneg;
u8 flowctrl;
@@ -2882,7 +2882,7 @@ struct tg3_link_config {
u8 active_flowctrl;
u8 active_duplex;
- u16 active_speed;
+ u32 active_speed;
u32 rmt_adv;
};
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] qlogic: qlcnic: fix use of SPEED_UNKNOWN ethtool constant
2019-04-08 14:48 [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Michael Zhivich
2019-04-08 14:48 ` [PATCH v2 1/3] ethtool: avoid signed-unsigned comparison in ethtool_validate_speed() Michael Zhivich
2019-04-08 14:48 ` [PATCH v2 2/3] broadcom: tg3: fix use of SPEED_UNKNOWN ethtool constant Michael Zhivich
@ 2019-04-08 14:48 ` Michael Zhivich
2019-04-08 17:53 ` Andrew Lunn
2019-04-08 17:54 ` [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Andrew Lunn
2019-04-08 23:30 ` David Miller
4 siblings, 1 reply; 9+ messages in thread
From: Michael Zhivich @ 2019-04-08 14:48 UTC (permalink / raw)
To: davem
Cc: siva.kallam, prashant, mchan, shshaikh, manishc, netdev, Michael Zhivich
qlcnic driver uses u16 to store SPEED_UKNOWN ethtool constant,
which is defined as -1, resulting in value truncation and
thus incorrect test results against SPEED_UNKNOWN.
For example, the following test will print "False":
u16 speed = SPEED_UNKNOWN;
if (speed == SPEED_UNKNOWN)
printf("True");
else
printf("False");
Change storage of speed to use u32 to avoid this issue.
Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 0c443ea..374a4d4 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -497,7 +497,7 @@ struct qlcnic_hardware_context {
u16 board_type;
u16 supported_type;
- u16 link_speed;
+ u32 link_speed;
u16 link_duplex;
u16 link_autoneg;
u16 module_type;
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] broadcom: tg3: fix use of SPEED_UNKNOWN ethtool constant
2019-04-08 14:48 ` [PATCH v2 2/3] broadcom: tg3: fix use of SPEED_UNKNOWN ethtool constant Michael Zhivich
@ 2019-04-08 17:52 ` Andrew Lunn
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-04-08 17:52 UTC (permalink / raw)
To: Michael Zhivich
Cc: davem, siva.kallam, prashant, mchan, shshaikh, manishc, netdev
On Mon, Apr 08, 2019 at 10:48:46AM -0400, Michael Zhivich wrote:
> tg3 driver uses u16 to store SPEED_UKNOWN ethtool constant,
> which is defined as -1, resulting in value truncation and
> thus incorrect test results against SPEED_UNKNOWN.
>
> For example, the following test will print "False":
>
> u16 speed = SPEED_UNKNOWN;
>
> if (speed == SPEED_UNKNOWN)
> printf("True");
> else
> printf("False");
>
> Change storage of speed to use u32 to avoid this issue.
>
> Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] qlogic: qlcnic: fix use of SPEED_UNKNOWN ethtool constant
2019-04-08 14:48 ` [PATCH v2 3/3] qlogic: qlcnic: " Michael Zhivich
@ 2019-04-08 17:53 ` Andrew Lunn
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-04-08 17:53 UTC (permalink / raw)
To: Michael Zhivich
Cc: davem, siva.kallam, prashant, mchan, shshaikh, manishc, netdev
On Mon, Apr 08, 2019 at 10:48:47AM -0400, Michael Zhivich wrote:
> qlcnic driver uses u16 to store SPEED_UKNOWN ethtool constant,
> which is defined as -1, resulting in value truncation and
> thus incorrect test results against SPEED_UNKNOWN.
>
> For example, the following test will print "False":
>
> u16 speed = SPEED_UNKNOWN;
>
> if (speed == SPEED_UNKNOWN)
> printf("True");
> else
> printf("False");
>
> Change storage of speed to use u32 to avoid this issue.
>
> Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant
2019-04-08 14:48 [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Michael Zhivich
` (2 preceding siblings ...)
2019-04-08 14:48 ` [PATCH v2 3/3] qlogic: qlcnic: " Michael Zhivich
@ 2019-04-08 17:54 ` Andrew Lunn
2019-04-08 18:40 ` Zhivich, Michael
2019-04-08 23:30 ` David Miller
4 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-04-08 17:54 UTC (permalink / raw)
To: Michael Zhivich
Cc: davem, siva.kallam, prashant, mchan, shshaikh, manishc, netdev
On Mon, Apr 08, 2019 at 10:48:44AM -0400, Michael Zhivich wrote:
> This patch series addresses 2 related issues:
>
> 1. ethtool_validate_speed() triggers a "signed-unsigned comparison"
> warning due to type difference of SPEED_UNKNOWN constant (int)
> and argument to ethtool_validate_speed (__u32).
>
> 2. some drivers use u16 storage for SPEED_UNKNOWN constant,
> resulting in value truncation and thus failure to test against
> SPEED_UNKNOWN correctly.
>
> This revised series addresses several feedback comments:
> - split up the patch in to series
> - do not unnecessarily change drivers that use "int" storage
> for speed values
Hi Michael
Your v1 had some changes which changed plain int value speeds to using
the SPEED_X macros. Those changes appear to of disappeared. You could
submit them as well.
Thanks
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant
2019-04-08 17:54 ` [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Andrew Lunn
@ 2019-04-08 18:40 ` Zhivich, Michael
0 siblings, 0 replies; 9+ messages in thread
From: Zhivich, Michael @ 2019-04-08 18:40 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem, siva.kallam, prashant, mchan, shshaikh, manishc, netdev
On 4/8/19, 1:55 PM, "Andrew Lunn" <andrew@lunn.ch> wrote:
>On Mon, Apr 08, 2019 at 10:48:44AM -0400, Michael Zhivich wrote:
>> This patch series addresses 2 related issues:
>>
>> 1. ethtool_validate_speed() triggers a "signed-unsigned comparison"
>> warning due to type difference of SPEED_UNKNOWN constant (int)
>> and argument to ethtool_validate_speed (__u32).
>>
>> 2. some drivers use u16 storage for SPEED_UNKNOWN constant,
>> resulting in value truncation and thus failure to test against
>> SPEED_UNKNOWN correctly.
>>
>> This revised series addresses several feedback comments:
>> - split up the patch in to series
>> - do not unnecessarily change drivers that use "int" storage
>> for speed values
>
>Hi Michael
>
>Your v1 had some changes which changed plain int value speeds to using
>the SPEED_X macros. Those changes appear to of disappeared. You could
>submit them as well.
>
> Thanks
> Andrew
Hi Andrew,
I dropped those changes since they were purely cosmetic and not
required for this patch series.
I'll resubmit that set of changes as a separate patch.
Thanks,
~ Michael
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant
2019-04-08 14:48 [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Michael Zhivich
` (3 preceding siblings ...)
2019-04-08 17:54 ` [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Andrew Lunn
@ 2019-04-08 23:30 ` David Miller
4 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2019-04-08 23:30 UTC (permalink / raw)
To: mzhivich; +Cc: siva.kallam, prashant, mchan, shshaikh, manishc, netdev
From: Michael Zhivich <mzhivich@akamai.com>
Date: Mon, 8 Apr 2019 10:48:44 -0400
> This patch series addresses 2 related issues:
>
> 1. ethtool_validate_speed() triggers a "signed-unsigned comparison"
> warning due to type difference of SPEED_UNKNOWN constant (int)
> and argument to ethtool_validate_speed (__u32).
>
> 2. some drivers use u16 storage for SPEED_UNKNOWN constant,
> resulting in value truncation and thus failure to test against
> SPEED_UNKNOWN correctly.
>
> This revised series addresses several feedback comments:
> - split up the patch in to series
> - do not unnecessarily change drivers that use "int" storage
> for speed values
Series applied.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-04-08 23:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-08 14:48 [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Michael Zhivich
2019-04-08 14:48 ` [PATCH v2 1/3] ethtool: avoid signed-unsigned comparison in ethtool_validate_speed() Michael Zhivich
2019-04-08 14:48 ` [PATCH v2 2/3] broadcom: tg3: fix use of SPEED_UNKNOWN ethtool constant Michael Zhivich
2019-04-08 17:52 ` Andrew Lunn
2019-04-08 14:48 ` [PATCH v2 3/3] qlogic: qlcnic: " Michael Zhivich
2019-04-08 17:53 ` Andrew Lunn
2019-04-08 17:54 ` [PATCH v2 0/3] ethtool: fix use of SPEED_UNKNOWN constant Andrew Lunn
2019-04-08 18:40 ` Zhivich, Michael
2019-04-08 23:30 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.