diff mbox

[ethtool,4/6] Add support for __be64 and bitops to ethtool

Message ID 20110421204035.23054.6918.stgit@gitlad.jf.intel.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Duyck, Alexander H April 21, 2011, 8:40 p.m. UTC 
This change is meant to add support for __be64 values and bitops to
ethtool.  These changes will be needed in order to support network flow
classifier rule configuration.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 ethtool-bitops.h |   25 +++++++++++++++++++++++++
 ethtool-util.h   |   30 ++++++++++++++++++++++++++----
 ethtool.c        |    7 -------
 3 files changed, 51 insertions(+), 11 deletions(-)
 create mode 100644 ethtool-bitops.h


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

Comments

Ben Hutchings April 27, 2011, 3:54 p.m. UTC | #1 
On Thu, 2011-04-21 at 13:40 -0700, Alexander Duyck wrote:
> This change is meant to add support for __be64 values and bitops to
> ethtool.  These changes will be needed in order to support network flow
> classifier rule configuration.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
>  ethtool-bitops.h |   25 +++++++++++++++++++++++++
>  ethtool-util.h   |   30 ++++++++++++++++++++++++++----
>  ethtool.c        |    7 -------
>  3 files changed, 51 insertions(+), 11 deletions(-)
>  create mode 100644 ethtool-bitops.h
> 
> diff --git a/ethtool-bitops.h b/ethtool-bitops.h
> new file mode 100644
> index 0000000..0ff14f1
> --- /dev/null
> +++ b/ethtool-bitops.h
> @@ -0,0 +1,25 @@
> +#ifndef ETHTOOL_BITOPS_H__
> +#define ETHTOOL_BITOPS_H__
> +
> +#define BITS_PER_BYTE		8
> +#define BITS_PER_LONG		(BITS_PER_BYTE * sizeof(long))
> +#define DIV_ROUND_UP(n, d)	(((n) + (d) - 1) / (d))
> +#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_LONG)
> +
> +static inline void set_bit(int nr, unsigned long *addr)
> +{
> +	addr[nr / BITS_PER_LONG] |= 1UL << (nr % BITS_PER_LONG);
> +}
> +
> +static inline void clear_bit(int nr, unsigned long *addr)
> +{
> +	addr[nr / BITS_PER_LONG] &= ~(1UL << (nr % BITS_PER_LONG));
> +}
> +
> +static __always_inline int test_bit(unsigned int nr, const unsigned long *addr)
> +{
> +	return !!((1UL << (nr % BITS_PER_LONG)) &
> +		  (((unsigned long *)addr)[nr / BITS_PER_LONG]));
> +}

Where is __always_inline supposed to be defined?

> +#endif
> diff --git a/ethtool-util.h b/ethtool-util.h
> index f053028..3d46faf 100644
> --- a/ethtool-util.h
> +++ b/ethtool-util.h
> @@ -5,15 +5,18 @@
>  
>  #include <sys/types.h>
>  #include <endian.h>
> +#include <sys/ioctl.h>
> +#include <net/if.h>
> +#include "ethtool-config.h"
> +#include "ethtool-copy.h"
>  
>  /* ethtool.h expects these to be defined by <linux/types.h> */
>  #ifndef HAVE_BE_TYPES
>  typedef __uint16_t __be16;
>  typedef __uint32_t __be32;
> +typedef unsigned long long __be64;
>  #endif
>  
> -#include "ethtool-copy.h"
> -

You can't move the inclusion of ethtool-copy.h; that defeats the whole
purpose of the HAVE_BE_TYPES feature test.

