LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH net] ipvs: Improve robustness to the ipvs sysctl

To: Florian Westphal <fw@xxxxxxxxx>
Subject: Re: [PATCH net] ipvs: Improve robustness to the ipvs sysctl
Cc: <wensong@xxxxxxxxxxxx>, <horms@xxxxxxxxxxxx>, <ja@xxxxxx>, <pablo@xxxxxxxxxxxxx>, <lvs-devel@xxxxxxxxxxxxxxx>, <netfilter-devel@xxxxxxxxxxxxxxx>, Mingfangsen <mingfangsen@xxxxxxxxxx>, <wangxiaogang3@xxxxxxxxxx>, <xuhanbing@xxxxxxxxxx>
From: hujunwei <hujunwei4@xxxxxxxxxx>
Date: Mon, 29 Jul 2019 22:58:45 +0800
Hi Julian, thank you for replay.

On 2019/7/29 8:49, Florian Westphal wrote:
> hujunwei <hujunwei4@xxxxxxxxxx> wrote:
> 
> [ trimmed CC list ]
> 
>> The ipvs module parse the user buffer and save it to sysctl,
>> then check if the value is valid. invalid value occurs
>> over a period of time.
>> Here, I add a variable, struct ctl_table tmp, used to read
>> the value from the user buffer, and save only when it is valid.
> 
> Does this cause any problems?  If so, what are those?
> 
For example, when a negative number value occurs over a period of time,
the func such as ip_vs_sync_conn_v0() will get invalid number
by sysctl_sync_threshold(), casue judge abnormal in ip_vs_sync_conn_needed().

>> Fixes: f73181c8288f ("ipvs: add support for sync threads")
>> Signed-off-by: Junwei Hu <hujunwei4@xxxxxxxxxx>
>> ---
>>  net/netfilter/ipvs/ip_vs_ctl.c | 61 +++++++++++++++++++++++-----------
>>  1 file changed, 42 insertions(+), 19 deletions(-)
>>
>> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
>> index 741d91aa4a8d..e78fd05f108b 100644
>> --- a/net/netfilter/ipvs/ip_vs_ctl.c
>> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
>> @@ -1680,12 +1680,18 @@ proc_do_defense_mode(struct ctl_table *table, int 
>> write,
>>      int val = *valp;
>>      int rc;
>>
>> -    rc = proc_dointvec(table, write, buffer, lenp, ppos);
>> +    struct ctl_table tmp = {
>> +            .data = &val,
>> +            .maxlen = sizeof(int),
>> +            .mode = table->mode,
>> +    };
>> +
>> +    rc = proc_dointvec(&tmp, write, buffer, lenp, ppos);
> 
> Wouldn't it be better do use proc_dointvec_minmax and set the
> constraints via .extra1,2 in the sysctl knob definition?
> 
You are right, proc_dointvec_minmax seems like a better choice, I will update 
the patch.

Regards,
Junwei


<Prev in Thread] Current Thread [Next in Thread>