Update setsockopt(IPV6_MULTICAST_IF) to support RFC 3493

Hi,

From RFC 3493, Section 5.2:

IPV6_MULTICAST_IF

Set the interface to use for outgoing multicast packets. The argument is the index of the interface to use. If the interface index is specified as zero, the system selects the interface (for example, by looking up the address in a routing table and using the resulting interface).

This patch adds support for (index == 0) to reset the value to it's original state, allowing the system to choose the best interface. IPv4 already behaves this way.

-Brian

index 532425d..309284e 100644
@@ -539,6 +539,13 @@ done:
 	case IPV6_MULTICAST_IF:
 		if (sk->sk_type == SOCK_STREAM)
 			goto e_inval;
+
+		if (val == 0) {
+			np->mcast_oif = 0;
+			retv = 0;
+			break;
+		}
+
 		if (sk->sk_bound_dev_if && sk->sk_bound_dev_if != val)
 			goto e_inval;
 

What about just checking for 0 in the later test?

        if (val && __dev_get_by_index(val) == NULL) {

...

                                        +-DLS

We could fail the next check right before that though:

          if (sk->sk_bound_dev_if && sk->sk_bound_dev_if != val)
                  goto e_inval;

I just mimicked what the IPv4 code does in do_ip_setsockopt().

-Brian

Brian Haley <[email protected]> wrote on 10/10/2007 02:20:45 PM:

Right, the semantics there would be "if we have a bound
dev if, that's the only legal value here." Setting it to '0' in that case doesn't really do anythng, anyway. But I don't care about that semantic difference-- could even add "val &&" to the bound_dev_if check.
What I don't like is that your "if" creates an identical
duplicate code path for the functional part of it. In this case it's trivial (the asignment), but makes the code look more complex than it really is. If v4 does it that way, I don't like that either. :-)
I agree with it in general, and may not be worth the
trouble, but I'd personally prefer something like:

        if (sk->sk_type == SOCK_STREAM)
                goto e_inval;
        if (val && sk->sk_bound_dev_if && sk->sk_bound_dev_if != val)
                goto e_inval;

        if (val && __dev_get_by_index(val) != NULL) {
                retv = -ENODEV;
                break;
        }
[at this point all validity checks are done and we're following
        one code path to do the work; each check is easily
        identifiable.]

        np->mcast_oif = val;
        retv = 0;
        break;

Or maybe:

        if (sk->sk_type == SOCK_STREAM)
                goto e_inval;

        if (val) {
                if (sk->sk_bound_dev_if && sk->sk_bound_dev_if != val)
                        goto e_inval;
                if (__dev_get_by_index(val != NULL) {
                        retv = -ENODEV;
                        break;
                }
        }
        np->mcast_oif = val;
        retv = 0;
        break;

But anyway, I made the comment; I think some form of it should go in. :-) If you like the original better, that's ok with me, too.

                                                +-DLS

From: David Stevens <[email protected]>
Date: Wed, 10 Oct 2007 14:48:38 -0700

Brian, please submit a new patch or resubmit the original one, the choice is your's :-)

In article <[email protected]> (at Wed, 10 Oct 2007 16:42:56 -0700 (PDT)), David Miller <[email protected]> says:

I agree, too. :-)

--yoshfuji