Are there any ABI or KBI concerns for stable/13?

In D31244#703721, @imp wrote:

ZFS is useful on these small memory beasts?

How is this performing versus the combined IRQ in your testing? I can do some basic smoke tests and suspect we'd want to couple it with the TX AIM patch.

In D31172#701482, @gallatin wrote:

In D31172#701479, @kbowling wrote:

In D31172#701478, @gallatin wrote:

iflib_stop() is pretty heavy-weight -- is this needed for all devices? Maybe add a flag to do this only on devices where its needed, or have the device do the disabling itself?

I'm not super familiar with this area, is there something that would cause media changes to be "common" like EEE or something? I'm trying to imagine a situation where it wouldn't be as drastic as a link flap already which would justify spending more time thinking about this.

I'm not sure what EEE is.

My point was that there are NICs where the whole thing does not need to be shut down and re-started to change media types, and the iflib_stop() is pretty heavy (2 DELAY(1000), interrupt disable, callout disable, etc.

Now that I'm thinking about this, it came up before with vlans. See IFLIB_RESTART_VLAN_CONFIG. I think we should add an IFLIB_RESTART_MEDIA_CONFIG and wrap the restart in iflib_media_change() in IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_MEDIA_CONFIG) like in iflib_vlan_register()

This will be handled completely by vendor imports

In D31172#701478, @gallatin wrote:

iflib_stop() is pretty heavy-weight -- is this needed for all devices? Maybe add a flag to do this only on devices where its needed, or have the device do the disabling itself?

@mav @mm if the upstream PR is merged this won't be necessary. The only question I have is how to MFC the change to stable/13? Do I need to have it merged back upstream first?

I've been sick and didn't make the x11 call, can this go in as planned?

LGTM (with iflib hat on)

The intent of this change looks fine to me after D30959 progresses.

In D30930#696470, @evgeniy_khramtsov.org wrote:

There is a lack of transparency here.

In D30930#696059, @evgeniy_khramtsov.org wrote:

While monitoring the FreeBSD-x11@, FreeBSD-desktop@, FreeBSD-multimedia@ and the FreeBSDDesktop gitter, I didn't see anything like "let's remove the ability to build without libglvnd".
Seems like this was discussed somewhere else, mind providing links to these discussions?

Commenting that zeising approved with the changes mentioned above on x11 call

In D30930#695949, @evgeniy_khramtsov.org wrote:

Per discussion with x11@ libglvnd will be the only GL provider supported by ports.

Context missing; is there a log of the discussion?

I realize this all might be awkward for ancient nvidia-drivers (<=390, and it is awkward now with mesa-libs as the deps too), although we still do the hard link dance on them and should keep the weight of that complexity in those ports since they don't play nicely with multi-vendor setups. The ancient multi-vendor case should be handled by the nvidia-secondary-driver port for multi-vendor or the nvidia-driver-{304, 340, 390} hardlink icky for single vendor.

Is there some variant of this that can be checked in like dropping Mk/Uses/gl.mk changes or can someone else do it to their liking?

Upstream MR https://gitlab.freedesktop.org/mesa/glu/-/merge_requests/5

Matt's suggestion is what @jbeich suggested in PR241568, I'll get a patch to freedesktop and amend this PR if it passes his review:

Add a dep on egl so we get needed mesa headers.

Sent a mail to Matt Turner (gentoo/nvidia/mesa) to get some clarification.

In D30869#694298, @jbeich wrote:

Did you test runtime on Wayland[1] or plain KMS[2]? osmesa seems to be for headless.

[1] Consumers that use GLU + GLEW may need glew-wayland (see its pkg-message)
[2] Similar to kmscube but something with GLU

https://cgit.freebsd.org/ports/commit/?id=da3162c7c9f01912ba1940e188b253f2b5c7ba77

@jbeich this should have gone in first, I was just doing the needful because portmgr had blessed the secondary driver work and it was still sitting unhandled. I am in progress on my own QA on the combination of this on top of that at the moment.

Yeah I agree with @jbeich this is a straight forward patch in the grand scheme of this software stack, so I'll merge it with maintainer timeout to fortify the secondary driver work mentioned above.

LGTM still will you push it?

Yes I apologize for my own misunderstanding, I saw this patch in BZ and was confused previously and didn't see it helping the related e1000 issues. Upon seeing it here again, it seemed like the if statement WRT ctx and arg would be incorrect if receiving an event from the vlan handler, but I see that we control the argument with our EVENTHANDLER_REGISTER here so it is consistent. @kaho_elam.kais.kyoto-u.ac.jp I believe the problems you are pursuing are a combination of filter management in e1000 and a couple issues in the em_txrx.c. I have today off so I'll see if I can make some progress, I've found some bugs in testing so nothing is ready for commit yet but here's my works in progress: D30002 and D30072

I see it clearly now, and agree. Thanks for the fix!

@stallamr_netapp.com thanks, there is a variable here in that I am running in two VMs amongst other things. I'm also diving into this code for the first time in 3 years so this is new, I'm just trying to understand the problem in the drivers and hopefully fix it or find someone who can. @gnn is getting me access to the project's network lab, and I'll use that to see if I can take a look at the problem on other types of hardware.

Thanks for catching those mistakes. Can you take a look, I'm happy with this at the moment.

Thanks for catching those mistakes. Can you take a look, I'm happy with this at the moment.

There are some optimizations in the iflib driver to decrease TX descriptor writeback txq_max_rs_deferred (I think @gallatin mentioned this earlier), I wonder if this is just a matter of the old AIM algorithm being too aggressive and needing to be tamped down a bit for this batching.

I have good news and bad news to report.

Ok I'll add that accounting and report my findings

@erj ok I agree with both of you about that change to the aim algorithm (See D30155). I just tested and by read it will be necessary but by testing it is not sufficient to fix the issue. We are still over-eager in iflib_fast_intr_rxtx with the change applied.

Some other random notes:

@stallamr_netapp.com @jeffrey.e.pieper_intel.com @erj I've looked pretty hard at the committed aim code for a couple days and haven't figured it out yet, just a bunch of false starts/dead ends. It's not really a problem with the committed aim code as far as I can tell, there seem to be a couple issues in iflib that somehow interact poorly with the driver, as it is re-arming way too frequently in iflib_fast_intr_rxtx with AIM enabled on the sender (DBG_COUNTER_INC(rx_intr_enables) captures this).

In D30094#676189, @jeffrey.e.pieper_intel.com wrote:

When this was originally implemented, Limelight (sbruno) experienced issues with this feature, as it was conflicting with iflib in some way, so it was then set to off by default.

@stallamr_netapp.com Sai, do you see the high rate of interrupts on the sender? If we can figure that out (which I think is important for you too), this patch to enable it by default will be ok.

Due to constraints with my testing environment (a xeon-d under my desk, with two bhyve VMs each with a passthru X552) I cannot provide great statistical results but I can do a basic functionality test.

This seems ok on I210 but I am having a link stability issue on I219 after I add a vlan. I will need to check out the txrx code.

Spotted one more error after the last

bugfix for em_if_vlan_filter_capable

Some improvements to SRRCTL related settings pointed out by Kaho Toshikazu <kaho@elam.kais.kyoto-u.ac.jp> in PR 230996

Enable the VME bit if IFCAP_VLAN_HWTAGGING. Some better logic around the VFE bit, disable it when promiscuous. Add some helper functions for filter stuff.