To netlink or not to netlink, that is the question

To netlink or not to netlink, that is the question

[Jason A. Donenfeld]

Hey folks,

A few months ago I switched away from using netlink in wireguard,
preferring instead to use ioctl. I had come up against limitations in
rtnetlink, and ioctl presented a straightforward hard to screw-up
alternative. The very simple API is documented here:
https://git.zx2c4.com/WireGuard/tree/src/uapi.h

This works well, and I'm reluctant to change it, but as I do more
complicated things, and as kernel submission time looms nearer, I'm
kept up at night by the notion that maybe I ought to give netlink
another chance. But how?

For each wireguard interface, there are three types of structures for
userspace to configure. There is one wgdevice for each interface. Each
wgdevice has a variable amount (up to 2^16) of wgpeers. Each wgpeer
has a variable amount (up to 2^16) of wgipmasks. I'd like an interface
to get and set all of these at once, atomically.

Presently, with the ioctl, I just have a simple get ioctl and a simple
set ioctl. The set one passes a user space pointer, which is read
incrementally in kernel space. The get one will first return how much
userspace should allocate, and then when called again will write
incrementally into a provided userspace buffer up to a passed-in
maximum number of bytes. Very basic, I'm quite happy.

When I had tried to do this priorly with netlink, I did it by defining
changelink and fill_info in rtnl_link_ops. For changelink, I iterated
through the netlink objects, and for fill_info, I filled in the skb
with netlink objects. This was a bit more complex but basically
worked. Except netlink skbs have a maximum size and are buffered,
which means things broke entirely when trying to read or write logs of
wgpeers or lots of wgipmasks. So, the meager interfaces afforded to me
by rtnl_link_ops are insufficient. Doing anything beyond this, either
by registering new rtnetlink messages, or by using generic netlink,
seemed overwhelmingly complex and undesirable.

So I'm wondering -- is there a good way to be doing this with netlink?
Or am I right to stay with ioctl?

Thanks,
Jason

Re: To netlink or not to netlink, that is the question

[Stephen Hemminger]

On Thu, 12 Jan 2017 14:01:07 +0100
"Jason A. Donenfeld" <Jason@zx2c4.com> wrote:

> Hey folks,
> 
> A few months ago I switched away from using netlink in wireguard,
> preferring instead to use ioctl. I had come up against limitations in
> rtnetlink, and ioctl presented a straightforward hard to screw-up
> alternative. The very simple API is documented here:
> https://git.zx2c4.com/WireGuard/tree/src/uapi.h
> 
> This works well, and I'm reluctant to change it, but as I do more
> complicated things, and as kernel submission time looms nearer, I'm
> kept up at night by the notion that maybe I ought to give netlink
> another chance. But how?
> 
> For each wireguard interface, there are three types of structures for
> userspace to configure. There is one wgdevice for each interface. Each
> wgdevice has a variable amount (up to 2^16) of wgpeers. Each wgpeer
> has a variable amount (up to 2^16) of wgipmasks. I'd like an interface
> to get and set all of these at once, atomically.
> 
> Presently, with the ioctl, I just have a simple get ioctl and a simple
> set ioctl. The set one passes a user space pointer, which is read
> incrementally in kernel space. The get one will first return how much
> userspace should allocate, and then when called again will write
> incrementally into a provided userspace buffer up to a passed-in
> maximum number of bytes. Very basic, I'm quite happy.
> 
> When I had tried to do this priorly with netlink, I did it by defining
> changelink and fill_info in rtnl_link_ops. For changelink, I iterated
> through the netlink objects, and for fill_info, I filled in the skb
> with netlink objects. This was a bit more complex but basically
> worked. Except netlink skbs have a maximum size and are buffered,
> which means things broke entirely when trying to read or write logs of
> wgpeers or lots of wgipmasks. So, the meager interfaces afforded to me
> by rtnl_link_ops are insufficient. Doing anything beyond this, either
> by registering new rtnetlink messages, or by using generic netlink,
> seemed overwhelmingly complex and undesirable.
> 
> So I'm wondering -- is there a good way to be doing this with netlink?
> Or am I right to stay with ioctl?
> 
> Thanks,
> Jason

It is up to you but I doubt that code with new private ioctl's will be
accepted upstream. If you want full review then post for inclusion upstream.
If you just want to maintain it is a private fork, go ahead and do what
you want and suffer the consequences.

Re: To netlink or not to netlink, that is the question

[Jason A. Donenfeld]

On Thu, Jan 12, 2017 at 6:14 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> It is up to you but I doubt that code with new private ioctl's will be
> accepted upstream. If you want full review then post for inclusion upstream.
> If you just want to maintain it is a private fork, go ahead and do what
> you want and suffer the consequences.

Obviously I'm going for upstream conclusion and not willing to "suffer
the consequences", hence my email in the first place. Given that you
seem most interested in netlink, might you have any constructive
suggestions on how netlink might be used with very large atomic
messages?

Re: To netlink or not to netlink, that is the question

[Dan Williams]

