[alsa-devel] [PATCH] Aseqnet, no nagle and dual stack
Hi there
Not an ALSA source patch, but a patch for an ALSA related util. Aseqnet sends ALSA sound_seq MIDI over TCP/IP. The patch below disables nagle, enables quickack and makes aseqnet dual-stack.
--- aseqnet.c.bak 2012-01-25 10:43:38.000000000 +0100 +++ aseqnet.c 2017-08-26 14:17:58.261868853 +0200 @@ -3,6 +3,8 @@ * ver.0.1 * * Copyright (C) 1999-2000 Takashi Iwai + * Modified by Rob van der Putten, Leiden, Holland, + * rob at sput dot nl. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -15,18 +17,21 @@ * */
+#include <alsa/asoundlib.h> +#include <arpa/inet.h> +#include <assert.h> +#include <ctype.h> +#include <getopt.h> +#include <locale.h> +#include <netdb.h> +#include <netinet/in.h> +#include <netinet/tcp.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> -#include <ctype.h> #include <string.h> -#include <netinet/in.h> #include <sys/socket.h> -#include <netdb.h> -#include <locale.h> -#include <alsa/asoundlib.h> -#include <getopt.h> -#include <signal.h> -#include <assert.h> +#include <sys/types.h> #include "aconfig.h" #include "gettext.h"
@@ -327,17 +332,24 @@ */ static void init_server(int port) { + /* + * RvdP, changed to support IPv6 + * Dual stack only! + */ int i; int curstate = 1; - struct sockaddr_in addr; + int ipv6only = 0; + int nodelay = 1; + int quickack = 1; + struct sockaddr_in6 addr;
memset(&addr, 0, sizeof(addr));
- addr.sin_family = AF_INET; - addr.sin_addr.s_addr = INADDR_ANY; - addr.sin_port = htons(port); + addr.sin6_family = AF_INET6; + inet_pton(AF_INET6, "::", &(addr.sin6_addr)); + addr.sin6_port = htons(port);
- sockfd = socket(AF_INET, SOCK_STREAM, 0); + sockfd = socket(PF_INET6, SOCK_STREAM, 0); if (sockfd < 0) { perror("create socket"); exit(1); @@ -345,7 +357,19 @@ setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &curstate, sizeof(curstate)); /* the return value is ignored.. */
- if (bind(sockfd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { + /* Force dual stack */ + setsockopt(sockfd, IPPROTO_IPV6, IPV6_V6ONLY, &ipv6only, sizeof(ipv6only)); + /* the return value is ignored.. */ + + /* Nagle and quickack */ + if ((setsockopt(sockfd, IPPROTO_TCP, TCP_NODELAY, &nodelay, sizeof(nodelay))) < 0) { + perror("Error setsockopt tcp_nodelay"); + } + if ((setsockopt(sockfd, IPPROTO_TCP, TCP_QUICKACK, &quickack, sizeof(quickack))) < 0) { + perror("Error setsockopt tcp_quickack"); + } + + if (bind(sockfd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { perror("can't bind"); exit(1); } @@ -394,30 +418,58 @@ */ static void init_client(char *server, int port) { - struct sockaddr_in addr; - struct hostent *host; - int curstate = 1; - int fd; + /* + * RvdP, changed to support IPv6 + */
- if ((fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0){ - perror("create socket"); + struct addrinfo hints; + struct addrinfo *result, *rp; + int curstate = 1; + int nodelay = 1; + int quickack = 1; + int fd, s; + char portstr[8]; + + memset(&hints, 0, sizeof(struct addrinfo)); + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_STREAM; + /* hints.ai_protocol = IPPROTO_TCP; */ + hints.ai_flags = 0; + + memset(portstr, 0, 8); + snprintf(portstr, 6, "%d", port); + + s = getaddrinfo(server, portstr, &hints, &result); + if (s != 0) { + fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(s)); + exit(1); + } + for (rp = result; rp != NULL; rp = rp->ai_next) { + fd = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol); + if (fd == -1) + continue; + if (connect(fd, rp->ai_addr, rp->ai_addrlen) != -1) + break; + close(fd); + } + if (rp == NULL) { + fprintf(stderr, "Could not connect\n"); exit(1); } + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &curstate, sizeof(curstate)) < 0) { perror("setsockopt"); exit(1); } - if ((host = gethostbyname(server)) == NULL){ - fprintf(stderr, _("can't get address %s\n"), server); - exit(1); + + /* RvdP, nagle and quickack */ + if ((setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &nodelay, sizeof(nodelay))) < 0) { + perror("Error setsockopt tcp_nodelay"); } - addr.sin_port = htons(port); - addr.sin_family = AF_INET; - memcpy(&addr.sin_addr, host->h_addr, host->h_length); - if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { - perror("connect"); - exit(1); + if ((setsockopt(fd, IPPROTO_TCP, TCP_QUICKACK, &quickack, sizeof(quickack))) < 0) { + perror("Error setsockopt tcp_quickack"); } + if (verbose) fprintf(stderr, _("ok.. connected\n")); netfd[0] = fd;
Regards, Rob
On Tue, 23 Apr 2019 20:25:58 +0200, Rob van der Putten wrote: On Tue, 23 Apr 2019 20:25:58 +0200, Rob van der Putten wrote:
Hi there
Not an ALSA source patch, but a patch for an ALSA related util. Aseqnet sends ALSA sound_seq MIDI over TCP/IP. The patch below disables nagle, enables quickack and makes aseqnet dual-stack.
Thanks for the patch. Could you repost with a proper patch change log and your Signed-off-by line so that one can apply to git repo?
About the changes:
--- aseqnet.c.bak 2012-01-25 10:43:38.000000000 +0100 +++ aseqnet.c 2017-08-26 14:17:58.261868853 +0200 @@ -3,6 +3,8 @@
- ver.0.1
- Copyright (C) 1999-2000 Takashi Iwai
- Modified by Rob van der Putten, Leiden, Holland,
- rob at sput dot nl.
We don't need to add each change in the source like that as all tracked in git.
@@ -15,18 +17,21 @@
*/
+#include <alsa/asoundlib.h> +#include <arpa/inet.h> +#include <assert.h> +#include <ctype.h> +#include <getopt.h> +#include <locale.h> +#include <netdb.h> +#include <netinet/in.h> +#include <netinet/tcp.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> -#include <ctype.h> #include <string.h> -#include <netinet/in.h> #include <sys/socket.h> -#include <netdb.h> -#include <locale.h> -#include <alsa/asoundlib.h> -#include <getopt.h> -#include <signal.h> -#include <assert.h> +#include <sys/types.h> #include "aconfig.h" #include "gettext.h"
Why these large rearrangement of include files? If it must be inevitably done, please describe the reason in the changelog, too.
@@ -327,17 +332,24 @@ */ static void init_server(int port) {
- /*
RvdP, changed to support IPv6
Dual stack only!- */
Wouldn't it potentially break things? IMO, it's better to keep the old behavior (ipv4-only), at least, with an option. Also, drop your initials in the commit, it's rather superfluous.
thanks,
Takashi
Hi there
On 28/04/2019 10:00, Takashi Iwai wrote:
On Tue, 23 Apr 2019 20:25:58 +0200, Rob van der Putten wrote: On Tue, 23 Apr 2019 20:25:58 +0200, Rob van der Putten wrote:
Hi there
Not an ALSA source patch, but a patch for an ALSA related util. Aseqnet sends ALSA sound_seq MIDI over TCP/IP. The patch below disables nagle, enables quickack and makes aseqnet dual-stack.
Thanks for the patch. Could you repost with a proper patch change log and your Signed-off-by line so that one can apply to git repo?
Added - Disabled Nagle's algorithm - Enbled quickack - IPv6 support
Signed-off-by: Rob van der Putten rob@sput.nl
About the changes:
--- aseqnet.c.bak 2012-01-25 10:43:38.000000000 +0100 +++ aseqnet.c 2017-08-26 14:17:58.261868853 +0200 @@ -3,6 +3,8 @@
- ver.0.1
- Copyright (C) 1999-2000 Takashi Iwai
- Modified by Rob van der Putten, Leiden, Holland,
- rob at sput dot nl.
We don't need to add each change in the source like that as all tracked in git.
@@ -15,18 +17,21 @@
*/
+#include <alsa/asoundlib.h> +#include <arpa/inet.h> +#include <assert.h> +#include <ctype.h> +#include <getopt.h> +#include <locale.h> +#include <netdb.h> +#include <netinet/in.h> +#include <netinet/tcp.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> -#include <ctype.h> #include <string.h> -#include <netinet/in.h> #include <sys/socket.h> -#include <netdb.h> -#include <locale.h> -#include <alsa/asoundlib.h> -#include <getopt.h> -#include <signal.h> -#include <assert.h> +#include <sys/types.h> #include "aconfig.h" #include "gettext.h"
Why these large rearrangement of include files? If it must be inevitably done, please describe the reason in the changelog, too.
I just put them into alphabetical order. I changed that back.
@@ -327,17 +332,24 @@ */ static void init_server(int port) {
- /*
RvdP, changed to support IPv6
Dual stack only!- */
Wouldn't it potentially break things? IMO, it's better to keep the old behavior (ipv4-only), at least, with an option.
Server: The behaviour is determined by /proc/sys/net/ipv6/bindv6only If this is one, the listening socket will be IPv6 only instead of dual stack. A socket option is used to set IPV6_V6ONLY to zero, which forces the listening socket dual stack.
Client: The 'for (rp = result; rp != NULL; rp = rp->ai_next) {' loop keeps trying to connect to the remote host: If IPv6 fails, it uses IPv4 instead.
Both: I added a -4 / --ipv4 option. This not in the previous patch. It also introduces the string '-4, --ipv4 : IPv4 only' for which there no translations. And a new bit in the man page, with an added man page patch.
Also, drop your initials in the commit, it's rather superfluous.
A new aseqnet patch. Note: This patches the original, not the previous patch. --- a/seq/aseqnet/aseqnet.c +++ b/seq/aseqnet/aseqnet.c @@ -29,6 +29,9 @@ #include <assert.h> #include "aconfig.h" #include "gettext.h" +#include <arpa/inet.h> +#include <netinet/tcp.h> +#include <sys/types.h>
/* * prototypes @@ -78,6 +81,8 @@ static int verbose = 0; static int info = 0;
+static int ipv4only = 0; +
/* * main routine @@ -90,6 +95,7 @@ {"help", 0, NULL, 'h'}, {"verbose", 0, NULL, 'v'}, {"info", 0, NULL, 'i'}, + {"ipv4", 0, NULL, '4'}, {NULL, 0, NULL, 0}, };
@@ -104,7 +110,7 @@ textdomain(PACKAGE); #endif
- while ((c = getopt_long(argc, argv, "p:s:d:vi", long_option, NULL)) != -1) { + while ((c = getopt_long(argc, argv, "p:s:d:vi4", long_option, NULL)) != -1) { switch (c) { case 'p': if (isdigit(*optarg)) @@ -124,6 +130,9 @@ case 'i': info++; break; + case '4': + ipv4only++; + break; default: usage(); exit(1); @@ -172,6 +181,7 @@ printf(_(" -d,--dest addr : write to given addr (client:port)\n")); printf(_(" -v, --verbose : print verbose messages\n")); printf(_(" -i, --info : print certain received events\n")); + printf(_(" -4, --ipv4 : IPv4 only\n")); }
@@ -329,25 +339,64 @@ { int i; int curstate = 1; - struct sockaddr_in addr; - - memset(&addr, 0, sizeof(addr)); - - addr.sin_family = AF_INET; - addr.sin_addr.s_addr = INADDR_ANY; - addr.sin_port = htons(port); + int ipv6only = 0; + int nodelay = 1; + int quickack = 1; + struct sockaddr_in addr4; + struct sockaddr_in6 addr; + + if (ipv4only == 0) { + memset(&addr, 0, sizeof(addr)); + + addr.sin6_family = AF_INET6; + inet_pton(AF_INET6, "::", &(addr.sin6_addr)); + addr.sin6_port = htons(port); + + sockfd = socket(PF_INET6, SOCK_STREAM, 0); + if (sockfd < 0) { + perror("create socket"); + exit(1); + } + } else { + memset(&addr4, 0, sizeof(addr4));
- sockfd = socket(AF_INET, SOCK_STREAM, 0); - if (sockfd < 0) { - perror("create socket"); - exit(1); + addr4.sin_family = AF_INET; + addr4.sin_addr.s_addr = INADDR_ANY; + addr4.sin_port = htons(port); + + sockfd = socket(AF_INET, SOCK_STREAM, 0); + if (sockfd < 0) { + perror("create socket"); + exit(1); + } } setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &curstate, sizeof(curstate)); /* the return value is ignored.. */
- if (bind(sockfd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { - perror("can't bind"); - exit(1); + if (ipv4only == 0) { + /* Force dual stack */ + setsockopt(sockfd, IPPROTO_IPV6, IPV6_V6ONLY, &ipv6only, sizeof(ipv6only)); + /* the return value is ignored.. */ + } + + /* Nagle and quickack */ + if ((setsockopt(sockfd, IPPROTO_TCP, TCP_NODELAY, &nodelay, sizeof(nodelay))) < 0) { + perror("Error setsockopt tcp_nodelay"); + } + if ((setsockopt(sockfd, IPPROTO_TCP, TCP_QUICKACK, &quickack, sizeof(quickack))) < 0) { + perror("Error setsockopt tcp_quickack"); + } + + if (ipv4only == 0) { + if (bind(sockfd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { + perror("can't bind"); + exit(1); + } + } else { + if (bind(sockfd, (struct sockaddr *) &addr4, sizeof(addr4)) < 0) { + perror("can't bind"); + exit(1); + } }
if (listen(sockfd, 5) < 0) { @@ -365,7 +414,8 @@ */ static void start_connection(void) { - struct sockaddr_in addr; + struct sockaddr_in6 addr; + struct sockaddr_in addr4; int i; socklen_t addr_len;
@@ -377,9 +427,15 @@ fprintf(stderr, _("too many connections!\n")); exit(1); } - memset(&addr, 0, sizeof(addr)); - addr_len = sizeof(addr); - netfd[i] = accept(sockfd, (struct sockaddr *)&addr, &addr_len); + if (ipv4only == 0) { + memset(&addr, 0, sizeof(addr)); + addr_len = sizeof(addr); + netfd[i] = accept(sockfd, (struct sockaddr *)&addr, &addr_len); + } else { + memset(&addr4, 0, sizeof(addr4)); + addr_len = sizeof(addr4); + netfd[i] = accept(sockfd, (struct sockaddr *)&addr4, &addr_len); + } if (netfd[i] < 0) { perror("accept"); exit(1); @@ -394,30 +450,57 @@ */ static void init_client(char *server, int port) { - struct sockaddr_in addr; - struct hostent *host; + struct addrinfo hints; + struct addrinfo *result, *rp; int curstate = 1; - int fd; + int nodelay = 1; + int quickack = 1; + int fd, s; + char portstr[8]; + struct hostent *host;
- if ((fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0){ - perror("create socket"); + memset(&hints, 0, sizeof(struct addrinfo)); + if(ipv4only == 0) + hints.ai_family = AF_UNSPEC; + else + hints.ai_family = AF_INET; + hints.ai_socktype = SOCK_STREAM; + /* hints.ai_protocol = IPPROTO_TCP; */ + hints.ai_flags = 0; + + memset(portstr, 0, 8); + snprintf(portstr, 6, "%d", port); + + s = getaddrinfo(server, portstr, &hints, &result); + if (s != 0) { + fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(s)); + exit(1); + } + for (rp = result; rp != NULL; rp = rp->ai_next) { + fd = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol); + if (fd == -1) + continue; + if (connect(fd, rp->ai_addr, rp->ai_addrlen) != -1) + break; + close(fd); + } + if (rp == NULL) { + fprintf(stderr, "Could not connect\n"); exit(1); } + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &curstate, sizeof(curstate)) < 0) { perror("setsockopt"); exit(1); } - if ((host = gethostbyname(server)) == NULL){ - fprintf(stderr, _("can't get address %s\n"), server); - exit(1); + + if ((setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &nodelay, sizeof(nodelay))) < 0) { + perror("Error setsockopt tcp_nodelay"); } - addr.sin_port = htons(port); - addr.sin_family = AF_INET; - memcpy(&addr.sin_addr, host->h_addr, host->h_length); - if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { - perror("connect"); - exit(1); + if ((setsockopt(fd, IPPROTO_TCP, TCP_QUICKACK, &quickack, sizeof(quickack))) < 0) { + perror("Error setsockopt tcp_quickack"); } + if (verbose) fprintf(stderr, _("ok.. connected\n")); netfd[0] = fd;
A man page patch; --- a/seq/aseqnet/aseqnet.1 +++ b/seq/aseqnet/aseqnet.1 @@ -1,4 +1,4 @@ -.TH aseqnet 1 "January 1, 2000" +.TH aseqnet 1 "May 1, 2019" .SH NAME aseqnet - ALSA sequencer connectors over network
@@ -72,6 +72,9 @@ .TP .B -v Verbose mode. +.TP +.B -4 +IPv4 only.
.SH "SEE ALSO" aconnect(1), pmidi(1)
Regards, Rob
On Sat, 04 May 2019 14:51:53 +0200, Rob van der Putten wrote:
Hi there
On 28/04/2019 10:00, Takashi Iwai wrote:
On Tue, 23 Apr 2019 20:25:58 +0200, Rob van der Putten wrote: On Tue, 23 Apr 2019 20:25:58 +0200, Rob van der Putten wrote:
Hi there
Not an ALSA source patch, but a patch for an ALSA related util. Aseqnet sends ALSA sound_seq MIDI over TCP/IP. The patch below disables nagle, enables quickack and makes aseqnet dual-stack.
Thanks for the patch. Could you repost with a proper patch change log and your Signed-off-by line so that one can apply to git repo?
Added
- Disabled Nagle's algorithm
- Enbled quickack
- IPv6 support
Signed-off-by: Rob van der Putten rob@sput.nl
About the changes:
--- aseqnet.c.bak 2012-01-25 10:43:38.000000000 +0100 +++ aseqnet.c 2017-08-26 14:17:58.261868853 +0200 @@ -3,6 +3,8 @@
- ver.0.1
- Copyright (C) 1999-2000 Takashi Iwai
- Modified by Rob van der Putten, Leiden, Holland,
- rob at sput dot nl.
We don't need to add each change in the source like that as all tracked in git.
@@ -15,18 +17,21 @@
*/
+#include <alsa/asoundlib.h> +#include <arpa/inet.h> +#include <assert.h> +#include <ctype.h> +#include <getopt.h> +#include <locale.h> +#include <netdb.h> +#include <netinet/in.h> +#include <netinet/tcp.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> -#include <ctype.h> #include <string.h> -#include <netinet/in.h> #include <sys/socket.h> -#include <netdb.h> -#include <locale.h> -#include <alsa/asoundlib.h> -#include <getopt.h> -#include <signal.h> -#include <assert.h> +#include <sys/types.h> #include "aconfig.h" #include "gettext.h"
Why these large rearrangement of include files? If it must be inevitably done, please describe the reason in the changelog, too.
I just put them into alphabetical order. I changed that back.
@@ -327,17 +332,24 @@ */ static void init_server(int port) {
- /*
RvdP, changed to support IPv6
Dual stack only!- */
Wouldn't it potentially break things? IMO, it's better to keep the old behavior (ipv4-only), at least, with an option.
Server: The behaviour is determined by /proc/sys/net/ipv6/bindv6only If this is one, the listening socket will be IPv6 only instead of dual stack. A socket option is used to set IPV6_V6ONLY to zero, which forces the listening socket dual stack.
Client: The 'for (rp = result; rp != NULL; rp = rp->ai_next) {' loop keeps trying to connect to the remote host: If IPv6 fails, it uses IPv4 instead.
Both: I added a -4 / --ipv4 option. This not in the previous patch. It also introduces the string '-4, --ipv4 : IPv4 only' for which there no translations. And a new bit in the man page, with an added man page patch.
Also, drop your initials in the commit, it's rather superfluous.
A new aseqnet patch. Note: This patches the original, not the previous patch.
Thanks, the code changes look much better now! But the patch embedded in your mail is broken because of your MUA that breaks lines. Please either fix your MUA setup or use an attachment if it's difficult.
Also, please give the patch description along with your sign-off line. That is, at best, please send a patch in the form that is applicable via git-am.
thanks,
Takashi
participants (2)
-
Rob van der Putten -
Takashi Iwai