netdev - Re: [patch net-next v2 00/10] Add support for resource abstraction

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: <bc69ccb0-b98a-8e40-9600-bd5dc07ff96e@mellanox.com>
Date:   Mon, 1 Jan 2018 14:23:00 +0200
From:   Arkadi Sharshevsky <arkadis@...lanox.com>
To:     David Ahern <dsa@...ulusnetworks.com>,
        Yuval Mintz <yuvalm@...lanox.com>,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        Jiri Pirko <jiri@...nulli.us>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>, mlxsw <mlxsw@...lanox.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Michael Chan <michael.chan@...adcom.com>,
        "ganeshgr@...lsio.com" <ganeshgr@...lsio.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Matan Barak <matanb@...lanox.com>,
        Leon Romanovsky <leonro@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>,
        "jakub.kicinski@...ronome.com" <jakub.kicinski@...ronome.com>,
        "ast@...nel.org" <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Simon Horman <simon.horman@...ronome.com>,
        "pieter.jansenvanvuuren@...ronome.com" 
        <pieter.jansenvanvuuren@...ronome.com>,
        "john.hurley@...ronome.com" <john.hurley@...ronome.com>,
        Alexander Duyck <alexander.h.duyck@...el.com>,
        "John W. Linville" <linville@...driver.com>,
        Andy Gospodarek <gospo@...adcom.com>,
        Steve Lin <steven.lin1@...adcom.com>,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Shrijeet Mukherjee <shm@...ulusnetworks.com>,
        Andy Roulin <aroulin@...ulusnetworks.com>
Subject: Re: [patch net-next v2 00/10] Add support for resource abstraction



On 12/31/2017 05:46 PM, David Ahern wrote:
> On 12/31/17 3:52 AM, Arkadi Sharshevsky wrote:
>>> [1] This is allowed by the current patch set and perhaps it should not be:
>>>
>>> $ ip ro ls vrf vrf1101
>>> unreachable default metric 8192
>>> 11.2.51.0/24 dev swp1s0.51 proto kernel scope link src 11.2.51.1 offload
>>> 11.3.51.0/24 dev swp1s1.51 proto kernel scope link src 11.3.51.1 offload
>>> 11.4.51.0/24 dev swp1s2.51 proto kernel scope link src 11.4.51.1 offload
>>> 11.5.51.0/24 dev swp1s3.51 proto kernel scope link src 11.5.51.1 offload
>>> 11.6.51.0/24 dev swp3s0.51 proto kernel scope link src 11.6.51.1 offload
>>> 11.7.51.0/24 dev swp3s1.51 proto kernel scope link src 11.7.51.1 offload
>>> 11.8.51.0/24 dev swp3s2.51 proto kernel scope link src 11.8.51.1 offload
>>> 11.9.51.0/24 dev swp3s3.51 proto kernel scope link src 11.9.51.1 offload
>>>
>>> $ devlink resource set pci/0000:03:00.0 path /kvd/linear size 0
>>
>> This line actually did nothing, because size zero is not acceptable
>> see patch 6. This is pure userpsace problem that error is not shown.
> 
> Then perhaps you have a kernel side bug. After the reload I get this:
> 
> $ devlink resource show pci/0000:03:00.0
> pci/0000:03:00.0:
>   name kvd size 245760 size_valid true
>   resources:
>     name linear size 0 occ 0
>     name hash_double size 60416
>     name hash_single size 87040
> 

Actually no bug here, the linear can be zero. This implies no nexthop
routes. The adj table uses it as you can see.

> 
>>
>> You can verify it by dumping the resources and see that there is no
>> pending change (only size and not size_new).
>>
>>> $ devlink reload pci/0000:03:00.0
>>> $ ip ro ls vrf vrf1101
>>> unreachable default metric 8192
>>>
>>
>> So you just performed full reload of the driver which includes
>> unregistration of all the netdevs and full init. KVD update requires
>> full teardown of the driver.
> 
> you are right, I forgot to do a networking reload. Because of the above
> (0 was actually taken) all kinds of errors are spewed on 'ifreload -av'
> and there is no change to the ro ls:
> 
> $  ip ro ls vrf vrf1101
> unreachable default metric 8192
> 
>>
>> The system will not get back to the same state after reloading,
>> It's should be done on init. But it doesn't have to be like this
>> this, each driver provides his own reload devlink op implementation
>> so in our case full blown reset is required.
>>
>>
>>> [2] Same exact result for setting hash_double to 0:
>>> $ ip ro ls vrf vrf1101
>>> unreachable default metric 8192
>>> 11.2.51.0/24 dev swp1s0.51 proto kernel scope link src 11.2.51.1 offload
>>> 11.3.51.0/24 dev swp1s1.51 proto kernel scope link src 11.3.51.1 offload
>>> 11.4.51.0/24 dev swp1s2.51 proto kernel scope link src 11.4.51.1 offload
>>> 11.5.51.0/24 dev swp1s3.51 proto kernel scope link src 11.5.51.1 offload
>>> 11.6.51.0/24 dev swp3s0.51 proto kernel scope link src 11.6.51.1 offload
>>> 11.7.51.0/24 dev swp3s1.51 proto kernel scope link src 11.7.51.1 offload
>>> 11.8.51.0/24 dev swp3s2.51 proto kernel scope link src 11.8.51.1 offload
>>> 11.9.51.0/24 dev swp3s3.51 proto kernel scope link src 11.9.51.1 offload
>>>
>>> $ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 0
> 
> On this command you are correct, 0 is not taken:
> $ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 0
> $ devlink resource show pci/0000:03:00.0
> pci/0000:03:00.0:
>   name kvd size 245760 size_valid true
>   resources:
>     name linear size 0 occ 0
>     name hash_double size 60416
>     name hash_single size 87040
> 
> but the 'set' command did not fail with a proper extack based error
> message, so consider this another a bug report.
> 

Yeah, this is a bug, but a userspace one. I actually sniffed the nl
messages with nlmon and saw the extended ack packet with the required
string.