All of lore.kernel.org
 help / color / mirror / Atom feed
* linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
@ 2010-05-04  1:33 enh
  2010-05-04  2:16 ` Brian Haley
  0 siblings, 1 reply; 23+ messages in thread
From: enh @ 2010-05-04  1:33 UTC (permalink / raw)
  To: netdev

RFC 3493 (http://tools.ietf.org/rfc/rfc3493.txt) says:

      IPV6_MULTICAST_HOPS

         Set the hop limit to use for outgoing multicast packets.  (Note
         a separate option - IPV6_UNICAST_HOPS - is provided to set the
         hop limit to use for outgoing unicast packets.)

         The interpretation of the argument is the same as for the
         IPV6_UNICAST_HOPS option:

            x < -1:        return an error of EINVAL
            x == -1:       use kernel default
            0 <= x <= 255: use x
            x >= 256:      return an error of EINVAL

            If IPV6_MULTICAST_HOPS is not set, the default is 1
            (same as IPv4 today)

         Argument type: int

but if i create a socket and call getsockopt, i get 64, not 1. this
happens both on Android (2.6.32) and on Ubuntu 8.04 (2.6.24).

actually, i get whatever i've written to
/proc/sys/net/ipv6/conf/all/hop_limit. but afaics, nothing writes that
during init, so i think i'm getting the kernel's fallback default.

anyway, here's a test program you can use to see what i mean. i've
included the ipv4 equivalents, which give the values i'd expect.

/tmp$ cat sock.cpp
/* checkopts.c - based on Stevens */

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

struct sock_opts {
  const char	 *opt_str;
  int	 opt_level;
  int	 opt_name;
} sock_opts[] =
  {
    "IP_TTL",		IPPROTO_IP,	IP_TTL,
    "IP_MULTICAST_TTL",		IPPROTO_IP,	IP_MULTICAST_TTL,
    "IPV6_MULTICAST_HOPS",		IPPROTO_IPV6,	IPV6_MULTICAST_HOPS,
    "IPV6_UNICAST_HOPS",		IPPROTO_IPV6,	IPV6_UNICAST_HOPS,
    NULL,		0,		0,
  };

int main(int argc, char* argv[]) {
  int fd4 = socket(AF_INET, SOCK_DGRAM, 0);
  int fd6 = socket(AF_INET6, SOCK_DGRAM, 0);
  for (struct sock_opts* ptr = sock_opts; ptr->opt_str != NULL; ++ptr) {
    int val;
    socklen_t len = sizeof(int);
    int rc = getsockopt(ptr->opt_level == IPPROTO_IP ? fd4 : fd6,
ptr->opt_level, ptr->opt_name, &val, &len);
    printf("%s default = %d\n", ptr->opt_str, val);
  }
  return 0;
}

/tmp$ make sock && ./sock
make: `sock' is up to date.
IP_TTL default = 64
IP_MULTICAST_TTL default = 1
IPV6_MULTICAST_HOPS default = 64
IPV6_UNICAST_HOPS default = 64
/tmp$

is this a bug? is this the right place to report it? thanks!

-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  1:33 linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1? enh
@ 2010-05-04  2:16 ` Brian Haley
  2010-05-04  3:58   ` enh
  2010-05-04  6:05   ` David Miller
  0 siblings, 2 replies; 23+ messages in thread
From: Brian Haley @ 2010-05-04  2:16 UTC (permalink / raw)
  To: enh; +Cc: netdev

enh wrote:
> RFC 3493 (http://tools.ietf.org/rfc/rfc3493.txt) says:
> 
>       IPV6_MULTICAST_HOPS
> 
>          Set the hop limit to use for outgoing multicast packets.  (Note
>          a separate option - IPV6_UNICAST_HOPS - is provided to set the
>          hop limit to use for outgoing unicast packets.)
> 
>          The interpretation of the argument is the same as for the
>          IPV6_UNICAST_HOPS option:
> 
>             x < -1:        return an error of EINVAL
>             x == -1:       use kernel default
>             0 <= x <= 255: use x
>             x >= 256:      return an error of EINVAL
> 
>             If IPV6_MULTICAST_HOPS is not set, the default is 1
>             (same as IPv4 today)
> 
>          Argument type: int
> 
> but if i create a socket and call getsockopt, i get 64, not 1. this
> happens both on Android (2.6.32) and on Ubuntu 8.04 (2.6.24).

<snip>

> is this a bug? is this the right place to report it? thanks!

It looks like a bug to me, feel free to send along a patch :)

-Brian


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  2:16 ` Brian Haley
@ 2010-05-04  3:58   ` enh
  2010-05-04  6:05   ` David Miller
  1 sibling, 0 replies; 23+ messages in thread
