All of lore.kernel.org
 help / color / mirror / Atom feed
* ulogd2 suggest
@ 2012-01-12 23:05 marty
  2012-01-13  7:40 ` Pierre Chifflier
  2012-01-13 14:28 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 8+ messages in thread
From: marty @ 2012-01-12 23:05 UTC (permalink / raw)
  To: netfilter-devel

Ulogd2 is NOT a prime-time candidate. Never has been.
The API has been broken a long time.
Too many issues unresolved.

The wise way to fix the endian issues I documented are to use
host byte order from the very beginning, as per my freely given patch.
That leaves all math options open.
That patch is absolutely correct. Won't break the core design.

NOW, to provide endian conversion simply write a tiny filter module
that does endian conversion of addresses. Duh! How hard could that be?
Then the user can add that mod to stack as needed for an instance,
but it is not global and he can still enjoy both formats.
Gaa, how simple can it get?

Then of course remove every line in every other module that changes
byte order. That is totally inconsistent behavior and wrong.

Likewise IPV6 is NOT the default protocol yet so any and ALL IPv6
conversions need to be done by a single module the user can add to
the stack only when/where desired.
It is stupid and redundant to hardwire that into every module.
Get that out of the other modules and into a single filter.

The database modules NEED to have more options in the config file,
or a LOT of capabilities are unavailable. This is particular to
each DBMS or DBI, so those need to be addressed individually.

After that we should have a much more logical API to work with.

I dare you to tell me I am wrong.

Marty B.













^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ulogd2 suggest
  2012-01-12 23:05 ulogd2 suggest marty
@ 2012-01-13  7:40 ` Pierre Chifflier
  2012-01-13 14:28 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 8+ messages in thread
From: Pierre Chifflier @ 2012-01-13  7:40 UTC (permalink / raw)
  To: marty; +Cc: netfilter-devel

On Thu, Jan 12, 2012 at 06:05:59PM -0500, marty wrote:
> Ulogd2 is NOT a prime-time candidate. Never has been.
> The API has been broken a long time.
> Too many issues unresolved.
> 
> The wise way to fix the endian issues I documented are to use
> host byte order from the very beginning, as per my freely given patch.
> That leaves all math options open.
> That patch is absolutely correct. Won't break the core design.
> 
> NOW, to provide endian conversion simply write a tiny filter module
> that does endian conversion of addresses. Duh! How hard could that be?
> Then the user can add that mod to stack as needed for an instance,
> but it is not global and he can still enjoy both formats.
> Gaa, how simple can it get?
> 
> Then of course remove every line in every other module that changes
> byte order. That is totally inconsistent behavior and wrong.
> 
> Likewise IPV6 is NOT the default protocol yet so any and ALL IPv6
> conversions need to be done by a single module the user can add to
> the stack only when/where desired.
> It is stupid and redundant to hardwire that into every module.
> Get that out of the other modules and into a single filter.
> 
> The database modules NEED to have more options in the config file,
> or a LOT of capabilities are unavailable. This is particular to
> each DBMS or DBI, so those need to be addressed individually.
> 
> After that we should have a much more logical API to work with.
> 
> I dare you to tell me I am wrong.
> 

Hi,

