iproute2: support dotted-quad netmask notation.
Subsystems (3)
PATCH 1/1 iproute2: support dotted-quad netmask notation.
Suggested patch for allowing netmask to be specified in dotted quad format. See http://bugs.debian.org/357172
(Known problem: this will not prevent some invalid syntaxes, ie. "255.0.255.0" will be treated as "255.255.255.0")
Comments? Suggestions? Improvements?
lib/utils.c +21 -1
@@ -47,6 +47,25 @@ int get_integer(int *val, const char *arg, int base)return 0 ; } +int get_netmask (unsigned *val,const char *arg,int base) +{ +inet_prefix addr; + +if (!get_unsigned (val, arg, base)) +return 0 ; + +/* try coverting dotted quad to CIDR */ +if (!get_addr_1 (&addr, arg,AF_INET )) { +u_int32_t mask; + *val=0 ; +for (mask = addr.data[0 ]; mask; mask >>=1 ) + (*val)++; +return 0 ; + } + +return -1 ; +} +int get_unsigned (unsigned *val,const char *arg,int base) {unsigned long res;
@@ -304,7 +323,8 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family) dst->bitlen =32 ; }if (slash) { -if (get_unsigned (&plen, slash+1 ,0 ) || plen > dst->bitlen) { +if (get_netmask (&plen, slash+1 ,0 ) + || plen > dst->bitlen) { err =-1 ; goto done; }
Signature · 2 lines
Regards, Andreas Henriksson
Quoted 7 lines · On Tue, 4 Dec 2007 14:58:18 +0100 Andreas Henriksson <[email protected]> wrote:
Suggested patch for allowing netmask to be specified in dotted quad format. See http://bugs.debian.org/357172
(Known problem: this will not prevent some invalid syntaxes, ie. "255.0.255.0" will be treated as "255.255.255.0")
Comments? Suggestions? Improvements?
Fix the bug you mentioned?
/* a valid netmask must be 2^n - 1 (n = 1..31) */ static int is_valid_netmask(const inet_prefix *addr)
{
uint32_t host;
if (addr->family != AF_INET)
return 0;
host = ~ntohl(addr->data[0]);
if (host == 0 || ~host == 0)
return 0;
return (host & (host + 1)) == 0;
}Quoted 6 lines · Stephen Hemminger
Quoted 3 lines · On Tue, 4 Dec 2007 14:58:18 +0100 Andreas Henriksson <[email protected]> wrote:
Suggested patch for allowing netmask to be specified in dotted quad format. See http://bugs.debian.org/357172
Quoted 8 lines
Quoted 4 lines
(Known problem: this will not prevent some invalid syntaxes, ie. "255.0.255.0" will be treated as "255.255.255.0")
Comments? Suggestions? Improvements?
Fix the bug you mentioned?
[... snip example code ...]
Updated patch, added your netmask validation code but without the check that made 0.0.0.0 (default) and 255.255.255.255 (one address) invalid netmasks as they are permitted in CIDR format.
lib/utils.c +37 -1
@@ -47,6 +47,41 @@ int get_integer(int *val, const char *arg, int base)return 0 ; } +/* a valid netmask must be 2^n - 1 (n = 1..31) */ +static int is_valid_netmask (const inet_prefix *addr) +{ +uint32_t host; + +if (addr->family !=AF_INET ) +return 0 ; + + host = ~ntohl (addr->data[0 ]); + +return (host & (host +1 )) ==0 ; +} + +static int get_netmask (unsigned *val,const char *arg,int base) +{ +inet_prefix addr; + +if (!get_unsigned (val, arg, base)) +return 0 ; + +/* try coverting dotted quad to CIDR */ +if (!get_addr_1 (&addr, arg,AF_INET )) { +u_int32_t mask; + + *val=0 ; +for (mask = addr.data[0 ]; mask; mask >>=1 ) + (*val)++; + +if (is_valid_netmask (&addr)) +return 0 ; + } + +return -1 ; +} +int get_unsigned (unsigned *val,const char *arg,int base) {unsigned long res;
@@ -304,7 +339,8 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family) dst->bitlen =32 ; }if (slash) { -if (get_unsigned (&plen, slash+1 ,0 ) || plen > dst->bitlen) { +if (get_netmask (&plen, slash+1 ,0 ) + || plen > dst->bitlen) { err =-1 ; goto done; }
Signature · 2 lines
Regards, Andreas Henriksson
Quoted 10 lines · Andreas Henriksson
Quoted 6 lines · Stephen Hemminger
Quoted 3 lines · On Tue, 4 Dec 2007 14:58:18 +0100 Andreas Henriksson <[email protected]> wrote:
Suggested patch for allowing netmask to be specified in dotted quad format. See http://bugs.debian.org/357172
Updated patch, added your netmask validation code but without the check that made 0.0.0.0 (default) and 255.255.255.255 (one address) invalid netmasks as they are permitted in CIDR format.
I think both previous patches where broken on big-endian platforms. Here's an updated patch again. I'm very sorry for the inconvenience!
lib/utils.c +37 -1
@@ -47,6 +47,41 @@ int get_integer(int *val, const char *arg, int base)return 0 ; } +/* a valid netmask must be 2^n - 1 (n = 1..31) */ +static int is_valid_netmask (const inet_prefix *addr) +{ +uint32_t host; + +if (addr->family !=AF_INET ) +return 0 ; + + host = ~ntohl (addr->data[0 ]); + +return (host & (host +1 )) ==0 ; +} + +static int get_netmask (unsigned *val,const char *arg,int base) +{ +inet_prefix addr; + +if (!get_unsigned (val, arg, base)) +return 0 ; + +/* try coverting dotted quad to CIDR */ +if (!get_addr_1 (&addr, arg,AF_INET )) { +u_int32_t mask; + + *val=0 ; +for (mask =ntohl (addr.data[0 ]); mask; mask <<=1 ) + (*val)++; + +if (is_valid_netmask (&addr)) +return 0 ; + } + +return -1 ; +} +int get_unsigned (unsigned *val,const char *arg,int base) {unsigned long res;
@@ -304,7 +339,8 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family) dst->bitlen =32 ; }if (slash) { -if (get_unsigned (&plen, slash+1 ,0 ) || plen > dst->bitlen) { +if (get_netmask (&plen, slash+1 ,0 ) + || plen > dst->bitlen) { err =-1 ; goto done; }
Signature · 2 lines
Regards, Andreas Henriksson
Stephen Hemminger
AppliedQuoted 70 lines · On Sun, 09 Dec 2007 18:10:22 +0100 Andreas Henriksson <[email protected]> wrote:
Quoted 10 lines · Andreas Henriksson
Quoted 6 lines · Stephen Hemminger
Quoted 3 lines · On Tue, 4 Dec 2007 14:58:18 +0100 Andreas Henriksson <[email protected]> wrote:
Suggested patch for allowing netmask to be specified in dotted quad format. See http://bugs.debian.org/357172
Updated patch, added your netmask validation code but without the check that made 0.0.0.0 (default) and 255.255.255.255 (one address) invalid netmasks as they are permitted in CIDR format.
I think both previous patches where broken on big-endian platforms. Here's an updated patch again. I'm very sorry for the inconvenience!
Signed-off-by Andreas Henriksson <[email protected]>lib/utils.c +35
-0@@ -47,6 +47,41 @@ int get_integer(int *val, const char *arg, int base)return 0 ; } +/* a valid netmask must be 2^n - 1 (n = 1..31) */ +static int is_valid_netmask (const inet_prefix *addr) +{ +uint32_t host; + +if (addr->family !=AF_INET ) +return 0 ; + + host = ~ntohl (addr->data[0 ]); + +return (host & (host +1 )) ==0 ; +} + +static int get_netmask (unsigned *val,const char *arg,int base) +{ +inet_prefix addr; + +if (!get_unsigned (val, arg, base)) +return 0 ; + +/* try coverting dotted quad to CIDR */ +if (!get_addr_1 (&addr, arg,AF_INET )) { +u_int32_t mask; + + *val=0 ; +for (mask =ntohl (addr.data[0 ]); mask; mask <<=1 ) + (*val)++; + +if (is_valid_netmask (&addr)) +return 0 ; + } + +return -1 ; +} +int get_unsigned (unsigned *val,const char *arg,int base) {unsigned long res;
lib/utils.c int get_prefix_1(inet_prefix *dst, char *arg, int family) · line 346
Quoted 9 lines
lib/utils.c +2
-1@@ -304,7 +339,8 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family) dst->bitlen =32 ; }if (slash) { -if (get_unsigned (&plen, slash+1 ,0 ) || plen > dst->bitlen) { +if (get_netmask (&plen, slash+1 ,0 ) + || plen > dst->bitlen) { err =-1 ; goto done; }
applied
Signature · 1 line
Stephen Hemminger <[email protected]>
Quoted 4 lines · Stephen Hemminger
Quoted 2 lines · On Sun, 09 Dec 2007 18:10:22 +0100 Andreas Henriksson <[email protected]> wrote:
I think both previous patches where broken on big-endian platforms. Here's an updated patch again. I'm very sorry for the inconvenience!
[...]
Quoted 3 lines
Quoted 3 lines
lib/utils.c +3
-0+ *val=0 ; +for (mask =ntohl (addr.data[0 ]); mask; mask <<=1 ) + (*val)++;
[...]
Quoted 3 lines
applied
Just to make sure.... It looks on git.kernel.org like you applied the wrong patch. (Maybe you just haven't pushed out the latest changes there yet.) Please double-check that you actually applied the latest version (which is the one in the mail you replied "applied" to, important part quoted above).
Additionally, there's still a couple of trivial patches pending in the "patches" branch ofgit://git.debian.org/git/collab-maint/pkg-iproute.git
Please see the original thread[1], where Patrick McHarding had some concerns about one of the patches. It's exactly the same changes you made in commit 660818498d0f5a3f52c05355a3e82c23f670fcc1 [2] though, so I don't really see the problem. I have an additional patch[3] available, that makes MAX_ROUNDS configurable which Patrick requested.
Please comment on the way forward there....
[1]: http://www.spinics.net/lists/netdev/msg44800.html
[2]: Where the comment seems to be wrong about "Limit ip route flush...",
since it's actually "ip neigh flush" that's being modified.
[3]: I have a slightly updated patch, but it's basically the same as
http://www.spinics.net/lists/netdev/msg45080.html
Will send updated version if the patches it's based on goes in.
Signature · 2 lines
Regards, Andreas Henriksson
Quoted 11 lines · On Wed, 12 Dec 2007 12:55:13 +0100 Andreas Henriksson <[email protected]> wrote:
Quoted 4 lines · Stephen Hemminger
Quoted 2 lines · On Sun, 09 Dec 2007 18:10:22 +0100 Andreas Henriksson <[email protected]> wrote:
I think both previous patches where broken on big-endian platforms. Here's an updated patch again. I'm very sorry for the inconvenience!
[...]
Quoted 3 lines
Quoted 3 lines
lib/utils.c +3
-0+ *val=0 ; +for (mask =ntohl (addr.data[0 ]); mask; mask <<=1 ) + (*val)++;[...]
Quoted 1 line
Quoted 7 lines
Quoted 2 lines
applied
Just to make sure.... It looks on git.kernel.org like you applied the wrong patch. (Maybe you just haven't pushed out the latest changes there yet.) Please double-check that you actually applied the latest version (which is the one in the mail you replied "applied" to, important part quoted above).
Actually, I took your logic and moved it to a new function:
static unsigned cidr(const inet_prefix *addr)
{
unsigned bits = 0;
u_int32_t mask;
for (mask = ntohl(addr->data[0]); mask; mask <<= 1)
++bits;
return bits;
}Signature · 1 line
Stephen Hemminger <[email protected]>