Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support AF_NETLINK address family #2980

Open
cohosh opened this issue May 30, 2023 · 16 comments
Open

Support AF_NETLINK address family #2980

cohosh opened this issue May 30, 2023 · 16 comments
Labels
Type: Enhancement New functionality or improved design

Comments

@cohosh
Copy link
Contributor

cohosh commented May 30, 2023

Describe the issue
Trying out snowflake simulations on the newest version of shadow and ran into some trouble with go library calls again. Seems like Snowflake's requirements have changed since I last tried a year ago :)

When I run a minimal snowflake example, all of the tgen streams fail and up inspecting the logs from the snowflake client, I see the following log messages:

2000/01/01 00:01:30 snowflake-client 2.5.1
2000/01/01 00:01:30 Started SOCKS listener at 127.0.0.1:9000.
2000/01/01 00:02:00 SOCKS accepted: {127.0.0.1:8080   map[]}
2000/01/01 00:02:00 


 --- Starting Snowflake Client ---
2000/01/01 00:02:00 Using ICE servers:
2000/01/01 00:02:00 url: stun:stun:3478
2000/01/01 00:02:00 Rendezvous using Broker at: http://broker:8080
2000/01/01 00:02:00 ---- SnowflakeConn: begin collecting snowflakes ---
2000/01/01 00:02:00 ---- SnowflakeConn: starting a new session ---
2000/01/01 00:02:00 WebRTC: Collecting a new Snowflake. Currently at [0/1]
2000/01/01 00:02:00 snowflake-9b4b05eedfb883bf  connecting...
2000/01/01 00:02:00 WebRTC: DataChannel created
2000/01/01 00:02:00 Failed to prepare offer failed to create network: route ip+net: netlinkrib: address family not supported by protocol
2000/01/01 00:02:00 WebRTC: closing DataChannel
2000/01/01 00:02:00 WebRTC: closing PeerConnection
2000/01/01 00:02:00 WebRTC: Closing
2000/01/01 00:02:00 WebRTC: failed to create network: route ip+net: netlinkrib: address family not supported by protocol  Retrying...
2000/01/01 00:02:00 redialing on same connection

These presumably correspond to the following warnings in shadow.log:

00:00:00.428894 [154689:shadow-worker] 00:02:00.000000001 [WARN] [snowflakeclient:11.0.0.3] [socket.c:1109] [syscallhandler_socket] unsupported socket domain "16", we only support AF_INET

The same thing appears to be happening with the proxy (which also uses WebRTC).

To Reproduce
Run a minimal Snowflake experiment: https://github.com/cohosh/shadow-snowflake-minimal

All of the tgen streams will fail

Operating System (please complete the following information):

  • OS and version: Debian GNU/Linux 12 (bookworm)
  • Kernel version:
    Linux 6.1.0-9-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.27-1 (2023-05-08) x86_64 GNU/Linux
  • Go version: go version go1.19.8 linux/amd64

Shadow (please complete the following information):

  • Version: Shadow v3.0.0-52-g787ed885 2023-05-25--19:22:30

  • Which plug-ins you are using: None

Additional context
I realize this might be a big ask, I'm still trying to figure out why the webrtc library needs this.

@cohosh cohosh added the Type: Bug Error or flaw producing unexpected results label May 30, 2023
@cohosh
Copy link
Contributor Author

cohosh commented May 30, 2023

Okay, it looks like this is triggered from the go standard library call to get addresses for network interfaces. For the client and proxy, this happens before they bind to a UDP address for candidate creation. Peers will automatically try to bind to all available addresses and apparently they do that by looking up addresses assigned to each interface.

@stevenengler
Copy link
Contributor

Would you be able to run the client or proxy under strace outside of shadow and include the related syscalls? Probably starting with something like socket(AF_NETLINK, ...) = fd and then any other syscalls that operate on that fd.

This is probably something that would be good for Shadow to support since other applications use it as well, and I don't think shadow would need any architectural changes / rewriting of anything to support it, but it might take a fair amount of work depending on exactly what syscalls and what netlink messages are used. This is indirectly tied to #2900, which also requires netlink support.

I think we'd have to write our own parsers for the protocol, similar to the existing libc macros. Or maybe we could write C wrappers around the libc macros and just use that, since I think the libc macros write the wire format directly so there shouldn't be any difference between the libc format and the kernel format.

@sporksmith
Copy link
Contributor

It would definitely be valuable to add netlink support, as it's come up repeatedly.

Okay, it looks like this is triggered from the go standard library call to get addresses for network interfaces.

