[jamal]

On Wed, 13 Feb 2002, Doron Oz wrote:

> True - for a module, where nf_register_hook is called from init_module.
> But if compiled-in, nf_register_hook is called whenever a new ingress
> qdisc is installed, by ingress_init - which will fail.

It will succeed the first time, in the caxe of a module in init_module and
in the case of static in ingress_init. It will fail always after that.
This last part is a little annoyance but considering thsi is
initialization, not costly.

cheers,
jamal

>
> Doron.
>
> -----Original Message-----
> From: jamal [mailto:hadi@cyberus.ca]
> Sent: Wednesday, February 13, 2002 3:16 PM
> To: Doron Oz
> Cc: 'Harald Welte'; kuznet@ms2.inr.ac.ru;
> netfilter-devel@lists.samba.org
> Subject: RE:
>
>
>
>
> On Wed, 13 Feb 2002, Doron Oz wrote:
>
> > That's it? But nf_register_hook will return an error, which will cause
>
> > ingress_init to fail, in case of more than one ingress qdisc. Or am I
> > missing something again?
> >
>
> The error is returned if you call nf_register_hook() more than once. The
> module only gets installed once. Does that sound reasonable?
>
> cheers,
> jamal
>
> > Doron.
> >
> >
> > -----Original Message-----
> > From: jamal [mailto:hadi@cyberus.ca]
> > Sent: Tuesday, February 12, 2002 2:39 PM
> > To: Doron Oz
> > Cc: 'Harald Welte'; 'David S. Miller'; kuznet@ms2.inr.ac.ru;
> > netfilter-devel@lists.samba.org
> > Subject: RE:
> >
> >
> >
> >
> > Doron,
> >
> > On Tue, 12 Feb 2002, Doron Oz wrote:
> >
> > > This seems insufficient. It solves just one side of the problem -
> > > not registering a hook twice. But the ingress code is not aware that
>
> > > it is
> >
> > > registered only once, and when an ingress qdisc is destroyed it will
>
> > > unregister the hook, even though other ingress qdiscs still exist.
> > > Or does it come with a patch to sch_ingress as well?
> >
> > yes, the last bit of the patch takes care of this:
> >
> > ---
> > -#ifndef MODULE
> > -       nf_unregister_hook(&ing_ops);
> > -#endif
> > ---
> >
> > a registered ingress hook never gets unregistered in the static setup.
>
> > This is fine in my opinion. In the case of a module, MOD_DEC_USE_COUNT
>
> > takes care of things. The module will be cleaned up once all the
> > ingress qdiscs are unregistered for all devices.
> >
> > cheers,
> > jamal
> >
> > >
> > > My original patch was a simple one - the ingress code registers
> > > itself
> >
> > > once, and only once, and never unregisters (same behaviour as loaded
>
> > > as a module).
> > >
> > > Doron.
> > >
> > > -----Original Message-----
> > > From: jamal [mailto:hadi@cyberus.ca]
> > > Sent: Monday, February 11, 2002 4:08 PM
> > > To: Harald Welte
> > > Cc: David S. Miller; kuznet@ms2.inr.ac.ru;
> > > netfilter-devel@lists.samba.org; Doron Oz
> > > Subject:
> > >
> > >
> > >
> > >
> > > Dave,
> > > Dont apply the ingress patch yet; I wanted Doron to comment (we had
> > > a long discussion but i cant find my archives on the subject).
> > > Doron, does this cover all grounds we discussed at the time?
> > >
> > > cheers,
> > > jamal
> > >
> > > On Mon, 11 Feb 2002, Harald Welte wrote:
> > >
> > > > On Sat, Feb 09, 2002 at 08:10:01AM -0500, Jamal Hadi Selim wrote:
> > > >
> > > > Hi Jamal & Dave.
> > > >
> > > > > Dave,
> > > > > If Harald would propose something for nf_register_hook() then
> > > > > the ifdef MODULE would dissapear as well.
> > > >
> > > > I do agree with jamal's original posting - we should return an
> > > > error
> >
> > > > in case somebody tries to register the same thing a second time.
> > > >
> > > > However, the additional checks about the valid range of hook
> > > > numbers
> >
> > > > introduced ipv4 specific code assumed that IPv4 is the only
> > > > protocol
> >
> > > > having netfilter hooks - which is an invalid assumption.
> > > >
> > > > I've reduced his patch to just the "don't register twice" part:
> > > >
> > > > --- linuxppc-190102-plain/net/core/netfilter.c	Thu Jan 17
> > > 21:44:29 2002
> > > > +++ linuxppc-190102-nfpom/net/core/netfilter.c	Mon Feb 11
> > > 12:14:12 2002
> > > > @@ -61,6 +61,15 @@
> > > >  	for (i = nf_hooks[reg->pf][reg->hooknum].next;
> > > >  	     i != &nf_hooks[reg->pf][reg->hooknum];
> > > >  	     i = i->next) {
> > > > +		/* assumption: If exact same thing already on the list
> > > > +		 * we dont want to add a new one
> > > > +		 */
> > > > +		if ((reg->priority == ((struct nf_hook_ops
> > > *)i)->priority)
> > > > +		&& (reg->hook == (((struct nf_hook_ops  *)i)->hook))) {
> > > > +			br_write_unlock_bh(BR_NETPROTO_LOCK);
> > > > +			return -1;
> > > > +		}
> > > > +
> > > >  		if (reg->priority < ((struct nf_hook_ops *)i)->priority)
> > > >  			break;
> > > >  	}
> > > >
> > > >
> > > > Please apply.
> > > >
> > > > > cheers,
> > > > > jamal
> > > >
> > > > --
> > > > Live long and prosper
> > > > - Harald Welte / laforge@gnumonks.org
> > > http://www.gnumonks.org/
> > > > ==================================================================
> > > > ==
> > > > ==
> > > > ======
> > > > GCS/E/IT d- s-: a-- C+++ UL++++$ P+++ L++++$ E--- W- N++ o? K-
> w---
> > O-
> > > M+
> > > > V-- PS++ PE-- Y++ PGP++ t+ 5-- !X !R tv-- b+++ !DI !D G+ e* h---
> > > > r++
> > > y+(*)
> > > >
> > >
> >
>