Bug 164265 - [netinet] [patch] tcp_lro_rx computes wrong checksum if 'csum' variable is 0
Summary: [netinet] [patch] tcp_lro_rx computes wrong checksum if 'csum' variable is 0
Status: Closed Feedback Timeout
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Only Me
Assignee: Kubilay Kocak
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-17 23:00 UTC by Dilip Chhetri
Modified: 2019-01-01 02:44 UTC (History)
1 user (show)

See Also:


Attachments
file.diff (950 bytes, patch)
2012-01-17 23:00 UTC, Dilip Chhetri
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dilip Chhetri 2012-01-17 23:00:19 UTC
While modifying "bge" driver to use software LRO API provided by tcp_lro.c.
I saw this piece of code, 


224         /* Get the TCP checksum if we dont have it */
225         if (!csum)
226                 csum = tcp->th_sum;       <=== this is wrong

reading code further down in this same function, looks like "csum" is
supposed to contain checksum of "TCP Header (with modified th_sum) +
TCP Payload", therefore "csum = tcp->th_sum" assignment is WRONG.


Since tcp_lro_flush() has this logic,
136                 lro->m_head->m_pkthdr.csum_flags = CSUM_IP_CHECKED |
137                         CSUM_IP_VALID | CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
138                 lro->m_head->m_pkthdr.csum_data = 0xffff;
139                 lro->m_head->m_pkthdr.len = lro->len;

tcp_input never looks at what is is in 'th_sum' field and things work
fine (assuming tcp_lro_rx was called ONLY for mbuf's validated/good
checksum). However, I don't know what else may broke due to wrong
'th_sum' filled in by tcp_lro_flush().


Even FreeBSD-9 has this same piece of logic.

Fix: refer patch file

Patch attached with submission follows:
How-To-Repeat: Setup,
  [client]<---->[FreeBSD server]


FreeBSD server is using NIC with software LRO.


use netperf to send packet from client to "freebsd server" and if you
capture tcpdump on server side, you will see TCP packets with wrong
checksum (wrong value of 'th_sum'), even though the packets are not
corrupted.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2012-01-18 01:06:41 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-net

Over to maintainer(s).
Comment 2 Hiren Panchasara freebsd_committer freebsd_triage 2016-12-23 03:08:24 UTC
Dilip,

LRO code has changed/improve lately. Can you please check if this problem still persists?
Comment 3 Eitan Adler freebsd_committer freebsd_triage 2018-05-28 19:48:59 UTC
batch change:

For bugs that match the following
-  Status Is In progress 
AND
- Untouched since 2018-01-01.
AND
- Affects Base System OR Documentation

DO:

Reset to open status.


Note:
I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2019-01-01 02:43:39 UTC
Closing due to lack of feedback.

If this issue is still reproducible on currently supported FreeBSD versions (11.2, 12.0, CURRENT at present), please-re-open this issue with additional system information, details and reproduction steps