linuxulator: add sendfile fallback for non-socket FDs

[james_mclgh.net]

Summary

In Linux < 2.6.33, sendfile could only be used to send from a file to a socket. This is the behaviour implemented by the FreeBSD Linuxulator. However, since 2.6.33, sendfile can send from any FD to any other FD, which is not implemented by FreeBSD sendfile or by the Linuxulator.

I ran into this problem trying to run the game "Factorio" using the Linuxulator, which relies on sendfile to copy files on disk. Here is another thread on freebsd-emulation describing the same problem:

https://www.mail-archive.com/[e-mail]/msg11340.html

Attached is a patch which changes the Linuxulator sendfile such that it will use FreeBSD sendfile where possible, but otherwise fallback on a simple read/write loop for compatibility.

I would very much appreciate some feedback on the patch, as I have never contributed to the FreeBSD kernel before. In particular, any suggestion for a better way to choose the buffer size than the arbitrary 8 KB I have hardcoded would be welcome.

See also:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=262535
https://github.com/freebsd/freebsd-src/pull/590

Test Plan

So far only manual testing has been completed (i.e., it fixed the game I was trying to play :-D). However, I am in the process of writing test cases for:

1. sendfile from file FDs to file FDs
2. sendfile from socket FDs to file FDs
3. sendfile from file FDs to socket FDs

where 1) and 2) make use of the new fallback, and 3) remains a pass through to FreeBSD sendfile as was implemented before.

I am interested to receive feedback on the code changes in the meantime.

[dchagin]

thank you, you can test using linux test project, also, please, read style(9) and fix patch

sys/compat/linux/linux_socket.c

2070

should be linux_ prefixed

2073

style(9), I think initialization is not needed here

2091

return (false), below too

2098

LINUX_ prefix

2101

linux_ prefix

2115

drop blank line

2124

not necessary, M_WAITOK specified

2129

whitespace

2130

Extra empty line

2136

Extra empty line

2139

Extra empty line

2144

Extra empty line

2152

extar brackets

2155

Extra empty line

2156

please add whitespace between if and (

2162

Extra empty line

2165

Extra empty line

2170

Extra empty line

2173

hmm, why not use if ()? More readable code

2180

extar {

2182

extra brackets

2187

whitespace

2189

extra brackets

2237

whitespace

2248

extra {

[emaste]

commented on a couple of style(9)-related things that IMO are fine to leave the way they are if @dchagin agrees

sys/compat/linux/linux_socket.c

2070

IMO it's OK for static functions to have no linux_ prefix, but it shouldn't start with _.

2180

Note that style(9) allows { } on one-line blocks now

[dchagin]

sys/compat/linux/linux_socket.c

2070

generally agree, but we always add a linux_ prefix in Linuxulator

2098

btw, I think instead of TMPBUF_SIZE is better to use MAXBSIZE and calc tmpbufsize like:

tmpbuflen = MIN(count, MAXBSIZE);
tmpbuf = malloc(tmpbuflen, , M_TEMP, M_WAITOK);

2180

yes, it's ugly in cases like this, IMO

[mjg]

So the specific use case is file to file copy? In that case the code should employ kern_copy_file_range (or something lower, to the same extent). That's aside of generic whatever-to-whatever copying added here.

[emaste]

Also if uploading an updated version please include full context (e.g. git diff -U999999 or git show -U999999)

[imp]

A pull request was opened some time ago:
https://github.com/freebsd/freebsd-src/pull/590
And there's no activity here or there since the original feedback, so I'm closing the PR