Bug 257642 - e1000(4): Does not check for bad Rx UDP checksum
Summary: e1000(4): Does not check for bad Rx UDP checksum
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 13.0-RELEASE
Hardware: Any Any
: --- Affects Many People
Assignee: Kevin Bowling
URL: https://reviews.freebsd.org/D31449
Keywords: IntelNetworking
Depends on:
Blocks:
 
Reported: 2021-08-05 18:51 UTC by Nick Reilly
Modified: 2021-08-16 01:03 UTC (History)
3 users (show)

See Also:
kbowling: mfc-stable13+
kbowling: mfc-stable12+
kbowling: mfc-stable11-

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Reilly 2021-08-05 18:51:14 UTC 
The e1000 driver doesn't check for checksum errors on Rx UDP checksum, it marks all packets as having a good checksum.

Suggested fix:

diff --git a/sys/dev/e1000/em_txrx.c b/sys/dev/e1000/em_txrx.c
index 458de96a7b9..1328e12daf4 100644
--- a/sys/dev/e1000/em_txrx.c
+++ b/sys/dev/e1000/em_txrx.c
@@ -797,7 +797,8 @@ em_receive_checksum(uint32_t status, if_rxd_info_t ri)
                ri->iri_csum_flags |= (CSUM_DATA_VALID | CSUM_PSEUDO_HDR);
                ri->iri_csum_data = htons(0xffff);
        }
-       if (status & E1000_RXD_STAT_UDPCS) {
+       if ((status & E1000_RXD_STAT_UDPCS | E1000_RXDEXT_STATERR_TCPE)) ==
+               E1000_RXD_STAT_UDPCS) {
                ri->iri_csum_flags |= (CSUM_DATA_VALID | CSUM_PSEUDO_HDR);
                ri->iri_csum_data = htons(0xffff);
        }
Comment 1 Kevin Bowling freebsd_committer freebsd_triage 2021-08-07 04:36:58 UTC 
(In reply to Nick Reilly from comment #0)
Nick, can you give this a try https://reviews.freebsd.org/D31449?
Comment 2 Nick Reilly 2021-08-09 15:43:40 UTC 
Gave the diff from https://reviews.freebsd.org/D31449 a try and it seems to be working.
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-08-09 21:33:06 UTC 
A commit in branch main references this bug:

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

commit 015075f383489fcbedbe8aae7c1c64a3d55ca75e
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-08-09 21:29:31 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-08-09 21:29:31 +0000

    e1000: Fix lem/em UDP rx csum offload

    Rebase on igb code and unify lem/em implementations.

    PR:             257642
    Reported by:    Nick Reilly <nreilly@blackberry.com>
    Reviewed by:    karels, emaste
    Tested by:      Nick Reilly <nreilly@blackberry.com>
    Approved by:    grehan
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D31449

 sys/dev/e1000/em_txrx.c | 67 ++++++++++++++++---------------------------------
 1 file changed, 21 insertions(+), 46 deletions(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2021-08-16 00:57:32 UTC 
A commit in branch stable/13 references this bug:

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

commit 0d0726a769a176be699b2ceeb5a417ffe88b0e64
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-08-09 21:29:31 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-08-16 00:56:45 +0000

    e1000: Fix lem/em UDP rx csum offload

    Rebase on igb code and unify lem/em implementations.

    PR:             257642
    Reported by:    Nick Reilly <nreilly@blackberry.com>
    Reviewed by:    karels, emaste
    Tested by:      Nick Reilly <nreilly@blackberry.com>
    Approved by:    grehan
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D31449

    (cherry picked from commit 015075f383489fcbedbe8aae7c1c64a3d55ca75e)

 sys/dev/e1000/em_txrx.c | 67 ++++++++++++++++---------------------------------
 1 file changed, 21 insertions(+), 46 deletions(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-08-16 00:59:34 UTC 
A commit in branch stable/12 references this bug:

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

commit ec56aaca91b243be590fdb3b8f0e6ee3b0b2e9f0
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2021-08-09 21:29:31 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-08-16 00:58:11 +0000

    e1000: Fix lem/em UDP rx csum offload

    Rebase on igb code and unify lem/em implementations.

    PR:             257642
    Reported by:    Nick Reilly <nreilly@blackberry.com>
    Reviewed by:    karels, emaste
    Tested by:      Nick Reilly <nreilly@blackberry.com>
    Approved by:    grehan
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D31449

    (cherry picked from commit 015075f383489fcbedbe8aae7c1c64a3d55ca75e)

 sys/dev/e1000/em_txrx.c | 67 ++++++++++++++++---------------------------------
 1 file changed, 21 insertions(+), 46 deletions(-)
Comment 6 Kevin Bowling freebsd_committer freebsd_triage 2021-08-16 01:03:12 UTC 
Thanks for the report Nick!