What svn rev are you using?

Is this with the driver in kernel or as a module? And with INVARIANTS or
without?

Jeffrey:

Is this the same or different as reported in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=211771 with the unmodified driver? The wording caught my eye as something we had already discussed a bit.

In D5213#172435, @kmacy wrote:

What svn rev are you using?

9e6f38428d632aa7be0c61c681627f0085399d3e convert ixgbe to iflib
d7d4326b414d043a46827d9d0ea449b119ed358c Increase timeouts so tests have more chances to succeed on MIPS64EB in QEMU.
ca21fba3dea2485ec44a5d32e3a82e068b78a87c Fix ipfw table lookup handler to return entry value, but not its index.
23dcf48f60666d39ff92b68e8c578ff41fbda16e Add FFS pager, which uses buffer cache read operation to validate pages. See the comments for more detailed description of the algorithm.
5579c4169c49e58a6a1e122d3c5b56637942d452 Add big-endian uzip file system and choose right file system to proceed tests with.
b4afcf142fd2513f22ed1c98088864b268d47693 hyperv/vmbus: Expose channel management taskqueue for driver to use.
38255c19df588d1ff0548be101755a7a756fb3a1 RPI3 is retired in preference to GENERIC-UP. Use that instead.
fcbea8e537b499aaf197be36cdfb34986232eec6 Use MODULES_EXTRA rather than MODULES_OVERRIDE for dtb.
8c4ba2c67a6e233659bf79b2fb35ae3c2f60d578 [net80211] Initial full-offload scan support.
c430547a157a39f3559c61424a7cae2d97360478 Fix typo in comment.

In D5213#172438, @kmacy wrote:

Is this with the driver in kernel or as a module? And with INVARIANTS or
without?

Loaded as a module without INVARIANTS.

In D5213#172449, @sbruno wrote:

Jeffrey:

Is this the same or different as reported in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=211771 with the unmodified driver? The wording caught my eye as something we had already discussed a bit.

FreeBSD u0807 12.0-CURRENT FreeBSD 12.0-CURRENT #0 9e6f384(arcpatch-D5213): Wed Oct 19 13:03:43 UTC 2016 root@s254:/usr/obj/diskless/os/FreeBSD/12-CURRENT_2016-08-29-D5213/usr/src/sys/VALIDATION amd64
[root@u0807 /]# kldload if_ix
[root@u0807 /]# ifconfig ix0 190.2.8.7 up
[root@u0807 /]# dmesg
ix0: <Intel(R) PRO/10GbE PCI-Express Network Driver, Version - 3.1.13-k> port 0xe020-0xe03f mem 0xec000000-0xec03ffff,0xec080000-0xec083fff irq 58 at device 0.0 numa-domain 1 on pci11
ix0: Using MSIX interrupts with 9 vectors
ix0: Ethernet address: 00:8c:fa:18:a1:28
ix0: PCI Express Bus: Speed 5.0GT/s Width x8
ix0: netmap queues/slots: TX 8/2048, RX 8/2048
ix1: <Intel(R) PRO/10GbE PCI-Express Network Driver, Version - 3.1.13-k> port 0xe000-0xe01f mem 0xec040000-0xec07ffff,0xec084000-0xec087fff irq 61 at device 0.1 numa-domain 1 on pci11
ix1: Using MSIX interrupts with 9 vectors
ix1: Ethernet address: 00:8c:fa:18:a1:29
ix1: PCI Express Bus: Speed 5.0GT/s Width x8
ix1: netmap queues/slots: TX 8/2048, RX 8/2048
ix0: link state changed to UP

Another data point:

Neither an X520-DA2 adapter (same device id) nor X552 show this behavior.

In D5213#172450, @jeffrey.e.pieper_intel.com wrote:

In D5213#172435, @kmacy wrote:

What svn rev are you using?

9e6f38428d632aa7be0c61c681627f0085399d3e convert ixgbe to iflib
d7d4326b414d043a46827d9d0ea449b119ed358c Increase timeouts so tests have more chances to succeed on MIPS64EB in QEMU.
ca21fba3dea2485ec44a5d32e3a82e068b78a87c Fix ipfw table lookup handler to return entry value, but not its index.
23dcf48f60666d39ff92b68e8c578ff41fbda16e Add FFS pager, which uses buffer cache read operation to validate pages. See the comments for more detailed description of the algorithm.
5579c4169c49e58a6a1e122d3c5b56637942d452 Add big-endian uzip file system and choose right file system to proceed tests with.
b4afcf142fd2513f22ed1c98088864b268d47693 hyperv/vmbus: Expose channel management taskqueue for driver to use.
38255c19df588d1ff0548be101755a7a756fb3a1 RPI3 is retired in preference to GENERIC-UP. Use that instead.
fcbea8e537b499aaf197be36cdfb34986232eec6 Use MODULES_EXTRA rather than MODULES_OVERRIDE for dtb.
8c4ba2c67a6e233659bf79b2fb35ae3c2f60d578 [net80211] Initial full-offload scan support.
c430547a157a39f3559c61424a7cae2d97360478 Fix typo in comment.

I assume this means you did a git pull (asked for svn rev, but this is git log information) to update to the "latest" head prior to these tests?

Correct.

Please try running worth INVARIANTS on the offending hardware.

@jeffrey.e.pieper_intel.com Please run with INVARIANTS. No one else has the offending hardware.

@jeffrey.e.pieper_intel.com and please post dmesg for the system it crashes on.

In D5213#172500, @kmacy wrote:

