Closed
Bug 132208
Opened 22 years ago
Closed 22 years ago
Implement PR_SendFile() with sendfile() and TCP_CORK
Categories
(NSPR :: NSPR, enhancement, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
4.2
People
(Reporter: wtc, Assigned: wtc)
References
Details
Attachments
(2 files, 3 obsolete files)
Linux 2.2 kernel has the sendfile() system call and the TCP_CORK socket option. We should implement PR_SendFile() with these.
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → 4.2
Assignee | ||
Comment 1•22 years ago
|
||
Comment 2•22 years ago
|
||
The patch has what appears to be an unrelated change to the way file_nbytes_to_send is calculated in the HPUX variant. The condition for setting TCP_CORK should instead be if (sfd->hlen != 0 || sfd->tlen != 0) if (count != -1 && (flags & PR_TRANSMITFILE_CLOSE_SOCKET)) then I don't think you have to call setsockopt() to clear TCP_CORK. If the kernel is implemented well, this could cause the FIN to be piggybacked on the last data packet. I don't know what the kernel's actual behavior is.
Assignee | ||
Comment 3•22 years ago
|
||
Thank you for the code review, John. You are right, there is an unrelated HP-UX change in the previous patch. I will put it in a separate patch. I considered using if (sfd->hlen != 0 || sfd->tlen != 0) as the condition for setting TCP_CORK. I didn't do that because both the sendfile(2) and tcp(7) man pages only mention the use of TCP_CORK with headers but not trailers: SENDFILE(2) If you plan to use sendfile for sending files to a TCP socket, but need to send some header data in front of the file contents, please see the TCP_CORK option in tcp(7) to minimize the number of packets and to tune performance. TCP(7) TCP_CORK If enabled don't send out partial frames. All queued partial frames are sent when the option is cleared again. This is useful for prepending head¡ ers before calling sendfile(2), or for throughput optimization. This option cannot be combined with TCP_NODELAY. Any thoughts? In this new patch, I use if (sfd->hlen != 0 || sfd->tlen != 0) as you suggested. As for your last suggestion: if (count != -1 && (flags & PR_TRANSMITFILE_CLOSE_SOCKET)) then I don't think you have to call setsockopt() to clear TCP_CORK. I did that originally but it led to a hang in our test on Red Hat Linux 6.2. The client thread in our test was hung in PR_Recv, and the server thread had exited. I have not seen this hang on Red Hat Linux 7.2. So it is possible that the glibc or kernel in Red Hat Linux 7.2 does this correctly. But I will leave my code as is for NSPR 4.2 so that it works on both Red Hat Linux 6.2 and 7.2 and we can investigate the exact conditions under which we can skip the "uncork" setsockopt call.
Attachment #75137 -
Attachment is obsolete: true
Assignee | ||
Comment 4•22 years ago
|
||
Assignee | ||
Comment 5•22 years ago
|
||
This patch is the full implementation of PR_SendFile using sendfile and TCP_CORK. The algorithm to decide whether TCP_CORK should be set is as follows. We attempt to set TCP_CORK if there are header or trailer data to be sent with the file. However, if we detect that TCP_NODELAY has been set on the socket, we do not set TCP_CORK. If we set TCP_CORK, we remember to turn TCP_CORK off before we return.
Attachment #75226 -
Attachment is obsolete: true
Assignee | ||
Comment 6•22 years ago
|
||
This is a new version of the patch. I verified with experiments that on Red Hat Linux 6.0, 6.2, 7.1, and 7.2, accepted sockets inherit the TCP_NODELAY socket option of the listening socket. So in pt_Accept I copy the 'tcp_nodelay' field of the listening socket to the newly accepted socket. I also added a warning log message if setsockopt(TCP_CORK) fails with the EINVAL error. This will help us find out if the socket has the TCP_NODELAY socket option set by a non-NSPR function.
Attachment #75698 -
Attachment is obsolete: true
Assignee | ||
Comment 7•22 years ago
|
||
Patch checked into the tip of NSPR.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•