Page MenuHomeFreeBSD

libsa: Fix IP recv timeout
ClosedPublic

Authored by kevans on Feb 13 2018, 3:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 28, 3:54 AM
Unknown Object (File)
Sat, Dec 21, 4:43 AM
Unknown Object (File)
Sat, Dec 21, 2:40 AM
Unknown Object (File)
Thu, Dec 19, 11:43 AM
Unknown Object (File)
Tue, Dec 10, 6:06 PM
Unknown Object (File)
Wed, Dec 4, 7:38 PM
Unknown Object (File)
Nov 29 2024, 9:20 PM
Unknown Object (File)
Nov 29 2024, 5:01 AM
Subscribers
None

Details

Summary

readip() doesn't, at the moment, properly indicate to callers that it's timed out. One can tell that it's timed out if errno == EAGAIN when it returns, but this is not ideal. Restructure it a little bit to explicitly set errno to ETIMEDOUT if we've exhausted tleft.

I found two places that care about where it timed out or not: sendrecv in net.c and sendrecv_tftp. Both are structured to pass smaller timeout values to readip while tracking a larger timeout. Neither of them were able to do this properly with readip not indicating ETIMEDOUT, so fix it.

While here, straighten out the time (t/t1) usage in sendrecv_tftp.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 15012

Event Timeline

kevans edited the summary of this revision. (Show Details)

Found an unrelated bug in the tftp section; if a send fails, we're supposed to wait before retrying. However, 't1' doesn't get reset, so if it times out a second time (and still before we reached MAXTMO) there won't be any further waiting before resending. because getsecs() - t1 by definition already passed after the first timeout period.

sbruno requested changes to this revision.Feb 13 2018, 8:51 PM

Ugh, phab seems to have not take the diffs from the root of the src tree. I'll munge a bit, but can you regenerate?

This revision now requires changes to proceed.Feb 13 2018, 8:53 PM
This revision is now accepted and ready to land.Feb 13 2018, 9:56 PM

For what it's worth, I would almost like to do something more like this: https://people.freebsd.org/~kevans/stand-ip.diff

It's a lot more invasive, but it gets rid sendrecv_tftp and consolidates MINTMO/MAXTMO timeout logic into one spot that everything honors.

For what it's worth, I would almost like to do something more like this: https://people.freebsd.org/~kevans/stand-ip.diff

It's a lot more invasive, but it gets rid sendrecv_tftp and consolidates MINTMO/MAXTMO timeout logic into one spot that everything honors.

take it as second round.

This revision was automatically updated to reflect the committed changes.