Re: [PATCH] ebtables: Port ebt_[u]log.c to nf[netlink]_log

On Mon, Oct 17, 2005 at 05:59:59PM +0000, Bart De Schuymer wrote:
> Op za, 08-10-2005 te 01:49 +0200, schreef Harald Welte:
> > Hi Bart!
> > 
> > The patch below is totally untested (though it compiles), and updates
> > ebtables to resemble the behaviour that we now have in ipv4 (and ipv6):
> > {ip,ip6,eb}tables just tell the nf_log core that they want to log a
> > packet, the mechanism (syslog, nfnetlink_log, ...) is actually decided
> > by nf_log.
> > 
> > By default, everything will behave like before.
> > 
> > Please review, and test that ebt_log and ebt_ulog are still working as
> > expected.  Thanks!
> 
> Sorry for the late reply, some hardware problems got in the way.

no problem, I probably hold the record of delayed responses, so I can
understand that completely ;)

> Apart from the comments below, the patch is fine by me (I tested both).

great.

> > +	nf_log_packet(PF_BRIDGE, hooknr, skb, in, out, &li, info->prefix);
> 
> Should be ebt_log_packet

why is that?  nf_log_packet() is a function provided by the netfilter
core in net/netfilter/.  Do you want an ebt_log_packet() wrapper function that just calls
nf_log_packet() ?

> >  {
> > -	return ebt_register_watcher(&log);
> > +	int ret;
> > +
> > +	ret = ebt_register_watcher(&log);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (nf_log_register(PF_BRIDGE, &ebt_log_logger) < 0) {
> > +		printk(KERN_WARNING "ebt_log: not logging via system console "
> > +		       "since somebody else already registered for PF_INET\n");
> > +		/* wecannot make module load fail here, since otherwise 
> > +		 * ebtables userspace would abort */
> > +	}
> 
> Since we're using PF_BRIDGE instead of PF_INET now, this if construct
> can be replaced by a simple call to nf_log_register.

No, I think we only fix the comment (state PF_BRIDGE in the comment) but
leave it like it is.

The issues is, when (in chronological order)

1) someone starts their logging daemon (e.g. ulogd2)
2) the daemon is configured to nf_log_register() for PF_BRIDGE
3) then the ruleset is loaded, which automatically modprobe's ebt_log.ko
4) ebt_log wants to nf_log_register() for PF_BRIDGE

I think we should print some message to syslog to tell the use (once)
that logging will not be done via the system console, even though he
uses the "log" watcher (which traditionally always logged via syslog).

Comments?

-- 
- Harald Welte <laforge-Cap9r6Oaw4JrovVCs/[email protected]>                 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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQFDVLlNXaXGVTD0i/8RApTfAJ9PCRWAyCfrRHGpYidTjD4RtZyqWgCeK6Ir
55etc3je4r1v/oTq+Na/gdk=
=TMIJ
-----END PGP SIGNATURE-----