SSH login takes very long time...sometimes

[Rostislav Krasny]

On Sat, 25 Feb 2006 17:22:07 +0300
Yar Tikhiy <yar at comp.chem.msu.su> wrote:

> > However I was unable to connect by ftp, even with only one unreachable
> > name server in resolv.conf. I got following error:
> > 
> > 421 Service not available, remote server timed out. Connection closed
> > 
> > I've found the problem in both: ftpd(8) and ftp(1). In the ftpd(8) a
> > getaddrinfo() is called in two places with hints.ai_socktype == 0 and
> > hints.ai_family == PF_UNSPEC. In the ftp(1) a command reply timeout is
> > only 60 seconds. Those things are what I've changed to fix the problem.
> > Two diffs are attached to this email. The ftpd.c.diff extends -4 and -6
> > ftpd options. So if this patch is good, the ftpd(8) manual page and the
> > default /etc/inetd.conf should also be changed appropriately.
> 
> Could you explain please how your patch would affect ftpd(8) semantics
> visible to the user?

ftpd(8) has -4 and -6 options, which originally are used in daemon mode
only. With my patch they are used in inetd mode too. But Hajimu showed
me that in the inetd mode the 'family' variable could be assigned
according to remote client's address family. Please read my recent reply
to his email.

> > Although I changed two getaddrinfo() calls in ftpd.c, only first of
> > them is really called on default FreeBSD configuration,
> > when /etc/ftphosts isn't existing yet. So there might be a need of
> > additional increase of the command reply timeout in ftp.c. Or better if
> > this timeout could be configurable by some new ftp(1) option, with 120
> > seconds by default.
> 
> > --- libexec/ftpd/ftpd.c.orig	Wed Feb  8 18:54:05 2006
> > +++ libexec/ftpd/ftpd.c	Sat Feb 25 00:30:26 2006
> > @@ -239,7 +239,7 @@
> >  	}
> >  
> >  #ifdef VIRTUAL_HOSTING
> > -static void	 inithosts(void);
> > +static void	 inithosts(int);
> >  static void	 selecthost(union sockunion *);
> >  #endif
> >  static void	 ack(char *);
> > @@ -424,7 +424,7 @@
> >  	}
> >  
> >  #ifdef VIRTUAL_HOSTING
> > -	inithosts();
> > +	inithosts(family);
> >  #endif
> >  
> >  	if (daemon_mode) {
> > @@ -663,7 +663,7 @@
> >   */
> >  
> >  static void
> > -inithosts(void)
> > +inithosts(int family)
> >  {
> >  	int insert;
> >  	size_t len;
> > @@ -689,7 +689,8 @@
> >  
> >  	memset(&hints, 0, sizeof(hints));
> >  	hints.ai_flags = AI_CANONNAME;
> > -	hints.ai_family = AF_UNSPEC;
> > +	hints.ai_family = family;
> > +	hints.ai_socktype = SOCK_STREAM;
> >  	if (getaddrinfo(hrp->hostname, NULL, &hints, &res) == 0)
> >  		hrp->hostinfo = res;
> >  	hrp->statfile = _PATH_FTPDSTATFILE;
> > @@ -759,9 +760,10 @@
> >  						/* NOTREACHED */
> >  					}
> >  
> > -			hints.ai_flags = 0;
> > -			hints.ai_family = AF_UNSPEC;
> > +			/* If no flag, assign hints.ai_flags to zero! */
> 
> Sorry, but I don't understand the purpose of this comment here.

The original code looked like that:

			hints.ai_flags = 0;
			hints.ai_family = AF_UNSPEC;
			hints.ai_flags = AI_PASSIVE;

It looked like someone wanted to say to future developers: "if you don't
whant hints.ai_flags = AI_PASSIVE keep the hints.ai_flags = 0".
My comment might be not well formulated or even unnecessary, sorry.