ipt_LOG and friends. Question

ipt_LOG and friends. Question

[Joakim Axelsson]

I am writing myself a new/better ipt_LOG (or xt_LOG). I won't go into why i
am not using ULOG. Its not the point here so.

Is there some special reason to call printk() in the logging code for every
little piece of info. This gives a problem when using "Networking console
logging support" (the netconsole module). Each printk will result in one
line of log (one UDP-packet) leaving the reiceving syslog-daemon on another
machine puting several lines of log for each log.

Also, there is a lock so only one can enter the dump_packet -code in order
to not get two packets logging at the same time. Locks are costly.

What so wrong with using a buffer of say around 1024 chars and a strcat-like
function (but smarter) keeping track of where to write next and not buffer
overflowing. And finally printk().

Like:

#define MAXLEN 1016
struct log_buffer {
	char log[MAXLEN]
	char *ptr
}

/* sizeof(log_buffer) <= 1024, even in 64bits */

int printlog(struct log_buffer *b, char *fmt, ...) 
{
	if (b->ptr + bytes need writing > log + MAXLEN) {
		return error
	else {
		add string to b->ptr
		update b->ptr for next call
	}
}


This struct should need allocated on stack. This might be a bad idea steal
that much memory on stack? So we could GPF_ATOMIC allocate it or use one
global and lock it. I think however that locking is more expensive than
memory alloc. 

Any problems with this? Have i missed something? Any good reason for calling
printk() that many times?

--
Joakim Axelsson

Re: ipt_LOG and friends. Question

[Joakim Axelsson]

2006-08-24 16:03:20+0200, Joakim Axelsson <gozem@gozem.se> ->
> I am writing myself a new/better ipt_LOG (or xt_LOG). I won't go into why i
> am not using ULOG. Its not the point here so.
> 
> Is there some special reason to call printk() in the logging code for every
> little piece of info. This gives a problem when using "Networking console
> logging support" (the netconsole module). Each printk will result in one
> line of log (one UDP-packet) leaving the reiceving syslog-daemon on another
> machine puting several lines of log for each log.
> 
> Also, there is a lock so only one can enter the dump_packet -code in order
> to not get two packets logging at the same time. Locks are costly.
> 
> What so wrong with using a buffer of say around 1024 chars and a strcat-like
> function (but smarter) keeping track of where to write next and not buffer
> overflowing. And finally printk().
> 
> Like:
> 
> #define MAXLEN 1016
> struct log_buffer {
> 	char log[MAXLEN]
> 	char *ptr
> }
> 
> /* sizeof(log_buffer) <= 1024, even in 64bits */
> 
> int printlog(struct log_buffer *b, char *fmt, ...) 
> {
> 	if (b->ptr + bytes need writing > log + MAXLEN) {
> 		return error
> 	else {
> 		add string to b->ptr
> 		update b->ptr for next call
> 	}
> }
> 
> 
> This struct should need allocated on stack. This might be a bad idea steal
> that much memory on stack? So we could GPF_ATOMIC allocate it or use one
> global and lock it. I think however that locking is more expensive than
> memory alloc. 
> 
> Any problems with this? Have i missed something? Any good reason for calling
> printk() that many times?
> 

Some additional thinking:
The default 29-32 chars as a prefix is far too little for me. Needs to
change to 128. But again i see some code allocating this on the stack. It
this a problem?

The code says that a maximum of 830 chars can be used (ICMP-pcaket with
one recursion level). 128 char prefix and some more on IN, OUT, PHYSIN/OUT
and MAC fields will give an overall figure of around 1024 chars a good
buffer length. Anything above that can probably be cut. It won't happen
alot.

Another idea of not allocating on the stack nor using GFP_ATOMIC is to have
a global buffer per CPU. Is this a good idea with the hot adding and
removing of CPUs? Are linux kernel guarantied to continue on the same CPU if
an interupt occurs and so on? 

--
Joakim Axelsson

Re: ipt_LOG and friends. Question

[Patrick McHardy]

Joakim Axelsson wrote:
> I am writing myself a new/better ipt_LOG (or xt_LOG). I won't go into why i
> am not using ULOG. Its not the point here so.
> 
> Is there some special reason to call printk() in the logging code for every
> little piece of info. This gives a problem when using "Networking console
> logging support" (the netconsole module). Each printk will result in one
> line of log (one UDP-packet) leaving the reiceving syslog-daemon on another
> machine puting several lines of log for each log.
> 
> Also, there is a lock so only one can enter the dump_packet -code in order
> to not get two packets logging at the same time. Locks are costly.

Logging to the ring buffer is not really very suitable for anything but
debugging anyway.

> What so wrong with using a buffer of say around 1024 chars and a strcat-like
> function (but smarter) keeping track of where to write next and not buffer
> overflowing. And finally printk().

That sounds like a good idea. Would you care to send a patch?

> Like:
> 
> #define MAXLEN 1016
>
> struct log_buffer {
> 	char log[MAXLEN]
> 	char *ptr
> }
> 
> /* sizeof(log_buffer) <= 1024, even in 64bits */
> 
> int printlog(struct log_buffer *b, char *fmt, ...) 
> {
> 	if (b->ptr + bytes need writing > log + MAXLEN) {
> 		return error
> 	else {
> 		add string to b->ptr
> 		update b->ptr for next call
> 	}
> }
> 
> 
> This struct should need allocated on stack. This might be a bad idea steal
> that much memory on stack? So we could GPF_ATOMIC allocate it or use one
> global and lock it. I think however that locking is more expensive than
> memory alloc. 

We can't use that much space on the stack, but you can simply keep
the current lock around and use a global structure.

Re: ipt_LOG and friends. Question

[Patrick McHardy]

Joakim Axelsson wrote:
> The default 29-32 chars as a prefix is far too little for me. Needs to
> change to 128. But again i see some code allocating this on the stack. It
> this a problem?

We can't change it due to binary compatibility. In your local tree you
can simply increase both sizes (not on the stack of course).

> The code says that a maximum of 830 chars can be used (ICMP-pcaket with
> one recursion level).

Are you sure? That sounds a bit high.

> 128 char prefix and some more on IN, OUT, PHYSIN/OUT
> and MAC fields will give an overall figure of around 1024 chars a good
> buffer length. Anything above that can probably be cut. It won't happen
> alot.
> 
> Another idea of not allocating on the stack nor using GFP_ATOMIC is to have
> a global buffer per CPU. Is this a good idea with the hot adding and
> removing of CPUs? Are linux kernel guarantied to continue on the same CPU if
> an interupt occurs and so on? 

That seems overkill, one global buffer protected by a lock will do fine.

Re: ipt_LOG and friends. Question

[Joakim Axelsson]

I am currently half way writing this up. However i'm modifying it after my
needs primarily. I think they might meet the netfilters general taste as
well. I am currently fixing the following:

1. Instead of calling printk() very here and there, it prints to a global
buffer (using a spinlock). To finally calling printk once. This works fine.
Its coded and tested.

2. I have merged ipt_LOG and ip6t_LOG into xt_LOG. With common
dump_tcp()/udp() and some other small code. This also works good. 

--
The first two doesn't break anything or changes anything in compability with
former ip(6)t_LOG.

However the next things will change slightly. The question is, how much can
i change. Using -j LOG and the ringbuffer is for debugging purpose? Or does
it actually exists people that parse this buffer for some need? (It surly
does, but can we kindly ignore them and point them to ULOG?).

3. The prefix is missing a space between the prefix and IN=interface. Very
annoying to always have to use -j LOG --log-prefix "one two three ". Notice
the last space. Will this break anything if i just add a space in between of
"%sIN=%s OUT..." for the prefix?

4. Using an ethernet interface is _very_ common. Can i if (skb->dev->type ==
ARPHRD_ETHER) just dump an ETHSRC and an ETHDST instead of MAC, or will
this break things for people? Again a matter of someone having scripts
parsing this debug log. I can add this as an extra flag and have LOG revison
1 use it.

--
The last suggestions are involving a revision 1 of LOG.

5. I my self have a good need for treating the prefix as a sulfix instead.
Much easier to read with the packet dump better aligned up, and an ending
reason. Can i add this as a flag? Having userspace revision 1 honoring it
with --log-sulfix (it will ofcouse not be allowed to have both --log-prefix and
--log-sulfix).

6. I am in the need to alter the prefix length to somewhat below 128 (so the
kernel space info-struct totally is 128 bytes would be good). Is it okey if
i add this in a revison 1? The packet dumping code doesn't depend on a
length. Only the target itself.

7. It would be very nice if we had a -m log. This is very easy todo in
kernel-space. Only that we have to call it -m LOG (uppercase) for userspace
to load the correct library and kernel module. Its just a matter of the
module registering both as a match and a target. Bad idea? It would be very
convinent. The match would just log the packet and always return true
(match). Compare this to the comment module that doesn't actually do any
match-work.

7. Currently i always use my LOGing with a limiter to not flood the
ringbuffer. I guess most people are. There would be very nice with a
--log-limit 5, meaning we will only log 5 packets per second. Also an
inverter for that so --log-limit ! 6 means to log everypacket hiting the
rule above 6 packet per second. This will go very well with LOG being a
match as well. As it simply selects which packets to log and always matches
them to continue in the firewall rules. Again this is a revison 1 thing. The
code for this in kernel is very simple. 

Comments wanted. I do not want to write anything that will be rejected.

Ulitmaly i am thinking of doing the same work for ip(6)t_ULOG to a xt_ULOG.

--
Joakim Axelsson

Re: ipt_LOG and friends. Question

[Patrick McHardy]

Joakim Axelsson wrote:
> I am currently half way writing this up. However i'm modifying it after my
> needs primarily. I think they might meet the netfilters general taste as
> well. I am currently fixing the following:
> 
> 1. Instead of calling printk() very here and there, it prints to a global
> buffer (using a spinlock). To finally calling printk once. This works fine.
> Its coded and tested.

That part I would like to put in mainline.

> 2. I have merged ipt_LOG and ip6t_LOG into xt_LOG. With common
> dump_tcp()/udp() and some other small code. This also works good. 

If it stays compatible and results in less code, that one too.

> The first two doesn't break anything or changes anything in compability with
> former ip(6)t_LOG.

Great :)

> However the next things will change slightly. The question is, how much can
> i change. Using -j LOG and the ringbuffer is for debugging purpose? Or does
> it actually exists people that parse this buffer for some need? (It surly
> does, but can we kindly ignore them and point them to ULOG?).

People do, we can't change anything in the output format.

> 3. The prefix is missing a space between the prefix and IN=interface. Very
> annoying to always have to use -j LOG --log-prefix "one two three ". Notice
> the last space. Will this break anything if i just add a space in between of
> "%sIN=%s OUT..." for the prefix?

Yes, people already add the extra space and it would at least change
how log files look, potentially even break parsers.

> 4. Using an ethernet interface is _very_ common. Can i if (skb->dev->type ==
> ARPHRD_ETHER) just dump an ETHSRC and an ETHDST instead of MAC, or will
> this break things for people? Again a matter of someone having scripts
> parsing this debug log. I can add this as an extra flag and have LOG revison
> 1 use it.

An extra flag would OK fine I guess.

> 
> --
> The last suggestions are involving a revision 1 of LOG.
> 
> 5. I my self have a good need for treating the prefix as a sulfix instead.
> Much easier to read with the packet dump better aligned up, and an ending
> reason. Can i add this as a flag? Having userspace revision 1 honoring it
> with --log-sulfix (it will ofcouse not be allowed to have both --log-prefix and
> --log-sulfix).

Mhh .. why not.

> 6. I am in the need to alter the prefix length to somewhat below 128 (so the
> kernel space info-struct totally is 128 bytes would be good). Is it okey if
> i add this in a revison 1? The packet dumping code doesn't depend on a
> length. Only the target itself.

That was already requested by multiple people (including myself), so go
ahead.

> 7. It would be very nice if we had a -m log. This is very easy todo in
> kernel-space. Only that we have to call it -m LOG (uppercase) for userspace
> to load the correct library and kernel module. Its just a matter of the
> module registering both as a match and a target. Bad idea? It would be very
> convinent. The match would just log the packet and always return true
> (match). Compare this to the comment module that doesn't actually do any
> match-work.

I think this is something that can wait for a infrastructure that is
meant to do things like that.

> 7. Currently i always use my LOGing with a limiter to not flood the
> ringbuffer. I guess most people are. There would be very nice with a
> --log-limit 5, meaning we will only log 5 packets per second. Also an
> inverter for that so --log-limit ! 6 means to log everypacket hiting the
> rule above 6 packet per second. This will go very well with LOG being a
> match as well. As it simply selects which packets to log and always matches
> them to continue in the firewall rules. Again this is a revison 1 thing. The
> code for this in kernel is very simple. 

Whats wrong with using the limit match?

Re: ipt_LOG and friends. Question

[Joakim Axelsson]

2006-08-28 12:57:13+0200, Patrick McHardy <kaber@trash.net> ->
> Joakim Axelsson wrote:
> > I am currently half way writing this up. However i'm modifying it after my
> > needs primarily. I think they might meet the netfilters general taste as
> > well. I am currently fixing the following:
> > 
> > 1. Instead of calling printk() very here and there, it prints to a global
> > buffer (using a spinlock). To finally calling printk once. This works fine.
> > Its coded and tested.
> 
> That part I would like to put in mainline.
> 

Okie, i'll se if i can just make a patch for that small part. Might be hard
as i merge ipt_LOG and ip6t_LOG into xt_LOG. Well, see below.

> > 2. I have merged ipt_LOG and ip6t_LOG into xt_LOG. With common
> > dump_tcp()/udp() and some other small code. This also works good. 
> 
> If it stays compatible and results in less code, that one too.

Yes, it _should_ be the same. The code is somewhat changed but the result
should produce exactly the same. It might have introduced some bug? So
handing it in an early rc-tree of 2.6 or give me time to run the code on our
router for a few weeks. What ever you like :-)

> 
> > The first two doesn't break anything or changes anything in compability with
> > former ip(6)t_LOG.
> 
> Great :)
> 
> > However the next things will change slightly. The question is, how much can
> > i change. Using -j LOG and the ringbuffer is for debugging purpose? Or does
> > it actually exists people that parse this buffer for some need? (It surly
> > does, but can we kindly ignore them and point them to ULOG?).
> 
> People do, we can't change anything in the output format.
> 
> > 3. The prefix is missing a space between the prefix and IN=interface. Very
> > annoying to always have to use -j LOG --log-prefix "one two three ". Notice
> > the last space. Will this break anything if i just add a space in between of
> > "%sIN=%s OUT..." for the prefix?
> 
> Yes, people already add the extra space and it would at least change
> how log files look, potentially even break parsers.
> 

Hm, hard one. I guess it's not enough to only change this to revision 1 as
the parameter is the same and so on. And the code i wrote doesn't make any
difference between revision 0 and 1 during the packet dumping functions.
They are the same.

I guess the only way to solve this is to add a new flag in revision 1
(beside keeping old --log-prefix with the missing space. --log-text-prefix,
--log-text-sulfix ? I'll think of something.

> > 4. Using an ethernet interface is _very_ common. Can i if (skb->dev->type ==
> > ARPHRD_ETHER) just dump an ETHSRC and an ETHDST instead of MAC, or will
> > this break things for people? Again a matter of someone having scripts
> > parsing this debug log. I can add this as an extra flag and have LOG revison
> > 1 use it.
> 
> An extra flag would OK fine I guess.
> 

Okey, i'll add the flag --log-macparse which removes the MAC= into something
more usefull for common interface types. The code is already written (for
ETHERNET and LOOPBACK) but i skipped the flag. I'll add it into revision 1.
The thing is that MAC= is only present when we have the IN-interface defined
and OUT not. Meaning that MAC isn't even there most of the time. ANyway,
i'll add the flag.

> > 
> > --
> > The last suggestions are involving a revision 1 of LOG.
> > 
> > 5. I my self have a good need for treating the prefix as a sulfix instead.
> > Much easier to read with the packet dump better aligned up, and an ending
> > reason. Can i add this as a flag? Having userspace revision 1 honoring it
> > with --log-sulfix (it will ofcouse not be allowed to have both --log-prefix and
> > --log-sulfix).
> 
> Mhh .. why not.
> 

Already implemented :-)

