Skip to content

Disable lwIP checksum-on-copy #4311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 18, 2017
Merged

Disable lwIP checksum-on-copy #4311

merged 1 commit into from
May 18, 2017

Conversation

kjbracey
Copy link
Contributor

@kjbracey kjbracey commented May 12, 2017

Current version of lwIP has a bug in its checksum-on-copy code - see

#4140 and https://savannah.nongnu.org/bugs/?50914

Pending a fix from lwIP, set LWIP_CHECKSUM_ON_COPY to 0 to work around.
Will impact performance.

@mikaleppanen , @geky - if there's not a patch for the bug in the next few days, I think we should do this.

Current version of lwIP has a bug in its checksum-on-copy code - see

      ARMmbed#4140
  and https://savannah.nongnu.org/bugs/?50914

Pending a fix from lwIP, set LWIP_CHECKSUM_ON_COPY to 0 to work around.
Will impact performance.
Copy link
Contributor

@geky geky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me 👍

This seems to just be an optimization, so we can drop it until a fix is available.

@0xc0170
Copy link
Contributor

0xc0170 commented May 15, 2017

/morph test-nightly

@mbed-bot
Copy link

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 222

All builds and test passed!

@sg-
Copy link
Contributor

sg- commented May 18, 2017

@kjbracey-arm given we merge this, will you be tracking an upstream fix that we will later pickup?

@sg- sg- merged commit 6a96481 into ARMmbed:master May 18, 2017
@kjbracey
Copy link
Contributor Author

There's a link to the lwIP bug tracker left in the code - we're not planning to investigate ourselves, and I'm not sure it's a significant enough issue to warrant cherry-picking a fix but we'll try to remember to recheck whether it's fixed next time we go for a general update.

I think it's arguable we should have the option disabled anyway, at least by default. There's a moderate amount of additional code and small RAM cost involved in doing the checksum-on-copy, and more mbed platforms are worried about space than CPU time spent on IP.

It's not that much extra memory, but every little helps. Does that makes sense?

@kjbracey
Copy link
Contributor Author

This one's probably backportable.

Some people will never see it, but some TCP connections will just die - the stack will persistently resend a packet with an incorrect checksum, as in the original report.

Looks like it may be the usual zero checksum confusion, which suggests 1 in 65536 packets may suffer from it. If you're lucky, there will be more data and hence different checksum in the retransmit. If not, connection over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants