Hi Catalin, Thank you for the patch, I've committed it. I've made a change so that it will start off in lowdelay mode for a pty client or any server - the initial connection involves lots of roundtrips, so delays matter there. It's dropping down to BULK if it ends up having a non-pty shell.
The only remaining question I can see is whether it needs special handling for the SSH socket in a situation without a shell, just used for tcp forwarding. Default TOS is probably better than BULK, but I'll leave it for now. I disabled the printing errors when setsockopt() failed since it can validly fail if a ipv4 socket is passed to the ipv6 call (or I assume vice versa), at least on OS X. Cheers, Matt On Mon, Dec 02, 2013 at 01:54:25AM -0800, Catalin Patulea wrote: > Signed-off-by: Catalin Patulea <[email protected]> > --- > cli-chansession.c | 1 + > dbutil.c | 29 +++++++++++++++++++++-------- > dbutil.h | 2 ++ > includes.h | 4 ++++ > svr-chansession.c | 2 ++ > 5 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/cli-chansession.c b/cli-chansession.c > index 0ee3e85..b99e073 100644 > --- a/cli-chansession.c > +++ b/cli-chansession.c > @@ -369,6 +369,7 @@ static int cli_initchansess(struct Channel *channel) { > > if (cli_opts.wantpty) { > send_chansess_pty_req(channel); > + set_sock_priority(ses.sock_out); > } > > send_chansess_shell_req(channel); > diff --git a/dbutil.c b/dbutil.c > index ce88731..4f15027 100644 > --- a/dbutil.c > +++ b/dbutil.c > @@ -177,28 +177,41 @@ void dropbear_trace2(const char* format, ...) { > } > #endif /* DEBUG_TRACE */ > > -static void set_sock_priority(int sock) { > - > +void set_sock_nodelay(int sock) { > int val; > > /* disable nagle */ > val = 1; > setsockopt(sock, IPPROTO_TCP, TCP_NODELAY, (void*)&val, sizeof(val)); > +} > + > +void set_sock_priority(int sock) { > + > + int val, rc; > > /* set the TOS bit for either ipv4 or ipv6 */ > #ifdef IPTOS_LOWDELAY > val = IPTOS_LOWDELAY; > #if defined(IPPROTO_IPV6) && defined(IPV6_TCLASS) > - setsockopt(sock, IPPROTO_IPV6, IPV6_TCLASS, (void*)&val, sizeof(val)); > + rc = setsockopt(sock, IPPROTO_IPV6, IPV6_TCLASS, (void*)&val, > sizeof(val)); > + if (rc < 0) > + dropbear_log(LOG_WARNING, "Couldn't set IPV6_TCLASS (%s)", > + strerror(errno)); > #endif > - setsockopt(sock, IPPROTO_IP, IP_TOS, (void*)&val, sizeof(val)); > + rc = setsockopt(sock, IPPROTO_IP, IP_TOS, (void*)&val, sizeof(val)); > + if (rc < 0) > + dropbear_log(LOG_WARNING, "Couldn't set IP_TOS (%s)", > + strerror(errno)); > #endif > > #ifdef SO_PRIORITY > /* linux specific, sets QoS class. > * 6 looks to be optimal for interactive traffic (see tc-prio(8) ). */ > - val = 6; > - setsockopt(sock, SOL_SOCKET, SO_PRIORITY, (void*) &val, sizeof(val)); > + val = TC_PRIO_INTERACTIVE; > + rc = setsockopt(sock, SOL_SOCKET, SO_PRIORITY, (void*) &val, > sizeof(val)); > + if (rc < 0) > + dropbear_log(LOG_WARNING, "Couldn't set SO_PRIORITY (%s)", > + strerror(errno)); > #endif > > } > @@ -290,7 +303,7 @@ int dropbear_listen(const char* address, const char* port, > } > #endif > > - set_sock_priority(sock); > + set_sock_nodelay(sock); > > if (bind(sock, res->ai_addr, res->ai_addrlen) < 0) { > err = errno; > @@ -429,7 +442,7 @@ int connect_remote(const char* remotehost, const char* > remoteport, > TRACE(("Error connecting: %s", strerror(err))) > } else { > /* Success */ > - set_sock_priority(sock); > + set_sock_nodelay(sock); > } > > freeaddrinfo(res0); > diff --git a/dbutil.h b/dbutil.h > index 7c7435c..7665845 100644 > --- a/dbutil.h > +++ b/dbutil.h > @@ -66,6 +66,8 @@ void get_socket_address(int fd, char **local_host, char > **local_port, > char **remote_host, char **remote_port, int host_lookup); > void getaddrstring(struct sockaddr_storage* addr, > char **ret_host, char **ret_port, int host_lookup); > +void set_sock_nodelay(int sock); > +void set_sock_priority(int sock); > int dropbear_listen(const char* address, const char* port, > int *socks, unsigned int sockcount, char **errstring, int > *maxfd); > int spawn_command(void(*exec_fn)(void *user_data), void *exec_data, > diff --git a/includes.h b/includes.h > index 62a8d73..bae82f5 100644 > --- a/includes.h > +++ b/includes.h > @@ -156,6 +156,10 @@ typedef unsigned int u_int32_t; > typedef u_int32_t uint32_t; > #endif /* HAVE_UINT32_T */ > > +#ifdef SO_PRIORITY > +#include <linux/pkt_sched.h> > +#endif > + > #include "fake-rfc2553.h" > > #ifndef LOG_AUTHPRIV > diff --git a/svr-chansession.c b/svr-chansession.c > index b585a9a..b912eaf 100644 > --- a/svr-chansession.c > +++ b/svr-chansession.c > @@ -580,6 +580,8 @@ static int sessionpty(struct ChanSess * chansess) { > /* Read the terminal modes */ > get_termmodes(chansess); > > + set_sock_priority(ses.sock_out); > + > TRACE(("leave sessionpty")) > return DROPBEAR_SUCCESS; > } > -- > 1.8.4.1 >
