Re: 2.6.15-rc1: NET_CLS_U32 not working?

Re: 2.6.15-rc1: NET_CLS_U32 not working?

[Adrian Bunk]

On Thu, Nov 17, 2005 at 12:10:17AM +0100, Christian wrote:
> hi,
> 
> i noticed that some of my QoS rules are not working any more. oh, i 
> forgot to enable CONFIG_NET_CLS_U32. but when enabled, i got the 
> following errors when compiling / installing the module:
> 
> * Warning: "unregister_tcf_proto_ops" [net/sched/cls_u32.ko] undefined!
> * Warning: "register_tcf_proto_ops" [net/sched/cls_u32.ko] undefined!
> * Warning: "tcf_exts_dump" [net/sched/cls_u32.ko] undefined!
> * Warning: "tcf_exts_dump_stats" [net/sched/cls_u32.ko] undefined!
> * Warning: "tcf_exts_change" [net/sched/cls_u32.ko] undefined!
> * Warning: "tcf_exts_validate" [net/sched/cls_u32.ko] undefined!
> * Warning: "tcf_exts_destroy" [net/sched/cls_u32.ko] undefined!
>...
> when i disabled CONFIG_NET_CLS_U32, everything compiles fine, but 
> cls_u32 is missing of course :-(
> 
> all the missing symbols seem to be defined in include/net/pkt_cls.h. but 
>  this file is #included by net/sched/cls_u32.c and other too, so i 
> don't really know, why it doesn't work.
> 
> FWIW, i see EXPORT_SYMBOLs at the very end of net/sched/cls_api.c, but i 
> can't see if/when cls_api.c is used (included?) at all.
>...


I'm assuming you are trying to insert the new module in your old kernel?

This is one of the unfortunate but hardly avoidable cases where adding a 
module requires installing a new kernel.


But there's a change in 2.6.15-rc1 that makes this issue much worse:
It is no longer user-visible.

tristate's select'ing bool's that do not change parts of the (modular) 
driver but compile additional code into the kernel are simply wrong.


> thanks for looking into that,
> Christian.
>...


cu
Adrian

BTW: Please Cc netdev@vger.kernel.org on networking issues.

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: 2.6.15-rc1: NET_CLS_U32 not working?

[Christian]

Adrian Bunk schrieb:
> 
> I'm assuming you are trying to insert the new module in your old kernel?

yes, i tried to "modprobe" the compiled cls_u32 module. but the "make 
modules" errors are there anyway. i tried to compile a fresh 2.6.15-rc1 
on a different machine (where i can't do "modprobe") and the errors were 
there too: http://nerdbynature.de/bits/sheep/2.6.15-rc1/make-modules.log

> This is one of the unfortunate but hardly avoidable cases where adding a 
> module requires installing a new kernel.

despite of the errors on "make modules" i'll reboot with the "new" 
kernel asap.

> BTW: Please Cc netdev@vger.kernel.org on networking issues.

ok, will do that.


thank you,
Christian.
-- 
BOFH excuse #442:

Trojan horse ran out of hay

Re: 2.6.15-rc1: NET_CLS_U32 not working?

[Christian]

Christian schrieb:
> Adrian Bunk schrieb:
>>
>> I'm assuming you are trying to insert the new module in your old kernel?

yes, rebuilding the whole kernel (after just enabling NET_CLS_U32 as a 
module) makes the warnings go away.

>> This is one of the unfortunate but hardly avoidable cases where adding 
>> a module requires installing a new kernel.

i wonder why/if this is really needed. although not critical, this 
behaviour is pretty annoying....

thanks,
Christian.
-- 
BOFH excuse #24:

network packets travelling uphill (use a carrier pigeon)

[2.6 patch] do not select NET_CLS

[Adrian Bunk]

>...
> But there's a change in 2.6.15-rc1 that makes this issue much worse:
> It is no longer user-visible.
> 
> tristate's select'ing bool's that do not change parts of the (modular) 
> driver but compile additional code into the kernel are simply wrong.

The patch below (should apply against 2.6.15-rc2) fixes this issue.

cu
Adrian


<--  snip  -->


2.6.15-rc changes NET_CLS to being automatically select'ed when needed.

This patch confuses users since NET_CLS is a bool, and compiling an 
additional module that select's NET_CLS causes unresolved symbols since 
it's not user-visible that adding a module changes the kernel image.

This patch therefore changes NET_CLS back to the 2.6.14 status quo of 
being an user-visible option.


Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

 net/sched/Kconfig |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

--- linux-2.6.15-rc1-mm2-full/net/sched/Kconfig.old	2005-11-21 16:39:04.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/net/sched/Kconfig	2005-11-21 16:41:59.000000000 +0100
@@ -254,11 +254,23 @@
 comment "Classification"
 
 config NET_CLS
-	boolean
+	bool "Packet classifier API"
+	help
+	  The CBQ scheduling algorithm requires that network packets which are
+	  scheduled to be sent out over a network device be classified
+	  according to some criterion. If you say Y here, you will get a
+	  choice of several different packet classifiers with the following
+	  questions.
+
+	  This will enable you to use Differentiated Services (diffserv) and
+	  Resource Reservation Protocol (RSVP) on your Linux router.
+	  Documentation and software is at
+	  <http://diffserv.sourceforge.net/>.
+
+if NET_CLS
 
 config NET_CLS_BASIC
 	tristate "Elementary classification (BASIC)"
-	select NET_CLS
 	---help---
 	  Say Y here if you want to be able to classify packets using
 	  only extended matches and actions.
@@ -268,7 +280,6 @@
 
 config NET_CLS_TCINDEX
 	tristate "Traffic-Control Index (TCINDEX)"
-	select NET_CLS
 	---help---
 	  Say Y here if you want to be able to classify packets based on
 	  traffic control indices. You will want this feature if you want
@@ -280,7 +291,6 @@
 config NET_CLS_ROUTE4
 	tristate "Routing decision (ROUTE)"
 	select NET_CLS_ROUTE
-	select NET_CLS
 	---help---
 	  If you say Y here, you will be able to classify packets
 	  according to the route table entry they matched.
@@ -293,7 +303,6 @@
 
 config NET_CLS_FW
 	tristate "Netfilter mark (FW)"
-	select NET_CLS
 	---help---
 	  If you say Y here, you will be able to classify packets
 	  according to netfilter/firewall marks.
@@ -303,7 +312,6 @@
 
 config NET_CLS_U32
 	tristate "Universal 32bit comparisons w/ hashing (U32)"
-	select NET_CLS
 	---help---
 	  Say Y here to be able to classify packetes using a universal
 	  32bit pieces based comparison scheme.
@@ -326,7 +334,6 @@
 
 config NET_CLS_RSVP
 	tristate "IPv4 Resource Reservation Protocol (RSVP)"
-	select NET_CLS
 	select NET_ESTIMATOR
 	---help---
 	  The Resource Reservation Protocol (RSVP) permits end systems to
@@ -341,7 +348,6 @@
 
 config NET_CLS_RSVP6
 	tristate "IPv6 Resource Reservation Protocol (RSVP6)"
-	select NET_CLS
 	select NET_ESTIMATOR
 	---help---
 	  The Resource Reservation Protocol (RSVP) permits end systems to
@@ -356,7 +362,6 @@
 
 config NET_EMATCH
 	bool "Extended Matches"
-	select NET_CLS
 	---help---
 	  Say Y here if you want to use extended matches on top of classifiers
 	  and select the extended matches below.
@@ -541,6 +546,8 @@
 	  automaticaly selected if needed but can be selected manually for
 	  statstical purposes.
 
+endif # NET_CLS
+
 endif # NET_SCHED
 
 endmenu

Re: [2.6 patch] do not select NET_CLS

[Patrick McHardy]

Adrian Bunk wrote:
> 2.6.15-rc changes NET_CLS to being automatically select'ed when needed.
> 
> This patch confuses users since NET_CLS is a bool, and compiling an 
> additional module that select's NET_CLS causes unresolved symbols since 
> it's not user-visible that adding a module changes the kernel image.
> 
> This patch therefore changes NET_CLS back to the 2.6.14 status quo of 
> being an user-visible option.

I disagree with this patch. NET_CLS enables the infrastructure support
for classifiers. Users generally don't care about infrastructure but
directly usable things, so I'd prefer to have it automatically selected. 
And there are lots of other cases where enabling a module causes changes
in the kernel image. Some examples include some of the netfilter stuff,
the IPsec transforms, NET_CLS_ROUTE4, the ieee80211 stuff, and a lot
more.

Re: [2.6 patch] do not select NET_CLS

[David S. Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 21 Nov 2005 17:16:18 +0100

> Adrian Bunk wrote:
> > This patch therefore changes NET_CLS back to the 2.6.14 status quo of 
> > being an user-visible option.
> 
> I disagree with this patch. NET_CLS enables the infrastructure support
> for classifiers. Users generally don't care about infrastructure but
> directly usable things, so I'd prefer to have it automatically selected. 
> And there are lots of other cases where enabling a module causes changes
> in the kernel image. Some examples include some of the netfilter stuff,
> the IPsec transforms, NET_CLS_ROUTE4, the ieee80211 stuff, and a lot
> more.

I agree with Patrick.

Changing config options of any kind can result in the main kernel
image needing to be rebuilt.  One thing we can do to prevent human
mistakes, is to make the "make modules" pass do a quick "is vmlinux
uptodate?" check, and if not print out an error message explaining the
situation and aborting the "make modules" attempt.

Re: [2.6 patch] do not select NET_CLS

[Sam Ravnborg]

On Tue, Nov 22, 2005 at 02:37:13PM -0800, David S. Miller wrote:
> 
> One thing we can do to prevent human
> mistakes, is to make the "make modules" pass do a quick "is vmlinux
> uptodate?" check, and if not print out an error message explaining the
> situation and aborting the "make modules" attempt.


I do not quite follow you here.

For a while I have considered implementing something that told why a
given file was compiled - like:

 CC     net/ipv4/ip_gre.o   due to net/dsfield.h, net/xfrm.h
 CC     net/ipv4/raw.c   due to include/config/ip/mroute.h

The latter is a config option that I do not see a possibility to change
back to a config option syntax (at least not without doing some effort).

My thinking was that 'make V=2' would give above printout.

But what you request is something that keep the dense printout without
building the kernel - right?

Any suggestion for an intuitive syntax to enable that?
'make -n V=2' will not do it.

	Sam

Re: [2.6 patch] do not select NET_CLS

[David S. Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Tue, 22 Nov 2005 23:49:14 +0100

> On Tue, Nov 22, 2005 at 02:37:13PM -0800, David S. Miller wrote:
> > 
> > One thing we can do to prevent human
> > mistakes, is to make the "make modules" pass do a quick "is vmlinux
> > uptodate?" check, and if not print out an error message explaining the
> > situation and aborting the "make modules" attempt.
> 
> I do not quite follow you here.

If the user tries to do a "make modules" without first rebuilding
their kernel image, then the make will fail if the dependencies
are not satisfied for the main kernel image and it is thus not
up to date.

Re: [2.6 patch] do not select NET_CLS

[Sam Ravnborg]

On Tue, Nov 22, 2005 at 03:00:41PM -0800, David S. Miller wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> Date: Tue, 22 Nov 2005 23:49:14 +0100
> 
> > On Tue, Nov 22, 2005 at 02:37:13PM -0800, David S. Miller wrote:
> > > 
> > > One thing we can do to prevent human
> > > mistakes, is to make the "make modules" pass do a quick "is vmlinux
> > > uptodate?" check, and if not print out an error message explaining the
> > > situation and aborting the "make modules" attempt.
> > 
> > I do not quite follow you here.
> 
> If the user tries to do a "make modules" without first rebuilding
> their kernel image, then the make will fail if the dependencies
> are not satisfied for the main kernel image and it is thus not
> up to date.

OK - so a simple 'make -q vmlinux' check, except that the way we utilise
make will let it fail at first build command.
That will obscufate things even more in kbuild - but I will give it a
try sometime. It will be easy to cover 95% but to reach 100%
predictability will be though.
- file dependencies is easy
- command line changes is relatively easy
- but the various scripts and user commands will be tricky..

Not on the top of the TODO list though.

	Sam

Re: [2.6 patch] do not select NET_CLS

[Randy.Dunlap]

On Wed, 23 Nov 2005 06:57:35 +0100 Sam Ravnborg wrote:

> On Tue, Nov 22, 2005 at 03:00:41PM -0800, David S. Miller wrote:
> > From: Sam Ravnborg <sam@ravnborg.org>
> > Date: Tue, 22 Nov 2005 23:49:14 +0100
> > 
> > > On Tue, Nov 22, 2005 at 02:37:13PM -0800, David S. Miller wrote:
> > > > 
> > > > One thing we can do to prevent human
> > > > mistakes, is to make the "make modules" pass do a quick "is vmlinux
> > > > uptodate?" check, and if not print out an error message explaining the
> > > > situation and aborting the "make modules" attempt.
> > > 
> > > I do not quite follow you here.
> > 
> > If the user tries to do a "make modules" without first rebuilding
> > their kernel image, then the make will fail if the dependencies
> > are not satisfied for the main kernel image and it is thus not
> > up to date.
> 
> OK - so a simple 'make -q vmlinux' check, except that the way we utilise
> make will let it fail at first build command.
> That will obscufate things even more in kbuild - but I will give it a
> try sometime. It will be easy to cover 95% but to reach 100%
> predictability will be though.
> - file dependencies is easy
> - command line changes is relatively easy
> - but the various scripts and user commands will be tricky..
> 
> Not on the top of the TODO list though.

So -q means "quick" ?

I wouldn't mind seeing a -quiet (even less quiet than V=0),
not even printing the CC, AS, LD, etc. commands -- just let the
tools print errors & warnings.  I always redirect output to a
disk file and filter it for errors and warnings anyway, so I
want hold my breath for this, but ISTM that V=0 could be even
quieter than it is right now.

---
~Randy

Re: [2.6 patch] do not select NET_CLS

[Sam Ravnborg]

On Wed, Nov 23, 2005 at 06:13:32PM -0800, Randy.Dunlap wrote:
> 
> So -q means "quick" ?
>From make help:
-q, --question Run no commands; exit status says if up to date

> I wouldn't mind seeing a -quiet (even less quiet than V=0),
> not even printing the CC, AS, LD, etc. commands -- just let the
> tools print errors & warnings.  I always redirect output to a
> disk file and filter it for errors and warnings anyway, so I
> want hold my breath for this, but ISTM that V=0 could be even
> quieter than it is right now.

make -s shuld give you this. I've not used it for long though...

	Sam

Re: [2.6 patch] do not select NET_CLS

[Adrian Bunk]

On Tue, Nov 22, 2005 at 02:37:13PM -0800, David S. Miller wrote:
>...
> Changing config options of any kind can result in the main kernel
> image needing to be rebuilt.  One thing we can do to prevent human
> mistakes, is to make the "make modules" pass do a quick "is vmlinux
> uptodate?" check, and if not print out an error message explaining the
> situation and aborting the "make modules" attempt.

This won't work with CONFIG_IKCONFIG=y.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] do not select NET_CLS

[Thomas Graf]

* David S. Miller <davem@davemloft.net> 2005-11-22 14:37
> From: Patrick McHardy <kaber@trash.net>
> Date: Mon, 21 Nov 2005 17:16:18 +0100
> 
> > Adrian Bunk wrote:
> > > This patch therefore changes NET_CLS back to the 2.6.14 status quo of 
> > > being an user-visible option.
> > 
> > I disagree with this patch. NET_CLS enables the infrastructure support
> > for classifiers. Users generally don't care about infrastructure but
> > directly usable things, so I'd prefer to have it automatically selected. 
> > And there are lots of other cases where enabling a module causes changes
> > in the kernel image. Some examples include some of the netfilter stuff,
> > the IPsec transforms, NET_CLS_ROUTE4, the ieee80211 stuff, and a lot
> > more.
> 
> I agree with Patrick.

In fact Patrick's explanation was exactly the motivation for me to
change it in the first place a few weeks back.

I thought about making cls_api aware to be built as module but then
the same would apply for ematches and the generic scheduling code
and it gets real complicated. It is possible with some heavy code
shuffling but not worth it I think.