[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