Bug 236119

Summary: Intel Ethernet 82547 device cannot transfer data
Product: Base System Reporter: Jeff Gibbons <jgibbons>
Component: kernAssignee: Kevin Bowling <kbowling>
Status: Closed FIXED    
Severity: Affects Some People CC: kbowling, net
Priority: --- Keywords: IntelNetworking, regression
Version: 12.0-STABLEFlags: kbowling: mfc-stable13+
kbowling: mfc-stable12+
kbowling: mfc-stable11-
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D29766

Description Jeff Gibbons 2019-03-01 02:59:30 UTC
When upgrading some of my machines from FreeBSD 11 to FreeBSD 12, I found that some of them could not transfer data via Ethernet after the upgrade.  ifconfig seemed to work normally, including showing "status: active", but that interface could neither send nor receive data packets, no matter what I tried.  And a few minutes after booting my systems, they would silently hang, with no response whatsoever, even on a serial console.

I eventually narrowed down the problem to my machines with Intel 82547GI chips, which look like this in "pciconf -lv":

em0@pci0:1:1:0: class=0x020000 card=0x10758086 chip=0x10758086 rev=0x00 hdr=0x00
    vendor     = 'Intel Corporation'
    device     = '82547GI Gigabit Ethernet Controller'
    class      = network
    subclass   = ethernet

By comparing the FreeBSD 11 code (which I knew worked) with the FreeBSD 12 code which had the problem, I found the solution: the 82547 is an "edge" case which slipped through the cracks when the Intel em driver was modified for FreeBSD 12.  Here is what I had to change in if_em.c to make the Intel 82547 Ethernet chips work:

root@prod:~ # svn diff /usr/src/sys/dev/e1000/if_em.c
Index: /usr/src/sys/dev/e1000/if_em.c
===================================================================
--- /usr/src/sys/dev/e1000/if_em.c      (revision 344229)
+++ /usr/src/sys/dev/e1000/if_em.c      (working copy)
@@ -31,13 +31,16 @@
 #include <sys/sbuf.h>
 #include <machine/_inttypes.h>
 
-#define em_mac_min e1000_82547
+// jagwas: #define em_mac_min e1000_82547
+#define em_mac_min e1000_82571   // jag; 25feb2019;
+                                 //   so (adapter->hw.mac.type < em_mac_min)
+                                 //   is true for 82547GI and below
 #define igb_mac_min e1000_82575
 
 /*********************************************************************
  *  Driver version:
  *********************************************************************/
-char em_driver_version[] = "7.6.1-k";
+char em_driver_version[] = "7.6.1-k+jag3";
 
 /*********************************************************************
  *  PCI Device ID Table
@@ -2476,6 +2479,24 @@
        case e1000_i211:
                pba = E1000_PBA_34K;
                break;
+                        // jag; 26feb2019 added this case section; adapted from
+                        //      FreeBSD 11 /usr/src/sys/dev/e1000/if_lem.c
+       case e1000_82547:
+       case e1000_82547_rev_2: /* 82547: Total Packet Buffer is 40K */
+               if (adapter->hw.mac.max_frame_size > 8192)
+                       pba = E1000_PBA_22K; /* 22K for Rx, 18K for Tx */
+               else
+                       pba = E1000_PBA_30K; /* 30K for Rx, 10K for Tx */
+
+               // jag; the following would be needed for plain 82547 (before GI)
+               // and would also require adding the elements set here to
+               // struct adapter in if_em.h:
+               // adapter->tx_fifo_head = 0;
+               // adapter->tx_head_addr = pba << EM_TX_HEAD_ADDR_SHIFT;
+               // adapter->tx_fifo_size =
+               //     (E1000_PBA_40K - pba) << EM_PBA_BYTES_SHIFT;
+
+               break;
        default:
                if (adapter->hw.mac.max_frame_size > 8192)
                        pba = E1000_PBA_40K; /* 40K for Rx, 24K for Tx */
root@prod:~ #

With that change, my interfaces all now work fine, and my systems don't hang (I think the hanging problem was because the pre-change if_em.c was assigning more packet buffer space than the 82547GI actually has).

I believe this change will also help some of the other 82547 models to work, but I don't have any of those chips and so cannot test them.  Those chips also seem to need some workarounds for special cases (like jumbo frames which cross buffer boundaries); those workarounds are implemented in FreeBSD 11 but not in FreeBSD 12, and I didn't implement them as part of this change because my 82547GI chip doesn't need them.  (For example, see the lem_82547_fifo_workaround() function; search for "82547" in the /usr/src/sys/dev/e1000/if_lem.c file in FreeBSD 11 to see other workarounds).

This problem seems to exist on CURRENT, too (from looking at the code; I have not tested that).
Comment 1 Kevin Bowling freebsd_committer freebsd_triage 2021-04-14 22:59:40 UTC
https://reviews.freebsd.org/D29766
Comment 2 commit-hook freebsd_committer freebsd_triage 2021-04-15 17:21:08 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=bb1b375fa7487ee5c3843121a0621ac8379c18e6

commit bb1b375fa7487ee5c3843121a0621ac8379c18e6
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-04-15 16:58:36 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-04-15 17:19:30 +0000

    e1000: fix em_mac_min and 82547 packet buffer

    The boundary differentiating "lem" vs "em" class devices was wrong
    after the iflib conversion of lem(4).

    The Packet Buffer size for 82547 class chips was not set correctly
    after the iflib conversion of lem(4).

    These changes restore functionality on an 82547 for the submitter.

    PR:             236119
    Reported by:    Jeff Gibbons <jgibbons@protogate.com>
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D29766

 sys/dev/e1000/if_em.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-05-18 07:47:25 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a7e6edc7d70fbe5d82faeda0bc6ae37550c2080f

commit a7e6edc7d70fbe5d82faeda0bc6ae37550c2080f
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-04-15 16:58:36 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-05-18 07:45:49 +0000

    e1000: fix em_mac_min and 82547 packet buffer

    The boundary differentiating "lem" vs "em" class devices was wrong
    after the iflib conversion of lem(4).

    The Packet Buffer size for 82547 class chips was not set correctly
    after the iflib conversion of lem(4).

    These changes restore functionality on an 82547 for the submitter.

    PR:             236119
    Reported by:    Jeff Gibbons <jgibbons@protogate.com>
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D29766

    (cherry picked from commit bb1b375fa7487ee5c3843121a0621ac8379c18e6)

 sys/dev/e1000/if_em.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2021-05-18 07:49:29 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=1a132077c2cb500410079f9120c3f676d15f7931

commit 1a132077c2cb500410079f9120c3f676d15f7931
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-04-15 16:58:36 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-05-18 07:48:49 +0000

    e1000: fix em_mac_min and 82547 packet buffer

    The boundary differentiating "lem" vs "em" class devices was wrong
    after the iflib conversion of lem(4).

    The Packet Buffer size for 82547 class chips was not set correctly
    after the iflib conversion of lem(4).

    These changes restore functionality on an 82547 for the submitter.

    PR:             236119
    Reported by:    Jeff Gibbons <jgibbons@protogate.com>
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D29766

    (cherry picked from commit bb1b375fa7487ee5c3843121a0621ac8379c18e6)

 sys/dev/e1000/if_em.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)
Comment 5 Kevin Bowling freebsd_committer freebsd_triage 2021-05-28 06:28:35 UTC
No MFC needed for stable11 it has a different driver.