Re: [PATCH] QoS: Do not try to unload any kernel modules

["Peter Müller" <peter.mueller@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 2079 bytes --]

Hello Michael,

thanks for your reply.

This makes sense to me.

Reviewed-by: Peter Müller <peter.mueller(a)ipfire.org>

Thanks, and best regards,
Peter Müller


> Hello,
> 
>> On 17 Nov 2021, at 20:15, Peter Müller <peter.mueller(a)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(a)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
>