[Xorp-users] Question on supporting multiple routing tables [PATCH]

Pavlin Radoslavov pavlin at icir.org
Fri Sep 7 04:08:01 PDT 2007


Ben Greear <greearb at candelatech.com> wrote:

> Ben Greear wrote:
> > I attempted to change my patch per your (off-list) comments,
> > basically to only bind when non-multicast and then immediately
> > un-bind.  Unfortunately, it appears that the Linux kernel logic
> > to un-bind does not work.  I sent a mail to the kernel mailing
> > list, so hopefully it can be fixed sometime soon (or I can be
> > properly beat around the head if it's my mistake.)
> 
> Well, at least part of the problem was self inflicted, as usual.
> I'm still not certain the kernel handles un-binds correctly, but
> as far as I can tell, that doesn't matter for Xorp since we always
> bind on uni-cast, and multi-cast doesn't care.
> 
> I have tested the attached patch, and it seems to work as planned.
> 
> Please review and apply if it meets your specifications.  There is
> a small change to the original logic:  It used to return on some
> error cases and not run the loopback multicast setup at the bottom
> of the method.  With the new patch, it will always run the loopback multicast
> test, as well as the un-bindtodevice logic.
> 
> If you want changes to the patch, let me know.

Ben,

Thank you for the patch.
I applied it with some additional modifications. Most notably,
the setsockopt(SO_BINDTODEVICE) mechanism is used only when it is
needed currently (i.e., only if the unicast forwarding table ID is
configured).
I just committed it so please let me know whether it works for you:

Revision  Changes                                Path
1.12      +57 -7;  commitid: 16e2f46e12aeb7ea6;  xorp/fea/data_plane/io/io_ip_socket.cc

AND

1.13      +4 -2;  commitid: 16f2946e1305c7ea6;  xorp/fea/data_plane/io/io_ip_socket.cc

FYI, I tested the unbinding, and it seems to work on Linux-2.6.20.1.
Though, while testing it I discovered that the value of the optlen
argument to setsockopt indeed needs to be at least 4.

Thanks,
Pavlin


> Thanks,
> Ben
> 
> -- 
> Ben Greear <greearb at candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
> 
> ? fea/data_plane/io/io_ip_socket.cc.tmp
> Index: fea/data_plane/io/io_ip_socket.cc
> ===================================================================
> RCS file: /cvs/xorp/fea/data_plane/io/io_ip_socket.cc,v
> retrieving revision 1.11
> diff -u -r1.11 io_ip_socket.cc
> --- fea/data_plane/io/io_ip_socket.cc	30 Aug 2007 17:37:51 -0000	1.11
> +++ fea/data_plane/io/io_ip_socket.cc	5 Sep 2007 00:26:02 -0000
> @@ -2347,6 +2347,12 @@
>  {
>      bool setloop = false;
>      int ret_value = XORP_OK;
> +#ifdef SO_BINDTODEVICE
> +    // NOTE:  DO NOT RETURN from this method until the very end.  Set return
> +    // value properly and 'goto out' instead.  This way we are certain to unbind
> +    // the interface as needed.
> +    bool unbind = false;
> +#endif
>  
>      //
>      // Adjust some IPv4 header fields
> @@ -2366,21 +2372,38 @@
>      // Multicast-related setting
>      //
>      if (dst_address.is_multicast()) {
> -	if (set_default_multicast_interface(ifp->ifname(),
> -					    vifp->vifname(),
> -					    error_msg)
> +	if ((ret_value = set_default_multicast_interface(ifp->ifname(),
> +							 vifp->vifname(),
> +							 error_msg))
>  	    != XORP_OK) {
> -	    return (XORP_ERROR);
> +	    goto out;
>  	}
>  	//
>  	// XXX: we need to enable the multicast loopback so other processes
>  	// on the same host can receive the multicast packets.
>  	//
> -	if (enable_multicast_loopback(true, error_msg) != XORP_OK) {
> -	    return (XORP_ERROR);
> +	if ((ret_value = enable_multicast_loopback(true, error_msg)) != XORP_OK) {
> +	    goto out;
>  	}
>  	setloop = true;
>      }
> +#ifdef SO_BINDTODEVICE
> +    else {
> +	if (!vifp->ifname().empty()) {
> +	    unbind = true;
> +	    if (setsockopt(_proto_socket_out, SOL_SOCKET, SO_BINDTODEVICE,
> +			   vifp->ifname().c_str(), IFNAMSIZ)) {
> +		error_msg += c_format("setsockopt(SO_BINDTODEVICE, %s) failed: %s",
> +				      vifp->ifname().c_str(), strerror(errno));
> +		XLOG_ERROR("BINDTODEVICE, dev: %s, this: %p src-addr: %s failed: %s",
> +			   vifp->ifname().c_str(), this, cstring(src_address),
> +			   strerror(errno));
> +		// Continue on, probably not fatal in most configurations.
> +	    }
> +	}
> +    }
> +#endif
> +
>  
>      //
>      // Transmit the packet
> @@ -2406,7 +2429,8 @@
>      default:
>  	XLOG_UNREACHABLE();
>  	error_msg = c_format("Invalid address family %d", family());
> -	return (XORP_ERROR);
> +	ret_value = XORP_ERROR;
> +	goto out;
>      }
>  
>      if (sendmsg(_proto_socket_out, &_sndmh, 0) < 0) {
> @@ -2453,7 +2477,8 @@
>      default:
>  	XLOG_UNREACHABLE();
>  	error_msg = c_format("Invalid address family %d", family());
> -	return (XORP_ERROR);
> +	ret_value = XORP_ERROR;
> +	goto out;
>      }
>  
>      error = WSASendTo(_proto_socket_out,
> @@ -2476,6 +2501,31 @@
>      }
>  #endif // HOST_OS_WINDOWS
>  
> +  out:
> +#ifdef SO_BINDTODEVICE
> +    if (unbind) {
> +	// Un-bind the interface
> +        // NOTE:  The man page indicates one can use a zero-length null-terminated string
> +        //    to un-bind, but as of Linux 2.6.20, optlen must be >= 0.  The code below
> +        //    appears to be the correct API.
> +        // NOTE2:  I am not sure if the 2.6.20 kernel code is correct for un-bind, since it does not
> +	//    explicitly free the cached route for the socket, but that route appears to be recalculated
> +	//    properly, because we always (re)bind when sending unicast, and if we are not (re)binding,
> +	//    then it's multicast, which isn't going to use the cached unicast route anyway.
> +        //    --Ben Sept 4, 2007
> +
> +	int z = 0;
> +	if (setsockopt(_proto_socket_out, SOL_SOCKET, SO_BINDTODEVICE,
> +		       &z, sizeof(z))) {
> +	    error_msg += c_format("setsockopt(SO_BINDTODEVICE, NULL) failed: %s",
> +				  strerror(errno));
> +	    XLOG_ERROR("un-BINDTODEVICE, dev: %s, this: %p src-addr: %s failed: %s",
> +		       vifp->ifname().c_str(), this, cstring(src_address),
> +		       strerror(errno));
> +	}
> +    }
> +#endif
> +
>      //
>      // Restore some multicast related settings
>      //
> _______________________________________________
> Xorp-users mailing list
> Xorp-users at xorp.org
> http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-users



More information about the Xorp-users mailing list