Re: [RFC PATCH] convert ebt_ulog to nfnetlink_log

(using webmail)

----- Oorspronkelijk bericht -----
Van: Harald Welte [mailto:laforge@netfilter.org]
Ok, so this is actually the opposite problem of the ipt_physdev.c
problem.  Here we have the physical device, but not the bridge group.

This means we have to handle two separate cases:

1) when nf_log_packet() gets called from the bridging code
indev == eth0
outdev == eth1
	-> we need to resolve br0 from dev->br_port member

2) when nf_log_packet() gets called from the ipv4 code:
indev == br0
outdev == br0
	-> we need to resolve ethX from skb->nf_bridge member

If I'm now correct, what about the following [reverse] patch (also
attached the whole resulting file for your reference, since you don't
have access to the tree).

There is one case missing: the brouter case. If br0=eth0+eth1 and a packet arrives at eth0 (not br0) in the IP code (not the bridge code), then the indev must be eth0, not br0.
How about something like this?

if (pf == PF_BRIDGE) { /* Called from ebtables */
	NFA_PUT(inst->skb, NFULA_IFINDEX_PHYSINDEV,
		sizeof(tmp_uint), &tmp_uint);
	tmp_uint = htonl(indev->br_port->br->dev->ifindex);
	NFA_PUT(inst->skb, NFULA_IFINDEX_INDEV,
		sizeof(tmp_uint), &tmp_uint);
} else {
	NFA_PUT(inst->skb, NFULA_IFINDEX_INDEV,
		sizeof(tmp_uint), &tmp_uint);

	if (skb->nf_bridge && skb->nf_bridge->physindev)
		/* Called inside bridge code, but not from ebtables. */
		tmp_uint = htonl(skb->nf_bridge->physindev->ifindex);
	NFA_PUT(inst->skb, NFULA_IFINDEX_PHYSINDEV,
		sizeof(tmp_uint), &tmp_uint);
}

cheers,
Bart

There is one case missing: the brouter case. If br0=eth0+eth1 and a
packet arrives at eth0 (not br0) in the IP code (not the bridge code),
then the indev must be eth0, not br0.  How about something like this?

Ok, I've implemented your suggested modifications now.

Dave: Please apply to your net-2.6.14 tree. Thanks!

-- 
- Harald Welte [off-list ref]                 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

On Mon, Aug 08, 2005 at 05:43:29PM +0000, Bart De Schuymer wrote:

Op ma, 08-08-2005 te 15:24 +0200, schreef Harald Welte:

quoted

quoted

There is one case missing: the brouter case. If br0=eth0+eth1 and a
packet arrives at eth0 (not br0) in the IP code (not the bridge code),
then the indev must be eth0, not br0.  How about something like this?

Ok, I've implemented your suggested modifications now.

There's still one small issue: if CONFIG_BRIDGE_NETFILTER isn't set in
the kernel configuration but ebtables is enabled, 

Doesn't ebtables attach to netfilter hooks? Ah, BRIDGE_NETFILTER
actually only selects the {ip,ip6,arp}tables emulation, not netfilter
support in bridging.  

then the physindev should still be filled in if ebt_ulog is used. I'm
afraid this will result in more ugly ifdef's.

well, If you can send me a patch for those ugly ifdef's after you get
back from holidays, I'll apply it.  Until then I think we can live
without that missing bit.

I don't mind making CONFIG_BRIDGE_NETFILTER mandatory for people wanting
to log the logical {in,out}put device, if you feel it would uglify the
code too much otherwise... 

No, I don't think we should force people to use certain config options
if they're technically not required.

-- 
- Harald Welte [off-list ref]                 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

Op ma, 08-08-2005 te 15:24 +0200, schreef Harald Welte:

quoted

There is one case missing: the brouter case. If br0=eth0+eth1 and a
packet arrives at eth0 (not br0) in the IP code (not the bridge code),
then the indev must be eth0, not br0.  How about something like this?

Ok, I've implemented your suggested modifications now.

There's still one small issue: if CONFIG_BRIDGE_NETFILTER isn't set in
the kernel configuration but ebtables is enabled, then the physindev
should still be filled in if ebt_ulog is used. I'm afraid this will
result in more ugly ifdef's.
I don't mind making CONFIG_BRIDGE_NETFILTER mandatory for people wanting
to log the logical {in,out}put device, if you feel it would uglify the
code too much otherwise... The {in,out}dev sent to userspace will then
be different depending on whether CONFIG_BRIDGE_NETFILTER is set or not.
People can still disable bridge-nf at runtime with the right /proc
entry. This should then be stated somewhere very clearly.

cheers,
Bart

From: Harald Welte <redacted>
Date: Mon, 8 Aug 2005 15:24:13 +0200

[NETFILTER] add correct bridging support to nfnetlink_{queue,log}

Applied.

Op ma, 08-08-2005 te 19:36 +0200, schreef Harald Welte:

quoted

then the physindev should still be filled in if ebt_ulog is used. I'm
afraid this will result in more ugly ifdef's.

well, If you can send me a patch for those ugly ifdef's after you get
back from holidays, I'll apply it.  Until then I think we can live
without that missing bit.

Perhaps I'd better wait then until I have access to the latest changes.
Transparency never has been the greatest asset of the netfilter
development.

cheers,
Bart