> > On Tue, Jan 16, 2018 at 02:18:14PM +0000, Frediano Ziglio wrote: > > Cork is a system interface implemented by Linux and some *BSD systems to > > tell the system that other data are expected to be written to a socket. > > This allows the system to reduce network fragmentation waiting a network > > 'waiting for network packets to be complete' I think. > > > packet to be complete. > > > > Using some replay capture and some instrumentation resulted in a > > bandwith reduction of 11% and a packet reduction of 56%. > > I would add a link to > https://lists.freedesktop.org/archives/spice-devel/2017-February/035577.html > and maybe even copy the data you put in there. > Added part of it, the TLS part was not strictly related (and surely the small TLS optimization tests does not belong here). Specifically I added: The tests was done using replay utility so results could be a bit different from real cases as: - replay goes as fast as it can, for instance packets could be merged by the kernel decreasing packet numbers and a bit byte spent (this actually make the following improves worse); - there are fewer channels (no much cursor, sound, etc). The following tests shows count packet and total bytes from server to client using a real network. I used a direct cable connection using 1gb connection and 2 laptops. cork: 537 1582240 cork: 681 1823754 cork: 524 1583287 cork: 538 1582350 no cork: 1329 1834630 no cork: 1290 1829094 no cork: 1289 1830164 no cork: 1317 1833589 no cork: 1320 1835705 > > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > --- > > server/red-stream.c | 34 +++++++++++++++++++++++++++++++++- > > 1 file changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/server/red-stream.c b/server/red-stream.c > > index 4812d8e4..4833077c 100644 > > --- a/server/red-stream.c > > +++ b/server/red-stream.c > > @@ -24,6 +24,7 @@ > > #include <unistd.h> > > #include <sys/socket.h> > > #include <fcntl.h> > > +#include <netinet/tcp.h> > > > > #include <glib.h> > > > > @@ -83,6 +84,8 @@ struct RedStreamPrivate { > > * deallocated when main_dispatcher handles the > > SPICE_CHANNEL_EVENT_DISCONNECTED > > * event, either from same thread or by call back from main thread. */ > > SpiceChannelEventInfo* info; > > + bool use_cork; > > + bool corked; > > > > ssize_t (*read)(RedStream *s, void *buf, size_t nbyte); > > ssize_t (*write)(RedStream *s, const void *buf, size_t nbyte); > > @@ -92,6 +95,15 @@ struct RedStreamPrivate { > > SpiceCoreInterfaceInternal *core; > > }; > > > > +/** > > + * Set TCP_CORK on socket > > + */ > > +/* NOTE: enabled must be int */ > > Maybe verify(sizeof(socket) == sizeof(int))? > added a SPICE_VERIFY inside the function, specifically SPICE_VERIFY(sizeof(enabled) == sizeof(int)); > > +static inline int socket_set_cork(int socket, int enabled) > > I'd drop the 'inline' > > > +{ > > + return setsockopt(socket, IPPROTO_TCP, TCP_CORK, &enabled, > > sizeof(enabled)); > > I suspect we'll need to add a configure check for this? It seems to be > called TCP_NOPUSH in OpenBSD? https://man.openbsd.org/tcp > I'll add a #ifndef TCP_CORK #define TCP_CORK TCP_NOPUSH #endif at the beginning of the file > > +} > > + > > static ssize_t stream_write_cb(RedStream *s, const void *buf, size_t size) > > { > > return write(s->socket, buf, size); > > @@ -205,11 +217,31 @@ bool red_stream_write_all(RedStream *stream, const > > void *in_buf, size_t n) > > > > bool red_stream_set_auto_flush(RedStream *s, bool enable) > > { > > - return enable; > > + if (s->priv->use_cork == !enable) { > > Might be slightly more readable if 'enable' is renamed to 'auto_flush' > Agreed, done > > Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> > Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel