Page MenuHomeFreeBSD

krpc: add kernel side client over netlink(4)
ClosedPublic

Authored by glebius on Mon, Jan 20, 9:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Feb 13, 9:11 AM
Unknown Object (File)
Thu, Feb 13, 12:37 AM
Unknown Object (File)
Wed, Feb 12, 4:20 AM
Unknown Object (File)
Wed, Feb 12, 1:55 AM
Unknown Object (File)
Tue, Feb 11, 5:09 AM
Unknown Object (File)
Mon, Feb 10, 9:03 PM
Unknown Object (File)
Sun, Feb 9, 11:28 PM
Unknown Object (File)
Sat, Feb 8, 7:14 PM
Subscribers

Details

Summary

This shall be the official transport to connect kernel side RPC clients
to userland side RPC servers. All current kernel side clients that
hijack unix(4) sockets will be converted to it. Some implementation
details are available inside new clnt_nl.c.

The complementary RPC server over netlink(4) coming in next commit.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

  • Fix logic bug in timeout/retries calculation.
  • Better default timeout and retries.

Should be no-op in this patch stack, cause all existing users set
timeout & retries to non-default values.

I'm not very familiar with netlink nor RPC internals, so it's hard to make useful comments.

include/rpc/clnt_nl.h
2 ↗(On Diff #149701)

Missing license header.

sys/rpc/clnt.h
376
sys/rpc/clnt_nl.c
303

Why the XXX?

453

Here, I think we have to ensure that the nl_pending list is empty before proceeding. Either it must already be empty (i.e., we should add an assertion), or we need some way to wait for pending requests to leave the list.

sys/rpc/clnt_nl.h
3

Missing license header.

include/rpc/clnt_nl.h
2 ↗(On Diff #149701)

This is not final version. As you see, I found out that I need to add the exactly same file twice. This is rue for all kernel rpc includes right now and I want to fix that. I already touched base with @imp , maybe he can help me first fix that for all existing headers. After that I will regenerate this review with a single instance of clnt_nl.h, and that one will have header.

sys/rpc/clnt_nl.c
303

This is pattern copied from other RPC clients. XXX here because we are messing with already marshaled data bypassing the xdr layer, which isn't pretty.

Added Warner, asking for help on resolving the problem that some include files need a copy in the source tree. This review illustrates the problem.

sys/rpc/clnt_nl.c
453

I have a change where the TAILQ_REMOVE() is performed by the thread that does the wakeup, either the clnt_nl_reply() or clnt_nl_close(). With that change clnt_nl_call() doesn't touch the softc after being woken up, it works solely with the request structure. I will not push it until I test it. Once tested & pushed, this concern will go away. clnt_nl_close() will leave nl_pending empty.

sys/rpc/clnt_nl.c
453

My experiment showed that it complicates things wrt retries loop inside clnt_nl_call(). So I will leave the TAILQ management as it is. I will assert that clnt_nl_destroy() is called only and only on a instance that doesn't have active calls through it. Obviously any sane use of this KPI must guarantee that.

  • Make single copy of clnt_nl.h. Note: RPC includes really need a thorough

cleanup and divorce between kernel RPC and userland RPC. This file basically
set the correct trend :)

  • Add copyright notice to clnt_nl.h

No functional changes.

This revision is now accepted and ready to land.Wed, Jan 29, 10:13 PM

Remove the "nl" suffix from the Netlink family name. This is unneeded
tautology. You will see just "rpc" in the genl(1) output. Application
will lookup just "rpc" in the Netlink.

The Linux habit (and our new developing habit) us that Netlink families
are named with what they do, no "nl" prefixes or suffixes, except the
special "netlink control" family which is "nlctrl".

This revision now requires review to proceed.Fri, Jan 31, 11:18 PM
This revision is now accepted and ready to land.Sat, Feb 1, 12:07 AM
This revision was automatically updated to reflect the committed changes.