Re: [PATCH -next] qlcnic: fix compiler warning

Re: [PATCH -next] qlcnic: fix compiler warning

[Martin Kaiser]

re-sent as requested by David M.
(I understand the concerns about portability that were raised. At the moment, I don't have a possiblity to do
further rework and testing)

Add an explicit cast to fix the following warning
(seen on Debian Wheezy, gcc 4.7.2)

CC [M]  drivers/net/wireless/rtlwifi/rtl8192ce/trx.o
    drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c: In function ‘qlcnic_send_filter’:
    drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:349:3: warning:                   
    passing argument 2 of ‘ether_addr_equal’ from incompatible pointer type [enabled by default]
    In file included from include/linux/if_vlan.h:16:0,                                         
    from drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:9:
    include/linux/etherdevice.h:244:20: note: expected ‘const u8 *’ but argument is of type ‘u64 *’

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
Acked-by: Shahed Shaikh <shahed.shaikh@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
index 6373f60..3557154 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
@@ -346,7 +346,7 @@ static void qlcnic_send_filter(struct qlcnic_adapter *adapter,
        head = &(adapter->fhash.fhead[hindex]);

        hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
-               if (ether_addr_equal(tmp_fil->faddr, &src_addr) &&
+               if (ether_addr_equal(tmp_fil->faddr, (const u8 *)&src_addr) &&
                    tmp_fil->vlan_id == vlan_id) {
                        if (jiffies > (QLCNIC_READD_AGE * HZ + tmp_fil->ftime))
                                qlcnic_change_filter(adapter, &src_addr,

Re: [PATCH -next] qlcnic: fix compiler warning

[David Miller]

From: Martin Kaiser <martin@kaiser.cx>
Date: Tue, 14 Jan 2014 16:21:28 +0100

> re-sent as requested by David M.

Sorry, dianthong sent his version of the fix meanwhile and I applied
his copy.

RE: [PATCH -next] qlcnic: fix compiler warning

[Shahed Shaikh]

Adding netdev.

> -----Original Message-----
> From: Martin Kaiser,,, [mailto:martin@reykholt.kaiser.cx] On Behalf Of
> Martin Kaiser
> Sent: Thursday, January 09, 2014 9:29 PM
> To: Himanshu Madhani; Rajesh Borundia
> Cc: linux-kernel; trivial@kernel.org
> Subject: [PATCH -next] qlcnic: fix compiler warning
> 
> Add an explicit cast to fix the following warning (seen on Debian Wheezy, gcc
> 4.7.2)
> 
> CC [M]  drivers/net/wireless/rtlwifi/rtl8192ce/trx.o
>     drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c: In function
> ‘qlcnic_send_filter’:
>     drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:349:3: warning:
>     passing argument 2 of ‘ether_addr_equal’ from incompatible pointer type
> [enabled by default]
>     In file included from include/linux/if_vlan.h:16:0,
>     from drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:9:
>     include/linux/etherdevice.h:244:20: note: expected ‘const u8 *’ but
> argument is of type ‘u64 *’
>

If I am not wrong, this patch should go to David's net-next tree.
 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

Acked-by: Shahed Shaikh <shahed.shaikh@qlogic.com>

Thanks,
Shahed
> ---
>  drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> index 6373f60..3557154 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> @@ -346,7 +346,7 @@ static void qlcnic_send_filter(struct qlcnic_adapter
> *adapter,
>  	head = &(adapter->fhash.fhead[hindex]);
> 
>  	hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
> -		if (ether_addr_equal(tmp_fil->faddr, &src_addr) &&
> +		if (ether_addr_equal(tmp_fil->faddr, (const u8 *)&src_addr)
> &&
>  		    tmp_fil->vlan_id == vlan_id) {
>  			if (jiffies > (QLCNIC_READD_AGE * HZ + tmp_fil-
> >ftime))
>  				qlcnic_change_filter(adapter, &src_addr,
> --
> 1.7.10.4

RE: [PATCH -next] qlcnic: fix compiler warning

[David Laight]

From: Shahed Shaikh
> Adding netdev.
> 
> > -----Original Message-----
> > From: Martin Kaiser,,, [mailto:martin@reykholt.kaiser.cx] On Behalf Of
> > Martin Kaiser
> > Sent: Thursday, January 09, 2014 9:29 PM
> > To: Himanshu Madhani; Rajesh Borundia
> > Cc: linux-kernel; trivial@kernel.org
> > Subject: [PATCH -next] qlcnic: fix compiler warning
> >
> > Add an explicit cast to fix the following warning (seen on Debian Wheezy, gcc
> > 4.7.2)
> >
> > CC [M]  drivers/net/wireless/rtlwifi/rtl8192ce/trx.o
> >     drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c: In function
> > qlcnic_send_filter:
> >     drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:349:3: warning:
> >     passing argument 2 of ether_addr_equal from incompatible pointer type
> > [enabled by default]
> >     In file included from include/linux/if_vlan.h:16:0,
> >     from drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:9:
> >     include/linux/etherdevice.h:244:20: note: expected const u8 * but
> > argument is of type u64 *
> >
> 
> If I am not wrong, this patch should go to David's net-next tree.
> 
> > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> 
> Acked-by: Shahed Shaikh <shahed.shaikh@qlogic.com>
> 
> Thanks,
> Shahed
> > ---
> >  drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> > b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> > index 6373f60..3557154 100644
> > --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> > +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
> > @@ -346,7 +346,7 @@ static void qlcnic_send_filter(struct qlcnic_adapter
> > *adapter,
> >  	head = &(adapter->fhash.fhead[hindex]);
> >
> >  	hlist_for_each_entry_safe(tmp_fil, n, head, fnode) {
> > -		if (ether_addr_equal(tmp_fil->faddr, &src_addr) &&
> > +		if (ether_addr_equal(tmp_fil->faddr, (const u8 *)&src_addr)
> > &&
> >  		    tmp_fil->vlan_id == vlan_id) {
> >  			if (jiffies > (QLCNIC_READD_AGE * HZ + tmp_fil-
> > >ftime))
> >  				qlcnic_change_filter(adapter, &src_addr,
> > --
> > 1.7.10.4

A quick look at the code seems to imply that this code is somewhat buggy.
'src_addr' is defined a u64 even though it is a 6 byte mac address.
On big-endian systems the wrong bytes get accessed all over the place.

Also when ether_addr_equal() in inlined the code ends up accessing src_addr
as 32bit and 16bit data - which don't have to be synchronised against
and writes to the 64bit item.

If might be that ether_addr_equal() (etc) should have a gcc asm
statement with a "memory" constraint against the mac address buffers.

	David

Re: [PATCH -next] qlcnic: fix compiler warning

[David Miller]

From: Shahed Shaikh <shahed.shaikh@qlogic.com>
Date: Fri, 10 Jan 2014 07:19:19 +0000

> 
> Adding netdev.
> 
>> -----Original Message-----
>> From: Martin Kaiser,,, [mailto:martin@reykholt.kaiser.cx] On Behalf Of
>> Martin Kaiser
>> Sent: Thursday, January 09, 2014 9:29 PM
>> To: Himanshu Madhani; Rajesh Borundia
>> Cc: linux-kernel; trivial@kernel.org
>> Subject: [PATCH -next] qlcnic: fix compiler warning
>> 
>> Add an explicit cast to fix the following warning (seen on Debian Wheezy, gcc
>> 4.7.2)
>> 
>> CC [M]  drivers/net/wireless/rtlwifi/rtl8192ce/trx.o
>>     drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c: In function
>> ‘qlcnic_send_filter’:
>>     drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:349:3: warning:
>>     passing argument 2 of ‘ether_addr_equal’ from incompatible pointer type
>> [enabled by default]
>>     In file included from include/linux/if_vlan.h:16:0,
>>     from drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c:9:
>>     include/linux/etherdevice.h:244:20: note: expected ‘const u8 *’ but
>> argument is of type ‘u64 *’
>>
> 
> If I am not wrong, this patch should go to David's net-next tree.
>  
>> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> 
> Acked-by: Shahed Shaikh <shahed.shaikh@qlogic.com>

Please post the original patch to netdev so that it properly gets
queued up in patchwork, thank you very much.