Page MenuHomeFreeBSD

e1000/em/igb: Update em to 7.6.1, update igb to 2.5.3.
ClosedPublic

Authored by erj on Jul 23 2015, 12:44 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 28, 1:21 PM
Unknown Object (File)
Thu, Oct 24, 12:59 AM
Unknown Object (File)
Wed, Oct 23, 5:46 AM
Unknown Object (File)
Tue, Oct 22, 5:55 PM
Unknown Object (File)
Tue, Oct 22, 5:55 PM
Unknown Object (File)
Tue, Oct 22, 5:55 PM
Unknown Object (File)
Tue, Oct 22, 5:54 PM
Unknown Object (File)
Tue, Oct 22, 5:54 PM

Details

Summary

Major changes:

  • Add Skylake/Sunrise Point chipset/i219 hardware support. Further to the last Skylake support diff, this one also includes support for the Lewisburg chipset (i219(3)).
  • Avoton (i354) PHY errata workaround added

And a bunch of minor fixes, as well as #defines for things that the current em/igb drivers don't implement.

Test Plan

We've compile tested (make kernel) this on r294499.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
This revision is now accepted and ready to land.Jul 25 2015, 5:57 PM
sbruno requested changes to this revision.Aug 24 2015, 4:44 PM
sbruno edited edge metadata.

it looks like we're removing max_frame size from the e1000_mac_info struct but its still being referenced all over the e1000 drivers (igb/em):

/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_igb.c:617:36: error: too few arguments to function call, expected 3, have 1
                                e1000_set_eee_i354(&adapter->hw);
                                ~~~~~~~~~~~~~~~~~~             ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/e1000_82575.h:499:1: note: 'e1000_set_eee_i354' declared here
s32 e1000_set_eee_i354(struct e1000_hw *hw, bool adv1G, bool adv100M);
^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_igb.c:619:36: error: too few arguments to function call, expected 3, have 1
                                e1000_set_eee_i350(&adapter->hw);
                                ~~~~~~~~~~~~~~~~~~             ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/e1000_82575.h:498:1: note: 'e1000_set_eee_i350' declared here
s32 e1000_set_eee_i350(struct e1000_hw *hw, bool adv1G, bool adv100M);
^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_igb.c:1406:35: error: too few arguments to function call, expected 3, have 1
                        e1000_set_eee_i354(&adapter->hw);
                        ~~~~~~~~~~~~~~~~~~             ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/e1000_82575.h:499:1: note: 'e1000_set_eee_i354' declared here
s32 e1000_set_eee_i354(struct e1000_hw *hw, bool adv1G, bool adv100M);
^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_igb.c:1408:35: error: too few arguments to function call, expected 3, have 1
                        e1000_set_eee_i350(&adapter->hw);
                        ~~~~~~~~~~~~~~~~~~             ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/e1000_82575.h:498:1: note: 'e1000_set_eee_i350' declared here
s32 e1000_set_eee_i350(struct e1000_hw *hw, bool adv1G, bool adv100M);
^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_em.c:657:18: error: no member named 'max_frame_size' in 'struct e1000_mac_info'
        adapter->hw.mac.max_frame_size =
        ~~~~~~~~~~~~~~~ ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_em.c:1194:19: error: no member named 'max_frame_size' in 'struct e1000_mac_info'
                adapter->hw.mac.max_frame_size =
                ~~~~~~~~~~~~~~~ ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_em.c:1389:22: error: no member named 'max_frame_size' in 'struct e1000_mac_info'
        if (adapter->hw.mac.max_frame_size <= 2048)
            ~~~~~~~~~~~~~~~ ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_em.c:1391:27: error: no member named 'max_frame_size' in 'struct e1000_mac_info'
        else if (adapter->hw.mac.max_frame_size <= 4096)
                 ~~~~~~~~~~~~~~~ ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_em.c:2916:23: error: no member named 'max_frame_size' in 'struct e1000_mac_info'
                if (adapter->hw.mac.max_frame_size > 4096)
                    ~~~~~~~~~~~~~~~ ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_em.c:2927:23: error: no member named 'max_frame_size' in 'struct e1000_mac_info'
                if (adapter->hw.mac.max_frame_size > 8192)
                    ~~~~~~~~~~~~~~~ ^
