Re: [PATCH] Aseqnet, no nagle and dual stack

[Takashi Iwai <tiwai@suse.de>]

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