@jeffrey.e.pieper_intel.com and please post dmesg for the system it crashes on.

I read it the first time, thank you.

@jeffrey.e.pieper_intel.com The panic you've hit means that the admin task was not assigned a taskqueue. In all likelihood we're hitting an initialization edge case on the system itself, not that particular adapter.

In D5213#172502, @kmacy wrote:

@jeffrey.e.pieper_intel.com The panic you've hit means that the admin task was not assigned a taskqueue. In all likelihood we're hitting an initialization edge case on the system itself, not that particular adapter.

That makes sense, as the device id is the same as a standard X520-DA2 adapter.

@jeffrey.e.pieper_intel.com are all your test systems multi-socket or just this one?

In D5213#172512, @kmacy wrote:

@jeffrey.e.pieper_intel.com are all your test systems multi-socket or just this one?

For the most part. Everything I've been using for testing iflib is MP, with the exception of The BDX-DE (X552). The systems in question are actually mainly used as clients (I have many). They are Dell C6220, all with X520 mezz cards. The only UP systems that I have (besides desktop/mobile systems) are Supermicro Microcloud servers w/ 8 nodes per chassis, which are also used as clients.

That makes sense, as the device id is the same as a standard X520-DA2 adapter.

ix_iflib_dmesg.txt30 KBDownload

In this case irq_alloc isn't getting called for the admin task. I'll take a closer look at ixgbe and fix the logic in iflib to handle this case.

In D5213#172663, @sbruno wrote:

Preinitialize admin task and add some INVARIANT debugging in case this
doesn't capture why Intel lab testing fails so miserably.

Jeffrey:

Give this a spin and lets see if we can make forward progress with it.

That seemed to do the trick. Ran some basic functionality on X520 (both adapter and mezz card) as well a couple BDX-DE skus and X540. I'm currently running netperf stress tests. I will do some more comprehensive testing (driver load/unload stress, mtu tests etc) after they've ran 24 hours.

Because this is a structural change of some magnitude, does anyone have a handle on how the man page should be updated?

I am confused by the question about the manual page. That can be put under a separate review if you like.

In D5213#173227, @gnn wrote:

I am confused by the question about the manual page. That can be put under a separate review if you like.

The changes to the driver are going to need an update to the man page for ix(4). That is my only point. I haven't done it, but I am noting that it needs to be done.

@sbruno what is this gated on now?

Also, I mentioned this earlier, but PLEASE print the driver version in dmesg at attach as well as in sysctl. It isn't very optimal to have no clue as to what driver version is being used.

hrm ... I had the same behavior on an em(4) system when executing an "ifconfig em0 down up" here. Are you sure this is an "iflib" problem?

In D5213#174653, @sbruno wrote:

hrm ... I had the same behavior on an em(4) system when executing an "ifconfig em0 down up" here. Are you sure this is an "iflib" problem?

Considering this doesn't reproduce on ix legacy, I'm pretty sure. The mtu test completes with no errors anyways.

In D5213#174654, @jeffrey.e.pieper_intel.com wrote:

In D5213#174653, @sbruno wrote:

hrm ... I had the same behavior on an em(4) system when executing an "ifconfig em0 down up" here. Are you sure this is an "iflib" problem?

Considering this doesn't reproduce on ix legacy, I'm pretty sure. The mtu test completes with no errors anyways.

ix_iflib usually fails within a few minutes. I've been running the script on ix legacy for 10 minutes with no failures.

ix_iflib usually fails within a few minutes. I've been running the script on ix legacy for 10 minutes with no failures.

I was able to make this happen this morning it I do *exactly* what Jeffrey is doing. There's some kind of race here that I think is related to handling of reinit being to "graceful".

Jeffrey, can you modify your iflib install with this? I'll update the review in a bit after I'm done with my testing, but so far, this seems to be "doing the same" if not "right" thing:

Index: sys/net/iflib.c
===================================================================
--- sys/net/iflib.c     (revision 308582)
+++ sys/net/iflib.c     (working copy)
@@ -3228,7 +3228,7 @@
                                        err = IFDI_PROMISC_SET(ctx, if_getflags(ifp));
                                }
                        } else
-                               reinit = 1;
+                               iflib_if_init(ctx);
                } else if (if_getdrvflags(ifp) & IFF_DRV_RUNNING) {
                        iflib_stop(ctx);
                }

The following 1 line change fixes this issue:

https://github.com/freebsd/freebsd/commit/bcebdde733b4104f437bc935647645901b909aab

This is now dependent on D8558

On a router/firewall use case: About -13% performance drop (4 cores Intel Xeon L5630 with Intel 82599EB NIC)

x r313730, paquets-per-second
+ r313730-D5213, paquets-per-second
+--------------------------------------------------------------------------+
|      +                                                        x          |
|     ++               x                   ++                 x xx         |
|                                    |__________________A_______M_________||
||_____M_____________A___________________|                                 |
+--------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   5       2439877     2923715.5     2907858.5     2816269.4     210659.57
+   5     2251528.5       2679501       2259267     2424025.7     229988.26
Difference at 95.0% confidence
        -392244 +/- 321639
        -13.9278% +/- 10.7244%
        (Student's t, pooled s = 220536)

Flamgraph before applying this patch:

Flamegraph after applying this patch:

Shouldn't D10293 supersede this anyways?

In D5213#213143, @jeffrey.e.pieper_intel.com wrote:

Shouldn't D10293 supersede this anyways?

Oh yeah, since I "own" this review. I'll abandon it in favor of 10293