> > 6. I am in the need to alter the prefix length to somewhat below 128 (so the
> > kernel space info-struct totally is 128 bytes would be good). Is it okey if
> > i add this in a revison 1? The packet dumping code doesn't depend on a
> > length. Only the target itself.
> 
> That was already requested by multiple people (including myself), so go
> ahead.
> 

Good.

> > 7. It would be very nice if we had a -m log. This is very easy todo in
> > kernel-space. Only that we have to call it -m LOG (uppercase) for userspace
> > to load the correct library and kernel module. Its just a matter of the
> > module registering both as a match and a target. Bad idea? It would be very
> > convinent. The match would just log the packet and always return true
> > (match). Compare this to the comment module that doesn't actually do any
> > match-work.
> 
> I think this is something that can wait for a infrastructure that is
> meant to do things like that.
> 

I already written it :-/. Havn't written userspace yet and not tested it.

> > 7. Currently i always use my LOGing with a limiter to not flood the
> > ringbuffer. I guess most people are. There would be very nice with a
> > --log-limit 5, meaning we will only log 5 packets per second. Also an
> > inverter for that so --log-limit ! 6 means to log everypacket hiting the
> > rule above 6 packet per second. This will go very well with LOG being a
> > match as well. As it simply selects which packets to log and always matches
> > them to continue in the firewall rules. Again this is a revison 1 thing. The
> > code for this in kernel is very simple. 
> 
> Whats wrong with using the limit match?