From: enh @ 2010-05-04  3:58 UTC (permalink / raw)
  To: Brian Haley; +Cc: netdev

On Mon, May 3, 2010 at 19:16, Brian Haley <brian.haley@hp.com> wrote:
> enh wrote:
>> RFC 3493 (http://tools.ietf.org/rfc/rfc3493.txt) says:
>>
>>       IPV6_MULTICAST_HOPS
>>
>>          Set the hop limit to use for outgoing multicast packets.  (Note
>>          a separate option - IPV6_UNICAST_HOPS - is provided to set the
>>          hop limit to use for outgoing unicast packets.)
>>
>>          The interpretation of the argument is the same as for the
>>          IPV6_UNICAST_HOPS option:
>>
>>             x < -1:        return an error of EINVAL
>>             x == -1:       use kernel default
>>             0 <= x <= 255: use x
>>             x >= 256:      return an error of EINVAL
>>
>>             If IPV6_MULTICAST_HOPS is not set, the default is 1
>>             (same as IPv4 today)
>>
>>          Argument type: int
>>
>> but if i create a socket and call getsockopt, i get 64, not 1. this
>> happens both on Android (2.6.32) and on Ubuntu 8.04 (2.6.24).
>
> <snip>
>
>> is this a bug? is this the right place to report it? thanks!
>
> It looks like a bug to me, feel free to send along a patch :)

a grep for IPV6_DEFAULT_MCASTHOPS suggests it isn't used:

http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fnext%2Flinux-next.git&a=search&h=HEAD&st=grep&s=IPV6_DEFAULT_MCASTHOPS

i assumed IPV6_DEFAULT_HOPLIMIT (the unicast hop limit) was being used
by accident where IPV6_DEFAULT_MCASTHOPS should be used.

looking at net/ipv6/ipv6_sockglue.c, i see that getsockopt for both
unicast and multicast hop limits defaults to the device's hop_limit:

1109         case IPV6_UNICAST_HOPS:
1110         case IPV6_MULTICAST_HOPS:
1111         {
1112                 struct dst_entry *dst;
1113
1114                 if (optname == IPV6_UNICAST_HOPS)
1115                         val = np->hop_limit;
1116                 else
1117                         val = np->mcast_hops;
1118
1119                 if (val < 0) {
1120                         rcu_read_lock();
1121                         dst = __sk_dst_get(sk);
1122                         if (dst)
1123                                 val = ip6_dst_hoplimit(dst);
1124                         rcu_read_unlock();
1125                 }
1126
1127                 if (val < 0)
1128                         val = sock_net(sk)->ipv6.devconf_all->hop_limit;
1129                 break;
1130         }

and look how net/ipv6/af_inet6.c initializes the two fields:

 202         np->hop_limit   = -1;
 203         np->mcast_hops  = -1;

so the easiest fix would be to change net/ipv6/af_inet6.c to:

 202         np->hop_limit   = -1; /* Use the configured device default. */
 203         np->mcast_hops  = IPV6_DEFAULT_MCASTHOPS; /* Use RFC 3493
default. */

userspace programmers still have the ability to ask for the device's
default by calling setsockopt with the value -1 (as mentioned in the
RFC). in practice, i'd imagine anyone who actually wanted to use that
feature would want a separate tunable from the existing unicast one.

> -Brian
>
>



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  2:16 ` Brian Haley
  2010-05-04  3:58   ` enh
@ 2010-05-04  6:05   ` David Miller
  2010-05-04  6:19     ` enh
  1 sibling, 1 reply; 23+ messages in thread
From: David Miller @ 2010-05-04  6:05 UTC (permalink / raw)
  To: brian.haley; +Cc: enh, netdev

From: Brian Haley <brian.haley@hp.com>
Date: Mon, 03 May 2010 22:16:39 -0400

> It looks like a bug to me, feel free to send along a patch :)

Is it?  The quoted text is only about setting the value and what
effect setting -1 or whatever has.

For getting the value, the behavior described sounds just fine.