[...]
> +#ifndef ARRAY_SIZE
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> +#endif
> +
> +#ifndef SIOCETHTOOL
> +#define SIOCETHTOOL     0x8946
>  #endif
>  
>  /* National Semiconductor DP83815, DP83816 */
> diff --git a/ethtool.c b/ethtool.c
> index 9ad7000..15af86a 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -45,16 +45,9 @@
>  #include <linux/sockios.h>
>  #include "ethtool-util.h"
>  
> -
> -#ifndef SIOCETHTOOL
> -#define SIOCETHTOOL     0x8946
> -#endif
>  #ifndef MAX_ADDR_LEN
>  #define MAX_ADDR_LEN	32
>  #endif
> -#ifndef ARRAY_SIZE
> -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> -#endif
>  
>  #ifndef HAVE_NETIF_MSG
>  enum {
> 

Presumably this is needed by the next patch, but it has nothing to do
with what the commit message says.

Ben.
Duyck, Alexander H April 27, 2011, 4:46 p.m. UTC | #2 
On 4/27/2011 8:54 AM, Ben Hutchings wrote:
> On Thu, 2011-04-21 at 13:40 -0700, Alexander Duyck wrote:
>> This change is meant to add support for __be64 values and bitops to
>> ethtool.  These changes will be needed in order to support network flow
>> classifier rule configuration.
>>
>> Signed-off-by: Alexander Duyck<alexander.h.duyck@intel.com>
>> ---
>>
>>   ethtool-bitops.h |   25 +++++++++++++++++++++++++
>>   ethtool-util.h   |   30 ++++++++++++++++++++++++++----
>>   ethtool.c        |    7 -------
>>   3 files changed, 51 insertions(+), 11 deletions(-)
>>   create mode 100644 ethtool-bitops.h
>>
>> diff --git a/ethtool-bitops.h b/ethtool-bitops.h
>> new file mode 100644
>> index 0000000..0ff14f1
>> --- /dev/null
>> +++ b/ethtool-bitops.h
>> @@ -0,0 +1,25 @@
>> +#ifndef ETHTOOL_BITOPS_H__
>> +#define ETHTOOL_BITOPS_H__
>> +
>> +#define BITS_PER_BYTE		8
>> +#define BITS_PER_LONG		(BITS_PER_BYTE * sizeof(long))
>> +#define DIV_ROUND_UP(n, d)	(((n) + (d) - 1) / (d))
>> +#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_LONG)
>> +
>> +static inline void set_bit(int nr, unsigned long *addr)
>> +{
>> +	addr[nr / BITS_PER_LONG] |= 1UL<<  (nr % BITS_PER_LONG);
>> +}
>> +
>> +static inline void clear_bit(int nr, unsigned long *addr)
>> +{
>> +	addr[nr / BITS_PER_LONG]&= ~(1UL<<  (nr % BITS_PER_LONG));
>> +}
>> +
>> +static __always_inline int test_bit(unsigned int nr, const unsigned long *addr)
>> +{
>> +	return !!((1UL<<  (nr % BITS_PER_LONG))&
>> +		  (((unsigned long *)addr)[nr / BITS_PER_LONG]));
>> +}
>
> Where is __always_inline supposed to be defined?

Sorry that should have just been inline.  I forgot we have to take tools 
other than gcc into account.

>> +#endif
>> diff --git a/ethtool-util.h b/ethtool-util.h
>> index f053028..3d46faf 100644
>> --- a/ethtool-util.h
>> +++ b/ethtool-util.h
>> @@ -5,15 +5,18 @@
>>
>>   #include<sys/types.h>
>>   #include<endian.h>
>> +#include<sys/ioctl.h>
>> +#include<net/if.h>
>> +#include "ethtool-config.h"
>> +#include "ethtool-copy.h"
>>
>>   /* ethtool.h expects these to be defined by<linux/types.h>  */
>>   #ifndef HAVE_BE_TYPES
>>   typedef __uint16_t __be16;
>>   typedef __uint32_t __be32;
>> +typedef unsigned long long __be64;
>>   #endif
>>
>> -#include "ethtool-copy.h"
>> -
>
> You can't move the inclusion of ethtool-copy.h; that defeats the whole
> purpose of the HAVE_BE_TYPES feature test.

You're correct. I will get that corrected so that it is located after 
the definitions of the types.  The key bit that mattered was getting 
ethtool-config.h in there before the type declarations.  I need it in 
place to address the test for HAVE_BE_TYPES.

> [...]
>> +#ifndef ARRAY_SIZE
>> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>> +#endif
>> +
>> +#ifndef SIOCETHTOOL
>> +#define SIOCETHTOOL     0x8946
>>   #endif
>>
>>   /* National Semiconductor DP83815, DP83816 */
>> diff --git a/ethtool.c b/ethtool.c
>> index 9ad7000..15af86a 100644
>> --- a/ethtool.c
>> +++ b/ethtool.c
>> @@ -45,16 +45,9 @@
>>   #include<linux/sockios.h>
>>   #include "ethtool-util.h"
>>
>> -
>> -#ifndef SIOCETHTOOL
>> -#define SIOCETHTOOL     0x8946
>> -#endif
>>   #ifndef MAX_ADDR_LEN
>>   #define MAX_ADDR_LEN	32
>>   #endif
>> -#ifndef ARRAY_SIZE
>> -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>> -#endif
>>
>>   #ifndef HAVE_NETIF_MSG
>>   enum {
>>
>
> Presumably this is needed by the next patch, but it has nothing to do
> with what the commit message says.
>
> Ben.
>

Yes. These two moves and the addition of certain headers to the 
ethtool-util.h were to address the needs of both the rxclass.c file and 
the ethtool.c file in one central location.  I will probably break those 
off into a separate patch.

On a side note, is there a git tree somewhere I can re-base off of?  At 
this point I know you have pulled in a number of patches and I figure it 
would be helpful for me to clean up my tree so I am not guessing what is 
there and what isn't.

Thanks,

Alex



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings April 27, 2011, 5:09 p.m. UTC | #3 
On Wed, 2011-04-27 at 09:46 -0700, Alexander Duyck wrote:
> On 4/27/2011 8:54 AM, Ben Hutchings wrote:
> > On Thu, 2011-04-21 at 13:40 -0700, Alexander Duyck wrote:
> >> This change is meant to add support for __be64 values and bitops to
> >> ethtool.  These changes will be needed in order to support network flow
> >> classifier rule configuration.
[...]
> > Where is __always_inline supposed to be defined?
> 
> Sorry that should have just been inline.  I forgot we have to take tools 
> other than gcc into account.

Oh, it's a gcc extension?  I read the code before trying to compile it.
I've never tested with anything other than gcc but I think it's worth
making a small effort to avoid gcc extensions.

[...]
> On a side note, is there a git tree somewhere I can re-base off of?  At 
> this point I know you have pulled in a number of patches and I figure it 
> would be helpful for me to clean up my tree so I am not guessing what is 
> there and what isn't.

git://git.kernel.org/pub/scm/network/ethtool/ethtool.git

Ben.
Duyck, Alexander H April 27, 2011, 6:33 p.m. UTC | #4 
On 4/27/2011 10:09 AM, Ben Hutchings wrote:
> On Wed, 2011-04-27 at 09:46 -0700, Alexander Duyck wrote:
>> On 4/27/2011 8:54 AM, Ben Hutchings wrote:
>>> On Thu, 2011-04-21 at 13:40 -0700, Alexander Duyck wrote:
>>>> This change is meant to add support for __be64 values and bitops to
>>>> ethtool.  These changes will be needed in order to support network flow
>>>> classifier rule configuration.
> [...]
>>> Where is __always_inline supposed to be defined?
>>
>> Sorry that should have just been inline.  I forgot we have to take tools
>> other than gcc into account.
>
> Oh, it's a gcc extension?  I read the code before trying to compile it.
> I've never tested with anything other than gcc but I think it's worth
> making a small effort to avoid gcc extensions.
>
> [...]
>> On a side note, is there a git tree somewhere I can re-base off of?  At
>> this point I know you have pulled in a number of patches and I figure it
>> would be helpful for me to clean up my tree so I am not guessing what is
>> there and what isn't.
>
> git://git.kernel.org/pub/scm/network/ethtool/ethtool.git
>
> Ben.
>

I just saw the updates pushed to the tree.  Thanks.  I'll start working 
on incorporating the suggestions you made and should have an updated 
version of the remaining patches ready in the next day or so.

Thanks,

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

Patch

diff --git a/ethtool-bitops.h b/ethtool-bitops.h
new file mode 100644
index 0000000..0ff14f1
--- /dev/null
+++ b/ethtool-bitops.h
@@ -0,0 +1,25 @@ 
+#ifndef ETHTOOL_BITOPS_H__
+#define ETHTOOL_BITOPS_H__
+
+#define BITS_PER_BYTE		8
+#define BITS_PER_LONG		(BITS_PER_BYTE * sizeof(long))
+#define DIV_ROUND_UP(n, d)	(((n) + (d) - 1) / (d))
+#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_LONG)
+
+static inline void set_bit(int nr, unsigned long *addr)
+{
+	addr[nr / BITS_PER_LONG] |= 1UL << (nr % BITS_PER_LONG);
+}
+
+static inline void clear_bit(int nr, unsigned long *addr)
+{
+	addr[nr / BITS_PER_LONG] &= ~(1UL << (nr % BITS_PER_LONG));
+}
+
+static __always_inline int test_bit(unsigned int nr, const unsigned long *addr)
+{
+	return !!((1UL << (nr % BITS_PER_LONG)) &
+		  (((unsigned long *)addr)[nr / BITS_PER_LONG]));
+}
+
+#endif
diff --git a/ethtool-util.h b/ethtool-util.h
index f053028..3d46faf 100644
--- a/ethtool-util.h
+++ b/ethtool-util.h
@@ -5,15 +5,18 @@ 
 
 #include <sys/types.h>
 #include <endian.h>
+#include <sys/ioctl.h>
+#include <net/if.h>
+#include "ethtool-config.h"
+#include "ethtool-copy.h"
 
 /* ethtool.h expects these to be defined by <linux/types.h> */
 #ifndef HAVE_BE_TYPES
 typedef __uint16_t __be16;
 typedef __uint32_t __be32;
+typedef unsigned long long __be64;
 #endif
 
-#include "ethtool-copy.h"
-
 typedef unsigned long long u64;
 typedef __uint32_t u32;
 typedef __uint16_t u16;
@@ -23,11 +26,15 @@  typedef __int32_t s32;
 #if __BYTE_ORDER == __BIG_ENDIAN
 static inline u16 cpu_to_be16(u16 value)
 {
-    return value;
+	return value;
 }
 static inline u32 cpu_to_be32(u32 value)
 {
-    return value;
+	return value;
+}
+static inline u64 cpu_to_be64(u64 value)
+{
+	return value;
 }
 #else
 static inline u16 cpu_to_be16(u16 value)
@@ -38,6 +45,21 @@  static inline u32 cpu_to_be32(u32 value)
 {
 	return cpu_to_be16(value >> 16) | (cpu_to_be16(value) << 16);
 }
+static inline u64 cpu_to_be64(u64 value)
+{
+	return cpu_to_be32(value >> 32) | ((u64)cpu_to_be32(value) << 32);
+}
+#endif
+
+#define ntohll cpu_to_be64
+#define htonll cpu_to_be64
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#endif
+
+#ifndef SIOCETHTOOL
+#define SIOCETHTOOL     0x8946
 #endif
 
 /* National Semiconductor DP83815, DP83816 */
diff --git a/ethtool.c b/ethtool.c
index 9ad7000..15af86a 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -45,16 +45,9 @@ 
 #include <linux/sockios.h>
 #include "ethtool-util.h"
 
-
-#ifndef SIOCETHTOOL
-#define SIOCETHTOOL     0x8946
-#endif
 #ifndef MAX_ADDR_LEN
 #define MAX_ADDR_LEN	32
 #endif
-#ifndef ARRAY_SIZE
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-#endif
 
 #ifndef HAVE_NETIF_MSG
 enum {