Re: [patch] net/mlx4: && vs & typo

2017-03-01 Thread David Miller 
From: Dan Carpenter 
Date: Tue, 28 Feb 2017 15:02:15 +0300

> Bitwise & was obviously intended here.
> 
> Fixes: 745d8ae4622c ("net/mlx4: Spoofcheck and zero MAC can't coexist")
> Signed-off-by: Dan Carpenter 

Applied.

Re: [patch] net/mlx4: && vs & typo

2017-03-01 Thread Tariq Toukan 



On 01/03/2017 8:52 AM, Julia Lawall wrote:

On Tue, 28 Feb 2017, Bart Van Assche wrote:


On 02/28/2017 02:23 PM, Joe Perches wrote:

On Tue, 2017-02-28 at 15:35 +, Bart Van Assche wrote:

On Tue, 2017-02-28 at 15:02 +0300, Dan Carpenter wrote:

Bitwise & was obviously intended here.

[]

diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h

[]

@@ -109,7 +109,7 @@ static inline void  (u8 *addr, u64 mac)
int i;

for (i = ETH_ALEN; i > 0; i--) {
-   addr[i - 1] = mac && 0xFF;
+   addr[i - 1] = mac & 0xFF;
mac >>= 8;
}
 }


Is this the only place where such a loop occurs?


Seems to be.


Should a put_unaligned_be48()
function be introduced?


Why?  This is used exactly once.


Really? Here is an example of another open-coded version of
put_unaligned_be48() from arch/mips/cavium-octeon/octeon-platform.c:

new_mac[0] = (mac >> 40) & 0xff;
new_mac[1] = (mac >> 32) & 0xff;
new_mac[2] = (mac >> 24) & 0xff;
new_mac[3] = (mac >> 16) & 0xff;
new_mac[4] = (mac >> 8) & 0xff;
new_mac[5] = mac & 0xff;


drivers/media/radio/radio-shark2.c:
for (i = 0; i < 6; i++)
   shark->transfer_buffer[i + 1] = (reg >> (40 - i * 8)) & 0xff;

drivers/rtc/rtc-ab3100.c
   buf[0] = (hw_counter) & 0xFF;
   buf[1] = (hw_counter >> 8) & 0xFF;
   buf[2] = (hw_counter >> 16) & 0xFF;
   buf[3] = (hw_counter >> 24) & 0xFF;
   buf[4] = (hw_counter >> 32) & 0xFF;
   buf[5] = (hw_counter >> 40) & 0xFF;

drivers/net/ethernet/sun/ldmvsw.c
for (i = 0; i < ETH_ALEN; i++)
   port->raddr[i] = (*rmac >> (5 - i) * 8) & 0xff;

drivers/net/ethernet/sun/sunvnet.c
for (i = 0; i < ETH_ALEN; i++)
   dev->dev_addr[i] = (*local_mac >> (5 - i) * 8) & 0xff;

for (i = 0; i < ETH_ALEN; i++)
   port->raddr[i] = (*rmac >> (5 - i) * 8) & 0xff;

julia



With these code replication examples, I agree that a function should be 
introduced.




Bart.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] net/mlx4: && vs & typo

2017-02-28 Thread Julia Lawall 
On Tue, 28 Feb 2017, Bart Van Assche wrote:

> On 02/28/2017 02:23 PM, Joe Perches wrote:
> > On Tue, 2017-02-28 at 15:35 +, Bart Van Assche wrote:
> >> On Tue, 2017-02-28 at 15:02 +0300, Dan Carpenter wrote:
> >>> Bitwise & was obviously intended here.
> > []
> >>> diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
> > []
> >>> @@ -109,7 +109,7 @@ static inline void(u8 *addr, u64 mac)
> >>>   int i;
> >>>
> >>>   for (i = ETH_ALEN; i > 0; i--) {
> >>> - addr[i - 1] = mac && 0xFF;
> >>> + addr[i - 1] = mac & 0xFF;
> >>>   mac >>= 8;
> >>>   }
> >>>  }
> >>
> >> Is this the only place where such a loop occurs?
> >
> > Seems to be.
> >
> >> Should a put_unaligned_be48()
> >> function be introduced?
> >
> > Why?  This is used exactly once.
>
> Really? Here is an example of another open-coded version of
> put_unaligned_be48() from arch/mips/cavium-octeon/octeon-platform.c:
>
>   new_mac[0] = (mac >> 40) & 0xff;
>   new_mac[1] = (mac >> 32) & 0xff;
>   new_mac[2] = (mac >> 24) & 0xff;
>   new_mac[3] = (mac >> 16) & 0xff;
>   new_mac[4] = (mac >> 8) & 0xff;
>   new_mac[5] = mac & 0xff;

drivers/media/radio/radio-shark2.c:
for (i = 0; i < 6; i++)
   shark->transfer_buffer[i + 1] = (reg >> (40 - i * 8)) & 0xff;

drivers/rtc/rtc-ab3100.c
   buf[0] = (hw_counter) & 0xFF;
   buf[1] = (hw_counter >> 8) & 0xFF;
   buf[2] = (hw_counter >> 16) & 0xFF;
   buf[3] = (hw_counter >> 24) & 0xFF;
   buf[4] = (hw_counter >> 32) & 0xFF;
   buf[5] = (hw_counter >> 40) & 0xFF;

drivers/net/ethernet/sun/ldmvsw.c
for (i = 0; i < ETH_ALEN; i++)
   port->raddr[i] = (*rmac >> (5 - i) * 8) & 0xff;