The default for a socket is whatever the kernel-wide default is.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  6:05   ` David Miller
@ 2010-05-04  6:19     ` enh
  2010-05-04  6:22       ` David Miller
  0 siblings, 1 reply; 23+ messages in thread
From: enh @ 2010-05-04  6:19 UTC (permalink / raw)
  To: David Miller; +Cc: brian.haley, netdev

On Mon, May 3, 2010 at 23:05, David Miller <davem@davemloft.net> wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Mon, 03 May 2010 22:16:39 -0400
>
>> It looks like a bug to me, feel free to send along a patch :)
>
> Is it?  The quoted text is only about setting the value and what
> effect setting -1 or whatever has.
>
> For getting the value, the behavior described sounds just fine.
>
> The default for a socket is whatever the kernel-wide default is.

for the *unicast* hops, a part of the RFC i didn't quote says:

   If the [IPV6_UNICAST_HOPS] option is not set, the
   system selects a default value.

but for the *multicast* hops, which is what i'm talking about, this
part of the quoted text seems pretty definitive:

           If IPV6_MULTICAST_HOPS is not set, the default is 1
           (same as IPv4 today)

this is what my test shows isn't true of linux; linux reuses its
unicast default instead.

-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  6:19     ` enh
@ 2010-05-04  6:22       ` David Miller
  2010-05-04  6:27         ` enh
  0 siblings, 1 reply; 23+ messages in thread
From: David Miller @ 2010-05-04  6:22 UTC (permalink / raw)
  To: enh; +Cc: brian.haley, netdev

From: enh <enh@google.com>
Date: Mon, 3 May 2010 23:19:22 -0700

> for the *unicast* hops, a part of the RFC i didn't quote says:
> 
>    If the [IPV6_UNICAST_HOPS] option is not set, the
>    system selects a default value.
> 
> but for the *multicast* hops, which is what i'm talking about, this
> part of the quoted text seems pretty definitive:
> 
>            If IPV6_MULTICAST_HOPS is not set, the default is 1
>            (same as IPv4 today)
> 
> this is what my test shows isn't true of linux; linux reuses its
> unicast default instead.

Ok, I see, so yeah this needs to be fixed to use "1" instead of
"-1" in the np->xxx ipv6 socket initialization.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  6:22       ` David Miller
@ 2010-05-04  6:27         ` enh
  2010-05-04  6:42           ` David Miller
  2010-05-04  7:48           ` David Stevens
  0 siblings, 2 replies; 23+ messages in thread
From: enh @ 2010-05-04  6:27 UTC (permalink / raw)
  To: David Miller; +Cc: brian.haley, netdev

On Mon, May 3, 2010 at 23:22, David Miller <davem@davemloft.net> wrote:
> From: enh <enh@google.com>
> Date: Mon, 3 May 2010 23:19:22 -0700
>
>> for the *unicast* hops, a part of the RFC i didn't quote says:
>>
>>    If the [IPV6_UNICAST_HOPS] option is not set, the
>>    system selects a default value.
>>
>> but for the *multicast* hops, which is what i'm talking about, this
>> part of the quoted text seems pretty definitive:
>>
>>            If IPV6_MULTICAST_HOPS is not set, the default is 1
>>            (same as IPv4 today)
>>
>> this is what my test shows isn't true of linux; linux reuses its
>> unicast default instead.
>
> Ok, I see, so yeah this needs to be fixed to use "1" instead of
> "-1" in the np->xxx ipv6 socket initialization.

i think so. there's already the IPV6_DEFAULT_MCASTHOPS constant
defined to 1 but unused according to gitweb, so you might want to
either use it or remove it.

thanks!

