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

Add support for tcp diags. #199

Merged
merged 1 commit into from Feb 5, 2017

Conversation

Projects
None yet
2 participants
@sebbov

sebbov commented Feb 2, 2017

Adds a single SocketGet function which gets the detail of a socket keyed by its local and remote
addresses.

Organized byte order functionality in a new order.go file.

#198

@sebbov

This comment has been minimized.

sebbov commented Feb 2, 2017

Not sure how to set up the test correctly. Without the setUpNetlinkTest, it works fine on my local machine. Any suggestions?

@vishvananda

This comment has been minimized.

Owner

vishvananda commented Feb 3, 2017

i don't think you want to add the setUpNetlinkTest to the teardown. Does travis give you an error if you remove that? Also is this something that the netlink c library or iproute cli has commands for? I'm curious how one generally uses this from c, as it doesn't look like iproute2 has any uses of SOCK_DIAG_BY_FAMILY. It is possible that the functionality should be in a separate library.

@vishvananda

This comment has been minimized.

Owner

vishvananda commented Feb 3, 2017

i see you had an initial failure. It looks like your issue might be a race between your goroutines. I don't see you waiting for your listen goroutine to get past accept before calling connect. I think if you add some sync there it will pass on travis.

@sebbov

This comment has been minimized.

sebbov commented Feb 3, 2017

libnl does seem to support NETLINK_INET_DIAG (which is the old name for NETLINK_SOCK_DIAG) iiuc. Re iproute2, i believe the "ss" util that is part of it uses this API. I hadn't looked at either or tried to mimic those existing interface in any way, but can take a look at that if you want. The one example use I had found of this is: https://github.com/kristrev/inet-diag-example/blob/master/inet_monitor.c

Right, the travis failure occurred before I used setUpNetlinkTest as well.

I don't see the race you mention: I do start the listener before i connect the client, which should be all that matters (accepting can occur later, right).

Is there anything in the travis environment preventing a test to just listen on a localhost address out of the box? It seems something is missing before I can do that (in this environment specifically; this works on my workstation).

thanks,
-seb.

@vishvananda

This comment has been minimized.

Owner

vishvananda commented Feb 3, 2017

It could be an intermittent failure. I can't rerun the test unless you remove that second commit. It also could have to do with one of the goroutines starting in a separate network namespace, which can happen. You might have some luck using
tearDown := setUpNetlinkTest(t)
defer tearDown()

at the beginning of your test. Although that might mess up your accept goroutine.

@sebbov

This comment has been minimized.

sebbov commented Feb 3, 2017

Sorry that second commit was intended to call setUpNetlinkTest from my test (not from SetupNetlinkTest itself. Fixing that didn't do it though. It does seem like my accepting go routine cannot accept (different "network namespace"?). However I realized that for the purpose of this test, we never need to accept the client's connection, so I removed that go routine altogether. With that and the setUpNetlinkTest removed, it now passes.

@vishvananda

This comment has been minimized.

Owner

vishvananda commented Feb 4, 2017

@sebbov sebbov force-pushed the sebbov:tcpdiags branch from c40f530 to 7253dc3 Feb 4, 2017

@sebbov

This comment has been minimized.

sebbov commented Feb 4, 2017

Done!

@vishvananda vishvananda merged commit a3f0be6 into vishvananda:master Feb 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sebbov

This comment has been minimized.

sebbov commented Feb 6, 2017

Thanks Vish!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment