Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config objects preceded by an inline destination are ignored #2820

Closed
MrAnno opened this issue Jul 5, 2019 · 5 comments · Fixed by #2989
Closed

Config objects preceded by an inline destination are ignored #2820

MrAnno opened this issue Jul 5, 2019 · 5 comments · Fixed by #2989
Labels

Comments

@MrAnno
Copy link
Collaborator

MrAnno commented Jul 5, 2019

syslog-ng

Version of syslog-ng

syslog-ng v3.22.1

Issue

The following configuration will not send any logs to d_asd:

@version: 3.22
destination d_asd {
	network("127.0.0.1" port(4448));
};

log {
	source { network(port(4444)); };
	destination {
		network("127.0.0.1" port(4447) );
	};
	destination(d_asd);
	flags(flow-control);
};

The cfg graph built from this config is incorrect. Changing the order of the 2 destinations (listing the inline destination last) fixes the issue.

Root cause

Inline destinations are compiled as sequences (cfg_tree_compile_sequence()), where no multiplexer is created for other objects. In case of general/non-inline destinations (cfg_tree_compile_reference()), the multiplexer is there:

https://github.com/balabit/syslog-ng/blob/1742b11e5cfa6544ece38aaecde96e9b423d61c5/lib/cfg-tree.c#L842-L851

Unfortunately, it seems syslog-ng creates a new multiplexer for every non-inline destination instead of adding a hop to the previous one. I don't know the reason, but we could improve on this too.

@MrAnno MrAnno added the bug label Jul 5, 2019
@MrAnno MrAnno changed the title Destinations preceding an inline destination are ignored by syslog-ng Destinations preceded by an inline destination are ignored Jul 5, 2019
@MrAnno MrAnno changed the title Destinations preceded by an inline destination are ignored Config objects preceded by an inline destination are ignored Jul 5, 2019
@bazsi
Copy link
Collaborator

bazsi commented Jul 5, 2019 via email

@furiel
Copy link
Collaborator

furiel commented Oct 22, 2019

Is it possible that the problem depends on the types of the config elements? I checked with an inline file destination and a network destination, and it works for me:

$ cat ../etc/tmp.conf 
@version: 3.24

options {
  time-reopen(5);
};

destination d_network {
    network("127.0.0.1" port(4447) template("NETWORK: $MESSAGE\n"));
};

log {
	source { example-msg-generator(); };
	destination { file(/dev/stdout template("FILE: $MESSAGE\n")); };
        destination(d_network);
	flags(flow-control);
};
$ nc -k -l 4447 &
[1] 8261
$ timeout 5 ./syslog-ng -Fe -f ../etc/tmp.conf 
[2019-10-22T08:34:25.696765] syslog-ng starting up; version='3.24.1.11.g126a19f'
[2019-10-22T08:34:25.696968] Syslog connection established; fd='11', server='AF_INET(127.0.0.1:4447)', local='AF_INET(0.0.0.0:0)'
FILE: -- Generated message. --
NETWORK: -- Generated message. --
FILE: -- Generated message. --
NETWORK: -- Generated message. --
FILE: -- Generated message. --
NETWORK: -- Generated message. --
FILE: -- Generated message. --
NETWORK: -- Generated message. --
FILE: -- Generated message. --
NETWORK: -- Generated message. --
[2019-10-22T08:34:30.671485] syslog-ng shutting down; version='3.24.1.11.g126a19f'
FILE: -- Generated message. --
NETWORK: -- Generated message. --

However, with the config example in the ticket, I also experience the problem.

@furiel
Copy link
Collaborator

furiel commented Oct 25, 2019

After looking into the matter deeper here is what I found out.

If I swap the order of an inline and normal destination, the config graph looks substantially different.

Showing a few examples:

  • inline file, referenced network version (the config I used int the previous comment)
    file-network

The file destination and the network destination are linked into the same chain serially. One would think how this can work at all. And the trick is that the affile_dd_queue function ends with a log_dest_driver_queue_method(s, msg, path_options);, that pushes forward the message in the chain.

  • referenced network - inline file
    If I swap the order, the two destinations are put to the two tips of a multiplexer:

network-file

  • inline network - referenced network
    So what is happening in case of the network-network version (the config that is posted in the issue)? The graph looks like this:
    network-network

Technically it should look like one long chain (similar to the file-network graph), but the lower part is disjoint. It is disjoint at:

log_pipe_append(&self->super.super.super, (LogPipe *) self->writer);

That code expects self->next_pipe to be null, but it points to the lower part of the chain. After overwriting the link with the pointer of the logwriter, the chain is broken.

I was thinking how this should be resolved but I do not have any plan yet.

The analogous solution would be that instead of log_pipe_append, I would join the logwriter into the chain. But that would mean the message should flow through log_writer_queue. So that to work I need to add a log_dest_driver_queue_method-ish call to the end of log_writer_queue. But log_writer_queue does kind of strange things to the message (possibly drops), so I do not think that is a good idea.

Another thing that feels a little off, that now no destinations attach their workers to themselves, because they might not be a terminal point in the graph. Conceptually, me feels natural that a destination should be a terminal point in the graph.

It also feels strange that if I change the order of two destinations, the config graph looks so differently (single chain vs fork).

@bazsi could you give me some hint how to fix this?

@MrAnno
Copy link
Collaborator Author

MrAnno commented Oct 25, 2019

I also feel it is right that destinations are terminal nodes in the graph. We obviously don't chain sources, but we should not chain destinations either in my opinion.

[I removed the misleading part of this comment, so I have now 0 real reason why we should use a multiplexer 😄.]

@MrAnno
Copy link
Collaborator Author

MrAnno commented Oct 25, 2019

Sorry, I was not right about the flow control part, add_ack() is working properly without the multiplexer between destinations.

furiel added a commit to furiel/syslog-ng that referenced this issue Nov 18, 2019
We want to leave pipe-next available to use for destinations. Prior
this patch, pipe-next was used by the config graph to pass messages
forward in a sequence layout. But pipe-next was overridden by network
destination (pointing to LogWriter), hence disjointing the config graph.

This patch links destinations in T form, instead of single link.

```
* (endpoint of sequence)
|
V
* (multiplexer) -(next-hop)-> destination - (pipe-next*) -> logwriter
|
(pipe-next)
|
V
* (rest of the sequence)
```

That way destinations are free to use the pipe-next*

Fixes: syslog-ng#2820

Signed-off-by: Antal Nemes <antal.nemes@balabit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants