Page MenuHomeFreeBSD

check so_error earlier in sosend_generic
AbandonedPublic

Authored by jason_eggnet.com on Oct 3 2017, 11:07 PM.

Details

Reviewers
gnn
glebius
Group Reviewers
transport
Summary

A simple ECONNRESET is reported as EPIPE because sosend_generic
is checking so_error after the EPIPE check.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 11866
Build 12202: arc lint + arc unit

Event Timeline

Before patch:

root@:~jason # ./packetdrill/gtests/net/packetdrill/packetdrill --tolerance_usecs=1000000 -v writerst.pkt
inbound injected packet:  0.102943 S 0:0(0) win 65535 <mss 1460,sackOK,eol,eol>
outbound sniffed packet:  0.103734 S. 3099687258:3099687258(0) ack 1 win 65535 <mss 1460,sackOK,eol,eol>
inbound injected packet:  0.155732 . 1:1(0) ack 3099687259 win 65535
inbound injected packet:  0.258126 R 1:1(0) win 65535
writerst.pkt:10: runtime error in write call: Expected errno 54 (Connection reset by peer) but got 32 (Broken pipe)

After patch:

root@:~jason # ./packetdrill/gtests/net/packetdrill/packetdrill --tolerance_usecs=1000000 -v writerst.pkt
inbound injected packet:  0.101978 S 0:0(0) win 65535 <mss 1460,sackOK,eol,eol>
outbound sniffed packet:  0.102505 S. 908754645:908754645(0) ack 1 win 65535 <mss 1460,sackOK,eol,eol>
inbound injected packet:  0.153256 . 1:1(0) ack 908754646 win 65535
inbound injected packet:  0.254186 R 1:1(0) win 65535

packetdrill test script:

# cat writerst.pkt
 0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0.000 bind(3, ..., ...) = 0
+0.000 listen(3, 3) = 0
+0.100 < S  0:0(0) win 65535 <mss 1460,sackOK,eol,eol>
+0.000 > S. 0:0(0) ack 1 win 65535 <mss 1460,sackOK,eol,eol>
+0.050 < . 1:1(0) ack 1 win 65535
+0.050 accept(3, ..., ...) = 4
+0.050 < R 1:1(0) ack 1 win 65535
+0.050 write(4, ..., 10) = -1 ECONNRESET (Connection reset by peer)
+0.050 close(4) = 0
jch added a subscriber: jch.Oct 4 2017, 7:23 AM
sbruno added a subscriber: sbruno.Oct 9 2017, 6:34 PM

This seems pretty reasonable to me. Any objections? Can I get a transport@ review accept on this?

gnn accepted this revision.Oct 9 2017, 11:02 PM
This revision is now accepted and ready to land.Oct 9 2017, 11:02 PM

Gleb wants us to hold off on this for the moment, with the first step of making sendfile() work like current send() / write() paths.

We'll probably be abandoning this review in favor of keeping the current behavior and ensuring sendfile behaves the same as send and write.

https://reviews.freebsd.org/D12633

It looks like I was incorrect in thinking that write and sendfile were supposed to return ECONNRESET in general.

SUS specifies ECONNRESET for write(2), but BSD systems never did that. SUS also specifies EPIPE, and doesn't provide any clue when ECONNRESET is preferred over EPIPE. They got very similar descriptions.

[ECONNRESET]

A write was attempted on a socket that is not connected.

[EPIPE]

A write was attempted on a socket that is shut down for writing, or is no longer connected. In the latter case, if the socket is of type SOCK_STREAM, a SIGPIPE signal shall also be sent to the thread.

Linux manual page says "Linux may return EPIPE instead of ENOTCONN. " What's funny - in the BUGS section :)

Taking all these into account, I believe many applications rely on the current behavior: the writing syscalls return EPIPE and doesn't clear error status, the reading syscalls return and clear ECONNRESET. So this behaviour shall not be changed.

glebius requested changes to this revision.Oct 10 2017, 8:10 PM
This revision now requires changes to proceed.Oct 10 2017, 8:10 PM
jason_eggnet.com abandoned this revision.Oct 10 2017, 10:09 PM

Sounds good, I'll abandon this.