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

Comments

Projects
None yet
6 participants
@mikioh
Copy link
Contributor

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?

@mikioh mikioh added the Proposal label Jun 10, 2017

@gopherbot gopherbot added this to the Proposal milestone Jun 10, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jun 10, 2017

What in the the standard library needs it?

/cc @mdlayher

@mikioh

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Member

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.

What bug is that?

Which existing functions/methods don't work?

Got a sample program?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jun 10, 2017

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Member

commented Jun 10, 2017

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

This comment has been minimized.

Copy link
Member

commented Jun 10, 2017

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

@mdlayher

This comment has been minimized.

Copy link
Member

commented Jun 10, 2017

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

This comment has been minimized.

Copy link
Member

commented Jun 10, 2017

Could somebody please succinctly state the problem?

@mdlayher

This comment has been minimized.

Copy link
Member

commented Jun 13, 2017

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

@rsc

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2018

Accepted for x/net/netlink (only).

@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
You can’t perform that action at this time.