QoS: Do not try to unload any kernel modules

Message ID 20211102105310.22809-1-michael.tremer@ipfire.org
State Accepted
Commit 7091738a5c3c5a95d358f5da498493914eeaf688
Headers show
Series QoS: Do not try to unload any kernel modules | expand

Commit Message

Michael Tremer Nov. 2, 2021, 10:53 a.m. UTC
Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---
 config/qos/makeqosscripts.pl | 2 --
 1 file changed, 2 deletions(-)

Comments

Peter Müller Nov. 17, 2021, 8:15 p.m. UTC | #1
Hello Michael,

could you elaborate more on this one?

 From my basic understanding, QoS should not try to unload kernel modules indeed,
but I did not get why it was doing so in the first place.

Thanks, and best regards,
Peter Müller


> Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
> ---
>   config/qos/makeqosscripts.pl | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/config/qos/makeqosscripts.pl b/config/qos/makeqosscripts.pl
> index 0694bb78c..3234ab366 100644
> --- a/config/qos/makeqosscripts.pl
> +++ b/config/qos/makeqosscripts.pl
> @@ -510,8 +510,6 @@ print <<END
>   	iptables -t mangle --flush  QOS-INC >/dev/null 2>&1
>   	iptables -t mangle --delete-chain QOS-INC >/dev/null 2>&1
>   
> -	rmmod sch_htb >/dev/null 2>&1
> -
>   	for i in \$(ls \$RRDLOG/class_*.rrd); do
>   		rrdtool update \$i \$(date +%s):
>   	done
>
Michael Tremer Nov. 18, 2021, 11:14 a.m. UTC | #2
Hello,

> On 17 Nov 2021, at 20:15, Peter Müller <peter.mueller@ipfire.org> wrote:
> 
> Hello Michael,
> 
> could you elaborate more on this one?

Sure.

> From my basic understanding, QoS should not try to unload kernel modules indeed,
> but I did not get why it was doing so in the first place.

Unloading kernel modules used to be an old thing to do. I have no idea really why what the motivation was.

Partly to save memory, and partly to not have unused module loaded since hardware for example was being detected by loading every single module and see if it recognised any hardware.

Here we unload the HTB scheduler simply because we do not need it any more after the QoS was stopped. However, during an update, it might happen that the QoS needs to be restarted. When the kernel has been updated in the same updater, we no longer have the kernel modules available for the running kernel. Since sch_htb was unloaded, we can no longer load it again. That was the motivation for this patch here. At least QoS continues to work until the system is being rebooted after the update.

Does this make sense?

-Michael

> 
> Thanks, and best regards,
> Peter Müller
> 
> 
>> Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
>> ---
>>  config/qos/makeqosscripts.pl | 2 --
>>  1 file changed, 2 deletions(-)
>> diff --git a/config/qos/makeqosscripts.pl b/config/qos/makeqosscripts.pl
>> index 0694bb78c..3234ab366 100644
>> --- a/config/qos/makeqosscripts.pl
>> +++ b/config/qos/makeqosscripts.pl
>> @@ -510,8 +510,6 @@ print <<END
>>  	iptables -t mangle --flush  QOS-INC >/dev/null 2>&1
>>  	iptables -t mangle --delete-chain QOS-INC >/dev/null 2>&1
>>  -	rmmod sch_htb >/dev/null 2>&1
>> -
>>  	for i in \$(ls \$RRDLOG/class_*.rrd); do
>>  		rrdtool update \$i \$(date +%s):
>>  	done
Peter Müller Nov. 19, 2021, 6:26 a.m. UTC | #3
Hello Michael,

thanks for your reply.

This makes sense to me.

Reviewed-by: Peter Müller <peter.mueller@ipfire.org>

Thanks, and best regards,
Peter Müller


> Hello,
> 
>> On 17 Nov 2021, at 20:15, Peter Müller <peter.mueller@ipfire.org> wrote:
>>
>> Hello Michael,
>>
>> could you elaborate more on this one?
> 
> Sure.
> 
>> From my basic understanding, QoS should not try to unload kernel modules indeed,
>> but I did not get why it was doing so in the first place.
> 
> Unloading kernel modules used to be an old thing to do. I have no idea really why what the motivation was.
> 
> Partly to save memory, and partly to not have unused module loaded since hardware for example was being detected by loading every single module and see if it recognised any hardware.
> 
> Here we unload the HTB scheduler simply because we do not need it any more after the QoS was stopped. However, during an update, it might happen that the QoS needs to be restarted. When the kernel has been updated in the same updater, we no longer have the kernel modules available for the running kernel. Since sch_htb was unloaded, we can no longer load it again. That was the motivation for this patch here. At least QoS continues to work until the system is being rebooted after the update.
> 
> Does this make sense?
> 
> -Michael
> 
>>
>> Thanks, and best regards,
>> Peter Müller
>>
>>
>>> Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
>>> ---
>>>  config/qos/makeqosscripts.pl | 2 --
>>>  1 file changed, 2 deletions(-)
>>> diff --git a/config/qos/makeqosscripts.pl b/config/qos/makeqosscripts.pl
>>> index 0694bb78c..3234ab366 100644
>>> --- a/config/qos/makeqosscripts.pl
>>> +++ b/config/qos/makeqosscripts.pl
>>> @@ -510,8 +510,6 @@ print <<END
>>>  	iptables -t mangle --flush  QOS-INC >/dev/null 2>&1
>>>  	iptables -t mangle --delete-chain QOS-INC >/dev/null 2>&1
>>>  -	rmmod sch_htb >/dev/null 2>&1
>>> -
>>>  	for i in \$(ls \$RRDLOG/class_*.rrd); do
>>>  		rrdtool update \$i \$(date +%s):
>>>  	done
>

Patch

diff --git a/config/qos/makeqosscripts.pl b/config/qos/makeqosscripts.pl
index 0694bb78c..3234ab366 100644
--- a/config/qos/makeqosscripts.pl
+++ b/config/qos/makeqosscripts.pl
@@ -510,8 +510,6 @@  print <<END
 	iptables -t mangle --flush  QOS-INC >/dev/null 2>&1
 	iptables -t mangle --delete-chain QOS-INC >/dev/null 2>&1
 
-	rmmod sch_htb >/dev/null 2>&1
-
 	for i in \$(ls \$RRDLOG/class_*.rrd); do
 		rrdtool update \$i \$(date +%s):
 	done