[PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[Joe Perches]

Change LOOPBACK MULTICAST LOCAL_MCAST BADCLASS and ZERONET
macros to inline functions is_ip4_[type](__be32 addr)
Adds some type safety and maybe some readability

No change in compiled image size

Signed-off-by: Joe Perches <joe@perches.com>

---

 include/linux/in.h |   45 ++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/linux/in.h b/include/linux/in.h
index 3975cbf..ac6eff1 100644
--- a/include/linux/in.h
+++ b/include/linux/in.h
@@ -246,12 +246,47 @@ struct sockaddr_in {
 #include <asm/byteorder.h> 
 
 #ifdef __KERNEL__
+
+static inline __be32 ip4_addr_octets(unsigned char a, unsigned char b, unsigned char c, unsigned char d)
+{
+	return htonl((((__u32)(a & 0xff)) << 24) |
+		     (((__u32)(b & 0xff)) << 16) |
+		     (((__u32)(c & 0xff)) << 8) |
+		     (((__u32)(d & 0xff)) << 0));
+}
+
+static inline bool is_ip4_loopback(__be32 addr)
+{
+	return (addr & ip4_addr_octets(255,0,0,0)) == ip4_addr_octets(127,0,0,0);
+}
+
+static inline bool is_ip4_multicast(__be32 addr)
+{
+	return (addr & ip4_addr_octets(240,0,0,0)) == ip4_addr_octets(224,0,0,0);
+}
+
+static inline bool is_ip4_local_multicast(__be32 addr)
+{
+	return (addr & ip4_addr_octets(255,255,255,0)) == ip4_addr_octets(224,0,0,0);
+}
+
+static inline bool is_ip4_badclass(__be32 addr)
+{
+	return (addr & ip4_addr_octets(240,0,0,0)) == ip4_addr_octets(240,0,0,0);
+}
+
+static inline bool is_ip4_zeronet(__be32 addr)
+{
+	return (addr & ip4_addr_octets(255,0,0,0)) == ip4_addr_octets(0,0,0,0);
+}
+
 /* Some random defines to make it easier in the kernel.. */
-#define LOOPBACK(x)	(((x) & htonl(0xff000000)) == htonl(0x7f000000))
-#define MULTICAST(x)	(((x) & htonl(0xf0000000)) == htonl(0xe0000000))
-#define BADCLASS(x)	(((x) & htonl(0xf0000000)) == htonl(0xf0000000))
-#define ZERONET(x)	(((x) & htonl(0xff000000)) == htonl(0x00000000))
-#define LOCAL_MCAST(x)	(((x) & htonl(0xFFFFFF00)) == htonl(0xE0000000))
+
+#define LOOPBACK(x)	is_ip4_loopback(x)
+#define MULTICAST(x)	is_ip4_multicast(x)
+#define LOCAL_MCAST(x)	is_ip4_local_multicast(x)
+#define BADCLASS(x)	is_ip4_badclass(x)
+#define ZERONET(x)	is_ip4_zeronet(x)
 
 #endif
 

Re: [PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[David Stevens]

Maybe I'm more used to hex, but I personally don't think those are
more readable. It replaces 1-line macroes with 4-line functions, and
I think more vgrepping to pick out the relevant constant data.

                                                +-DLS

Re: [PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[Joe Perches]

On Wed, 2007-11-14 at 08:53 -0800, David Stevens wrote: 
> Maybe I'm more used to hex, but I personally don't think those are
> more readable. It replaces 1-line macroes with 4-line functions, and
> I think more vgrepping to pick out the relevant constant data.

The rfcs generally use decimal octets. 
To me htonl(0xC0586300) isn't as readable as ip4_addr_octets(192,88,99,0)

Fred Templin has some patches that add several more macros
to in.h.  I just forwarded my patches to him too.

is_ip4_multicast seems a lot more readable to me than MULTICAST(x)
I prefer to eliminate the current use of macros for ip address tests
and also get no new macros added.  The old macros are just for
compatibility until the exis

http://marc.info/?l=linux-netdev&m=119490504022261&w=2

His patch adds:

+#define PRIVATE_10(x) (((x) & htonl(0xff000000)) == htonl(0x0A000000)) 
+#define LINKLOCAL_169(x) (((x) & htonl(0xffff0000)) == htonl(0xA9FE0000))
+#define PRIVATE_172(x)	(((x) & htonl(0xfff00000)) == htonl(0xAC100000))
+#define TEST_192(x)	(((x) & htonl(0xffffff00)) == htonl(0xC0000200))
+#define ANYCAST_6TO4(x)	(((x) & htonl(0xffffff00)) == htonl(0xC0586300))
+#define PRIVATE_192(x)	(((x) & htonl(0xffff0000)) == htonl(0xC0A80000))
+#define TEST_198(x)	(((x) & htonl(0xfffe0000)) == htonl(0xC6120000))

cheers, Joe

Re: [PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[Joe Perches]

Sorry, evolution died sending my mail and I didn't fully
read the "recovered" mail before hitting send.

Here's a more readable one.

---------

On Wed, 2007-11-14 at 08:53 -0800, David Stevens wrote: 
> Maybe I'm more used to hex, but I personally don't think those are
> more readable. It replaces 1-line macroes with 4-line functions, and
> I think more vgrepping to pick out the relevant constant data.

The rfcs generally use decimal octets. 

To me htonl(0xC0586300) isn't as readable as ip4_addr_octets(192,88,99,0)
is_ip4_multicast(addr) more intelligible than MULTICAST(x).

I hope to eliminate the current use of macros for ip address tests
and have no new macros added.  The existing macros are just for
compatibility until the current code uses are converted.

Fred Templin has some patches that add several more macros
to in.h.  I just forwarded my patches to him too.

http://marc.info/?l=linux-netdev&m=119490504022261&w=2

His patch adds:

+#define PRIVATE_10(x) (((x) & htonl(0xff000000)) == htonl(0x0A000000)) 
+#define LINKLOCAL_169(x) (((x) & htonl(0xffff0000)) == htonl(0xA9FE0000))
+#define PRIVATE_172(x)	(((x) & htonl(0xfff00000)) == htonl(0xAC100000))
+#define TEST_192(x)	(((x) & htonl(0xffffff00)) == htonl(0xC0000200))
+#define ANYCAST_6TO4(x)	(((x) & htonl(0xffffff00)) == htonl(0xC0586300))
+#define PRIVATE_192(x)	(((x) & htonl(0xffff0000)) == htonl(0xC0A80000))
+#define TEST_198(x)	(((x) & htonl(0xfffe0000)) == htonl(0xC6120000))

cheers, Joe

Re: [PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[David Miller]

From: David Stevens <dlstevens@us.ibm.com>
Date: Wed, 14 Nov 2007 08:53:13 -0800

> Maybe I'm more used to hex, but I personally don't think those are
> more readable. It replaces 1-line macroes with 4-line functions, and
> I think more vgrepping to pick out the relevant constant data.

Yes, the readability is agruable but I like the type checking.

I would also change the names, it's a better idea to prefix the
names with a namespace for the stuff being operated on, therefore
I'd prefer names like "ipv4_is_loopback()" etc.

I'm dropping these patches for now.

Re: [PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[Joe Perches]

On Mon, 2007-11-19 at 21:46 -0800, David Miller wrote:
> I would also change the names, it's a better idea to prefix the
> names with a namespace for the stuff being operated on, therefore
> I'd prefer names like "ipv4_is_loopback()" etc.

I used:

+static inline bool is_ip4_loopback(__be32 addr)

and converted the current uses in net of macros
to use those functions directly.  I'd prefer to drop
the macros entirely, but there are uses in
drivers/infiniband and drivers/parisc to convert too.

> I'm dropping these patches for now.

Is the use of "is_ip4_<level>" not "ipv4_is_<level>"
your reason to drop the patches?

Re: [PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Mon, 19 Nov 2007 21:59:58 -0800

> > I'm dropping these patches for now.
> 
> Is the use of "is_ip4_<level>" not "ipv4_is_<level>"
> your reason to drop the patches?

Yes.

[PATCH net-2.6.25 1/4] include - Convert IP4 address class macros to inline functions

[Joe Perches]

Change LOOPBACK MULTICAST LOCAL_MCAST BADCLASS and ZERONET
macros to inline functions ipv4_is_<type>(__be32 addr)

Adds some type safety and arguably some readability

No change in compiled image size

Signed-off-by: Joe Perches <joe@perches.com>

---

 include/linux/in.h |   45 ++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/linux/in.h b/include/linux/in.h
index 3975cbf..ac6eff1 100644
--- a/include/linux/in.h
+++ b/include/linux/in.h
@@ -246,12 +246,47 @@ struct sockaddr_in {
 #include <asm/byteorder.h> 
 
 #ifdef __KERNEL__
+
+static inline __be32 ipv4_addr_octets(unsigned char a, unsigned char b, unsigned char c, unsigned char d)
+{
+	return htonl((((__u32)(a & 0xff)) << 24) |
+		     (((__u32)(b & 0xff)) << 16) |
+		     (((__u32)(c & 0xff)) << 8) |
+		     (((__u32)(d & 0xff)) << 0));
+}
+
+static inline bool ipv4_is_loopback(__be32 addr)
+{
+	return (addr & ipv4_addr_octets(255,0,0,0)) == ipv4_addr_octets(127,0,0,0);
+}
+
+static inline bool ipv4_is_multicast(__be32 addr)
+{
+	return (addr & ipv4_addr_octets(240,0,0,0)) == ipv4_addr_octets(224,0,0,0);
+}
+
+static inline bool ipv4_is_local_multicast(__be32 addr)
+{
+	return (addr & ipv4_addr_octets(255,255,255,0)) == ipv4_addr_octets(224,0,0,0);
+}
+
+static inline bool ipv4_is_badclass(__be32 addr)
+{
+	return (addr & ipv4_addr_octets(240,0,0,0)) == ipv4_addr_octets(240,0,0,0);
+}
+
+static inline bool ipv4_is_zeronet(__be32 addr)
+{
+	return (addr & ipv4_addr_octets(255,0,0,0)) == ipv4_addr_octets(0,0,0,0);
+}
+
 /* Some random defines to make it easier in the kernel.. */
-#define LOOPBACK(x)	(((x) & htonl(0xff000000)) == htonl(0x7f000000))
-#define MULTICAST(x)	(((x) & htonl(0xf0000000)) == htonl(0xe0000000))
-#define BADCLASS(x)	(((x) & htonl(0xf0000000)) == htonl(0xf0000000))
-#define ZERONET(x)	(((x) & htonl(0xff000000)) == htonl(0x00000000))
-#define LOCAL_MCAST(x)	(((x) & htonl(0xFFFFFF00)) == htonl(0xE0000000))
+
+#define LOOPBACK(x)	ipv4_is_loopback(x)
+#define MULTICAST(x)	ipv4_is_multicast(x)
+#define LOCAL_MCAST(x)	ipv4_is_local_multicast(x)
+#define BADCLASS(x)	ipv4_is_badclass(x)
+#define ZERONET(x)	ipv4_is_zeronet(x)
 
 #endif