[Spice-devel] [PATCH spice-server 2/4] Do not set TCP_NODELAY flag twice

[Frediano Ziglio]

> 
> On Wed, Feb 15, 2017 at 11:31:59AM +0000, Frediano Ziglio wrote:
> > TCP_NODELAY flag is set by default for all connection inside
> > reds.c so there's no need to set again for the single
> > client channel.
> 
> Would have been nice to keep "Do not set not blocking flag twice"
> together with this patch, and push them at the same time..
> 
> > 
> > Note that there are still some call to setsockopt to set this
> > option but in this case the flag can reset the flag.
> 
> 'some calls'
> the flag can reset the flag?
> 

Yes, confusing, what about:

"Note that there are still some call to setsockopt to change this
option."

shorter and (I hope) clearer.

> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
> 
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/inputs-channel.c | 15 ---------------
> >  server/spicevmc.c       | 19 -------------------
> >  2 files changed, 34 deletions(-)
> > 
> > diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> > index 3672d7e..223f46f 100644
> > --- a/server/inputs-channel.c
> > +++ b/server/inputs-channel.c
> > @@ -19,11 +19,7 @@
> >  #include <config.h>
> >  #endif
> >  
> > -#include <netinet/in.h> // IPPROTO_TCP
> > -#include <netinet/tcp.h> // TCP_NODELAY
> > -#include <fcntl.h>
> >  #include <stddef.h> // NULL
> > -#include <errno.h>
> >  #include <stdbool.h>
> >  #include <spice/macros.h>
> >  #include <spice/vd_agent.h>
> > @@ -490,17 +486,6 @@ static void inputs_pipe_add_init(RedChannelClient
> > *rcc)
> >  
> >  static int inputs_channel_config_socket(RedChannelClient *rcc)
> >  {
> > -    int delay_val = 1;
> > -    RedsStream *stream = red_channel_client_get_stream(rcc);
> > -
> > -    if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY,
> > -            &delay_val, sizeof(delay_val)) == -1) {
> > -        if (errno != ENOTSUP && errno != ENOPROTOOPT) {
> > -            spice_printerr("setsockopt failed, %s", strerror(errno));
> > -            return FALSE;
> > -        }
> > -    }
> > -
> >      return TRUE;
> >  }
> >  
> > diff --git a/server/spicevmc.c b/server/spicevmc.c
> > index 4c9f442..4b46e54 100644
> > --- a/server/spicevmc.c
> > +++ b/server/spicevmc.c
> > @@ -23,10 +23,7 @@
> >  #endif
> >  
> >  #include <assert.h>
> > -#include <errno.h>
> >  #include <string.h>
> > -#include <netinet/in.h> // IPPROTO_TCP
> > -#include <netinet/tcp.h> // TCP_NODELAY
> >  #ifdef USE_LZ4
> >  #include <lz4.h>
> >  #endif
> > @@ -445,22 +442,6 @@ static void
> > spicevmc_char_dev_remove_client(RedCharDevice *self,
> >  
> >  static int spicevmc_red_channel_client_config_socket(RedChannelClient
> >  *rcc)
> >  {
> > -    int delay_val = 1;
> > -    RedsStream *stream = red_channel_client_get_stream(rcc);
> > -    RedChannel *channel = red_channel_client_get_channel(rcc);
> > -    uint32_t type;
> > -
> > -    g_object_get(channel, "channel-type", &type, NULL);
> > -    if (type == SPICE_CHANNEL_USBREDIR) {
> > -        if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY,
> > -                &delay_val, sizeof(delay_val)) != 0) {
> > -            if (errno != ENOTSUP && errno != ENOPROTOOPT) {
> > -                spice_printerr("setsockopt failed, %s", strerror(errno));
> > -                return FALSE;
> > -            }
> > -        }
> > -    }
> > -
> >      return TRUE;
> >  }
> >  

Frediano