It fits with -m LOG (the match). You then can write: 

iptables -m match ... -m LOG --log-prefix 'monkey' --log-limit 5/s -j DROP 

Mening that we will drop every match matching the 'match' but only log a few
of them. This saves rules (and therefore CPU). I have tons of small
terminating chains just for LOGing and then DROPing the packet. Two targets.

Old iptables needs:
iptables -N logging_droping_chain
iptables -A chain -m match ... -j logging_droping_chain
iptables -A logging_droping_chain -m limit --limit 5/s -j (U)LOG --log-prefix 'monkey'
iptables -A logging_droping_chain -j DROP

Now, if you have a really large firewall, you have alot of terminating
matches likes these. Having a -m LOG (with a limiter to not flood of the
logging path) will save alot of memory and CPU-cycles oof already stressed
routers/firewalls.

The time to get -a action (or sometime similar) added isn't soon here. :-/
I'm not saying we need to get this in mainline kernel. I'll keep it as a
patch in my svn-repository if needed.

--
Joakim Axelsson

Re: ipt_LOG and friends. Question

[Patrick McHardy]

Joakim Axelsson wrote:
> 2006-08-28 12:57:13+0200, Patrick McHardy <kaber@trash.net> ->
> 
>>>2. I have merged ipt_LOG and ip6t_LOG into xt_LOG. With common
>>>dump_tcp()/udp() and some other small code. This also works good. 
>>
>>If it stays compatible and results in less code, that one too.
> 
> 
> Yes, it _should_ be the same. The code is somewhat changed but the result
> should produce exactly the same. It might have introduced some bug? So
> handing it in an early rc-tree of 2.6 or give me time to run the code on our
> router for a few weeks. What ever you like :-)