This looks like functionality implemented in libc by getifaddrs; I think we've seen these netlink errors before when we fail to intercept getifaddrs at the libc level. I was pleasantly surprised we didn't have this problem before in golang; maybe something there changed s.t. it uses its own implementation now instead of libc's getifaddrs.

It might be worth trying to reproduce, and seeing if it works again with an older version of golang (or shadow, in case something broke there).

@cohosh
Copy link
Contributor Author

cohosh commented May 30, 2023

Hmm, I get a full bootstrap without any calls to socket for the AF_NETLINK address family when running it outside of shadow: strace.log

I'll do some more digging to see if I can figure out why this is different inside shadow.

@sporksmith
Copy link
Contributor

It might be helpful to enable strace-tracing in shadow and take a look at that output. --strace-logging-mode=standard

@cohosh
Copy link
Contributor Author

cohosh commented May 30, 2023

Oh, my bad. I forgot to run strace -f and it missed some threads. It is indeed calling socket(AF_NETLINK, SOCK_RAW|SOCK_CLOEXEC, [...]: strace.log

@cohosh
Copy link
Contributor Author

cohosh commented May 30, 2023

I tracked down the "breaking" change to a change in the webrtc library network stack. Modifying snowflake's go.mod to use an older version of pion/webrtc and pion/ice works with the most recent versions of both shadow and go:

diff --git a/go.mod b/go.mod
index 8f18d6f..2ef577f 100644
--- a/go.mod
+++ b/go.mod
@@ -5,10 +5,11 @@ go 1.15
 require (
        github.com/clarkduvall/hyperloglog v0.0.0-20171127014514-a0107a5d8004
        github.com/gorilla/websocket v1.5.0
-       github.com/pion/ice/v2 v2.3.1
+       github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
+       github.com/pion/ice/v2 v2.2.16
        github.com/pion/sdp/v3 v3.0.6
        github.com/pion/stun v0.4.0
-       github.com/pion/webrtc/v3 v3.1.57
+       github.com/pion/webrtc/v3 v3.1.53
        github.com/prometheus/client_golang v1.10.0
        github.com/prometheus/client_model v0.2.0
        github.com/refraction-networking/utls v1.0.0

This change comes with some settings for the library that might allow us to get around needing the AF_NETLINK sockets, I'll try that as a temporary workaround. But it'd be nice to have this feature eventually so we don't need to patch snowflake for shadow.

@sporksmith
Copy link
Contributor

Thanks for tracking that down! Agreed that ultimately it's desirable to support netlink for this use-case, but glad you found a workaround in the meantime.

@cohosh
Copy link
Contributor Author

cohosh commented May 30, 2023

Documenting the workaround here: https://gitlab.torproject.org/cohosh/snowflake/-/commit/ec14599c22fd7dd61ae2191c8b9001a61b7b0fe2

It was easier than I thought, it turns out that both the older working version of the library and the new versions were calling the same functions under the hood, it's just that the old version silently ignored an error when net.Interfaces() was called.

@stevenengler
Copy link
Contributor

stevenengler commented May 30, 2023

From the strace:

socket(AF_NETLINK, SOCK_RAW|SOCK_CLOEXEC, NETLINK_ROUTE) = 8

(new AF_NETLINK socket in the NETLINK_ROUTE family)

 bind(8, {sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, 12) = 0

(bind to a random port ID)

 sendto(8, [{nlmsg_len=17, nlmsg_type=RTM_GETLINK, nlmsg_flags=NLM_F_REQUEST|NLM_F_DUMP, nlmsg_seq=1, nlmsg_pid=0}, {ifi_family=AF_UNSPEC, ...}], 17, 0, {sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, 12) = 17

(send a RTM_GETLINK request)

getsockname(8, {sa_family=AF_NETLINK, nl_pid=181719, nl_groups=00000000}, [112 => 12]) = 0

(get the assigned port ID)

recvfrom(8, [[{nlmsg_len=1388, nlmsg_type=RTM_NEWLINK, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1, nlmsg_pid=181719}, {ifi_family=AF_UNSPEC, ifi_type=ARPHRD_LOOPBACK, ifi_index=if_nametoindex("lo"), ifi_flags=IFF_UP|IFF_LOOPBACK|IFF_RUNNING|IFF_LOWER_UP, ifi_change=0}, ...,  4096, 0, {sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, [112 => 12]) = 2824
recvfrom(8, [[{nlmsg_len=1820, nlmsg_type=RTM_NEWLINK, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1, nlmsg_pid=181719}, {ifi_family=AF_UNSPEC, ifi_type=ARPHRD_ETHER, ifi_index=if_nametoindex("virbr0"), ifi_flags=IFF_UP|IFF_BROADCAST|IFF_MULTICAST, ifi_change=0}, ..., , 4096, 0, {sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, [112 => 12]) = 3640
recvfrom(8, [{nlmsg_len=20, nlmsg_type=NLMSG_DONE, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1, nlmsg_pid=181719}, 0], 4096, 0, {sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, [112 => 12]) = 20
close(8) = 0

(read the RTM_GETLINK responses)

Then a new AF_NETLINK socket in the NETLINK_ROUTE family is created and bound, and then:

sendto(9, [{nlmsg_len=17, nlmsg_type=RTM_GETADDR, nlmsg_flags=NLM_F_REQUEST|NLM_F_DUMP, nlmsg_seq=1, nlmsg_pid=0}, {ifa_family=AF_UNSPEC, ...}], 17, 0, {sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, 12) = 17

(send a RTM_GETADDR request)

Then another getsockname and more recvfrom calls.

This cycle continues with more RTM_GETADDR requests.

I may have missed some things since I don't know much about netlink, but this gives a general overview of what parts of the netlink api are being used.

@stevenengler stevenengler added Type: Enhancement New functionality or improved design and removed Type: Bug Error or flaw producing unexpected results labels Jun 16, 2023
@sporksmith
Copy link
Contributor

If/when we implement this, the linux_raw_sys crate has Rust definitions for netlink data structures: https://docs.rs/linux-raw-sys/latest/linux_raw_sys/netlink/index.html

@ppopth
Copy link
Contributor

ppopth commented Aug 8, 2023

Fyi, I have implemented AF_NETLINK in Shadow a long time ago. See ppopth@4fd34

Currently, it has no test and I use it only in my use case, i.e. to simulate the Ethereum network. See https://github.com/ppopth/ethereum-shadow

I'm happy to clean it up and write the test to merge it to the upstream repo. Are you willing to take it? If so, I will start working on it.

@sporksmith
Copy link
Contributor

Fyi, I have implemented AF_NETLINK in Shadow a long time ago. See ppopth@4fd34

Cool! This implementation looks plausible to me at first glance, though it might conflict with @stevenengler's current work on the networking stack. @stevenengler wdyt?

@stevenengler
Copy link
Contributor

Fyi, I have implemented AF_NETLINK in Shadow a long time ago. See ppopth@4fd34

Cool! This implementation looks plausible to me at first glance, though it might conflict with @stevenengler's current work on the networking stack. @stevenengler wdyt?

There shouldn't be any conflicts with other networking code, and it looks like this code doesn't access the network interface at all (the interface names are hardcoded into the netlink socket impl). There will be some small merge conflicts when rebasing on shadow@HEAD and possibly with the new TCP code (PR should hopefully be up this week), but I think all of the conflicts would be small.

As for upstreaming it as a PR, it sounds good to me! There might be some small comments, but overall I think it's good. If you make a PR it would be good to have some tests, and a description about what netlink features are supported by this PR, and maybe what some limitations are. (For example, bind() is only partially supported, which I think is okay but it would be good to have that documented.) I don't know much about netlink, and netlink encompasses a lot of different features, so the more documentation the better.

@ppopth
Copy link
Contributor

ppopth commented Aug 18, 2023

If/when we implement this, the linux_raw_sys crate has Rust definitions for netlink data structures: https://docs.rs/linux-raw-sys/latest/linux_raw_sys/netlink/index.html

How about using netlink-packet-core and netlink-packet-route crates from https://github.com/rust-netlink instead?

linux_raw_sys provides only the equivalent C structs rather than the complete reasonable Rust structs and enums.

The other one that we can consider is neli https://docs.rs/neli/latest/neli/

@sporksmith
Copy link
Contributor

@ppopth at first glance either of netlink-packet-* or neli look reasonable to me. Both look like fairly popular and actively maintained crates.

I dropped the note about linux_raw_sys when I noticed it had the raw netlink definitions, so was at least one possible approach, but haven't really surveyed the landscape of alternative crates. I agree that using a more idiomatic and higher level interface than the raw bindings provided by linux_raw_sys is preferable if a suitable one exists.

tgragnato pushed a commit to tgragnato/snowflake that referenced this issue Aug 30, 2023
@ppopth ppopth mentioned this issue Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New functionality or improved design
Projects
None yet
Development

No branches or pull requests

4 participants