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

x/net/netlink, vendor/golang.org/x/net/netlink: new package #20637

Open
mikioh opened this issue Jun 10, 2017 · 13 comments
Open

x/net/netlink, vendor/golang.org/x/net/netlink: new package #20637

mikioh opened this issue Jun 10, 2017 · 13 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Milestone

Comments

@mikioh
Copy link
Contributor

mikioh commented Jun 10, 2017

I propose to have a x/net/netlink package and vendor it to the standard library for maintenance purposes.

Netlink is a Linux-specific service for manipulating networking facilities inside the kernel. Fortunately, unlike routing messages and sockets on BSD variants, it doesn't have much ABI incompatibility between major kernel releases, but its complexity like TLV-in-AVP-in-TLV data representation could be a root cause of issues like #18714, #16681. So I think it's better to have and maintain a small separated package like the existing x/net/route package.

The exposed API would be like the following:

  • minimal netlink message types mostly for the use of standard library,
  • minimal netlink message parsers mostly for the use of standard library,
  • both types and parsers are ABI agnostic and safe from potential misaligned memory access.

Thoughts?

@gopherbot gopherbot added this to the Proposal milestone Jun 10, 2017
@bradfitz
Copy link
Contributor

What in the the standard library needs it?

/cc @mdlayher

@mikioh
Copy link
Contributor Author

mikioh commented Jun 10, 2017

As far as I can tell, the net package of standard library requires at least a) fetching netlink routing information base, b) parsing netlink routing messages related to on-link adjacencies for now.

@bradfitz
Copy link
Contributor

As far as I can tell, the net package of standard library requires at least a) fetching netlink routing information base, b) parsing netlink routing messages related to on-link adjacencies for now.

What bug is that?

Which existing functions/methods don't work?

Got a sample program?

@bradfitz
Copy link
Contributor

In other words, this proposal is a solution but you haven't yet sufficiently identified any problem. We don't accept proposals if there's not a problem.

@mikioh
Copy link
Contributor Author

mikioh commented Jun 10, 2017

a solution

Yup, I'm guessing that @mdlayher has another perspective on this stuff because he poked me in #10565; once we make external socket and its addresses work together with the net package.

@mdlayher
Copy link
Member

I've got a netlink package that does its best to just focus on pure netlink: not conflating it with rtnetlink, genetlink, etc. I would be happy to contribute parts or all of it to become x/net/netlink. [1]

That being said, because of #10565, I don't have an easy way of configuring timeouts without doing all of the epoll work myself. This isn't such a big deal because netlink is Linux only, but I'd be happy to see that issue solved for the sake of other sockets packages which may attempt to provide some cross platform functionality. [2]

As for the original point, I think it could make sense to provide a high quality netlink package in x/net that could be useful both inside and outside the standard library.

The biggest potential problem I can think of with the existing netlink functionality in syscall is that netlink does not seem to cooperate with Go's scheduler moving goroutines between threads. This is a problem I'd also like to tackle in my own package, but have not yet had the chance to do so.

I'm happy to help make this happen if there is a consensus that this proposal should move forward.

[1] https://github.com/mdlayher/netlink
[2] https://github.com/mdlayher/raw

@mdlayher
Copy link
Member

More info on my concurrency woes with netlink and Go: http://lists.infradead.org/pipermail/libnl/2017-February/002293.html

@mdlayher
Copy link
Member

One more thing worth noting: I haven't seen any concurrency problems with the netlink usage in the standard library. I can't say for sure if it could be an issue or not.

Perhaps @mikioh has other problems in mind?

@bradfitz
Copy link
Contributor

Could somebody please succinctly state the problem?

@mdlayher
Copy link
Member

Would love to hear if you had any specific pain points in mind, @mikioh .

@rsc
Copy link
Contributor

rsc commented Jun 19, 2017

Having x/net/netlink seems fine. Vendoring it into stdlib seems less fine: x/net grows faster than most of the stnadard library, and I'd rather not give that growth a route into the stdlib. What specifically needs fixing in the standard library?

@rsc
Copy link
Contributor

rsc commented Oct 9, 2017

No answer to "what specifically need fixing in the standard library?"
If nothing does, then let's say we'll add a x/net/netlink but not vendor it into the standard library.

/cc @mikioh @mdlayher

@ianlancetaylor
Copy link
Contributor

Accepted for x/net/netlink (only).

@ianlancetaylor ianlancetaylor added Proposal-Accepted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 19, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Unplanned Mar 19, 2018
@ianlancetaylor ianlancetaylor changed the title proposal: x/net/netlink, vendor/golang.org/x/net/netlink: new package x/net/netlink, vendor/golang.org/x/net/netlink: new package Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

6 participants