-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  6:27         ` enh
@ 2010-05-04  6:42           ` David Miller
  2010-05-04  7:48           ` David Stevens
  1 sibling, 0 replies; 23+ messages in thread
From: David Miller @ 2010-05-04  6:42 UTC (permalink / raw)
  To: enh; +Cc: brian.haley, netdev

From: enh <enh@google.com>
Date: Mon, 3 May 2010 23:27:23 -0700

> On Mon, May 3, 2010 at 23:22, David Miller <davem@davemloft.net> wrote:
>> Ok, I see, so yeah this needs to be fixed to use "1" instead of
>> "-1" in the np->xxx ipv6 socket initialization.
> 
> i think so. there's already the IPV6_DEFAULT_MCASTHOPS constant
> defined to 1 but unused according to gitweb, so you might want to
> either use it or remove it.

I've applied the following, thanks Elliot.

--------------------
ipv6: Fix default multicast hops setting.

As per RFC 3493 the default multicast hops setting
for a socket should be "1" just like ipv4.

Ironically we have a IPV6_DEFAULT_MCASTHOPS macro
it just wasn't being used.

Reported-by: Elliot Hughes <enh@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv6/af_inet6.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 3192aa0..3f9e86b 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -200,7 +200,7 @@ lookup_protocol:
 
 	inet_sk(sk)->pinet6 = np = inet6_sk_generic(sk);
 	np->hop_limit	= -1;
-	np->mcast_hops	= -1;
+	np->mcast_hops	= IPV6_DEFAULT_MCASTHOPS;
 	np->mc_loop	= 1;
 	np->pmtudisc	= IPV6_PMTUDISC_WANT;
 	np->ipv6only	= net->ipv6.sysctl.bindv6only;
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  6:27         ` enh
  2010-05-04  6:42           ` David Miller
@ 2010-05-04  7:48           ` David Stevens
  2010-05-04  7:57             ` David Miller
  1 sibling, 1 reply; 23+ messages in thread
From: David Stevens @ 2010-05-04  7:48 UTC (permalink / raw)
  To: enh; +Cc: brian.haley, David Miller, netdev, netdev-owner

It's set to -1 by default, but the common code for unicast and
multicast in getsockopt is falling through to use the dst_entry.

I believe (though I haven't actually tried it recently) it actually
uses "1" for the default value for multicast; it just doesn't
report it correctly. It should distinguish multicast from unicast
in the <0 check in getsockopt.
                                        +-DLS


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  7:48           ` David Stevens
@ 2010-05-04  7:57             ` David Miller
  2010-05-04 14:40               ` Brian Haley
  0 siblings, 1 reply; 23+ messages in thread
From: David Miller @ 2010-05-04  7:57 UTC (permalink / raw)
  To: dlstevens; +Cc: enh, brian.haley, netdev, netdev-owner


From: David Stevens <dlstevens@us.ibm.com>
Date: Tue, 4 May 2010 00:48:46 -0700

> It's set to -1 by default, but the common code for unicast and
> multicast in getsockopt is falling through to use the dst_entry.
> 
> I believe (though I haven't actually tried it recently) it actually
> uses "1" for the default value for multicast;

It doesn't, all of the uses in the ipv6 stack say something like:

	if (multicast)
		hlimit = np->mcast_hops;
	else
		hlimit = np->hop_limit;
	if (hlimit < 0)
		hlimit = ip6_dst_hoplimit(dst);

Therefore, the change suggested by Elliot and which I committed is the
way to get the correct behavior and fix this.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04  7:57             ` David Miller
@ 2010-05-04 14:40               ` Brian Haley
  2010-05-04 16:12                 ` David Stevens
                                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Brian Haley @ 2010-05-04 14:40 UTC (permalink / raw)
  To: David Miller; +Cc: dlstevens, enh, netdev, netdev-owner

David Miller wrote:
> From: David Stevens <dlstevens@us.ibm.com>
> Date: Tue, 4 May 2010 00:48:46 -0700
> 
>> It's set to -1 by default, but the common code for unicast and
>> multicast in getsockopt is falling through to use the dst_entry.
>>
>> I believe (though I haven't actually tried it recently) it actually
>> uses "1" for the default value for multicast;

No, on-the-wire it's actually 64.

> It doesn't, all of the uses in the ipv6 stack say something like:
> 
> 	if (multicast)
> 		hlimit = np->mcast_hops;
> 	else
> 		hlimit = np->hop_limit;
> 	if (hlimit < 0)
> 		hlimit = ip6_dst_hoplimit(dst);
> 
> Therefore, the change suggested by Elliot and which I committed is the
> way to get the correct behavior and fix this.

Not exactly.  It fixes the case where it's wrong by default, but
the corner case of setting it to -1 via setsockopt() says:

    x == -1:       use kernel default

But that will revert back to the kernel using 64 on the next transmit.
I can work on an update to this that makes a new mcast_hops per-interface
setting and makes ip6_dst_hoplimit() aware of it.  Or even easier, just
have setsockopt() trap the -1 and set np->mcast_hops to 1.  Built but
untested patch below.

-Brian

--


Specifying -1 for setsockopt(IPV6_MULTICAST_HOPS) should set the socket
value back to the system default value of IPV6_DEFAULT_MCASTHOPS (1).

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

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index bd43f01..fa6875b 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -486,7 +486,10 @@ done:
 			goto e_inval;
 		if (val > 255 || val < -1)
 			goto e_inval;
-		np->mcast_hops = val;
+		if (val == -1)
+			np->mcast_hops = IPV6_DEFAULT_MCASTHOPS;
+		else
+			np->mcast_hops = val;
 		retv = 0;
 		break;
 

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 14:40               ` Brian Haley
@ 2010-05-04 16:12                 ` David Stevens
  2010-05-04 16:43                   ` Brian Haley
  2010-05-04 21:39                   ` David Miller
  2010-05-04 21:38                 ` David Miller
  2010-05-04 21:46                 ` David Miller
  2 siblings, 2 replies; 23+ messages in thread
From: David Stevens @ 2010-05-04 16:12 UTC (permalink / raw)
  To: Brian Haley; +Cc: David Miller, enh, netdev, netdev-owner

I think the original code was intending to do late binding -- carry "-1" 
as
meaning "not set by user" and use the default value _at_the_time_of_
_the_send_, and in its context. For that to have worked, the checks for
"<0" in the send paths should've checked for multicast and used the
multicast default as you're saying, Brian. And doing that not on the
set, but when generating packets, is what I would've expected.

I don't see anything that's broken by changing it to use the default at
the time of the set since for mcast the default is really a constant,
and in fact, it looks like in addition to not actually using the default 
of 1,
it was returning "-1" in the cmsg when not set by the user (and it, too,
should've been "1", which it would return now).

But if the default is different for each destination or interface in
the multicast case (ie, by adding conf settings for mcast), then
it really should do late binding and leave it as "-1" in the set, right?
That's what I thought it was already doing, but apparently not;
I think it used to, but maybe I just didn't notice.

                                        +-DLS


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 16:12                 ` David Stevens
@ 2010-05-04 16:43                   ` Brian Haley
  2010-05-04 17:05                     ` David Stevens
  2010-05-04 21:39                   ` David Miller
  1 sibling, 1 reply; 23+ messages in thread
From: Brian Haley @ 2010-05-04 16:43 UTC (permalink / raw)
  To: David Stevens; +Cc: David Miller, enh, netdev, netdev-owner

David Stevens wrote:
> I think the original code was intending to do late binding -- carry "-1" 
> as
> meaning "not set by user" and use the default value _at_the_time_of_
> _the_send_, and in its context. For that to have worked, the checks for
> "<0" in the send paths should've checked for multicast and used the
> multicast default as you're saying, Brian. And doing that not on the
> set, but when generating packets, is what I would've expected.

Right, we could do it that way, but then how far do we unravel the thread?
Unicast hoplimit is settable in the route, do we add a mcast_hops there
too, in addition to the per-interface tunable?  I think just having it
the recommended default is good enough here, until someone shows they
have the need to do more.

> I don't see anything that's broken by changing it to use the default at
> the time of the set since for mcast the default is really a constant,
> and in fact, it looks like in addition to not actually using the default 
> of 1,
> it was returning "-1" in the cmsg when not set by the user (and it, too,
> should've been "1", which it would return now).
> 
> But if the default is different for each destination or interface in
> the multicast case (ie, by adding conf settings for mcast), then
> it really should do late binding and leave it as "-1" in the set, right?
> That's what I thought it was already doing, but apparently not;
> I think it used to, but maybe I just didn't notice.

Yes, that would be the ideal fix, and give the admin more control over
the value, but it seems like overkill to me.  It's been 64 for a while,
and it's always been changeable by apps.  I guess the only thing to
think about is there could be an app that works because it being 64
today, but will break tomorrow.  Having a tunable parameter will let
you get the app working without re-writing it.

-Brian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 16:43                   ` Brian Haley
@ 2010-05-04 17:05                     ` David Stevens
  0 siblings, 0 replies; 23+ messages in thread
From: David Stevens @ 2010-05-04 17:05 UTC (permalink / raw)
  To: Brian Haley; +Cc: David Miller, enh, netdev, netdev-owner

 > Yes, that would be the ideal fix, and give the admin more control over
> the value, but it seems like overkill to me.  It's been 64 for a while,
> and it's always been changeable by apps.  I guess the only thing to
> think about is there could be an app that works because it being 64
> today, but will break tomorrow.  Having a tunable parameter will let
> you get the app working without re-writing it.

        Well, it should've been 1, and any app relying on having
multicast routing really should set it explicitly.
        I think per-interface defaulting to 1 should be ok. I'd
prefer carrying the "-1" so apps that set it get what they want
and apps that don't carry the current default, rather than the
value at the time the socket was created, but practically it probably
doesn't matter. In reality, apps that need more than one will
already be setting it to a non-default value.

                                                        +-DLS


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 14:40               ` Brian Haley
  2010-05-04 16:12                 ` David Stevens
@ 2010-05-04 21:38                 ` David Miller
  2010-05-04 21:46                 ` David Miller
  2 siblings, 0 replies; 23+ messages in thread
From: David Miller @ 2010-05-04 21:38 UTC (permalink / raw)
  To: brian.haley; +Cc: dlstevens, enh, netdev, netdev-owner

From: Brian Haley <brian.haley@hp.com>
Date: Tue, 04 May 2010 10:40:58 -0400

> Not exactly.  It fixes the case where it's wrong by default, but
> the corner case of setting it to -1 via setsockopt() says:
> 
>     x == -1:       use kernel default
> 
> But that will revert back to the kernel using 64 on the next transmit.
> I can work on an update to this that makes a new mcast_hops per-interface
> setting and makes ip6_dst_hoplimit() aware of it.  Or even easier, just
> have setsockopt() trap the -1 and set np->mcast_hops to 1.  Built but
> untested patch below.

I thought that we agreed that when the user explicitly asks for "-1"
it should get the behavior right now, with is to use
ip6_dst_hoplimit()?  I think I even acknowledged when Elliot mentioned
this explicitly, and I think it's a good idea.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 16:12                 ` David Stevens
  2010-05-04 16:43                   ` Brian Haley
@ 2010-05-04 21:39                   ` David Miller
  1 sibling, 0 replies; 23+ messages in thread
From: David Miller @ 2010-05-04 21:39 UTC (permalink / raw)
  To: dlstevens; +Cc: brian.haley, enh, netdev, netdev-owner

From: David Stevens <dlstevens@us.ibm.com>
Date: Tue, 4 May 2010 09:12:59 -0700

> But if the default is different for each destination or interface in
> the multicast case (ie, by adding conf settings for mcast), then
> it really should do late binding and leave it as "-1" in the set, right?
> That's what I thought it was already doing, but apparently not;
> I think it used to, but maybe I just didn't notice.

Unlike other people in this thread who sometimes aren't even checking
how the current code works, I checked all of the available source
control history in this area and this code has always behaved this way.

Right from day one.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 14:40               ` Brian Haley
  2010-05-04 16:12                 ` David Stevens
  2010-05-04 21:38                 ` David Miller
@ 2010-05-04 21:46                 ` David Miller
  2010-05-04 22:26                   ` enh
  2010-05-05 15:36                   ` Brian Haley
  2 siblings, 2 replies; 23+ messages in thread
From: David Miller @ 2010-05-04 21:46 UTC (permalink / raw)
  To: brian.haley; +Cc: dlstevens, enh, netdev, netdev-owner

From: Brian Haley <brian.haley@hp.com>
Date: Tue, 04 May 2010 10:40:58 -0400

> Specifying -1 for setsockopt(IPV6_MULTICAST_HOPS) should set the socket
> value back to the system default value of IPV6_DEFAULT_MCASTHOPS (1).
> 
> Signed-off-by: Brian Haley <brian.haley@hp.com>

In cast it wasn't clear from my other reply, I'm not applying this
patch because I intentionally left this behavior there based upon
some comments from Elliot in that this lets developers get the
old default by asking for "-1" explicitly with a setsockopt.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 21:46                 ` David Miller
@ 2010-05-04 22:26                   ` enh
  2010-05-04 23:07                     ` David Miller
  2010-05-05 15:36                   ` Brian Haley
  1 sibling, 1 reply; 23+ messages in thread
From: enh @ 2010-05-04 22:26 UTC (permalink / raw)
  To: David Miller; +Cc: brian.haley, dlstevens, netdev, netdev-owner

On Tue, May 4, 2010 at 14:46, David Miller <davem@davemloft.net> wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Tue, 04 May 2010 10:40:58 -0400
>
>> Specifying -1 for setsockopt(IPV6_MULTICAST_HOPS) should set the socket
>> value back to the system default value of IPV6_DEFAULT_MCASTHOPS (1).
>>
>> Signed-off-by: Brian Haley <brian.haley@hp.com>
>
> In cast it wasn't clear from my other reply, I'm not applying this
> patch because I intentionally left this behavior there based upon
> some comments from Elliot in that this lets developers get the
> old default by asking for "-1" explicitly with a setsockopt.

(for the record, i don't need that behavior myself, and have no
opinion on whether or not it makes sense for you guys... i'll only
ever call setsockopt with 0 <= value <= 255. all i need is for the
default when i never call setsockopt to be 1. for now, i've added a
work-around where i explicitly call setsockopt with 1 when i create
the socket.)

-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 22:26                   ` enh
@ 2010-05-04 23:07                     ` David Miller
  0 siblings, 0 replies; 23+ messages in thread
From: David Miller @ 2010-05-04 23:07 UTC (permalink / raw)
  To: enh; +Cc: brian.haley, dlstevens, netdev, netdev-owner

From: enh <enh@google.com>
Date: Tue, 4 May 2010 15:26:52 -0700

> On Tue, May 4, 2010 at 14:46, David Miller <davem@davemloft.net> wrote:
>> From: Brian Haley <brian.haley@hp.com>
>> Date: Tue, 04 May 2010 10:40:58 -0400
>>
>>> Specifying -1 for setsockopt(IPV6_MULTICAST_HOPS) should set the socket
>>> value back to the system default value of IPV6_DEFAULT_MCASTHOPS (1).
>>>
>>> Signed-off-by: Brian Haley <brian.haley@hp.com>
>>
>> In cast it wasn't clear from my other reply, I'm not applying this
>> patch because I intentionally left this behavior there based upon
>> some comments from Elliot in that this lets developers get the
>> old default by asking for "-1" explicitly with a setsockopt.
> 
> (for the record, i don't need that behavior myself, and have no
> opinion on whether or not it makes sense for you guys... i'll only
> ever call setsockopt with 0 <= value <= 255. all i need is for the
> default when i never call setsockopt to be 1. for now, i've added a
> work-around where i explicitly call setsockopt with 1 when i create
> the socket.)

It's more of an issue of having at least some kind of compatability
story when we change this.

With what's in the tree now we can at least say "if you explicitly
setsockopt() the value to '-1' you will get the same behavior now
as beforehand"

Whereas with what others are suggesting, we can't give people a way
in their applications to do that other than to suggest they use
disgusting concoctions like "set non-multicast hoplimit to '-1',
getsockopt() that, then set the multicast hop explicitly to that"

And even that won't work the same as now, in that changes to the
per-route metric will be ignored.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-04 21:46                 ` David Miller
  2010-05-04 22:26                   ` enh
@ 2010-05-05 15:36                   ` Brian Haley
  2010-05-05 22:00                     ` David Miller
  1 sibling, 1 reply; 23+ messages in thread
From: Brian Haley @ 2010-05-05 15:36 UTC (permalink / raw)
  To: David Miller; +Cc: dlstevens, enh, netdev, netdev-owner

David Miller wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Tue, 04 May 2010 10:40:58 -0400
> 
>> Specifying -1 for setsockopt(IPV6_MULTICAST_HOPS) should set the socket
>> value back to the system default value of IPV6_DEFAULT_MCASTHOPS (1).
>>
>> Signed-off-by: Brian Haley <brian.haley@hp.com>
> 
> In cast it wasn't clear from my other reply, I'm not applying this
> patch because I intentionally left this behavior there based upon
> some comments from Elliot in that this lets developers get the
> old default by asking for "-1" explicitly with a setsockopt.

I now see that in Elliot's email, but I think it's incorrect.  The RFC
says that setting it to -1 should get you the kernel default, which is
now 1.  Without this change, setting it to -1 will get you 64, the
old behavior.  If the user wants to, they can always just set it to
64 themselves, that's better than assuming when you set it to -1
you're going to get 64.

I'm just trying to make this follow the RFC and behave like other OSes
for consistency.

-Brian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-05 15:36                   ` Brian Haley
@ 2010-05-05 22:00                     ` David Miller
  2010-05-06  1:50                       ` Brian Haley
  0 siblings, 1 reply; 23+ messages in thread
From: David Miller @ 2010-05-05 22:00 UTC (permalink / raw)
  To: brian.haley; +Cc: dlstevens, enh, netdev, netdev-owner

From: Brian Haley <brian.haley@hp.com>
Date: Wed, 05 May 2010 11:36:31 -0400

> I now see that in Elliot's email, but I think it's incorrect.  The RFC
> says that setting it to -1 should get you the kernel default, which is
> now 1.  Without this change, setting it to -1 will get you 64, the
> old behavior.  If the user wants to, they can always just set it to
> 64 themselves, that's better than assuming when you set it to -1
> you're going to get 64.

It's not 64, it's whatever the per-route metric is.

> I'm just trying to make this follow the RFC and behave like other OSes
> for consistency.

I'm just trying to have a real compatability story, and we don't with
any of the proposals you guys are giving me because it complete ignores
the fact that the old default comes from the route and is not some
constant.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-05 22:00                     ` David Miller
@ 2010-05-06  1:50                       ` Brian Haley
  2010-05-06  7:10                         ` David Miller
  0 siblings, 1 reply; 23+ messages in thread
From: Brian Haley @ 2010-05-06  1:50 UTC (permalink / raw)
  To: David Miller; +Cc: dlstevens, enh, netdev, netdev-owner

David Miller wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Wed, 05 May 2010 11:36:31 -0400
> 
>> I now see that in Elliot's email, but I think it's incorrect.  The RFC
>> says that setting it to -1 should get you the kernel default, which is
>> now 1.  Without this change, setting it to -1 will get you 64, the
>> old behavior.  If the user wants to, they can always just set it to
>> 64 themselves, that's better than assuming when you set it to -1
>> you're going to get 64.
> 
> It's not 64, it's whatever the per-route metric is.

Not unless that metric's been set via RTAX_HOPLIMIT (and I believe
this is the unicast hop limit value anyways), and that metric
defaults to -1.  Routes added via a Router Advertisement are most
likely going to have a hop limit of 64, but I believe that's only
supposed to apply to unicast.

