netdev - Re: [PATCH net 1/3] net_sched: sch_sfq: use a temporary work area for validating configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5f493420-d7ff-43ab-827f-30e66b7df2c9@redhat.com>
Date: Tue, 1 Apr 2025 11:26:55 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Cong Wang <xiyou.wangcong@...il.com>, Octavian Purdila <tavip@...gle.com>
Cc: jhs@...atatu.com, jiri@...nulli.us, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, horms@...nel.org, shuah@...nel.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH net 1/3] net_sched: sch_sfq: use a temporary work area for
 validating configuration

On 3/31/25 1:49 AM, Cong Wang wrote:
> On Fri, Mar 28, 2025 at 01:16:32PM -0700, Octavian Purdila wrote:
>> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
>> index 65d5b59da583..027a3fde2139 100644
>> --- a/net/sched/sch_sfq.c
>> +++ b/net/sched/sch_sfq.c
>> @@ -631,6 +631,18 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
>>  	struct red_parms *p = NULL;
>>  	struct sk_buff *to_free = NULL;
>>  	struct sk_buff *tail = NULL;
>> +	/* work area for validating changes before committing them */
>> +	struct {
>> +		int limit;
>> +		unsigned int divisor;
>> +		unsigned int maxflows;
>> +		int perturb_period;
>> +		unsigned int quantum;
>> +		u8 headdrop;
>> +		u8 maxdepth;
>> +		u8 flags;
>> +	} tmp;
> 
> Thanks for your patch. It reminds me again about the lacking of complete
> RCU support in TC. ;-)
> 
> Instead of using a temporary struct, how about introducing a new one
> called struct sfq_sched_opt and putting it inside struct sfq_sched_data?
> It looks more elegant to me.

I agree with that. It should also make the code more compact. @Octavian,
please update the patch as per Cong's suggestion.

Thanks,

Paolo