[PATCH net 1/2] net: add eth_addr_inc in etherdevice.h

[PATCH net 1/2] net: add eth_addr_inc in etherdevice.h

[Tao Ren]

Add eth_addr_inc function in etherdevice.h to increment MAC address. One
of the use cases is in ncsi stack, where the host's MAC address needs to
be incremented to get BMC's MAC address.

Signed-off-by: Tao Ren <taoren@fb.com>
---
 include/linux/etherdevice.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index e2f3b21cd72a..d48e3a724c54 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -448,6 +448,19 @@ static inline void eth_addr_dec(u8 *addr)
 	u64_to_ether_addr(u, addr);
 }
 
+/**
+ * eth_addr_inc - Increment the given MAC address
+ *
+ * @addr: Pointer to a six-byte array containing Ethernet address to increment
+ */
+static inline void eth_addr_inc(u8 *addr)
+{
+	u64 u = ether_addr_to_u64(addr);
+
+	u++;
+	u64_to_ether_addr(u, addr);
+}
+
 /**
  * is_etherdev_addr - Tell if given Ethernet address belongs to the device.
  * @dev: Pointer to a device structure
-- 
2.17.1

Re: [PATCH net 1/2] net: add eth_addr_inc in etherdevice.h

[Jakub Kicinski]

On Tue, 23 Apr 2019 22:41:56 +0000, Tao Ren wrote:
> Add eth_addr_inc function in etherdevice.h to increment MAC address. One
> of the use cases is in ncsi stack, where the host's MAC address needs to
> be incremented to get BMC's MAC address.
> 
> Signed-off-by: Tao Ren <taoren@fb.com>

Please squash this patch with the next one, IMHO they form a single
logical change together.

>  include/linux/etherdevice.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index e2f3b21cd72a..d48e3a724c54 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -448,6 +448,19 @@ static inline void eth_addr_dec(u8 *addr)
>  	u64_to_ether_addr(u, addr);
>  }
>  
> +/**
> + * eth_addr_inc - Increment the given MAC address
> + *
> + * @addr: Pointer to a six-byte array containing Ethernet address to increment

Please see:  Documentation/doc-guide/kernel-doc.rst

Here we need:
 - "()" after function name;
 - no extra line between function name and argument description.

> + */
> +static inline void eth_addr_inc(u8 *addr)
> +{
> +	u64 u = ether_addr_to_u64(addr);
> +
> +	u++;
> +	u64_to_ether_addr(u, addr);
> +}
> +
>  /**
>   * is_etherdev_addr - Tell if given Ethernet address belongs to the device.
>   * @dev: Pointer to a device structure

Re: [PATCH net 1/2] net: add eth_addr_inc in etherdevice.h

[Jakub Kicinski]

On Tue, 23 Apr 2019 22:41:56 +0000, Tao Ren wrote:
> Add eth_addr_inc function in etherdevice.h to increment MAC address. One
> of the use cases is in ncsi stack, where the host's MAC address needs to
> be incremented to get BMC's MAC address.
> 
> Signed-off-by: Tao Ren <taoren@fb.com>

Please squash this patch with the next one, IMHO they form a single
logical change together.

>  include/linux/etherdevice.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index e2f3b21cd72a..d48e3a724c54 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -448,6 +448,19 @@ static inline void eth_addr_dec(u8 *addr)
>  	u64_to_ether_addr(u, addr);
>  }
>  
> +/**
> + * eth_addr_inc - Increment the given MAC address
> + *
> + * @addr: Pointer to a six-byte array containing Ethernet address to increment

Please see:  Documentation/doc-guide/kernel-doc.rst

Here we need:
 - "()" after function name;
 - no extra line between function name and argument description.

> + */
> +static inline void eth_addr_inc(u8 *addr)
> +{
> +	u64 u = ether_addr_to_u64(addr);
> +
> +	u++;
> +	u64_to_ether_addr(u, addr);
> +}
> +
>  /**
>   * is_etherdev_addr - Tell if given Ethernet address belongs to the device.
>   * @dev: Pointer to a device structure

Re: [PATCH net 1/2] net: add eth_addr_inc in etherdevice.h

[Tao Ren]

On 4/23/19 4:15 PM, Jakub Kicinski wrote:
> On Tue, 23 Apr 2019 22:41:56 +0000, Tao Ren wrote:
>> Add eth_addr_inc function in etherdevice.h to increment MAC address. One
>> of the use cases is in ncsi stack, where the host's MAC address needs to
>> be incremented to get BMC's MAC address.
>>
>> Signed-off-by: Tao Ren <taoren@fb.com>
> 
> Please squash this patch with the next one, IMHO they form a single
> logical change together.

Sure. Will send out updated patch soon.

>>  include/linux/etherdevice.h | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
>> index e2f3b21cd72a..d48e3a724c54 100644
>> --- a/include/linux/etherdevice.h
>> +++ b/include/linux/etherdevice.h
>> @@ -448,6 +448,19 @@ static inline void eth_addr_dec(u8 *addr)
>>  	u64_to_ether_addr(u, addr);
>>  }
>>  
>> +/**
>> + * eth_addr_inc - Increment the given MAC address
>> + *
>> + * @addr: Pointer to a six-byte array containing Ethernet address to increment
> 
> Please see:  Documentation/doc-guide/kernel-doc.rst
> 
> Here we need:
>  - "()" after function name;
>  - no extra line between function name and argument description.

Thank you for pointing it out (I didn't know the doc-guide). Given I copied the function comment from eth_addr_dec(), I will also fix the format for eth_addr_eth() then.

BTW, "()" is missing from all the other functions' comment in etherdevice.h, so maybe we should fix that in a separate patch?


Thanks,

- Tao

Re: [PATCH net 1/2] net: add eth_addr_inc in etherdevice.h

[Jakub Kicinski]

On Wed, 24 Apr 2019 00:59:52 +0000, Tao Ren wrote:
> > Please see:  Documentation/doc-guide/kernel-doc.rst
> > 
> > Here we need:
> >  - "()" after function name;
> >  - no extra line between function name and argument description.  
> 
> Thank you for pointing it out (I didn't know the doc-guide). Given I copied the function comment from eth_addr_dec(), I will also fix the format for eth_addr_eth() then.
> 
> BTW, "()" is missing from all the other functions' comment in etherdevice.h, so maybe we should fix that in a separate patch?

Possibly, I'm just trying to make sure the new stuff we add follows the
official guidelines :)  For the old stuff we need to way it with
potential merge conflicts and muddied git history so the case is not as
clear cut.

Re: [PATCH net 1/2] net: add eth_addr_inc in etherdevice.h

[Jakub Kicinski]

On Wed, 24 Apr 2019 00:59:52 +0000, Tao Ren wrote:
> > Please see:  Documentation/doc-guide/kernel-doc.rst
> > 
> > Here we need:
> >  - "()" after function name;
> >  - no extra line between function name and argument description.  
> 
> Thank you for pointing it out (I didn't know the doc-guide). Given I copied the function comment from eth_addr_dec(), I will also fix the format for eth_addr_eth() then.
> 
> BTW, "()" is missing from all the other functions' comment in etherdevice.h, so maybe we should fix that in a separate patch?

Possibly, I'm just trying to make sure the new stuff we add follows the
official guidelines :)  For the old stuff we need to way it with
potential merge conflicts and muddied git history so the case is not as
clear cut.

[PATCH net 1/2] net: add eth_addr_inc in etherdevice.h

[Tao Ren]

Add eth_addr_inc function in etherdevice.h to increment MAC address. One
of the use cases is in ncsi stack, where the host's MAC address needs to
be incremented to get BMC's MAC address.

Signed-off-by: Tao Ren <taoren@fb.com>
---
 include/linux/etherdevice.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index e2f3b21cd72a..d48e3a724c54 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -448,6 +448,19 @@ static inline void eth_addr_dec(u8 *addr)
 	u64_to_ether_addr(u, addr);
 }
 
+/**
+ * eth_addr_inc - Increment the given MAC address
+ *
+ * @addr: Pointer to a six-byte array containing Ethernet address to increment
+ */
+static inline void eth_addr_inc(u8 *addr)
+{
+	u64 u = ether_addr_to_u64(addr);
+
+	u++;
+	u64_to_ether_addr(u, addr);
+}
+
 /**
  * is_etherdev_addr - Tell if given Ethernet address belongs to the device.
  * @dev: Pointer to a device structure
-- 
2.17.1