I'll look over it when you send it to make sure.

>>>3. The prefix is missing a space between the prefix and IN=interface. Very
>>>annoying to always have to use -j LOG --log-prefix "one two three ". Notice
>>>the last space. Will this break anything if i just add a space in between of
>>>"%sIN=%s OUT..." for the prefix?
>>
>>Yes, people already add the extra space and it would at least change
>>how log files look, potentially even break parsers.
>>
> 
> 
> Hm, hard one. I guess it's not enough to only change this to revision 1 as
> the parameter is the same and so on. And the code i wrote doesn't make any
> difference between revision 0 and 1 during the packet dumping functions.
> They are the same.
> 
> I guess the only way to solve this is to add a new flag in revision 1
> (beside keeping old --log-prefix with the missing space. --log-text-prefix,
> --log-text-sulfix ? I'll think of something.

That doesn't look to useful .. if I have to say --trailing-prefix-space
I can just as well just specify it in the prefix itself.

>>>7. It would be very nice if we had a -m log. This is very easy todo in
>>>kernel-space. Only that we have to call it -m LOG (uppercase) for userspace
>>>to load the correct library and kernel module. Its just a matter of the
>>>module registering both as a match and a target. Bad idea? It would be very
>>>convinent. The match would just log the packet and always return true
>>>(match). Compare this to the comment module that doesn't actually do any
>>>match-work.
>>
>>I think this is something that can wait for a infrastructure that is
>>meant to do things like that.
>>
> 
> 
> I already written it :-/. Havn't written userspace yet and not tested it.

Please leave it away for now. We can discuss this as a next step,
I guess it doesn't affect the other changes.

>>>7. Currently i always use my LOGing with a limiter to not flood the
>>>ringbuffer. I guess most people are. There would be very nice with a
>>>--log-limit 5, meaning we will only log 5 packets per second. Also an
>>>inverter for that so --log-limit ! 6 means to log everypacket hiting the
>>>rule above 6 packet per second. This will go very well with LOG being a
>>>match as well. As it simply selects which packets to log and always matches
>>>them to continue in the firewall rules. Again this is a revison 1 thing. The
>>>code for this in kernel is very simple. 
>>
>>Whats wrong with using the limit match?
> 
> 
> It fits with -m LOG (the match). You then can write: 
> 
> iptables -m match ... -m LOG --log-prefix 'monkey' --log-limit 5/s -j DROP 
> 
> Mening that we will drop every match matching the 'match' but only log a few
> of them. This saves rules (and therefore CPU). I have tons of small
> terminating chains just for LOGing and then DROPing the packet. Two targets.
> 
> Old iptables needs:
> iptables -N logging_droping_chain
> iptables -A chain -m match ... -j logging_droping_chain
> iptables -A logging_droping_chain -m limit --limit 5/s -j (U)LOG --log-prefix 'monkey'
> iptables -A logging_droping_chain -j DROP
> 
> Now, if you have a really large firewall, you have alot of terminating
> matches likes these. Having a -m LOG (with a limiter to not flood of the
> logging path) will save alot of memory and CPU-cycles oof already stressed
> routers/firewalls.
> 
> The time to get -a action (or sometime similar) added isn't soon here. :-/
> I'm not saying we need to get this in mainline kernel. I'll keep it as a
> patch in my svn-repository if needed.

Thats fine of course. Adding support for actions wouldn't be too hard
btw, but I don't want to open new construction sites in that area
while we haven't even got support for x_tables in userspace.

Re: ipt_LOG and friends. Question

[Joakim Axelsson]

> >>>3. The prefix is missing a space between the prefix and IN=interface. Very
> >>>annoying to always have to use -j LOG --log-prefix "one two three ". Notice
> >>>the last space. Will this break anything if i just add a space in between of
> >>>"%sIN=%s OUT..." for the prefix?
> >>
> >>Yes, people already add the extra space and it would at least change
> >>how log files look, potentially even break parsers.
> >>
> > 
> > Hm, hard one. I guess it's not enough to only change this to revision 1 as
> > the parameter is the same and so on. And the code i wrote doesn't make any
> > difference between revision 0 and 1 during the packet dumping functions.
> > They are the same.
> > 
> > I guess the only way to solve this is to add a new flag in revision 1
> > (beside keeping old --log-prefix with the missing space. --log-text-prefix,
> > --log-text-sulfix ? I'll think of something.
> 
> That doesn't look to useful .. if I have to say --trailing-prefix-space
> I can just as well just specify it in the prefix itself.
> 

I fixed this using some smartness. If the log prefix doesn't end in an space
and isn't empty, i'll add the space. This should break a minimum of scripts
as the only this will result in is an easier space to parse between prefix
and IN=iface. Should have been there before in prefix if the log was parsed
with scripts.


Next big problem: ip6tables doesn't have revisions. So xt_LOG in userspace
has problems in libip6t_LOG.c. When i try to just ignore this and register
revision 1 (ipv6) as the one and only revision i get the following error
when using ip6tables: 
"ip6tables: Protocol wrong type for socket."
What have i missed?
Greping after the error in iptables userspace code gives me nothing. I guess
this is a libc error. Where should i look?

(I atucally created the file libip46t_LOG.c that both libipt_LOG.c (ipv4)
and libipt6t_LOG.c (ipv6) #includes after defining either LIBIPT_LOG_IPV4 or
LIBIP6T_LOG_IPV6. Since the userspace code is the very same except for some
structs in the functions headers. This might be a somewhat ugly solution,
but atleast better to have two files with more or less the same code.)

Anyway, what is the best solution here. Adding revision-control for
ip6tables? Add x_tables for userspace? Huge work. Atleast for me. Were can I
cut some corners? Is it a big deal not keeping backward compability in
ip6tables (since we don't have revisions)?

--
Joakim Axelsson

Re: ipt_LOG and friends. Question

[Patrick McHardy]

Joakim Axelsson wrote:
> I fixed this using some smartness. If the log prefix doesn't end in an space
> and isn't empty, i'll add the space. This should break a minimum of scripts
> as the only this will result in is an easier space to parse between prefix
> and IN=iface. Should have been there before in prefix if the log was parsed
> with scripts.


That sounds like a good idea, but people might just as well expect not
to see any spaces. The LOG target has behaved this way for many years
now, and this minor annoyance is not worth breaking things. I consider
changing the default output format as not possible. Adding an option
for this doesn't make much sense either.

> Next big problem: ip6tables doesn't have revisions. So xt_LOG in userspace
> has problems in libip6t_LOG.c. When i try to just ignore this and register
> revision 1 (ipv6) as the one and only revision i get the following error
> when using ip6tables: 
> "ip6tables: Protocol wrong type for socket."
> What have i missed?
> Greping after the error in iptables userspace code gives me nothing. I guess
> this is a libc error. Where should i look?
> 
> (I atucally created the file libip46t_LOG.c that both libipt_LOG.c (ipv4)
> and libipt6t_LOG.c (ipv6) #includes after defining either LIBIPT_LOG_IPV4 or
> LIBIP6T_LOG_IPV6. Since the userspace code is the very same except for some
> structs in the functions headers. This might be a somewhat ugly solution,
> but atleast better to have two files with more or less the same code.)
> 
> Anyway, what is the best solution here. Adding revision-control for
> ip6tables? Add x_tables for userspace? Huge work. Atleast for me. Were can I
> cut some corners? Is it a big deal not keeping backward compability in
> ip6tables (since we don't have revisions)?


Same here, userspace needs to be kept compatible. ip6tables doesn't have
revision support currently, so the only way would be to add it. I know
Harald had some unfinished work for adding x_tables support (probably
including revision support for ip6tables) for userspace. I've CCed him,
maybe you can pick up on his work.

He's quite busy these days though, so it might take some time. If you
want to split of the printk logbuffer patch I would already submit it
for 2.6.18, its a good idea for netconsole independant of any xtables
unification.