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.

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; 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 edited edge metadata.Aug 24 2015, 4:44 PM
sbruno requested changes to this revision.

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.Aug 24 2015, 5:29 PM
sbruno commandeered this revision.

Update incoming

sbruno edited edge metadata.Aug 24 2015, 5:47 PM
sbruno updated this revision to Diff 8174.

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.

erj edited edge metadata.Aug 24 2015, 5:58 PM
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.Aug 24 2015, 10:48 PM
sbruno updated this revision to Diff 8184.

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.

sbruno updated this revision to Diff 8185.Aug 24 2015, 11:06 PM

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.Sep 4 2015, 5:09 PM
erj accepted this revision.
This revision is now accepted and ready to land.Sep 4 2015, 5:09 PM
sbruno closed this revision.Sep 4 2015, 5:18 PM
sbruno reopened this revision.Sep 7 2015, 8:42 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 planned changes to this revision.Sep 11 2015, 9:32 PM
sbruno edited edge metadata.Sep 12 2015, 10:04 PM
sbruno updated this revision to Diff 8693.
sbruno reopened this revision.Sep 19 2015, 6:25 PM

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.Jan 7 2016, 5:19 PM
sbruno updated this revision to Diff 12012.

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.Jan 8 2016, 12:20 AM
erj commandeered this revision.

Update incoming...

erj edited edge metadata.Jan 8 2016, 1:55 AM
erj added a subscriber: jeffrey.e.pieper_intel.com.
op added a subscriber: op.Jan 25 2016, 6:07 PM
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..Jan 26 2016, 12:02 AM
erj updated this object.
erj edited the test plan for this revision. (Show Details)
erj updated this revision to Diff 12701.

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.

op added a comment.Jan 26 2016, 1:09 AM

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?

erj added a comment.Jan 26 2016, 1:27 AM

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.

sbruno added inline comments.Jan 26 2016, 2:33 PM
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 edited edge metadata.Jan 26 2016, 3:58 PM
sbruno added a subscriber: glebius.
erj added inline comments.Jan 26 2016, 6:08 PM
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.

erj added inline comments.Jan 26 2016, 6:10 PM
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.

erj updated this revision to Diff 12731.EditedJan 26 2016, 6:38 PM

Change SYSCTL_ADD_UQUADs back to SYSCTL_ADD_U64s.

flo added a subscriber: flo.Jan 26 2016, 8:34 PM

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
op added a comment.Jan 26 2016, 8:40 PM

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.

op added a comment.Jan 26 2016, 9:42 PM

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.

op added a comment.Jan 26 2016, 9:45 PM

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?

sbruno edited edge metadata.Jan 27 2016, 3:28 PM
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.Jan 27 2016, 3:29 PM
sbruno accepted this revision.

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

op added a comment.Jan 31 2016, 1:06 PM

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 added a subscriber: marius.Jan 31 2016, 10:45 PM
marius edited edge metadata.Jan 31 2016, 10:51 PM
marius accepted this revision.
sbruno added a comment.Feb 1 2016, 1:56 AM

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

erj added a comment.Feb 1 2016, 6:21 PM

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

erj added a comment.Feb 3 2016, 5:21 PM

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

gnn edited edge metadata.Feb 3 2016, 7:54 PM
gnn accepted this revision.

Approved.

erj added a comment.Feb 5 2016, 5:06 PM

Firing...

This revision was automatically updated to reflect the committed changes.
op added a comment.Feb 5 2016, 5:23 PM

Cool! Thank you very much guys!