drivers/net/ethernet/sun/sunvnet.c
for (i = 0; i < ETH_ALEN; i++)
   dev->dev_addr[i] = (*local_mac >> (5 - i) * 8) & 0xff;

for (i = 0; i < ETH_ALEN; i++)
   port->raddr[i] = (*rmac >> (5 - i) * 8) & 0xff;

julia

>
> Bart.
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [patch] net/mlx4: && vs & typo

2017-02-28 Thread Bart Van Assche 
On 02/28/2017 02:23 PM, Joe Perches wrote:
> On Tue, 2017-02-28 at 15:35 +, Bart Van Assche wrote:
>> On Tue, 2017-02-28 at 15:02 +0300, Dan Carpenter wrote:
>>> Bitwise & was obviously intended here.
> []
>>> diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
> []
>>> @@ -109,7 +109,7 @@ static inline void  (u8 *addr, u64 mac)
>>> int i;
>>>  
>>> for (i = ETH_ALEN; i > 0; i--) {
>>> -   addr[i - 1] = mac && 0xFF;
>>> +   addr[i - 1] = mac & 0xFF;
>>> mac >>= 8;
>>> }
>>>  }
>>
>> Is this the only place where such a loop occurs?
> 
> Seems to be.
> 
>> Should a put_unaligned_be48()
>> function be introduced?
> 
> Why?  This is used exactly once.

Really? Here is an example of another open-coded version of
put_unaligned_be48() from arch/mips/cavium-octeon/octeon-platform.c:

new_mac[0] = (mac >> 40) & 0xff;
new_mac[1] = (mac >> 32) & 0xff;
new_mac[2] = (mac >> 24) & 0xff;
new_mac[3] = (mac >> 16) & 0xff;
new_mac[4] = (mac >> 8) & 0xff;
new_mac[5] = mac & 0xff;

Bart.

Re: [patch] net/mlx4: && vs & typo

2017-02-28 Thread Joe Perches 
On Tue, 2017-02-28 at 15:35 +, Bart Van Assche wrote:
> On Tue, 2017-02-28 at 15:02 +0300, Dan Carpenter wrote:
> > Bitwise & was obviously intended here.
[]
> > diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
[]
> > @@ -109,7 +109,7 @@ static inline void  (u8 *addr, u64 mac)
> > int i;
> >  
> > for (i = ETH_ALEN; i > 0; i--) {
> > -   addr[i - 1] = mac && 0xFF;
> > +   addr[i - 1] = mac & 0xFF;
> > mac >>= 8;
> > }
> >  }
> 
> Is this the only place where such a loop occurs?

Seems to be.

> Should a put_unaligned_be48()
> function be introduced?

Why?  This is used exactly once.

Re: [patch] net/mlx4: && vs & typo

2017-02-28 Thread Tariq Toukan 



On 28/02/2017 2:02 PM, Dan Carpenter wrote:

Bitwise & was obviously intended here.


Sure! Thanks for your patch.



Fixes: 745d8ae4622c ("net/mlx4: Spoofcheck and zero MAC can't coexist")
Signed-off-by: Dan Carpenter 
---
Applies to net.git.

diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
index e965e5090d96..a858bcb6220b 100644
--- a/include/linux/mlx4/driver.h
+++ b/include/linux/mlx4/driver.h
@@ -109,7 +109,7 @@ static inline void mlx4_u64_to_mac(u8 *addr, u64 mac)
int i;

for (i = ETH_ALEN; i > 0; i--) {
-   addr[i - 1] = mac && 0xFF;
+   addr[i - 1] = mac & 0xFF;
mac >>= 8;
}
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Reviewed-by: Tariq Toukan

Re: [patch] net/mlx4: && vs & typo

2017-02-28 Thread Bart Van Assche 
On Tue, 2017-02-28 at 15:02 +0300, Dan Carpenter wrote:
> Bitwise & was obviously intended here.
> 
> Fixes: 745d8ae4622c ("net/mlx4: Spoofcheck and zero MAC can't coexist")
> Signed-off-by: Dan Carpenter 
> ---
> Applies to net.git.
> 
> diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
> index e965e5090d96..a858bcb6220b 100644
> --- a/include/linux/mlx4/driver.h
> +++ b/include/linux/mlx4/driver.h
> @@ -109,7 +109,7 @@ static inline void mlx4_u64_to_mac(u8 *addr, u64 mac)
>   int i;
>  
>   for (i = ETH_ALEN; i > 0; i--) {
> - addr[i - 1] = mac && 0xFF;
> + addr[i - 1] = mac & 0xFF;
>   mac >>= 8;
>   }
>  }

Is this the only place where such a loop occurs? Should a put_unaligned_be48()
function be introduced?

Bart.

[patch] net/mlx4: && vs & typo

2017-02-28 Thread Dan Carpenter 
Bitwise & was obviously intended here.

Fixes: 745d8ae4622c ("net/mlx4: Spoofcheck and zero MAC can't coexist")
Signed-off-by: Dan Carpenter 
---
Applies to net.git.

diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
index e965e5090d96..a858bcb6220b 100644
--- a/include/linux/mlx4/driver.h
+++ b/include/linux/mlx4/driver.h
@@ -109,7 +109,7 @@ static inline void mlx4_u64_to_mac(u8 *addr, u64 mac)
int i;
 
for (i = ETH_ALEN; i > 0; i--) {
-   addr[i - 1] = mac && 0xFF;
+   addr[i - 1] = mac & 0xFF;
mac >>= 8;
}
 }