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

From: David Laight
Date: Fri Jan 10 2014 - 05:17:55 EST


From: Shahed Shaikh
> Adding netdev.
>
> > -----Original Message-----
> > From: Martin Kaiser,,, [mailto:martin@xxxxxxxxxxxxxxxxxx] On Behalf Of
> > Martin Kaiser
> > Sent: Thursday, January 09, 2014 9:29 PM
> > To: Himanshu Madhani; Rajesh Borundia
> > Cc: linux-kernel; trivial@xxxxxxxxxx
> > 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@xxxxxxxxx>
>
> Acked-by: Shahed Shaikh <shahed.shaikh@xxxxxxxxxx>
>
> 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

èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸ ¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðà nÆàþY&—