Eric Leblond wrote:
Hello,
On Sat, 2013-03-16 at 14:14 +0000, Mr Dash Four wrote:
I'm wondering if it is not more convenient to simply pass a field with raw
data to the stack and have it used as binary in output plugins.
This way, we will treat all cases (I mean all protocols, ipsec or gre
are not supported in your patch) and the impact on the rest of the
plugins will be smaller.
I don't think so.
The very nature of the information contained within strictly depends on
the protocol used in the original connection. If that was udp for
example, then the only "additional information" (in other words, what my
patch can extract from the secondary header) available to the plugins
would be the KEY_O_UDP_* keys and nothing else.
Besides, if a binary data is passed to the plugins, that data also needs
to be processed - by *all* plugins connected to the chain, so I don't
see this as a benefit at all.
I really fear changes on all output plugins will be huge without
handling all cases.
Have you done a performance analysis to make that assumption? If so,
could you post the results for all of us to see what that impact really is?
Do I ever use the word "performance" ? I'm talking about developing
work
This will put work on interface and maybe it is a bit too much for them.
Same question as above?
And same answer, I'm talking here about user interface that use ulogd
provided data to give information to user. They will have to decode all
these fields.
The way I read your response, it was clear to me that by "this will put
work on interface and maybe it is a bit too much for them" you had some
performance concerns with regards to my patch, hence why I asked the
questions I did in the previous post. The clue for me was in the "too
much for them" bit.
If I understood you now correctly, you've explained that you have
concerns over the re-development of the rest of the plugins necessary to
accommodate these changes, is that right?
If so, again, from experience, I know that at least 2 plugins will
require light-to-no-work - the GPRINT (which works well without *any*
modifications) and the PGSQL plugin. I suspect for the other db plugins
the situation won't be any different.
Admittedly, I am using a heavily-modified version of the PGSQL plugin
where it took me about 10 minutes to accommodate these changes, as the
existing PGSQL plugin (and accompanying sql script) in the ulogd2
repository is ... well, lets just say that I won't recommend this to my
competitors, let alone use this myself.
I can't really vouch for the rest of the plugins though, but if I go
ahead with your suggestion above and distribute the new properties I
listed in my patch as a binary object, then there will be needed the
same amount of effort required at the very least, if not more, to modify
the existing plugins to accommodate these changes, as well as be able to
process the binary data into something more meaningful, so I don't see
how this would be any different from what is already done in my patch?
I have been using this addition for over 7 months now (mainly as an
extension to the PGSQL and GPRINT plugins as they are most-widely
deployed on my servers here) and from experience I can tell you that, at
least for the TCP part of the original connection, I need the TCP flags
(in addition to everything else), simply because a connection can be
rejected with icmp unreachable because of illegal flags combination. So
in that particular case, I need to have all TCP flags available as keys.
This information is also available in the binary data I suggest to add.
And you could get it from there if needed.
Except that you didn't. You suggested that I just include the src/dst ip
addresses and ports, protocol and nothing more.To quote your previous
response:
If we consider the problem from an admin point of view the most
meaningful information is the tuple (IP src and dest, proto, src and dst
port) the rest is a bit too much.
Care to highlight where in the above you mention TCP flags?
[1] - http://www.spinics.net/lists/netfilter-devel/msg23982.html
Can you rebase it to current tree and resubmit with white space fixed as
discussed before ?
Care to address the questions I asked you in that post?
[2] - http://www.spinics.net/lists/netfilter-devel/msg23120.html
Do you have provided a patchset containing the Null dereference fix and
the SSL feature as asked by Pablo Neira Ayuso ?
I'll do that when I get some answers. Besides, the main purpose of this
patch wasn't fixing the null dereference bug, but to introduce the SSL
feature and I am yet to hear from you whether you are happy with that
part of the patch I provided a couple of months ago.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
