From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arkadi Sharshevsky Subject: Re: [patch net-next v2 00/10] Add support for resource abstraction Date: Mon, 1 Jan 2018 14:23:00 +0200 Message-ID: References: <20171226112359.5313-1-jiri@resnulli.us> <20171227080902.GA1997@nanopsycho> <409891dc-4223-c037-d669-7ee85845d331@cumulusnetworks.com> <9c4f0583-3e5d-5684-87d5-1eaadfdb75b5@mellanox.com> <46bb1c77-9a0c-6a4b-2ea9-8cacfbe7eef6@cumulusnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "netdev@vger.kernel.org" , David Miller , mlxsw , Andrew Lunn , Vivien Didelot , Florian Fainelli , Michael Chan , "ganeshgr@chelsio.com" , Saeed Mahameed , Matan Barak , Leon Romanovsky , Ido Schimmel , "jakub.kicinski@netronome.com" , "ast@kernel.org" , Daniel Borkmann , Simon Horman , "pieter.jansenvanvuuren@netronome.com" , "john.hurley@netronome To: David Ahern , Yuval Mintz , Roopa Prabhu , Jiri Pirko Return-path: Received: from mail-eopbgr00064.outbound.protection.outlook.com ([40.107.0.64]:45058 "EHLO EUR02-AM5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750819AbeAAMXM (ORCPT ); Mon, 1 Jan 2018 07:23:12 -0500 In-Reply-To: <46bb1c77-9a0c-6a4b-2ea9-8cacfbe7eef6@cumulusnetworks.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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.