Rebase

Address inline comments (kib)

Versioning

In D58776#1349473, @glebius wrote:

In D58776#1349436, @gallatin wrote:

LGTM, but please wait for somebody to review the netlink part, which I did not do.

Yesterday I talked to Alexander melifaro@ and he promised to take look at all reviews he is subscribed to that touch netlink this week.

Did you inadvertently revert most of the change in the latest update?

Hi, symbol versioning patch is merged. Please rebase and do necessary change.

In D58789#1349424, @ziaee wrote:

Oh man, this is a feature I have wanted!

Move storyline to [new] ATF test (markj, inquire_JohnEricson.me)

Thanks Devin!

Fix -h / -v combinations with -hold / -release (jrm)

In D58776#1349436, @gallatin wrote:

LGTM, but please wait for somebody to review the netlink part, which I did not do.

The fontconfig setup is really a fix for CURRENT that deals with broken symlink resolution at-present -- that needs to be bisected separately, but until then keeping the setting is harmless and produces the desired results even on CURRENT

Tested dbus integration with working pause/play, rewind, and fast-forward integration, and screensaver inhibit all working regardless of whether WM is bus-less or not. In a bus-less WM session, it brings its own bus run that tears down on browser exit

1. Fix dbus integration (dbus-launch -> dbus-run-session)
2. Remove BRAVE_WIDEVINE from SUB_LIST (arrowd)
3. Fix pulseaudio detection (pre-existing issue)
4. Favor angle-on-vulkan over Mesa GL (w/ pkg-message updates)
5. Remove unnecessary FONTCONFIG_FILE (rely on FONTCONFIG_PATH) (~arrowd)

Add copyright for diff addition and output "Comparing" line to stderr

Caught a few more technical debt issues in the wrapper:

LGTM, but please wait for somebody to review the netlink part, which I did not do.

LGTM, but holding back approval as I don't feel like I own this driver..

In D58784#1349422, @adrian wrote:

In D58784#1349402, @dteske wrote:

In D58784#1349393, @arrowd wrote:

In D58784#1349385, @dteske wrote:

Also, currently battling an issue where dbus-launch sticks around after browser exit. Expect a fix for that to come in the next update too.

Is it really needed, by the way? Applications should only create buses if they want to have a separate bus for their own needs. Does Brave talk to its children via dbus?

The value-add we bring with dbus-launch is inhibition of screensaver invocation when Netflix is playing.

All part of my master plan to replace my Apple TV with a Framework Laptop 12

sure but shouldn't your X environment be starting dbus-launch and setting up the XDG stuff for its window manager and child processes to run in?

In D58803#1349423, @pouria wrote:

Thank you for the hint.
At this point, not sure how to test it.

Oh man, this is a feature I have wanted!

In D58803#1349400, @markj wrote:

In D58803#1349394, @pouria wrote:

In D58803#1349372, @markj wrote:

In D58803#1349353, @pouria wrote:

In D58803#1349330, @markj wrote:

But why do this? What do we gain? These operations are rare. In such code, it is better to keep things simpler and "obviously correct." You're right that we can now probably drop that one use of the fib module lock, but now the code is harder to reason about and debug, and we do not gain anything in return (that I can see, at least).

Unfortunately, if you're using the radix[6]_lockless algorithm, which is usually true for IPv6, these operations happen frequently.
This is what I found while debugging PR297339.

You can test it yourself, set sysctl net.route.algo.debug_level=8 and try to keep a simple routing daemon for routes under 10k.
Then you see these functions are frequently run must of the time.
Since radix[6]_lockless rebuilds itself on every change.

I'm trying to cleanup fib_algo file.
It's too hard to work on it with radix_lockless.

What does "frequently" mean? Once a second? Ten thousand times a second? Does the change actually help performance in some measurable way?

Ok, I just run the dtrace below on my IXP router (unmodified CURRENT kernel).
This might answer your question:
[...]
dtrace.s:

fbt:kernel:fib_unref_nhop:entry
{
fib_calls++;
}

BTW, this is racy, see the note here https://docs.oracle.com/en/operating-systems/solaris/oracle-solaris/11.4/dtrace-guide/variables-dtrace.html

Better would be to use an aggregation: @calls = count();

tick-1s
{
printf("%Y | fib_unref_nhop calls/sec: %d\n", walltimestamp, fib_calls);
fib_calls = 0;
}

Ok, but fib_unref_nhop() doesn't acquire the module lock.

In D58784#1349402, @dteske wrote:

In D58784#1349393, @arrowd wrote:

In D58784#1349385, @dteske wrote:

Also, currently battling an issue where dbus-launch sticks around after browser exit. Expect a fix for that to come in the next update too.

Is it really needed, by the way? Applications should only create buses if they want to have a separate bus for their own needs. Does Brave talk to its children via dbus?

The value-add we bring with dbus-launch is inhibition of screensaver invocation when Netflix is playing.

All part of my master plan to replace my Apple TV with a Framework Laptop 12

In D46062#1101472, @kgalazka wrote:

@kbowling Do you want me to make any additional changes in this patch?

In D58784#1349393, @arrowd wrote:

In D58784#1349385, @dteske wrote:

Also, currently battling an issue where dbus-launch sticks around after browser exit. Expect a fix for that to come in the next update too.

Is it really needed, by the way? Applications should only create buses if they want to have a separate bus for their own needs. Does Brave talk to its children via dbus?

Yes, I've noticed it also and is addressed in one of my staged commits. I'm AFK till the end of the month, though.