Re: [PATCH] iproute2: support dotted-quad netmask notation.
On Wed, 12 Dec 2007 12:55:13 +0100 Andreas Henriksson <[EMAIL PROTECTED]> wrote: > On Tue, Dec 11, 2007 at 05:14:06PM -0800, Stephen Hemminger wrote: > > 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! > [...] > > > + *val=0; > > > + for (mask = ntohl(addr.data[0]); mask; mask <<= 1) > > > + (*val)++; > [...] > > > > 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; } -- Stephen Hemminger <[EMAIL PROTECTED]> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iproute2: support dotted-quad netmask notation.
On Tue, Dec 11, 2007 at 05:14:06PM -0800, Stephen Hemminger wrote: > 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! [...] > > + *val=0; > > + for (mask = ntohl(addr.data[0]); mask; mask <<= 1) > > + (*val)++; [...] > > 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 of git://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. -- Regards, Andreas Henriksson -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iproute2: support dotted-quad netmask notation.
On Sun, 09 Dec 2007 18:10:22 +0100 Andreas Henriksson <[EMAIL PROTECTED]> wrote: > > On lör, 2007-12-08 at 00:41 +0100, Andreas Henriksson wrote: > > On tor, 2007-12-06 at 11:53 -0800, Stephen Hemminger wrote: > > > 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]> > > > diff --git a/lib/utils.c b/lib/utils.c > index 4c42dfd..bb88cf7 100644 > --- a/lib/utils.c > +++ b/lib/utils.c > @@ -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; > } > > > > applied -- Stephen Hemminger <[EMAIL PROTECTED]> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iproute2: support dotted-quad netmask notation.
On lör, 2007-12-08 at 00:41 +0100, Andreas Henriksson wrote: > On tor, 2007-12-06 at 11:53 -0800, Stephen Hemminger wrote: > > 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]> diff --git a/lib/utils.c b/lib/utils.c index 4c42dfd..bb88cf7 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -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; } -- Regards, Andreas Henriksson -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iproute2: support dotted-quad netmask notation.
On tor, 2007-12-06 at 11:53 -0800, Stephen Hemminger wrote: > 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? > > [... 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. Signed-off-by: Andreas Henriksson <[EMAIL PROTECTED]> diff --git a/lib/utils.c b/lib/utils.c index 4c42dfd..b4a6125 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -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; } -- Regards, Andreas Henriksson -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iproute2: support dotted-quad netmask notation.
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; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] 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? Signed-off-by: Andreas Henriksson <[EMAIL PROTECTED]> diff --git a/lib/utils.c b/lib/utils.c index 4c42dfd..277ab45 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -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; } -- Regards, Andreas Henriksson -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html