| Message ID | 4AD1E169.6090705@linux-ipv6.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
YOSHIFUJI Hideaki a écrit : > ipv6 sit: Set relay to 0.0.0.0 directly if relay_prefixlen == 0. > > Do not use bit-shift if relay_prefixlen == 0; > relay_prefix << 32 does not result in 0. > > Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> > --- > net/ipv6/sit.c | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c > index 193d0c6..510d31f 100644 > --- a/net/ipv6/sit.c > +++ b/net/ipv6/sit.c > @@ -1014,9 +1014,12 @@ ipip6_tunnel_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd) > ip6rd.prefixlen); > if (!ipv6_addr_equal(&prefix, &ip6rd.prefix)) > goto done; > - relay_prefix = ip6rd.relay_prefix & > - htonl(0xffffffffUL << > - (32 - ip6rd.relay_prefixlen)); > + if (ip6rd.relay_prefixlen) > + relay_prefix = ip6rd.relay_prefix & > + htonl(0xffffffffUL << > + (32 - ip6rd.relay_prefixlen)); > + else > + relay_prefix = 0; > if (relay_prefix != ip6rd.relay_prefix) > goto done; > Sorry I dont get it u32 val = any_value ; u32 relay_prefix = val & htonl(0xffffffffUL << 32) should give 0 If not, something is broken and should be fixed. -- 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
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 12 Oct 2009 09:32:41 +0200 > Sorry I dont get it > > u32 val = any_value ; > u32 relay_prefix = val & htonl(0xffffffffUL << 32) should give 0 > > If not, something is broken and should be fixed. Indeed, it's "x >> 32" which is undefined and has to be done as something like "(x >> 31) >> 1" when performed on a u32 object. Yoshfuji is this patch really necessary? -- 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
Eric Dumazet wrote: > YOSHIFUJI Hideaki a écrit : > > ipv6 sit: Set relay to 0.0.0.0 directly if relay_prefixlen == 0. > > > > Do not use bit-shift if relay_prefixlen == 0; > > relay_prefix << 32 does not result in 0. > > > > Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> > > --- > > net/ipv6/sit.c | 9 ++++++--- > > 1 files changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c > > index 193d0c6..510d31f 100644 > > --- a/net/ipv6/sit.c > > +++ b/net/ipv6/sit.c > > @@ -1014,9 +1014,12 @@ ipip6_tunnel_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd) > > ip6rd.prefixlen); > > if (!ipv6_addr_equal(&prefix, &ip6rd.prefix)) > > goto done; > > - relay_prefix = ip6rd.relay_prefix & > > - htonl(0xffffffffUL << > > - (32 - ip6rd.relay_prefixlen)); > > + if (ip6rd.relay_prefixlen) > > + relay_prefix = ip6rd.relay_prefix & > > + htonl(0xffffffffUL << > > + (32 - ip6rd.relay_prefixlen)); > > + else > > + relay_prefix = 0; > > if (relay_prefix != ip6rd.relay_prefix) > > goto done; > > > > > Sorry I dont get it > > u32 val = any_value ; > u32 relay_prefix = val & htonl(0xffffffffUL << 32) should give 0 > > If not, something is broken and should be fixed. Unfortunately, on x86 architecture (80286 and later at least), lower 5 bits (& 0x1f) are used for shift operation. Thus, 0xffffffffUL << 32 gives 0xffffffffUL. --yoshfuji -- 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
David Miller wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Mon, 12 Oct 2009 09:32:41 +0200 > > > Sorry I dont get it > > > > u32 val = any_value ; > > u32 relay_prefix = val & htonl(0xffffffffUL << 32) should give 0 > > > > If not, something is broken and should be fixed. > > Indeed, it's "x >> 32" which is undefined and has to be done as > something like "(x >> 31) >> 1" when performed on a u32 object. > > Yoshfuji is this patch really necessary? Well, yes. 6rd allows 32 bit suffix (0 bit prefix). --yoshfuji -- 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
From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> Date: Mon, 12 Oct 2009 17:50:54 +0900 > Unfortunately, on x86 architecture (80286 and later at least), > lower 5 bits (& 0x1f) are used for shift operation. > Thus, 0xffffffffUL << 32 gives 0xffffffffUL. Indeed, thanks for the explanation, I thought only right shift mattered for this case of "shift count larger than type". -- 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
From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> Date: Sun, 11 Oct 2009 22:45:13 +0900 > ipv6 sit: Set relay to 0.0.0.0 directly if relay_prefixlen == 0. > > Do not use bit-shift if relay_prefixlen == 0; > relay_prefix << 32 does not result in 0. > > Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> Applied. -- 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 --git a/net/ipv6/sit.c b/net/ipv6/sit.c index 193d0c6..510d31f 100644 --- a/net/ipv6/sit.c +++ b/net/ipv6/sit.c @@ -1014,9 +1014,12 @@ ipip6_tunnel_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd) ip6rd.prefixlen); if (!ipv6_addr_equal(&prefix, &ip6rd.prefix)) goto done; - relay_prefix = ip6rd.relay_prefix & - htonl(0xffffffffUL << - (32 - ip6rd.relay_prefixlen)); + if (ip6rd.relay_prefixlen) + relay_prefix = ip6rd.relay_prefix & + htonl(0xffffffffUL << + (32 - ip6rd.relay_prefixlen)); + else + relay_prefix = 0; if (relay_prefix != ip6rd.relay_prefix) goto done;
ipv6 sit: Set relay to 0.0.0.0 directly if relay_prefixlen == 0. Do not use bit-shift if relay_prefixlen == 0; relay_prefix << 32 does not result in 0. Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> --- net/ipv6/sit.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) -- 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