2021-01-02 20:17:42

by Valdis Klētnieks

[permalink] [raw]
Subject: Kconfig, DEFAULT_NETSCH, and shooting yourself in the foot..

Consider the following own goal I just discovered I scored:

[~] zgrep -i fq_codel /proc/config.gz
CONFIG_NET_SCH_FQ_CODEL=m
CONFIG_DEFAULT_FQ_CODEL=y
CONFIG_DEFAULT_NET_SCH="fq_codel"

Obviously, fq_codel didn't get set as the default, because that happens
before the module gets loaded (which may never happen if the sysadmin
thinks the DEFAULT_NET_SCH already made it happen)

Whoops. My bad, probably - but....

The deeper question, part 1:

There's this chunk in net/sched/Kconfig:

config DEFAULT_NET_SCH
string
default "pfifo_fast" if DEFAULT_PFIFO_FAST
default "fq" if DEFAULT_FQ
default "fq_codel" if DEFAULT_FQ_CODEL
default "fq_pie" if DEFAULT_FQ_PIE
default "sfq" if DEFAULT_SFQ
default "pfifo_fast"
endif

(And a similar chunk right above it with a similar issue)

Should those be "if (foo=y)" so =m can't be chosen? (I'll be
happy to write the patch if that's what we want)

Deeper question, part 2:

Should there be a way in the Kconfig language to ensure that
these two chunks can't accidentally get out of sync? There's other
places in the kernel where similar issues arise - a few days ago I was
chasing a CPU governor issue where it looked like it was possible
to set a default that was a module and thus possibly not actually loaded.



Attachments:
(No filename) (849.00 B)

2021-01-03 06:34:37

by Masahiro Yamada

[permalink] [raw]
Subject: Re: Kconfig, DEFAULT_NETSCH, and shooting yourself in the foot..

On Sun, Jan 3, 2021 at 5:14 AM Valdis Klētnieks <[email protected]> wrote:
>
> Consider the following own goal I just discovered I scored:
>
> [~] zgrep -i fq_codel /proc/config.gz
> CONFIG_NET_SCH_FQ_CODEL=m
> CONFIG_DEFAULT_FQ_CODEL=y
> CONFIG_DEFAULT_NET_SCH="fq_codel"
>
> Obviously, fq_codel didn't get set as the default, because that happens
> before the module gets loaded (which may never happen if the sysadmin
> thinks the DEFAULT_NET_SCH already made it happen)
>
> Whoops. My bad, probably - but....
>
> The deeper question, part 1:
>
> There's this chunk in net/sched/Kconfig:
>
> config DEFAULT_NET_SCH
> string
> default "pfifo_fast" if DEFAULT_PFIFO_FAST
> default "fq" if DEFAULT_FQ
> default "fq_codel" if DEFAULT_FQ_CODEL
> default "fq_pie" if DEFAULT_FQ_PIE
> default "sfq" if DEFAULT_SFQ
> default "pfifo_fast"
> endif
>
> (And a similar chunk right above it with a similar issue)
>
> Should those be "if (foo=y)" so =m can't be chosen? (I'll be
> happy to write the patch if that's what we want)
>
> Deeper question, part 2:
>
> Should there be a way in the Kconfig language to ensure that
> these two chunks can't accidentally get out of sync? There's other
> places in the kernel where similar issues arise - a few days ago I was
> chasing a CPU governor issue where it looked like it was possible
> to set a default that was a module and thus possibly not actually loaded.
>


If there is a restriction where a modular discipline cannot be the default,
I think you can add 'depends on FOO = y'.



For example,


choice
prompt "Default"

config DEFAULT_FOO
bool "Use foo for default"
depends on FOO = y

config DEFAULT_BAR
bool "Use bar for default"
depends on BAR = y

config DEFAULT_FALLBACK
bool "fallback when nothing else is builtin"

endchoice





--
Best Regards
Masahiro Yamada

2021-01-03 20:11:24

by Stephen Hemminger

[permalink] [raw]
Subject: Re: Kconfig, DEFAULT_NETSCH, and shooting yourself in the foot..

