[PATCH]: Syslog remote logging fails if network not available on first attempt

Denys Vlasenko vda.linux at googlemail.com
Tue Aug 3 02:27:55 UTC 2010


Hi Daniel,

Sorry for the huge delay :(

On Sunday 27 June 2010 15:05, Daniel Dickinson wrote:
> The attached patch fixes a bug (or missing feature) in busybox's syslog
> implementation.  The current implementation only makes one connection
> attempt to the remote host,

Not true. We do not make even *one* connection attempt.
Show me where in syslogd.c I can find connect() call.
There is none.

syslog uses UDP protocol. UDP is connectionless.

Even if we'd use connect(), that only sets fixed destination
address and makes it possible to use plain write() on the socket,
but on the wire it would not change one iota: UDP has no concept
of "connection", every UDP packet is a separate bit of data.

What we really do once is DNS resolution. Perhaps it does make
sense to retry it if sendto() fails...

> so that if the network or host is  
> unavailable when syslog first makes an attempt to send a messages to the
> remote host messages are never sent.

No, in this case the messages will be sent.

The only case when they will not be sent is when you use
non-existent hostname with -R HOSTNAME option. In this case,
hostname resolution will fail, and will be retried every 120 seconds.
Until it succeeds, we don't know the dst address, and thus don't know
where to sendto() data.

> The attached patch considers certain failures when attempt to transmit a
> log message, (ECONNRESET,

This one might be possible if remote host sends ICMP "port unreachable" pkts
or the host is down and we get ICMP "host unreachable" from routers
(or internally via kernel's ARP resolver).

> EDESTADDRREQ,

Impossible: we do provide the address to sendto() call.

> EISCONN,

Impossible: UDP is connectionless

> ENOTCONN,

Impossible: UDP is connectionless

> EPIPE

I think this might be possible (local firewall rules? Local delivery?)
But then we'll be killed by SIGPIPE. You need to prevent that
if you want to catch EPIPE.

> cause to close and remove the connection socket so that on the next
> syslog message to be sent, another connection will be tried.

Regarding the code itself:

Why do you need the send_err variable? You use it only in one
place - right after sendto() fails. You can simply do:

	if (sendto() fails) {
		switch (errno) {
		...
		}
	}

Thus, no new variable needed.

The comment
	/* send message to remote logger, ignore possible error */
needs changing: we do not ignore errors anymore.

You forgot to free G.remoteAddr.

Indentation is broken.

I applied the patch after much editing:

http://git.busybox.net/busybox/commit/?id=e74d79866c6d125527e3ba69245a087a28fd19ce

-- 
vda


More information about the busybox mailing list