/home/sbruno/bsd/fbsd_head/sys/dev/e1000/if_em.c:2950:31: error: no member named 'max_frame_size' in 'struct e1000_mac_info'
            roundup2(adapter->hw.mac.max_frame_size, 1024);
                     ~~~~~~~~~~~~~~~ ^
/home/sbruno/bsd/fbsd_head/sys/sys/param.h:297:27: note: expanded from macro 'roundup2'
#define roundup2(x, y)  (((x)+((y)-1))&(~((y)-1))) /* if y is powers of two */
                           ^
4 errors generated.
--- if_igb.o ---
*** [if_igb.o] Error code 1
This revision now requires changes to proceed.Aug 24 2015, 4:44 PM
sbruno edited reviewers, added: erj; removed: sbruno.

Update incoming

sbruno edited edge metadata.

restore 'max_frame_size' for em(4) lem(4) devices

Add additional arguments to calls to e1000_set_eee_i350/i354 that I
just grabbed from linux. Is TRUE valid? I have no idea.

In D3162#70947, @sbruno wrote:

restore 'max_frame_size' for em(4) lem(4) devices

Add additional arguments to calls to e1000_set_eee_i350/i354 that I
just grabbed from linux. Is TRUE valid? I have no idea.

I had the updated e1000_set_eee_i350/i354 calls in D3165 already. If you're doing the right thing and applying these one at a time in order, then that's probably why you got the build error. :p

As for max_frame_size, I accidentally stripped that code out since it was in a #define block somewhere. I can upload a version with that stuff back in.

It also looks like you've got a mix of old and new shared code in your new diff, too. Do you want to try to fix that, or maybe try combining all of the remaining differential revisions into one new giant revision? I don't think Phabricator likes the way I tried to split them up.

sbruno edited edge metadata.

Regenerate from diff id 7205.

add additional TRUE/TRUE arguments to eee settings

put max_frame_size back for em(4).

Nope, phabricator has still lost its mind here. Standby.

Regenerate from raw diff id 7205. Still restore max_frame_size for em(4)
and add additional BOOL arguments to eee functions.

Ok, this compiles cleanly and should do the right thing.

erj edited edge metadata.
This revision is now accepted and ready to land.Sep 4 2015, 5:09 PM

This patchset works on 82574L.

Reports of various chipsets locking up and failing to attach.

  • reproduced on my Dell laptop using i218-lm
This revision is now accepted and ready to land.Sep 7 2015, 8:42 PM
sbruno edited edge metadata.

Breakage reported on igb(4) devices due to this revision. I've reverted this from head and we need to come up with a significant set of hardware regressions before we proceed.

sbruno edited edge metadata.

Bump this so its buildable again. Phabricator changes the paths
of the patch when it gets a triggered commit from svn.

erj edited reviewers, added: sbruno; removed: erj.

Update incoming...

erj retitled this revision from e1000: Shared code updates to e1000/em/igb: Update em to 7.6.1, update igb to 2.5.3..
erj updated this object.
erj edited the test plan for this revision. (Show Details)

Mostly gave up on trying to split this up into separate commits -- this patch is a combo of ~94 commits to three repositories, and I don't think it would be a good idea to waste more of the community's time trying to get this split up.

We're going to regression test this on the adapters we have, but if something is still broken on 82575/i210/i219, update this diff.

I have an i219-V (not working) and an i211-at (working) adapter on my mobo (gigabyte h170n-wifi) , so feel free to ping me if you have a working or testable patch.

And is there any chance to MFC these updates to 10.3-RELENG or into 10-STABLE after the 10.3-RELESE?

The current form should be testable -- just arc patch D3162 and see if your i219 starts working. We had it working on our internal version of the code, but that's using a different version of the shared code that doesn't include anything igb-specific.

Jeff will be testing it on the adapters we have here soon.