[I'm slowly getting back to the subject)

ulogd2 is not wrong, mysql is the cause of the problem: it does not
provide any way to store IP addresses.
You can store them as integers, but that will only work for IPv4. Using
big-endian is only a workaround to avoid some conversions during a query
- you can also change the procedures for insertions, that will give the
  same result (though I agree byte swapping in SQL is really ugly).
If you want a good way to store IP addresses, use PostgreSQL, it
provides the type inet_addr which allows subnet queries, IPv6/IPv4
handling etc.

That said, I'm not against the idea of controlling the byte order or the
IPv6 conversion.
Adding some options to output modules may be a solution (or do we have
something more generic ?).

Regards,
Pierre

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ulogd2 suggest
  2012-01-12 23:05 ulogd2 suggest marty
  2012-01-13  7:40 ` Pierre Chifflier
@ 2012-01-13 14:28 ` Pablo Neira Ayuso
  2012-01-13 16:39   ` marty
  1 sibling, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2012-01-13 14:28 UTC (permalink / raw)
  To: marty; +Cc: netfilter-devel

On Thu, Jan 12, 2012 at 06:05:59PM -0500, marty wrote:
> Ulogd2 is NOT a prime-time candidate. Never has been.
> The API has been broken a long time.
> Too many issues unresolved.

Bah.

You don't even point to evindences. You don't even specify a
list of reasons why you affirm this or a list of unresolved
issues.

This is completely gratuitous.

> The wise way to fix the endian issues I documented are to use
> host byte order from the very beginning, as per my freely given patch.

Your patch breaks other plugins, I already told you. It's imcomplete.

If you change the endianness of the address, you have to propagate the
changes to other plugins.

[...]
> The database modules NEED to have more options in the config file,
> or a LOT of capabilities are unavailable. This is particular to
> each DBMS or DBI, so those need to be addressed individually.

Stop ranting, send patches.

This is a community project, if you feel something is broken, contribute
it, improve it and fix it, or if you don't know how to make it,
try to convince anyone to get it done. This is how it works.

So far, your overall contribution to the Netfilter project is ZERO
lines of code by now and tons of long emails with lots of complains.

Disregarding others is not the way to go, definitely not.

I'm very upset with your attitute, it's not coollaborative at all.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ulogd2 suggest
  2012-01-13 14:28 ` Pablo Neira Ayuso
@ 2012-01-13 16:39   ` marty
  2012-01-13 20:14     ` Pablo Neira Ayuso
  2012-01-13 21:43     ` Jozsef Kadlecsik
  0 siblings, 2 replies; 8+ messages in thread
From: marty @ 2012-01-13 16:39 UTC (permalink / raw)
  To: netfilter-devel

On 01/13/2012 09:28 AM, Pablo Neira Ayuso wrote:
> On Thu, Jan 12, 2012 at 06:05:59PM -0500, marty wrote:
>> Ulogd2 is NOT a prime-time candidate. Never has been.
>> The API has been broken a long time.
>> Too many issues unresolved.
>
> Bah.
>
> You don't even point to evindences. You don't even specify a
> list of reasons why you affirm this or a list of unresolved
> issues.
>
> This is completely gratuitous.
>

1. when ulogd logs to mysql, and a failure occurs, the last
auto_increment value may still contain a previous, used value.
So mysql will hang terminally. Transactions should prevent this.

2. transactions cannot be used in a mysql function, only a procedure.

3. When you call a stored procedure from a function, that puts the sproc 
into the same execution environment as the function, with the same 
restrictions, e.g. transactions don't work.

4. mysql provides a warning but ulogd2 ignores it.
mysql is not logging those warnings but they can be seen on command line

5. You cannot execute a procedure with a select statement.
    Mysql will look for a function, not a procedure.

6. Ulogd is hardwired to use SELECT statements for ALL DBMS in db.c.
So this breaks things and therfor I consider it wrong..

7. All key values, except "addresses" are in host byte order.
This inconsistency is problematic on little endian systems because
it breaks the native math processes for all client applications.

eg: You cannot do a SQL geo_ip lookup with wrong_endian numbers.
Converting a numeric address to host order is not practical in SQL.

8. IPv6 may be important to many, but in fact it is not the standard.
Almost every filter plugin is sprinkled with the same protocol
conversion code. That is redundant.
This can be accommodated on a per-instance basis by a single plugin.
AFAIK this is the only reason for the byte order mess anyway.

>> The wise way to fix the endian issues I documented are to use
>> host byte order from the very beginning, as per my freely given patch.
>
> Your patch breaks other plugins, I already told you. It's imcomplete.

Of course it breaks other plugins and thus incomplete.
I did not design or write ulogd2. I just tried to make it work for me.
I have limited resources and cannot test every available DBMS, or other 
output plugin.

> If you change the endianness of the address, you have to propagate the
> changes to other plugins.

That's exactly the problem!
Ulogd2 uses big-endian addresses on all host architectures,
so all the plugins are affected.
To fix this almost everything needs change, and I can't do that.

>
> [...]
>> The database modules NEED to have more options in the config file,
>> or a LOT of capabilities are unavailable. This is particular to
>> each DBMS or DBI, so those need to be addressed individually.
>
> Stop ranting, send patches.
>
> This is a community project, if you feel something is broken, contribute
> it, improve it and fix it, or if you don't know how to make it,
> try to convince anyone to get it done. This is how it works.

That is what I was trying to do.
I reported the issues clearly and asked for feedback
All I heard was send patches.
I sent a patch because that hack fixed my issue.
All I heard is that it wasn't acceptable, pretty enough, whatever.

I have fixed all my issues, making only 3 minor changes.
True enough, things I don't use are probably broken.
But it works for me.

>
> So far, your overall contribution to the Netfilter project is ZERO
> lines of code by now and tons of long emails with lots of complains.
>
> Disregarding others is not the way to go, definitely not.
>
> I'm very upset with your attitute, it's not coollaborative at all.

Sorry, didn't mean to offend.
Collaboration is not a 1 person act.
Don't expect me to re-write the entire program as a patch.

Marty B.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ulogd2 suggest
  2012-01-13 16:39   ` marty
@ 2012-01-13 20:14     ` Pablo Neira Ayuso
  2012-01-13 21:06       ` marty
  2012-01-13 21:43     ` Jozsef Kadlecsik
  1 sibling, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2012-01-13 20:14 UTC (permalink / raw)
  To: marty; +Cc: netfilter-devel

On Fri, Jan 13, 2012 at 11:39:07AM -0500, marty wrote:
> >You don't even point to evindences. You don't even specify a
> >list of reasons why you affirm this or a list of unresolved
> >issues.
> >
> >This is completely gratuitous.
> 
> 1. when ulogd logs to mysql, and a failure occurs, the last
> auto_increment value may still contain a previous, used value.
> So mysql will hang terminally. Transactions should prevent this.
>
> 2. transactions cannot be used in a mysql function, only a procedure.
> 
> 3. When you call a stored procedure from a function, that puts the
> sproc into the same execution environment as the function, with the
> same restrictions, e.g. transactions don't work.
> 
> 4. mysql provides a warning but ulogd2 ignores it.
> mysql is not logging those warnings but they can be seen on command line
> 
> 5. You cannot execute a procedure with a select statement.
>    Mysql will look for a function, not a procedure.
> 
> 6. Ulogd is hardwired to use SELECT statements for ALL DBMS in db.c.
> So this breaks things and therfor I consider it wrong..
> 
> 7. All key values, except "addresses" are in host byte order.
> This inconsistency is problematic on little endian systems because
> it breaks the native math processes for all client applications.
> 
> eg: You cannot do a SQL geo_ip lookup with wrong_endian numbers.
> Converting a numeric address to host order is not practical in SQL.
> 
> 8. IPv6 may be important to many, but in fact it is not the standard.
> Almost every filter plugin is sprinkled with the same protocol
> conversion code. That is redundant.
> This can be accommodated on a per-instance basis by a single plugin.
> AFAIK this is the only reason for the byte order mess anyway.

Bah.

You're asking for enhancements for ulogd2's mysql plugin.

However, you decided to start your email telling that ulogd2 (which
can make *way more things than logging to mysql*) is broken because
the current mysql support doesn't fit your needs.

[...]
> >If you change the endianness of the address, you have to propagate the
> >changes to other plugins.
> 
> That's exactly the problem!
> Ulogd2 uses big-endian addresses on all host architectures,
> so all the plugins are affected.
> To fix this almost everything needs change, and I can't do that.

[...]
> I have fixed all my issues, making only 3 minor changes.
> True enough, things I don't use are probably broken.
> But it works for me.

This comment shows that you know nothing about how Free/OpenSource
community projects work.

You just want to grab the software and provide ZERO return to the
existing codebase. Very sad.

Please, stop trolling us.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ulogd2 suggest
  2012-01-13 20:14     ` Pablo Neira Ayuso
@ 2012-01-13 21:06       ` marty
  0 siblings, 0 replies; 8+ messages in thread
From: marty @ 2012-01-13 21:06 UTC (permalink / raw)
  To: netfilter-devel

On 01/13/2012 03:14 PM, Pablo Neira Ayuso wrote:
> [...]
>> >  I have fixed all my issues, making only 3 minor changes.
>> >  True enough, things I don't use are probably broken.
>> >  But it works for me.
> This comment shows that you know nothing about how Free/OpenSource
> community projects work.
>
> You just want to grab the software and provide ZERO return to the
> existing codebase. Very sad.
>
> Please, stop trolling us.
>

Hardly true, and a very unprofessional reply I might add.
You obviously have a very distorted picture of me in your mind.

I know how open source works, and have contributed to many projects 
since 1975. I am happy to contribute where I am capable.
But I am not always capable of the work that needs to be done.
I can only test what I use, and I do it online at some risk.
But I won't submit untested code due to ethics.
I have a git repo fork, but it might take me over a year before I
could fix all the things that are wrong with ulogd2.

I may work for 2 weeks to fix something others might fix in minutes.
I have been struggling with ulogd2 for a very long time...
Now I have more understanding, and more capabilities.
Wrong time for to start flinging insults amigo.
Don't be a fool and alienate friends.

Marty B.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ulogd2 suggest
  2012-01-13 16:39   ` marty
  2012-01-13 20:14     ` Pablo Neira Ayuso
@ 2012-01-13 21:43     ` Jozsef Kadlecsik
  1 sibling, 0 replies; 8+ messages in thread
From: Jozsef Kadlecsik @ 2012-01-13 21:43 UTC (permalink / raw)
  To: marty; +Cc: netfilter-devel

Hi Marty,

On Fri, 13 Jan 2012, marty wrote:

> 1. when ulogd logs to mysql, and a failure occurs, the last
> auto_increment value may still contain a previous, used value.
> So mysql will hang terminally. Transactions should prevent this.

You wrote about it earlier, but I still don't understand. As far as I see, 
ulogd2 doesn't use any auto_increment value and doesn't expect such a 
parameter to be returned when inserting entries to the database. So 
where's this auto_increment value which may store a previous value after a 
failure?
 
> 2. transactions cannot be used in a mysql function, only a procedure.
> 
> 3. When you call a stored procedure from a function, that puts the sproc into
> the same execution environment as the function, with the same restrictions,
> e.g. transactions don't work.
> 
> 4. mysql provides a warning but ulogd2 ignores it.
> mysql is not logging those warnings but they can be seen on command line
> 
> 5. You cannot execute a procedure with a select statement.
>    Mysql will look for a function, not a procedure.
> 
> 6. Ulogd is hardwired to use SELECT statements for ALL DBMS in db.c.
> So this breaks things and therfor I consider it wrong..

As we discussed, this is (partly) a documentation issue. In PostgresSQL 
there's no real difference between a stored procedure and a stored 
function. In mySQL the two things are quite different, as you described 
above. The current doc is unfortunately quite misleading because it talks 
about stored procedures but internally invokes SELECT, in which case only 
stored functions are allowed in mySQL.

It could be solved by querying first the database whether the named object 
is a stored procedure or a function and issuing the proper SQL command 
(which is, again, pgsql/mysql dependent).

> 7. All key values, except "addresses" are in host byte order.
> This inconsistency is problematic on little endian systems because
> it breaks the native math processes for all client applications.
> 
> eg: You cannot do a SQL geo_ip lookup with wrong_endian numbers.
> Converting a numeric address to host order is not practical in SQL.

Your patch modified the current behaviour: the proper way is to add 
supporting new table fields which the filled out with the host ordered 
data.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ulogd2 suggest
@ 2012-01-14  3:27 marty
  0 siblings, 0 replies; 8+ messages in thread
From: marty @ 2012-01-14  3:27 UTC (permalink / raw)
  To: netfilter-devel

> Hi Marty,
>
>> 1. when ulogd logs to mysql, and a failure occurs, the last
>> auto_increment value may still contain a previous, used value.
>> So mysql will hang terminally. Transactions should prevent this.
 >
 > You wrote about it earlier, but I still don't understand. As far as I 
see,
 > ulogd2 doesn't use any auto_increment value and doesn't expect such a
 > parameter to be returned when inserting entries to the database. So
 > where's this auto_increment value which may store a previous value 
after a
 > failure?
 >
Sorry, if I didn't make that understandable.
First this is NOT a ulogd2 bug, and I didn't intend to infer that.
And I cannot consider it a mysql bug.
I just wanted to describe why mysql is broken by ulogd2.
Atomic transactions are essential which ulogd does not support by design!
Ulogd2 does have last_key/auto_increment values which "might"
be helpful in future; that is not what I was specifically addressing.

I have seen this happen a couple times, and it was very perplexing.
NFLOG was sending packets constantly, but nothing is being logged.
As soon as I remove last entry in table logging resumes.
Why? I cannot confirm this but I will clarify what I believe:

1. suppose we have a uncontrolled system shutdown; lets say reset.
2. the innodb logs contain last insert id logged, and will automatically
restore last logged state. That is very nice in most cases.
3. If an insert was done to a table with an auto_increment column
and the system fails before the log was written to disk, that ID is lost.
4. On restart the system is restored orderly from the logs, but only 
contains the LAST_INSERT_ID that was logged, not the LAST_INSERT_ID that 
actually was used! It cannot INSERT anything into a unique index
that was already used. DEADLOCK occurs terminally.
So I assume the innodb log is buffered but the insert is not.

Using transactions results in a synchronous write only after ALL
the SQL has been executed and if it crashes in the middle of the
log write it will be rolled back to the last completed transaction
  when state is restored.
That's as close to perfect as we can expect.

But as you know, that is not possible with ulogd2 as it is today.

 > As we discussed, this is (partly) a documentation issue. In PostgresSQL
 > there's no real difference between a stored procedure and a stored
 > function. In mySQL the two things are quite different, as you described
 > above. The current doc is unfortunately quite misleading because it talks
 > about stored procedures but internally invokes SELECT, in which case only
 > stored functions are allowed in mySQL.

Correct. And what about DBI and SQlite?
ALL DBMS have unique differences and should be addressed individually.
Instead they all use util/db.c which will not supports mysql 
transactions and perhaps causes issues with other DBMS.

 > Your patch modified the current behaviour: the proper way is to add
 > supporting new table fields which the filled out with the host ordered
 > data.

I tried that however it didn't work because I didn't hack every module
that used the keys. And the more I did, the worse it got.
So I hacked it for my needs alone. At best I got proof of concept ...

Marty B.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-01-14  3:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-12 23:05 ulogd2 suggest marty
2012-01-13  7:40 ` Pierre Chifflier
2012-01-13 14:28 ` Pablo Neira Ayuso
2012-01-13 16:39   ` marty
2012-01-13 20:14     ` Pablo Neira Ayuso
2012-01-13 21:06       ` marty
2012-01-13 21:43     ` Jozsef Kadlecsik
2012-01-14  3:27 marty

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.