On Thu, 2017-01-12 at 19:28 +0100, Jason A. Donenfeld wrote:
> On Thu, Jan 12, 2017 at 6:14 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > It is up to you but I doubt that code with new private ioctl's will
> > be
> > accepted upstream. If you want full review then post for inclusion
> > upstream.
> > If you just want to maintain it is a private fork, go ahead and do
> > what
> > you want and suffer the consequences.
> 
> Obviously I'm going for upstream conclusion and not willing to
> "suffer
> the consequences", hence my email in the first place. Given that you
> seem most interested in netlink, might you have any constructive
> suggestions on how netlink might be used with very large atomic
> messages?

Typically kernel code doesn't pass very large atomic messages back and
forth.  From the description you mailed, I would guess a better fit API
would be to have specific netlink mechanisms for add/remove wgpeers,
and a specific mechanism for add/remove ipmasks for each peer.  And
don't forget netlink events to indicate when peers and ipmasks have
been added/removed, so userspace is aware of that.

Besides, perhaps multiple programs may wish to manipulate a wgdevice,
which could include adding/removing peers or ipmasks from separate
userspace processes.  That's not really possible with a huge buffer
that includes all the information, as a process would have to read the
buffer, update it, and send it back, which would then potentially
clobber whatever another process did between the read/write.

In short, if the API doesn't quite fit, then perhaps the API could be
broken up into smaller, more discrete operations that better fit the
netlink API.

Just some thoughts...

Dan

Re: To netlink or not to netlink, that is the question

[Jason A. Donenfeld]

Hi Dan,

Thanks for your response. I'd thought about this, at least for
adding/removing wgpeers/wgipmasks and for configuring wgdevices. This
would fit into multiple smaller messages indeed.

But what about fetching the list of all existing peers and ipmasks
atomically? It seems like with multiple calls, if I'm using some kind
of pagination, things could change in the process. That's why using
one big buffer was most appealing... Any ideas about this?

Jason

Re: To netlink or not to netlink, that is the question

[David Miller]

From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Thu, 12 Jan 2017 20:02:14 +0100

> But what about fetching the list of all existing peers and ipmasks
> atomically? It seems like with multiple calls, if I'm using some kind
> of pagination, things could change in the process. That's why using
> one big buffer was most appealing... Any ideas about this?

This is a fact of life, dumps are always chopped into suitable
numbers of responses as necessary.  We do this for IPV4 routes,
network interfaces, etc. and it all works out just fine.

The thing you should be asking yourself is, if something as heavily
used and fundamental as IPV4 can handle this, probably your scenerio
can be handled just fine as well.

Re: To netlink or not to netlink, that is the question

[Jason A. Donenfeld]

On Thu, Jan 12, 2017 at 8:11 PM, David Miller <davem@davemloft.net> wrote:
> This is a fact of life, dumps are always chopped into suitable
> numbers of responses as necessary.  We do this for IPV4 routes,
> network interfaces, etc. and it all works out just fine.
>
> The thing you should be asking yourself is, if something as heavily
> used and fundamental as IPV4 can handle this, probably your scenerio
> can be handled just fine as well.

Okay, fair enough. I'll suck it up, then, and just use netlink in this
way. David - you concur with Stephen that ioctl is really not okay and
I should absolutely do netlink?

Re: To netlink or not to netlink, that is the question

[David Miller]

From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Thu, 12 Jan 2017 21:07:43 +0100

> David - you concur with Stephen that ioctl is really not okay and I
> should absolutely do netlink?

Yes.

Re: To netlink or not to netlink, that is the question

[Johannes Berg]

On Thu, 2017-01-12 at 20:02 +0100, Jason A. Donenfeld wrote:
> Hi Dan,
> 
> Thanks for your response. I'd thought about this, at least for
> adding/removing wgpeers/wgipmasks and for configuring wgdevices. This
> would fit into multiple smaller messages indeed.
> 
> But what about fetching the list of all existing peers and ipmasks
> atomically? It seems like with multiple calls, if I'm using some kind
> of pagination, things could change in the process. That's why using
> one big buffer was most appealing... Any ideas about this?

In addition to what others have said - netlink typically includes (and
has helpers to do so) a generation counter that's updated whenever this
list changes, and included in each message, so if userspace really
cares (often not) it can retry the dump until the system was idle
enough to get a consistent snapshot.

It also looks to me like your existing API isn't even compat-safe due
to u64 alignment (e.g. in wgpeer), proving once again that ioctl is a
bad idea.

johannes

Re: To netlink or not to netlink, that is the question

[Nicolas Dichtel]

Le 13/01/2017 à 08:17, Johannes Berg a écrit :
[snip]
> In addition to what others have said - netlink typically includes (and
> has helpers to do so) a generation counter that's updated whenever this
> list changes, and included in each message, so if userspace really
> cares (often not) it can retry the dump until the system was idle
> enough to get a consistent snapshot.
Look at NLM_F_DUMP_INTR to get details.

> 
> It also looks to me like your existing API isn't even compat-safe due
> to u64 alignment (e.g. in wgpeer), proving once again that ioctl is a
> bad idea.
+1


Regards,
Nicolas