@jhb Is there anything else you'd like me to change in this patch?

Remove also 2-part ID for 0x12D3 devices.

I would assume if you hold the fw semaphore you'd have exclusive access to the adapter..

@adrian Would you mind posting device id and hw->phy.type from the system you're using for testing?

In D50128#1147046, @kbowling wrote:

I would be somewhat curious why we need to serialize like this. Note that nothing else prodding the swfw in ixgbe is explicitly locked or asserted.

In D50067#1149007, @erj wrote:

In the description:

Support exclusively for x64 operating systems; no 32-bit OS support

Doesn't the in-kernel ixgbe driver still build for i386? Does this just mean no "official" support when it's running on non amd64 kernels?

Add missing newline

@erj, @kbowling: We're still awaiting for feedback from validation team, but would you mind taking a look?

@erj, @kbowling Could you please take a look?

In D50243#1145941, @kbowling wrote:

It has the approver-this is a bit of a race condition as you need me or erj or both to approve so you will always need to update it right before push.

In D50243#1145872, @kbowling wrote:

It will be helpful, especially early on, to add a fully formatted commit message to the review description so we can have a look and make sure you have the git config right.

In D50127#1144438, @erj wrote:

I'm going to assume there are some hidden #ifdefs that are getting stripped out that require the write() function to be more complex for some reason. @krzysztof.galazka_intel.com ?

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