netdev - Re: [RFC PATCH net-next] failover: allow name change on IFF

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>] [day] [month] [year] [list]

Message-Id: <D8A06856-62A4-44BC-920E-9BE60011C9C1@oracle.com>
Date:   Thu, 7 Mar 2019 01:36:10 +0200
From:   Liran Alon <liran.alon@...cle.com>
To:     si-wei liu <si-wei.liu@...cle.com>
Cc:     "Samudrala, Sridhar" <sridhar.samudrala@...el.com>,
        Jiri Pirko <jiri@...nulli.us>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Jakub Kicinski <kubakici@...pl>,
        David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org,
        boris.ostrovsky@...cle.com, vijay.balakrishna@...cle.com
Subject: Re: [RFC PATCH net-next] failover: allow name change on IFF_UP slave
 interfaces



> On 6 Mar 2019, at 23:42, si-wei liu <si-wei.liu@...cle.com> wrote:
> 
> 
> 
> On 3/6/2019 1:36 PM, Samudrala, Sridhar wrote:
>> 
>> On 3/6/2019 1:26 PM, si-wei liu wrote:
>>> 
>>> 
>>> On 3/6/2019 4:04 AM, Jiri Pirko wrote:
>>>>> --- a/net/core/failover.c
>>>>> +++ b/net/core/failover.c
>>>>> @@ -16,6 +16,11 @@
>>>>> 
>>>>> static LIST_HEAD(failover_list);
>>>>> static DEFINE_SPINLOCK(failover_lock);
>>>>> +static bool slave_rename_ok = true;
>>>>> +
>>>>> +module_param(slave_rename_ok, bool, (S_IRUGO | S_IWUSR));
>>>>> +MODULE_PARM_DESC(slave_rename_ok,
>>>>> +		 "If set allow renaming the slave when failover master is up");
>>>>> 
>>>> No module parameters please. If you need to set something do it using
>>>> rtnl_link_ops. Thanks.
>>>> 
>>>> 
>>> I understand what you ask for, but without module parameters userspace don't work. During boot (dracut) the virtio netdev gets enslaved earlier than when userspace comes up, so failover has to determine the setting during initialization/creation. This config is not dynamic, at least for the life cycle of a particular failover link it shouldn't be changed. Without module parameter, how does the userspace specify this value during kernel initialization? 
>>> 
>> Can we enable this by default and not make it configurable via module parameter?
>> Is there any  usecase where someone expects rename to fail with failover slaves?
> Probably just cater for those application that assumes fixed name on UP interface?
> 
> It's already the default for the configurable. I myself don't think that's a big problem for failover users. So far there's not even QEMU support I think everything can be changed. I don't feel strong to just fix it without introducing configurable. But maybe Michael or others think it differently...
> 
> If no one objects, I don't feel strong to make it fixed behavior.
> 
> -Siwei
> 

I agree we should just remove the module parameter.

-Liran