Fw: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1

Fw: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1

[Andrew Morton]

Begin forwarded message:

Date: Mon, 11 Dec 2006 07:18:20 -0800
From: bugme-daemon@bugzilla.kernel.org
To: bugme-new@lists.osdl.org
Subject: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1


http://bugzilla.kernel.org/show_bug.cgi?id=7665

           Summary: getsockopt(IPV6_*CAST_HOPS) returns -1
    Kernel Version: 2.6.19
            Status: NEW
          Severity: normal
             Owner: yoshfuji@linux-ipv6.org
         Submitter: rdenis@simphalempin.com


Most recent kernel where this bug did *NOT* occur: N/A
Distribution: Debian Sid
Hardware Environment: i386
Software Environment: glibc 2.3.6
Problem Description:

Where fd is a socket (datagram or raw) with IPv6 protocol family,
getsockopt(fd, IPPROTO_IPV6, IPV6_UNICAST_HOPS, ...) succeeds, but the returned 
hop limit is -1. connect()'ing the socket first does not solve the problem.

Same problem with IPV6_MULTICAST_HOPS.

Steps to reproduce:

Compile and run the following test case:

#include <sys/socket.h>
#include <netinet/in.h>
#include <unistd.h>
#include <stdio.h>
#include <string.h>

int main (void)
{
        int fd, val;
        socklen_t len = sizeof (val);
        struct sockaddr_in6 addr;

        memset (&addr, 0, sizeof (addr));
        fd = socket (AF_INET6, SOCK_RAW, IPPROTO_UDP);
        addr.sin6_family = AF_INET6;
        addr.sin6_addr.s6_addr[15] = 1;
        connect (fd, (struct sockaddr *)&addr, sizeof (addr));

        if (fd == -1) return 1;
        if (getsockopt (fd, IPPROTO_IPV6, IPV6_UNICAST_HOPS, &val, &len) == 0)
                printf ("Default unicast hops limit: %d\n", val);
        if (getsockopt (fd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &val, &len) == 
0)
                printf ("Default multicast hops limit: %d\n", val);
        return 0;
}

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

Re: Fw: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1

[Brian Haley]

Andrew Morton wrote:
> Where fd is a socket (datagram or raw) with IPv6 protocol family,
> getsockopt(fd, IPPROTO_IPV6, IPV6_UNICAST_HOPS, ...) succeeds, but the returned 
> hop limit is -1. connect()'ing the socket first does not solve the problem.

An IPv6 socket's hoplimit value is not set at creation time, instead, 
the hoplimit in an outgoing packet is set dynamically at transmit time 
to one of the following (in this order):

1. Hoplimit route metric (if set)
2. Outgoing interface value (/proc/sys/net/ipv6/conf/ethX/hop_limit)
3. Global IPv6 value (/proc/sys/net/ipv6/conf/all/hop_limit)

A setsockopt() value *will* override this.

Some *nixes have a different behavior and do set it at socket() creation 
time.

-Brian

Re: Fw: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1

[Rémi Denis-Courmont]

	Hello,

Le lundi 11 décembre 2006 22:55, Brian Haley a écrit :
> Andrew Morton wrote:
> > Where fd is a socket (datagram or raw) with IPv6 protocol family,
> > getsockopt(fd, IPPROTO_IPV6, IPV6_UNICAST_HOPS, ...) succeeds, but
> > the returned hop limit is -1. connect()'ing the socket first does
> > not solve the problem.
>
> An IPv6 socket's hoplimit value is not set at creation time, instead,
> the hoplimit in an outgoing packet is set dynamically at transmit
> time to one of the following (in this order):
>
> 1. Hoplimit route metric (if set)
> 2. Outgoing interface value (/proc/sys/net/ipv6/conf/ethX/hop_limit)
> 3. Global IPv6 value (/proc/sys/net/ipv6/conf/all/hop_limit)
>
> A setsockopt() value *will* override this.

Relevant standard (RFC 3493) notes:

   The IPV6_UNICAST_HOPS option may be used with getsockopt() to
   determine the hop limit value that the system will use for subsequent
   unicast packets sent via that socket.

I don't reckon -1 could be the hop limit value. IMHO, the value from 
case 1 (if socket is connected to some destination), otherwise case 2 
(if bound to a scope interface) or ultimately the default hop limit 
ought to be returned instead, as it will be most often correct, while 
the current behavior is always wrong, unless setsockopt() has been used 
first. I don't if some people may think doing a route lookup in 
getsockopt might be overly expensive, but at least the two other cases 
should be ok, particularly the last one.

-- 
Rémi Denis-Courmont

Re: Fw: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1

[Brian Haley]

