From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5D920F4F2; Tue, 2 Jan 2024 12:21:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="o6Pnu8tb" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 68BC3C433C7; Tue, 2 Jan 2024 12:21:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704198070; bh=vQv1f+Php0cWB9KLjYNv0YlDWbsApg3u1Zg0uonTTP8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=o6Pnu8tb/b6BkKvwKRyZg03sI23WqFc3OabZZrlcKFYmy7eowWjZf4pkHCYLNmFK8 iyEIA6jhFe0psI7nS4kKWu7AlfnuDPRSQovwijopXhWJvkD9VkyRtoLSvcLnIc5e9S 5hZsijjvmaaNQIKD15srPDWy0ME3pyZQYiucWemNobjCEP6ueQ6QFpE21r84x8xySg t2h71fVfTROjRjCMnUJ10FWqQPWZu/AqdEAzWWScYodSy9PNNDl3o+5N8jCYt69ud8 imJG/P6ZffgeI7aOWMc/lmKM4kMoRaV6HFR5KaHW4T04aDyIxaIv7QDFY96jIcmkLX 4/kBUb32514KA== Date: Tue, 2 Jan 2024 14:21:06 +0200 From: Leon Romanovsky To: Chengchang Tang Cc: Stephen Hemminger , Junxian Huang , jgg@ziepe.ca, dsahern@gmail.com, netdev@vger.kernel.org, linux-rdma@vger.kernel.org, linuxarm@huawei.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH iproute2-rc 1/2] rdma: Fix core dump when pretty is used Message-ID: <20240102122106.GI6361@unreal> References: <20231229065241.554726-1-huangjunxian6@hisilicon.com> <20231229065241.554726-2-huangjunxian6@hisilicon.com> <20231229092129.25a526c4@hermes.local> <30d8c237-953a-8794-9baa-e21b31d4d88c@huawei.com> <20240102083257.GB6361@unreal> <29146463-6d0e-21c5-af42-217cee760b3f@huawei.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <29146463-6d0e-21c5-af42-217cee760b3f@huawei.com> On Tue, Jan 02, 2024 at 08:06:19PM +0800, Chengchang Tang wrote: > > > On 2024/1/2 16:32, Leon Romanovsky wrote: > > On Tue, Jan 02, 2024 at 03:44:29PM +0800, Chengchang Tang wrote: > > > > > > On 2023/12/30 1:21, Stephen Hemminger wrote: > > > > On Fri, 29 Dec 2023 14:52:40 +0800 > > > > Junxian Huang wrote: > > > > > > > > > From: Chengchang Tang > > > > > > > > > > There will be a core dump when pretty is used as the JSON object > > > > > hasn't been opened and closed properly. > > > > > > > > > > Before: > > > > > $ rdma res show qp -jp -dd > > > > > [ { > > > > > "ifindex": 1, > > > > > "ifname": "hns_1", > > > > > "port": 1, > > > > > "lqpn": 1, > > > > > "type": "GSI", > > > > > "state": "RTS", > > > > > "sq-psn": 0, > > > > > "comm": "ib_core" > > > > > }, > > > > > "drv_sq_wqe_cnt": 128, > > > > > "drv_sq_max_gs": 2, > > > > > "drv_rq_wqe_cnt": 512, > > > > > "drv_rq_max_gs": 1, > > > > > rdma: json_writer.c:130: jsonw_end: Assertion `self->depth > 0' failed. > > > > > Aborted (core dumped) > > > > > > > > > > After: > > > > > $ rdma res show qp -jp -dd > > > > > [ { > > > > > "ifindex": 2, > > > > > "ifname": "hns_2", > > > > > "port": 1, > > > > > "lqpn": 1, > > > > > "type": "GSI", > > > > > "state": "RTS", > > > > > "sq-psn": 0, > > > > > "comm": "ib_core",{ > > > > > "drv_sq_wqe_cnt": 128, > > > > > "drv_sq_max_gs": 2, > > > > > "drv_rq_wqe_cnt": 512, > > > > > "drv_rq_max_gs": 1, > > > > > "drv_ext_sge_sge_cnt": 256 > > > > > } > > > > > } ] > > > > > > > > > > Fixes: 331152752a97 ("rdma: print driver resource attributes") > > > > > Signed-off-by: Chengchang Tang > > > > > Signed-off-by: Junxian Huang > > > > This code in rdma seems to be miking json and newline functionality > > > > which creates bug traps. > > > > > > > > Also the json should have same effective output in pretty and non-pretty mode. > > > > It looks like since pretty mode add extra object layer, the nesting of {} would be > > > > different. > > > > > > > > The conversion to json_print() was done but it isn't using same conventions > > > > as ip or tc. > > > > > > > > The correct fix needs to go deeper and hit other things. > > > > > > > Hi, Stephen, > > > > > > The root cause of this issue is that close_json_object() is being called in > > > newline_indent(), resulting in a mismatch > > > of {}. > > > > > > When fixing this problem, I was unsure why a newline() is needed in pretty > > > mode, so I simply kept this logic and > > > solved the issue of open_json_object() and close_json_object() not matching. > > > However, If the output of pretty mode > > > and not-pretty mode should be the same, then this problem can be resolved by > > > deleting this newline_indent(). > > Stephen didn't say that output of pretty and not-pretty should be the > > same, but he said that JSON logic should be the same. > > > > Thanks > > Hi, Leon, > > Thank you for your reply. But I'm not sure what you mean by JSON logic? I > understand that > pretty and not-pretty JSON should have the same content, but just difference > display effects. > Do you mean that they only need to have the same structure? > > Or, let's get back to this question. In the JSON format output, the > newline() here seems > unnecessary, because json_print() can solve the line break problems during > printing. > So I think the newline() here can be removed at least when outputting in > JSON format. I think that your original patch is correct way to fix the mismatch as it is not related to pretty/non-pretty. Thanks > > Thanks, > Chengchang Tang > > > > > I believe the original developer may not have realized that > > > close_json_object() is being called in newline(), which leads > > > to this problem. To improve the code's readability, I would try to strip out > > > close_json_obejct() from newline(). > > > > > > Thanks, > > > Chengchang Tang > > > > > . > > > >