On Sun, 3 Jan 2021 15:30:30 +0900
Masahiro Yamada <[email protected]> wrote:

> On Sun, Jan 3, 2021 at 5:14 AM Valdis Klētnieks <[email protected]> wrote:
> >
> > Consider the following own goal I just discovered I scored:
> >
> > [~] zgrep -i fq_codel /proc/config.gz
> > CONFIG_NET_SCH_FQ_CODEL=m
> > CONFIG_DEFAULT_FQ_CODEL=y
> > CONFIG_DEFAULT_NET_SCH="fq_codel"
> >
> > Obviously, fq_codel didn't get set as the default, because that happens
> > before the module gets loaded (which may never happen if the sysadmin
> > thinks the DEFAULT_NET_SCH already made it happen)
> >
> > Whoops. My bad, probably - but....
> >
> > The deeper question, part 1:
> >
> > There's this chunk in net/sched/Kconfig:
> >
> > config DEFAULT_NET_SCH
> > string
> > default "pfifo_fast" if DEFAULT_PFIFO_FAST
> > default "fq" if DEFAULT_FQ
> > default "fq_codel" if DEFAULT_FQ_CODEL
> > default "fq_pie" if DEFAULT_FQ_PIE
> > default "sfq" if DEFAULT_SFQ
> > default "pfifo_fast"
> > endif
> >
> > (And a similar chunk right above it with a similar issue)
> >
> > Should those be "if (foo=y)" so =m can't be chosen? (I'll be
> > happy to write the patch if that's what we want)
> >
> > Deeper question, part 2:
> >
> > Should there be a way in the Kconfig language to ensure that
> > these two chunks can't accidentally get out of sync? There's other
> > places in the kernel where similar issues arise - a few days ago I was
> > chasing a CPU governor issue where it looked like it was possible
> > to set a default that was a module and thus possibly not actually loaded.
> >
>
>
> If there is a restriction where a modular discipline cannot be the default,
> I think you can add 'depends on FOO = y'.
>
>
>
> For example,
>
>
> choice
> prompt "Default"
>
> config DEFAULT_FOO
> bool "Use foo for default"
> depends on FOO = y
>
> config DEFAULT_BAR
> bool "Use bar for default"
> depends on BAR = y
>
> config DEFAULT_FALLBACK
> bool "fallback when nothing else is builtin"
>
> endchoice
>
>
>
>
>

You can use a qdisc that is a module, it just has to be available when device
is loaded. Typically that means putting it in initramfs.

2021-01-03 20:14:43

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: Kconfig, DEFAULT_NETSCH, and shooting yourself in the foot..

On Sun, 03 Jan 2021 10:20:11 -0800, Stephen Hemminger said:
> You can use a qdisc that is a module, it just has to be available when device
> is loaded. Typically that means putting it in initramfs.

Apparently, that's not *quite* true regarding the default qdisc, because I
hit this situation (copying from another email):

---
[/proc/sys/net] cat core/default_qdisc
fq_codel

[/proc/sys/net] tc -s qdisc show
qdisc noqueue 0: dev lo root refcnt 2
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
qdisc pfifo_fast 0: dev eth0 root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
qdisc noqueue 0: dev wlan0 root refcnt 2
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

But if I give it a kick in the head...

[/proc/sys/net] tc qdisc add dev wlan0 root fq_codel
[/proc/sys/net] tc -s qdisc show dev wlan0
qdisc fq_codel 8001: root refcnt 2 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn
drop_batch 64
Sent 812 bytes 12 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
maxpacket 0 drop_overlimit 0 new_flow_count 0 ecn_mark 0
new_flows_len 0 old_flows_len 0
---

Note that wlan0 doesn't actually get plumbed up until after the initramfs
has done its thing and we're quite late in the boot process and the /lib/modules
on the production system is accessible, so it doesn't look like an implicit modprobe
gets done in that case - fq_codel.ko was very much available when the device
was brought up.


Attachments:
(No filename) (849.00 B)