Page MenuHomeFreeBSD

netlink: add snl(3) - simple netlink library
ClosedPublic

Authored by melifaro on Dec 18 2022, 5:35 PM.
Tags
None
Referenced Files
F102150269: D37736.id114257.diff
Fri, Nov 8, 6:08 AM
F102150198: D37736.id114509.diff
Fri, Nov 8, 6:07 AM
Unknown Object (File)
Wed, Nov 6, 2:43 PM
Unknown Object (File)
Wed, Oct 30, 12:19 PM
Unknown Object (File)
Tue, Oct 29, 11:57 AM
Unknown Object (File)
Tue, Oct 22, 6:24 PM
Unknown Object (File)
Oct 6 2024, 2:51 PM
Unknown Object (File)
Oct 3 2024, 2:02 PM

Details

Summary

Overview

With netlink landed. it is desired to have some netlink library in base, to simplify netlink interactions for both base and third-party applications.
There are the following goals:

  1. the library should be small and simple (inlined) so it can be used in the critical tools like route(8).
  2. the library should have a compatible license to be included in base
  3. there should be an easy way to push FreeBSD-specific changes to the library

The popular libraries like libnl or libml does not look positive when evaluating (2) and (3).

As a result, the snl(3) library is written to address all 3 requirements. De-facto it is a userland port of the sys/netlink/netlink_message_parser.[ch]

Diff Detail

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

Event Timeline

pauamma_gundo.com added inline comments.
share/man/man3/snl.3
98
99–100
102
109
112
120

Shouldn't the example declare ss somewhere?

Also noting that ss is a struct snl_state here but a struct snl_state * in function declarations above and below. This could confuse unwary readers.

125

(or "unique message numbers")

143

"XXX"?

144
159
161

Perhaps a reminder or a clarification of which values <type> can take?

165
167
170
171
173
174
176
180
182
185
189
200
202

Same question about listing values of <type> as above.

231

Unless my rusty C reading skills mislead me.

241–254

Is there a rule for the order in which declarations and statements can or should appear? Here I see a declaration, statements, another declaration, then more statements.

264

Should this be continue or break?

268

Same question.

275

Empty section?

287
This revision now requires changes to proceed.Dec 18 2022, 7:08 PM
lwhsu added inline comments.
share/man/man3/snl.3
125

"an" ?

share/man/man3/snl.3
125

"an" ?

https://www.merriam-webster.com/dictionary/unique says "a" in quoted examples, eg "a unique identifier".

melifaro marked 20 inline comments as done.
melifaro edited the summary of this revision. (Show Details)
  • Address comments.
  • Update snl(3) library.
share/man/man3/snl.3
120

I omitted it for brevity.
The full working example exists at the end of the man page.
Switched to ss instead of &ss, does it look better now?

241–254

IIRC relaxed style(9) rules some time ago -

Variables may be initialized where
    declared especially when they are constant for the rest of the scope.
    Declarations may be placed before executable lines at the start of any
    block.
264

Can be both. Maybe break is more logical here.

268

It depends on the app failure model - is partial failure okay (e.g. getting "some" result) or not.

Also: thank you for the review & feedback! It helps greatly.

Fixable on commit if there are no other changes.

share/man/man3/snl.3
120

Fair enough re brevity. And yes, it looks better to my no-longer-trained eye.

279

Missed it the first time.

This revision is now accepted and ready to land.Dec 20 2022, 12:34 AM
sys/netlink/netlink_snl.h
359

Isn't all the RT* things specific to netlink_route ? should snl impose the inclusion of netlink_route.h?

I manage to convert my code to use snl.h its ok for me.

Quick remarks, I plan to add snl_genl.h for genl specific case, would that be ok for you?
Doesn't the routing specific part needs to be moved to a snl_route.h?

melifaro marked 2 inline comments as done.

Add a bit more accessors.
Move NETLINK_ROUTE specific helpers to a separate file.
Update the manpage.

This revision now requires review to proceed.Dec 23 2022, 2:56 PM
In D37736#859376, @bapt wrote:

I manage to convert my code to use snl.h its ok for me.

Awesome!

Quick remarks, I plan to add snl_genl.h for genl specific case, would that be ok for you?

Of course.

Doesn't the routing specific part needs to be moved to a snl_route.h?

Yep, that makes sense. Moved to netlink_snl_route.h.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 23 2022, 3:03 PM
This revision was automatically updated to reflect the committed changes.