libsa: Allow a tftp retry if a sendrecv returns ETIMEDOUT
AbandonedPublic
Actions

Authored by sbruno on Feb 12 2018, 6:25 PM.

Details

Reviewers

kevans
tsoome
imp

Summary

Allow a sendrecv retry on ETIMEDOUT

After https://svnweb.freebsd.org/base?view=revision&revision=318320 sendrecv returns ETIMEDOUT a lot more often causing TFTP of files to fail. Retrying when there is no data available seems to fix this for my testcase. However, it may be masking something else.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 15022
Build 15125: arc lint + arc unit

Event Timeline

sbruno created this revision.Feb 12 2018, 6:25 PM

Harbormaster completed remote builds in B14978: Diff 39211.Feb 12 2018, 6:25 PM

sbruno edited the summary of this revision. (Show Details)Feb 12 2018, 6:26 PM

sbruno added subscribers: j-nitrology.com, kbowling.

sbruno added inline comments.

stand/libsa/tftp.c
507	Having the actual return code was useful, so I left it in.
624	"debug" didn't exist in the tree and invoking this on every packet is pointless in debug mode.

nice catch.

This revision is now accepted and ready to land.Feb 12 2018, 6:36 PM

Seems reasonable to me

imp accepted this revision.Feb 12 2018, 8:41 PM

imp added inline comments.

stand/libsa/tftp.c
644	I'd be tempted to limit this to 10 retries or something like that...

imp added inline comments.Feb 12 2018, 9:01 PM

stand/libsa/tftp.c
644–649	On second thought, maybe just if (errno == EAGAIN) { cc = 0; errno = 0; } and then letting it fall through to the current retry logic might be a lot less disruptive.

imp added inline comments.Feb 12 2018, 9:04 PM

stand/libsa/tftp.c
649	Or even simpler: if (cc != -1 \|\| (errno != 0 && errno != EAGAIN))

Use @kevans version of the patch from private email. Add a debug statement so
I can detect the ETIMEDOUT condition, which does get hit and resumes TFTP without
failing.

This revision now requires review to proceed.Feb 13 2018, 8:45 PM

sbruno retitled this revision from libsa: Allow a tftp retry if a sendrecv returns EAGAIN to libsa: Allow a tftp retry if a sendrecv returns ETIMEDOUT.Feb 13 2018, 8:47 PM

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

Heh, sorry, I ended up creating another review with a different version of that patch: https://reviews.freebsd.org/D14350 -- it turned out there was also a case over in net.c that needed fixing, and we have some timing weirdness happening in the tftp bits to go with it.

In D14332#300876, @kevans wrote:

Heh, sorry, I ended up creating another review with a different version of that patch: https://reviews.freebsd.org/D14350 -- it turned out there was also a case over in net.c that needed fixing, and we have some timing weirdness happening in the tftp bits to go with it.

Ok, let me abandon this and switch over to D14350

imp added inline comments.Feb 13 2018, 8:57 PM