sys/dev/e1000/if_igb.c
5920 ↗(On Diff #12701)

This code block does not apply cleanly. Can you update it to match head when you get a chance?

sbruno added a subscriber: glebius.
sys/dev/e1000/if_igb.c
5920 ↗(On Diff #12701)

Are you sure you're using something that's newer than rS294327? LRO statistics were made 64-bit in that commit, so you can't use the 32-bit sysctl's anymore.

sys/dev/e1000/if_igb.c
5920 ↗(On Diff #12701)

Wait, nvm, you're right -- upstream got changed to U64. I'll update this revision.

Change SYSCTL_ADD_UQUADs back to SYSCTL_ADD_U64s.

In contrast to the last version from 08.01.2016 this version (25.01.2016) of the patch works fine on:

em0@pci0:0:31:6: class=0x020000 card=0x121f1734 chip=0x15b78086 rev=0x31 hdr=0x00

vendor     = 'Intel Corporation'
device     = 'Ethernet Connection (2) I219-LM'
class      = network
subclass   = ethernet

I will test the patch now.

Touch-tested (so far) on:

em: i219, 82579 and 82572
igb: 82575, i350, i354, and i210

Continuing to work through other devices.

Cool! Working fine with the following chips:

em0@pci0:0:31:6: class=0x020000 card=0xe0001458 chip=0x15b88086 rev=0x31 hdr=0x00

vendor     = 'Intel Corporation'
device     = 'Ethernet Connection (2) I219-V'
class      = network
subclass   = ethernet

igb0@pci0:4:0:0: class=0x020000 card=0xe0001458 chip=0x15398086 rev=0x03 hdr=0x00

vendor     = 'Intel Corporation'
device     = 'I211 Gigabit Network Connection'
class      = network
subclass   = ethernet

Both of the chip able to saturate the link, I tested with dd if=/dev/random bs=1M | nc IP port <-> nc -l 9999 > /dev/null
between two machine, and in both direction. Everything works as expected.

Btw, when I'm there, is from the Intel side any plan to bring in to the tree newer wireless drivers drivers too?

At least basic a/b/g/n support for the latest wireless cards?

none4@pci0:5:0:0: class=0x028000 card=0x00108086 chip=0x24f38086 rev=0x3a hdr=0x00

vendor     = 'Intel Corporation'
device     = 'Wireless 8260'
class      = network

Or when I port the code from iwlwifi (what is dual licensed: GPL + BSDL-like), then someone could review the work?

In D3162#108113, @op wrote:

Btw, when I'm there, is from the Intel side any plan to bring in to the tree newer wireless drivers drivers too?

At least basic a/b/g/n support for the latest wireless cards?

none4@pci0:5:0:0: class=0x028000 card=0x00108086 chip=0x24f38086 rev=0x3a hdr=0x00

vendor     = 'Intel Corporation'
device     = 'Wireless 8260'
class      = network

Or when I port the code from iwlwifi (what is dual licensed: GPL + BSDL-like), then someone could review the work?

The folks who work on the ethernet drivers do not have any visibility or work on the wireless drivers. :-(

sbruno edited edge metadata.

So far ... so good. I'm going to try and get some igb(4) testing done to validate things.

This revision is now accepted and ready to land.Jan 27 2016, 3:29 PM

Touch-tested (so far) on:

em: i219, 82579 and 82572
igb: 82575, i350, i354, and i210

Continuing to work through other devices.

Also tested on :
em: i217, ich10 (82576LM), and ESB2 Goshen
lem: 82546EB
igb: 82576 and 82580

Test Pass:
Basic TCP/UDP IPv4/IPv6 traffic using netperf (short term stress)
ftp tx/rx with CSUM verification

Hello guys!

Any update with this differential?

I've been running head plus this driver on a new desktop with an I219-V for a few days, and I haven't had a single problem.

marius edited edge metadata.

This looks ready to land, you may fire when ready.

I'd still need to get approval from one of my mentors, right?

@gnn hasn't said anything. Would someone else like to commit this?

gnn edited edge metadata.

Approved.

This revision was automatically updated to reflect the committed changes.

Cool! Thank you very much guys!