lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fbdc3741b28a0174e3058a998e253439@codethink.co.uk>
Date: Fri, 23 Jun 2023 10:43:12 +0100
From: Ben Dooks <ben.dooks@...ethink.co.uk>
To: Simon Horman <simon.horman@...igine.com>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, kuba@...nel.org,
 edumazet@...gle.com, davem@...emloft.net, linux-kernel@...r.kernel.org,
 claudiu.beznea@...rochip.com, nicolas.ferre@...rochip.com
Subject: Re: [PATCH 3/3] net: macb: fix __be32 warnings in debug code



On 2023-06-22 16:44, Simon Horman wrote:
> On Thu, Jun 22, 2023 at 02:05:07PM +0100, Ben Dooks wrote:
>> The netdev_dbg() calls in gem_add_flow_filter() and 
>> gem_del_flow_filter()
>> call ntohl() on the ipv4 addresses, which will put them into the host 
>> order
>> but not the right type (returns a __be32, not an u32 as would be 
>> expected).
>> 
>> Chaning the htonl() to nthol() should still do the right conversion, 
>> return
>> the correct u32 type and  should not change any functional to remove 
>> the
>> following sparse warnings:
>> 
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: incorrect 
>> type in argument 1 (different base types)
>> drivers/net/ethernet/cadence/macb_main.c:3568:9:    expected unsigned 
>> int [usertype] val
>> drivers/net/ethernet/cadence/macb_main.c:3568:9:    got restricted 
>> __be32 [usertype] ip4src
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: incorrect 
>> type in argument 1 (different base types)
>> drivers/net/ethernet/cadence/macb_main.c:3568:9:    expected unsigned 
>> int [usertype] val
>> drivers/net/ethernet/cadence/macb_main.c:3568:9:    got restricted 
>> __be32 [usertype] ip4dst
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3568:9: warning: cast from 
>> restricted __be32
>> d
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: incorrect 
>> type in argument 1 (different base types)
>> drivers/net/ethernet/cadence/macb_main.c:3622:25:    expected unsigned 
>> int [usertype] val
>> drivers/net/ethernet/cadence/macb_main.c:3622:25:    got restricted 
>> __be32 [usertype] ip4src
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: incorrect 
>> type in argument 1 (different base types)
>> drivers/net/ethernet/cadence/macb_main.c:3622:25:    expected unsigned 
>> int [usertype] val
>> drivers/net/ethernet/cadence/macb_main.c:3622:25:    got restricted 
>> __be32 [usertype] ip4dst
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from 
>> restricted __be32
>> drivers/net/ethernet/cadence/macb_main.c:3622:25: warning: cast from 
>> restricted __be32
>> 
>> Signed-off-by: Ben Dooks <ben.dooks@...ethink.co.uk>
> 
> Hi Ben,
> 
> this code-change looks good to me, but I have a few minor nits for your
> consideration.
> 
> 1. Please specify the target tree, in this case net-next, for patch 
> sets
>    for Networking code.
> 
> 	Subject: [PATCH net-next ...] ...

Ah, was using net, but I assume net-next is probably ok

> 2. It might be nicer to write '.../macb_main.c' or similar,
>    rather tha nthe full path, in the patch description.
> 
> 3. checkpatch --codespell says: 'Chaning' -> 'Chaining'

Ok, thank you. I didn't know about that.

Since there's another patch that needs work I'll re-send this early next 
week
with the fixes in.

>> ---
>>  drivers/net/ethernet/cadence/macb_main.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
>> b/drivers/net/ethernet/cadence/macb_main.c
>> index 56e202b74bd7..59a90c2b307f 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -3568,8 +3568,8 @@ static int gem_add_flow_filter(struct net_device 
>> *netdev,
>>  	netdev_dbg(netdev,
>>  			"Adding flow filter 
>> entry,type=%u,queue=%u,loc=%u,src=%08X,dst=%08X,ps=%u,pd=%u\n",
>>  			fs->flow_type, (int)fs->ring_cookie, fs->location,
>> -			htonl(fs->h_u.tcp_ip4_spec.ip4src),
>> -			htonl(fs->h_u.tcp_ip4_spec.ip4dst),
>> +			ntohl(fs->h_u.tcp_ip4_spec.ip4src),
>> +			ntohl(fs->h_u.tcp_ip4_spec.ip4dst),
>>  			be16_to_cpu(fs->h_u.tcp_ip4_spec.psrc),
>>  			be16_to_cpu(fs->h_u.tcp_ip4_spec.pdst));
>> 
>> @@ -3622,8 +3622,8 @@ static int gem_del_flow_filter(struct net_device 
>> *netdev,
>>  			netdev_dbg(netdev,
>>  					"Deleting flow filter 
>> entry,type=%u,queue=%u,loc=%u,src=%08X,dst=%08X,ps=%u,pd=%u\n",
>>  					fs->flow_type, (int)fs->ring_cookie, fs->location,
>> -					htonl(fs->h_u.tcp_ip4_spec.ip4src),
>> -					htonl(fs->h_u.tcp_ip4_spec.ip4dst),
>> +					ntohl(fs->h_u.tcp_ip4_spec.ip4src),
>> +					ntohl(fs->h_u.tcp_ip4_spec.ip4dst),
>>  					be16_to_cpu(fs->h_u.tcp_ip4_spec.psrc),
>>  					be16_to_cpu(fs->h_u.tcp_ip4_spec.pdst));
>> 
>> --
>> 2.40.1
>> 
>> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