Page MenuHomeFreeBSD

Support AF_NETLINK in ktrstruct.
ClosedPublic

Authored by artemhevorhian_gmail.com on Nov 3 2024, 1:24 PM.
Tags
None
Referenced Files
F106272701: D47411.id145917.diff
Sat, Dec 28, 8:11 AM
Unknown Object (File)
Thu, Dec 12, 6:15 PM
Unknown Object (File)
Sun, Dec 8, 11:07 PM
Unknown Object (File)
Nov 25 2024, 12:55 PM
Unknown Object (File)
Nov 21 2024, 6:55 PM
Unknown Object (File)
Nov 21 2024, 9:44 AM
Unknown Object (File)
Nov 21 2024, 7:55 AM
Unknown Object (File)
Nov 19 2024, 10:26 PM
Subscribers

Details

Summary

Right now, sockaddr_nl parameters are not displayed in kdump output,
however they are present in the structure in ktrace.out file when
ktrace is run.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 60356
Build 57240: arc lint + arc unit

Event Timeline

sys/sys/ktrace.h
39 ↗(On Diff #145917)

This include should be in kdump.c, not here. In general we try to avoid including headers within headers, when possible.

Update the revision according the comments of markj@

Looks ok

usr.bin/kdump/kdump.c
51

This should be sorted after the sys/ includes. The style(9) man page has some examples.

In this case, netlink/netlink.h should come after netinet/in.h below.

1928

I think it's not useful to print the length and family, those will always be the same. We don't print them for other sockaddr types.

Something like netlink[pid=%u, groups=0x%x] makes more sense to me as a format string.

artemhevorhian_gmail.com marked an inline comment as done.

Sort the includes according to style(9) and modify

the netlink printf() string.

This revision is now accepted and ready to land.Nov 3 2024, 2:35 PM

Generally LGTM

usr.bin/kdump/kdump.c
1928

It depends on what are the goals of printing.
In my mental model, we start to trace app when something is wrong with the application or we just want to dump the interactions with the kernel. While 99.99% of the cases we'll indeed see the AF_NETLINK and sizeof(struct sockaddr_nl), in the rest cases it may be filled in differently. The thing is that we may be looking at the kdump output to find exactly that..
I definitely agree that in most cases we don't care - so one of the options would be to skip those fields _unless_ they have unexpected values.
It's also with noting that kdump format is not exactly space-optimised and we're still spending a whole line for printing STRU anyway - so maybe being verbose isn't too bad.

usr.bin/kdump/kdump.c
1928

Well, this case won't match at all if nl_family != AF_NETLINK. Probably the "unknown address family" message below should include the family.

If the length is wrong, we may indeed care, but probably not: classic unix socket interfaces take the sockaddr length as a separate parameter and typically overwrite the user-supplied length. Indeed, Linux sockaddrs don't have a length field at all. So it is rarely interesting, and it's not printed for the other sockaddr types handled above, which is why I suggested to omit it. I have no objection to including it though.

This revision now requires review to proceed.Nov 4 2024, 9:36 PM
This revision was not accepted when it landed; it landed in state Needs Review.Nov 19 2024, 10:21 PM
This revision was automatically updated to reflect the committed changes.