Implement feedback from jhb.

Update with recent changes from Github PR: https://github.com/freebsd/freebsd-src/pull/1573

Looks good to me. Unfortunately I don't have any EM HW to test it.

LGTM

The one thing I think drivers would like to do is allocate additional MSI-X IRQs on the fly

Ah, that makes sense. Thanks a lot for clarification!

In D48812#1114102, @jhb wrote:

I'm worried that this might have other side effects for things like EA caps. However, I think it's also true that caching the MSI counts is probably wrong in general. It's not just VFs, in theory writing to some other register in a PF can change the value of the MSI control registers and we should just always read them.

In D48812#1113788, @jhb wrote:

I believe pci_msix_count() is returning a cached value from when the VF was first created in pci_read_cap() called from pci_fill_devinfo() called from pci_add_iov_child() called from PCI_CREATE_IOV_CHILD. If the MSI-X count is then adjusted by the driver in the PCI_IOV_ADD_VF callback, the cached value will be wrong.

I'm not sure why pci_msix_count returns incorrect number. pciconf output looks fine:

TX filter for LLDP frames is added to prevent i.e. VFs from messing up with DCB config. My colleagues suggest using an override in TX path like it's done in the Linux driver: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/ice/ice_txrx.c?h=v6.13#n2360 instead of removing this filter. I'm still digging in the docs to better understand if this is the only option when SR-IOV is in use. Could you please wait until ice(4): Add host SR-IOV support is ready?

LGTM

We're trying a different approach to make the work more transparent and enable receiving feedback earlier during development. This patch will be updated with bug fixes and missing features. There is also a pull request on Github if you'd like to see commits history: https://github.com/freebsd/freebsd-src/pull/1573

LGTM but let me ask @bartosz.sobczak_intel.com to take a look at irdma changes.

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

Prefix taskqueue name with 'if_'.

Add forgotten task init for IFLIB_INTR_IOV soft irq type.

In D46062#1076736, @kbowling wrote:

Seems reasonable to me. If there is any work that depends or interacts with these changes it would be nice to see them linked/referenced so we can look at that too but I can't foresee any problems.

Rebased on top of master.
Free taskqueue after releasing interrupts per jhb comment on Github.

In D12142#1068558, @kbowling wrote:

In D12142#1068530, @krzysztof.galazka_intel.com wrote:

In D12142#1066726, @kbowling wrote:

@krzysztof.galazka_intel.com can you rebase this on main?

Did autocompletion tricked you, or am I really the target of your message? I'm not sure if @kmacy will be happy with me messing with his patch.

Sorry I was looking at another review and got the wires crossed, for some reason I thought this was an Intel patch but I was looking at something else.

In D12142#1066726, @kbowling wrote:

@krzysztof.galazka_intel.com can you rebase this on main?

@franco_opnsense.org Output from sysctl dev.ixl.<N>.debug.filter_list and ifmcstat may shed some light on the problem. IPv4/v6 addresses can be removed. I'd like to compare mcast-macaddr list and filters configured by driver. Could you please open also a PR on FreeBSD bugzilla?

Fixed typo in device id, was 5G instead of 1G.

Update patch to apply on top of HEAD

Update patch to apply on top of HEAD

In D30733#701137, @stallamr_netapp.com wrote:

• Do not rely on information from link event

I'm still not able to reproduce this issue with any of my switches. Could you, please, modify printf in the ixl_link_event function to dump hex values of status->link_info, status->an_info, hw->phy.link_info.link_info and hw->phy.link_info.an_info, and send me the log?

Thank you Sai! Now I understand what I should be looking for.

In D30733#693043, @stallamr_netapp.com wrote:

I have connected the cable back-to-back between two servers and rebooted them both at a time. During boot, on both the nodes, immediately after the driver is attached, receives link-event and notices MEDIA_AVAILABLE + IFF_UP + UNQUALIFIED + NO_LINK_UP.

In D30733#691657, @stallamr_netapp.com wrote:

So, thinking on this, my guess is that when you reboot the machine, we would be finding an "unqualified" for a qualified cable because FW see this as link down. Also, my guess, the cable will show up as unqualified when you shut the link on link-partner.

Actually using separate sysctl_ctx was necessary in non-iflib version of the driver to allow releasing and re-allocating queues while handling EMP reset. In this version Rx queues memory is managed by the iflib with IFDI interface so using separate syssctl context may be not required. Let me double check that.

In D30733#690739, @stallamr_netapp.com wrote:

Also, in original change, https://reviews.freebsd.org/D28028, I noticed that after executing ixl_set_link(pf, false), the PHY capabilities query for an_info has I40E_AQ_QUALIFIED_MODULE unset. So, the same supported/qualified module becomes unqualified.
I think the crux of the problem is with ixl_set_link() unsetting I40E_AQ_QUALIFIED_MODULE.

Fixed with D21924 by marius

• Fix unqualified transceivers reporting

In D27344#611363, @gallatin wrote:

At least on real hardware I've used with iflib, with both ix and ixl, one needs to adjust irq moderation manually in order to get a decent amount of packets per irq. I honestly think AIM might be the best path forward for less tuning knobs, if it were the default with a wider range of min and max irq rates.

Fix PHY configuration parameters when enabling EEE

@lwhsu: Could you, please, remove the declaration in line 610 instead? The first occurrence is in a group of other LED functions.

• Add missing cases in ixl_if_media_status for 2.5 and 5G

• Replace 'if 1' with 'ifdef IXL_DEBUG' in ixl_pf.h to match with if_ixl.c

• Fix sysctl ctx handling in IAVF

Backported disable rings fix implemented by Eric Joyner in ixl-iflib version of the driver. It prevents queue hangs when interface is reconfigured while receiving heavy traffic.

• Remove redundant PF reset after EMP reset

• Don't process admin queue if its not configured

• Fix receiving broadcast frames after reset

In D21712#473977, @gallatin wrote:

I'm not sure, but I think the only way this can happen is when the link is down. Is that the caee here?

ixlv: Use old implementation of _msec_pause for reset flow to avoid panic.

Changes in Netmap support for ixl caused that ixlv cannot be loaded when built as KLD. Move ixl specific Netmap includes from ixl_txrx.c file, which is shared by ixl and ixlv, to ixl_pf_main.c which is used only by ixl.

This limitation came from our out-of-tree driver, where we had to keep backwards compatibility, but there is no reason to keep it in the in-kernel version. Thanks Vincenzo for the patch. It looks good to me but let's wait for Eric to have a look too.