[-- Attachment #1: Type: text/plain, Size: 1664 bytes --]

>>> Where fd is a socket (datagram or raw) with IPv6 protocol family,
>>> getsockopt(fd, IPPROTO_IPV6, IPV6_UNICAST_HOPS, ...) succeeds, but
>>> the returned hop limit is -1. connect()'ing the socket first does
>>> not solve the problem.
>> An IPv6 socket's hoplimit value is not set at creation time, instead,
>> the hoplimit in an outgoing packet is set dynamically at transmit
>> time to one of the following (in this order):
>>
>> 1. Hoplimit route metric (if set)
>> 2. Outgoing interface value (/proc/sys/net/ipv6/conf/ethX/hop_limit)
>> 3. Global IPv6 value (/proc/sys/net/ipv6/conf/all/hop_limit)
>>
>> A setsockopt() value *will* override this.
> 
> Relevant standard (RFC 3493) notes:
> 
>    The IPV6_UNICAST_HOPS option may be used with getsockopt() to
>    determine the hop limit value that the system will use for subsequent
>    unicast packets sent via that socket.
> 
> I don't reckon -1 could be the hop limit value.

-1 means un-initialized.

> IMHO, the value from 
> case 1 (if socket is connected to some destination), otherwise case 2 
> (if bound to a scope interface) or ultimately the default hop limit 
> ought to be returned instead, as it will be most often correct, while 
> the current behavior is always wrong, unless setsockopt() has been used 
> first. I don't if some people may think doing a route lookup in 
> getsockopt might be overly expensive, but at least the two other cases 
> should be ok, particularly the last one.

The following patch seems to work for me, but this code has behaved this 
way for a while, so don't know if it will break any existing apps.

-Brian


Signed-off-by: Brian Haley <brian.haley@hp.com>


[-- Attachment #2: ipv6hops.patch --]
[-- Type: text/x-patch, Size: 776 bytes --]

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 1eafcfc..352690e 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -978,12 +978,27 @@ static int do_ipv6_getsockopt(struct soc
 		break;
 
 	case IPV6_UNICAST_HOPS:
-		val = np->hop_limit;
-		break;
-
 	case IPV6_MULTICAST_HOPS:
-		val = np->mcast_hops;
+	{
+		struct dst_entry *dst;
+
+		if (optname == IPV6_UNICAST_HOPS)
+			val = np->hop_limit;
+		else
+			val = np->mcast_hops;
+
+		dst = sk_dst_get(sk);
+		if (dst) {
+			if (val < 0)
+				val = dst_metric(dst, RTAX_HOPLIMIT);
+			if (val < 0)
+				val = ipv6_get_hoplimit(dst->dev);
+			dst_release(dst);
+		}
+		if (val < 0)
+			val = ipv6_devconf.hop_limit;
 		break;
+	}
 
 	case IPV6_MULTICAST_LOOP:
 		val = np->mc_loop;

Re: Fw: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1

[Rémi Denis-Courmont]

Le mardi 12 décembre 2006 22:16, vous avez écrit :
> > I don't reckon -1 could be the hop limit value.
>
> -1 means un-initialized.

Sure, -1 means "kernel default" for setsockopt(), but it is not 
specified for getsockopt().

> The following patch seems to work for me, but this code has behaved
> this way for a while, so don't know if it will break any existing
> apps.

Google Codesearch for "getsockopt IPV6_MULTICAST_HOPS" yields a bunch of 
apps that seem to assume a valid hop limit is returned, none of which 
detects -1. I believe applying your patch would fix much more apps than 
it is going to break. Hopefully those not handling -1 will somehow cast 
it to 255 if it is ever re-used, but I can also imagine some broken 
SDP-or-similar with "/-1" or "/4294967295" as a hop limit.

Thanks,

-- 
Rémi Denis-Courmont

Re: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1

[David Miller]

From: Brian Haley <brian.haley@hp.com>
Date: Tue, 12 Dec 2006 16:16:27 -0500

> The following patch seems to work for me, but this code has behaved this 
> way for a while, so don't know if it will break any existing apps.
> 
> Signed-off-by: Brian Haley <brian.haley@hp.com>

I like this patch so I have applied it, thanks Brian.  I'll
push it to -stable too.

I wonder, since the most accurate return value is tied to the route,
what is expected of this getsockopt() before a socket's identity
(and therefore route) is known?

I suppose returning the default value, which Brian's patch does,
is a reasonable best-effort.

Re: [Bugme-new] [Bug 7665] New: getsockopt(IPV6_*CAST_HOPS) returns -1

[Brian Haley]

David Miller wrote:
> I wonder, since the most accurate return value is tied to the route,
> what is expected of this getsockopt() before a socket's identity
> (and therefore route) is known?

A search for RTAX_HOPLIMIT found very little code that ever sets it, 
iproute2 was the only important one, so the interface/system default is 
probably the only one that's ever used.

-Brian