Page MenuHomeFreeBSD

libsa: Fix IP recv timeout
ClosedPublic

Authored by kevans on Feb 13 2018, 3:32 PM.

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kevans created this revision.Feb 13 2018, 3:32 PM
kevans updated this revision to Diff 39257.Feb 13 2018, 4:16 PM
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.

kevans updated this revision to Diff 39260.Feb 13 2018, 4:19 PM
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
kevans updated this revision to Diff 39278.Feb 13 2018, 9:00 PM

Whoops, my bad. =)

sbruno accepted this revision.Feb 13 2018, 9:56 PM

BINGO

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.

tsoome accepted this revision.Feb 14 2018, 6:16 AM
Closed by commit rS329264: libsa: Fix IP recv timeout (authored by kevans, committed by ). · Explain WhyFeb 14 2018, 3:40 PM
This revision was automatically updated to reflect the committed changes.