Regression in 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

Regression in 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

[Heiner Kallweit]

Recently ethtool started to give incomplete values for supported and
advertised modes. There seems to be a regression in this commit:
The bit number parameter in the calls to bitmap_to_arr32() in
store_link_ksettings_for_user() should be __ETHTOOL_LINK_MODE_MASK_NBITS
instead of __ETHTOOL_LINK_MODE_MASK_NU32.

After having changed this ethtool behaves normally again.

Heiner

Re: Regression in 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

[Yury Norov]

On Mon, Feb 05, 2018 at 07:21:32AM +0100, Heiner Kallweit wrote:
> Recently ethtool started to give incomplete values for supported and
> advertised modes. There seems to be a regression in this commit:
> The bit number parameter in the calls to bitmap_to_arr32() in
> store_link_ksettings_for_user() should be __ETHTOOL_LINK_MODE_MASK_NBITS
> instead of __ETHTOOL_LINK_MODE_MASK_NU32.
> 
> After having changed this ethtool behaves normally again.
> 
> Heiner

Hi Heiner,

Thank you for discovering this, and sorry for my error. 
The patch is below.

Yury

>From af960f7ae1be8e86abfaa39300ba690c24a80e5b Mon Sep 17 00:00:00 2001
From: Yury Norov <ynorov@caviumnetworks.com>
Date: Mon, 5 Feb 2018 10:01:59 +0300
Subject: [PATCH] Fix 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

During rework, bitmap_to_arr32() was erroneously passed with
__ETHTOOL_LINK_MODE_MASK_NU32 instead of __ETHTOOL_LINK_MODE_MASK_NBITS.

Fixes: 39461af74125fd268 (bitmap: replace bitmap_{from,to}_u32array)
Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 net/core/ethtool.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1822335518ea..494e6a5d7306 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -642,13 +642,13 @@ store_link_ksettings_for_user(void __user *to,
 	memcpy(&link_usettings.base, &from->base, sizeof(link_usettings));
 	bitmap_to_arr32(link_usettings.link_modes.supported,
 			from->link_modes.supported,
-			__ETHTOOL_LINK_MODE_MASK_NU32);
+			__ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_to_arr32(link_usettings.link_modes.advertising,
 			from->link_modes.advertising,
-			__ETHTOOL_LINK_MODE_MASK_NU32);
+			__ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_to_arr32(link_usettings.link_modes.lp_advertising,
 			from->link_modes.lp_advertising,
-			__ETHTOOL_LINK_MODE_MASK_NU32);
+			__ETHTOOL_LINK_MODE_MASK_NBITS);
 
 	if (copy_to_user(to, &link_usettings, sizeof(link_usettings)))
 		return -EFAULT;
-- 
2.14.1

Re: Regression in 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Mon, 5 Feb 2018 07:21:32 +0100

> Recently ethtool started to give incomplete values for supported and
> advertised modes. There seems to be a regression in this commit:
> The bit number parameter in the calls to bitmap_to_arr32() in
> store_link_ksettings_for_user() should be __ETHTOOL_LINK_MODE_MASK_NBITS
> instead of __ETHTOOL_LINK_MODE_MASK_NU32.
> 
> After having changed this ethtool behaves normally again.

I do not see the commit with the SHA1 ID from the Subject line in
either Linus's nor my networking tree.

Where is that change?

[davem@dhcp-10-15-49-227 linux]$ git show 39461af74125 >x.diff
fatal: ambiguous argument '39461af74125': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
[davem@dhcp-10-15-49-227 linux]$ cd ../net
[davem@dhcp-10-15-49-227 net]$ git show 39461af74125 >x.diff
fatal: ambiguous argument '39461af74125': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
[davem@dhcp-10-15-49-227 net]$

Re: Regression in 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

[Yury Norov]

On Tue, Feb 06, 2018 at 11:17:36AM -0500, David Miller wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Date: Mon, 5 Feb 2018 07:21:32 +0100
> 
> > Recently ethtool started to give incomplete values for supported and
> > advertised modes. There seems to be a regression in this commit:
> > The bit number parameter in the calls to bitmap_to_arr32() in
> > store_link_ksettings_for_user() should be __ETHTOOL_LINK_MODE_MASK_NBITS
> > instead of __ETHTOOL_LINK_MODE_MASK_NU32.
> > 
> > After having changed this ethtool behaves normally again.
> 
> I do not see the commit with the SHA1 ID from the Subject line in
> either Linus's nor my networking tree.
> 
> Where is that change?

It's in next-20180202.

Yury

Re: Regression in 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

[David Miller]

From: Yury Norov <ynorov@caviumnetworks.com>
Date: Tue, 6 Feb 2018 19:26:23 +0300

> On Tue, Feb 06, 2018 at 11:17:36AM -0500, David Miller wrote:
>> From: Heiner Kallweit <hkallweit1@gmail.com>
>> Date: Mon, 5 Feb 2018 07:21:32 +0100
>> 
>> > Recently ethtool started to give incomplete values for supported and
>> > advertised modes. There seems to be a regression in this commit:
>> > The bit number parameter in the calls to bitmap_to_arr32() in
>> > store_link_ksettings_for_user() should be __ETHTOOL_LINK_MODE_MASK_NBITS
>> > instead of __ETHTOOL_LINK_MODE_MASK_NU32.
>> > 
>> > After having changed this ethtool behaves normally again.
>> 
>> I do not see the commit with the SHA1 ID from the Subject line in
>> either Linus's nor my networking tree.
>> 
>> Where is that change?
> 
> It's in next-20180202.

Hmmm, then the fix should probably go into whatever tree that commit
ultimately came from.

Re: Regression in 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

[Yury Norov]

On Tue, Feb 06, 2018 at 11:42:18AM -0500, David Miller wrote:
> From: Yury Norov <ynorov@caviumnetworks.com>
> Date: Tue, 6 Feb 2018 19:26:23 +0300
> 
> > On Tue, Feb 06, 2018 at 11:17:36AM -0500, David Miller wrote:
> >> From: Heiner Kallweit <hkallweit1@gmail.com>
> >> Date: Mon, 5 Feb 2018 07:21:32 +0100
> >> 
> >> > Recently ethtool started to give incomplete values for supported and
> >> > advertised modes. There seems to be a regression in this commit:
> >> > The bit number parameter in the calls to bitmap_to_arr32() in
> >> > store_link_ksettings_for_user() should be __ETHTOOL_LINK_MODE_MASK_NBITS
> >> > instead of __ETHTOOL_LINK_MODE_MASK_NU32.
> >> > 
> >> > After having changed this ethtool behaves normally again.
> >> 
> >> I do not see the commit with the SHA1 ID from the Subject line in
> >> either Linus's nor my networking tree.
> >> 
> >> Where is that change?
> > 
> > It's in next-20180202.
> 
> Hmmm, then the fix should probably go into whatever tree that commit
> ultimately came from.

It's already in Andrew's tree. The original patch is not about networking,
and Andrew's tree is probably better place for fix. My apologies for noise
in your tree.

http://ozlabs.org/~akpm/mmots/broken-out/bitmap-replace-bitmap_fromto_u32array-fix-2.patch

Re: Regression in 39461af74125 "bitmap: replace bitmap_{from,to}_u32array"

[David Miller]

From: Yury Norov <ynorov@caviumnetworks.com>
Date: Tue, 6 Feb 2018 20:03:13 +0300

> On Tue, Feb 06, 2018 at 11:42:18AM -0500, David Miller wrote:
>> From: Yury Norov <ynorov@caviumnetworks.com>
>> Date: Tue, 6 Feb 2018 19:26:23 +0300
>> 
>> > On Tue, Feb 06, 2018 at 11:17:36AM -0500, David Miller wrote:
>> >> From: Heiner Kallweit <hkallweit1@gmail.com>
>> >> Date: Mon, 5 Feb 2018 07:21:32 +0100
>> >> 
>> >> > Recently ethtool started to give incomplete values for supported and
>> >> > advertised modes. There seems to be a regression in this commit:
>> >> > The bit number parameter in the calls to bitmap_to_arr32() in
>> >> > store_link_ksettings_for_user() should be __ETHTOOL_LINK_MODE_MASK_NBITS
>> >> > instead of __ETHTOOL_LINK_MODE_MASK_NU32.
>> >> > 
>> >> > After having changed this ethtool behaves normally again.
>> >> 
>> >> I do not see the commit with the SHA1 ID from the Subject line in
>> >> either Linus's nor my networking tree.
>> >> 
>> >> Where is that change?
>> > 
>> > It's in next-20180202.
>> 
>> Hmmm, then the fix should probably go into whatever tree that commit
>> ultimately came from.
> 
> It's already in Andrew's tree. The original patch is not about networking,
> and Andrew's tree is probably better place for fix. My apologies for noise
> in your tree.
> 
> http://ozlabs.org/~akpm/mmots/broken-out/bitmap-replace-bitmap_fromto_u32array-fix-2.patch

Thanks for the clarification.