ipt_ulog.h versus ebt_ulog.h

ipt_ulog.h versus ebt_ulog.h

[Amin Azez]

Is there a strong reason why ebt_ulog.h log structure has

       char physindev[IFNAMSIZ];
       char physoutdev[IFNAMSIZ];

and ipt_ulog.h structure does not support physdev?

Also interesting that ebt_ulog has
       struct timeval stamp;

but ipt_ulog has
       long timestamp_sec;
       long timestamp_usec;


I'd like to supply a patch to address these issues for kernel space and
user-space ulog tools, but I am soliciting feedback first.

I think the first addition is more important than the second change.

As a general principle although ebt_* /ipt_* network layer associations
exist, these exist for matching criteria and it seems a bit strong to
assume that just because the selection critera for the -J ULOG target
was impemented in ipt_* or ebt_* that the ULOG reader is not interested
in the entire frame. I mean this only to justify the change. I do also
realise the ulog is to some degree now depracated.

Sam

Re: ipt_ulog.h versus ebt_ulog.h

[Amin Azez]

This becomes more important when I see that ebt_ulog and upt_ULOG
modules can't both be loaded into the kernel at the same time, and it
must be confusing for ULOG clients to have different structs passed
through depending whether ebtables or iptables sent the message.

Sam

Amin Azez wrote:
> Is there a strong reason why ebt_ulog.h log structure has
> 
>        char physindev[IFNAMSIZ];
>        char physoutdev[IFNAMSIZ];
> 
> and ipt_ulog.h structure does not support physdev?
> 
> Also interesting that ebt_ulog has
>        struct timeval stamp;
> 
> but ipt_ulog has
>        long timestamp_sec;
>        long timestamp_usec;
> 
> 
> I'd like to supply a patch to address these issues for kernel space and
> user-space ulog tools, but I am soliciting feedback first.
> 
> I think the first addition is more important than the second change.
> 
> As a general principle although ebt_* /ipt_* network layer associations
> exist, these exist for matching criteria and it seems a bit strong to
> assume that just because the selection critera for the -J ULOG target
> was impemented in ipt_* or ebt_* that the ULOG reader is not interested
> in the entire frame. I mean this only to justify the change. I do also
> realise the ulog is to some degree now depracated.
> 
> Sam
> 
> 
> 

PATCH Re: ipt_ulog.h versus ebt_ulog.h

[Amin Azez]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

Do we need an equivalent patch for ulog2?

ipt_ULOG was logging the bridge device instead of the physical devices
(for bridged packets at least).

This patch does it's best to cause logging of the physical devices
involved. I realise the patch is not going to be handled as it addresses
ulog1 but I should ask if the problem of not logging physical devices is
also a problem with ulog2.

Harald; also does it make sense with ulog2 to talk about merging
ebt_ulog_packet_msg and ulog_packet_msg?

Sam



[-- Attachment #2: ulog.physdev2.patch --]
[-- Type: text/x-patch, Size: 1017 bytes --]

--- ./net/ipv4/netfilter/ipt_ULOG.c.prephysdev	2005-10-19 10:46:52.000000000 +0100
+++ ./net/ipv4/netfilter/ipt_ULOG.c	2005-10-19 11:55:49.000000000 +0100
@@ -244,14 +244,26 @@
 	} else
 		pm->mac_len = 0;
 
-	if (in)
-		strncpy(pm->indev_name, in->name, sizeof(pm->indev_name));
-	else
+	if (in) {
+#ifdef CONFIG_BRIDGE_NETFILTER
+		if (skb->nf_bridge) 
+			strncpy(pm->indev_name, &(skb->nf_bridge->physindev->name), sizeof(pm->indev_name));
+		else if (skb->input_dev) 
+			strncpy(pm->indev_name, &(skb->input_dev.name), sizeof(pm->indev_name));
+		else 
+#endif
+			strncpy(pm->indev_name, in->name, sizeof(pm->indev_name));
+	} else
 		pm->indev_name[0] = '\0';
 
-	if (out)
+	if (out) {
+#ifdef CONFIG_BRIDGE_NETFILTER
+		if (skb->nf_bridge) 
+			strncpy(pm->outdev_name, &(skb->nf_bridge->physoutdev->name), sizeof(pm->outdev_name));
+		else 
+#endif
 		strncpy(pm->outdev_name, out->name, sizeof(pm->outdev_name));
-	else
+	} else
 		pm->outdev_name[0] = '\0';
 
 	/* copy_len <= skb->len, so can't fail. */

Re: PATCH Re: ipt_ulog.h versus ebt_ulog.h

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

On Wed, Oct 19, 2005 at 02:05:27PM +0100, Amin Azez wrote:
> Do we need an equivalent patch for ulog2?

there is no ulog2 inside the kernel.  nfnetlink_log is the system that
obsoletes ULOG.  nfnetlink_log transports both, the indev/outdev _and_
the phys{in,out}dev to userspace.

> ipt_ULOG was logging the bridge device instead of the physical devices
> (for bridged packets at least).

yes, which is the correct behaviour from a technical point of view.

> Harald; also does it make sense with ulog2 to talk about merging
> ebt_ulog_packet_msg and ulog_packet_msg?

please look at nfnetlink_log.  It supersedes any of the old in-kernel
ulog stuff, and is totally independent of any fixed messages.

Since ipt_ULOG (and as far as my opinion goes, ebt_ulog as well) is
scheduled for feature removal,  I will not apply your patch.
-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: ipt_ulog.h versus ebt_ulog.h

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

On Wed, Oct 19, 2005 at 09:49:01AM +0100, Amin Azez wrote:
> Is there a strong reason why ebt_ulog.h log structure has
> 
>        char physindev[IFNAMSIZ];
>        char physoutdev[IFNAMSIZ];
> 
> and ipt_ulog.h structure does not support physdev?

it's historical, and we can't change it without hurting compatibility.

Since with 2.6.14, both ebt_ulog and ipt_ULOG are deprecated by
nfnetlink_log, I don't think it's worth fixing this up.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]