I *did* search the kernel code and test this before my original reply - it
uses the unicast hop limit from the interface as Elliot originally showed.

~# sysctl net.ipv6.conf.eth2.hop_limit
net.ipv6.conf.eth2.hop_limit = 64

21:04:48.766181 IP6 (hlim 64, next-header UDP (17) payload length: 108)
    fe80::21f:29ff:fef0:2f46.48914 > ip6-allrouters.7639: UDP, length 100

~# sysctl net.ipv6.conf.eth2.hop_limit=63
net.ipv6.conf.eth2.hop_limit = 63

21:05:09.670190 IP6 (hlim 63, next-header UDP (17) payload length: 108)
    fe80::21f:29ff:fef0:2f46.48914 > ip6-allrouters.7639: UDP, length 100

At this point in time I'll gladly implement a per-interface sysctl
to end this discussion.

-Brian

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
  2010-05-06  1:50                       ` Brian Haley
@ 2010-05-06  7:10                         ` David Miller
  0 siblings, 0 replies; 23+ messages in thread
From: David Miller @ 2010-05-06  7:10 UTC (permalink / raw)
  To: brian.haley; +Cc: dlstevens, enh, netdev, netdev-owner

From: Brian Haley <brian.haley@hp.com>
Date: Wed, 05 May 2010 21:50:18 -0400

> David Miller wrote:
>> From: Brian Haley <brian.haley@hp.com>
>> Date: Wed, 05 May 2010 11:36:31 -0400
>> 
>>> I now see that in Elliot's email, but I think it's incorrect.  The RFC
>>> says that setting it to -1 should get you the kernel default, which is
>>> now 1.  Without this change, setting it to -1 will get you 64, the
>>> old behavior.  If the user wants to, they can always just set it to
>>> 64 themselves, that's better than assuming when you set it to -1
>>> you're going to get 64.
>> 
>> It's not 64, it's whatever the per-route metric is.
> 
> Not unless that metric's been set via RTAX_HOPLIMIT (and I believe
> this is the unicast hop limit value anyways), and that metric
> defaults to -1.

Right, if it is, and anyone who does set it and expects the default
multicast hop limit to follow along have no portable way to code their
application in a way that works before and after fixing the RFC
issues.

I gave them a way, by making explicit setting of "-1" do what it's
always done.

> At this point in time I'll gladly implement a per-interface sysctl
> to end this discussion.

The game is over, the result decided, and this is just post-game
discussion as far as I'm concerned. :-)


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2010-05-06  7:10 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-04  1:33 linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1? enh
2010-05-04  2:16 ` Brian Haley
2010-05-04  3:58   ` enh
2010-05-04  6:05   ` David Miller
2010-05-04  6:19     ` enh
2010-05-04  6:22       ` David Miller
2010-05-04  6:27         ` enh
2010-05-04  6:42           ` David Miller
2010-05-04  7:48           ` David Stevens
2010-05-04  7:57             ` David Miller
2010-05-04 14:40               ` Brian Haley
2010-05-04 16:12                 ` David Stevens
2010-05-04 16:43                   ` Brian Haley
2010-05-04 17:05                     ` David Stevens
2010-05-04 21:39                   ` David Miller
2010-05-04 21:38                 ` David Miller
2010-05-04 21:46                 ` David Miller
2010-05-04 22:26                   ` enh
2010-05-04 23:07                     ` David Miller
2010-05-05 15:36                   ` Brian Haley
2010-05-05 22:00                     ` David Miller
2010-05-06  1:50                       ` Brian Haley
2010-05-06  7:10                         ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.