From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 347F743F3A; Mon, 29 Apr 2024 16:48:36 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 087274025C; Mon, 29 Apr 2024 16:48:36 +0200 (CEST) Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by mails.dpdk.org (Postfix) with ESMTP id 3E61B400D6 for ; Mon, 29 Apr 2024 16:48:35 +0200 (CEST) Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-1e5c7d087e1so39268795ad.0 for ; Mon, 29 Apr 2024 07:48:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1714402114; x=1715006914; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=+vbodCdfWd4a0vIXb3ijUpkgAfWELYYBcMMJgeAdkqg=; b=eqLgU/q4qsmr0k61UcvuDe597VxoRk7GgbDfWkpeav1EkBNn4im/Q7ycVBUJE7C2ib 4p/8QdeESlNb687D2u8+BSEg0QPs4S1ZQE//CN7BzBwndCZi5nxViSNMCnrqXUAUxg6f xKznwMQeVRGfQuV5b6ud6p3yP3yvCC6PvlULg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714402114; x=1715006914; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+vbodCdfWd4a0vIXb3ijUpkgAfWELYYBcMMJgeAdkqg=; b=KaeZa2xJ5SI/OH5YwErkMt6VTx6ywXvK2QGFRU+hxDV6HBEQTwn9IEXUzDGzLgcIPM Om6ke9+4viWjtfAid5Kahs/MeapDgR64W01xwoTVu5rQogQmWbVq5lQ3e0rDGsSXfxv4 d356k2Uoq34Nl+87h5/Mtgihego1PtuWPBVSgB1WG4tlzUXrJM+Y7GAVgdf8JyZrBDSi hxJRue7S67eK9+QtB7V+EpAbQJ03yeh97pHXj/KtEoisk2zpH3rB/1RgKcWL/xyYLGJp dRysXmoRexqsp0tUMz5oqRdD+kTN81DLL4qaL42SoqnJma21t1dtgwP2Ffq208YHGo/0 6PXA== X-Forwarded-Encrypted: i=1; AJvYcCWJOEKhJ2zDJhTIMDxW+Cvpvqrbx0xV+5z8JesQCeW/RsFWaI4fLcip7TP7eqsPzJdMOocEqorwxKYNu8k= X-Gm-Message-State: AOJu0YyKGVrxWxxz+3V3F186eJlk+DJu27LZuBO7sRgHO9ZEqpjszjLH PD8+0knbCOKE3nU7q5jXYQzc6Td+sDCou0FFfyFTF2ck3SWMKAYmvIAMO1zdk9OF147yA8rUENo ntYqPTje74x2BneI/s0ZNh0a8r0/lDvAC558M9A== X-Google-Smtp-Source: AGHT+IExpoiLtuEpJ/5XTD6zBmWrz5VEl2botnQAGOk6uTrIxHsFDuHykyYgYNyRCptp+wUdWR9U36eRRFKB+fnn6U8= X-Received: by 2002:a17:902:9684:b0:1e0:a3dd:82df with SMTP id n4-20020a170902968400b001e0a3dd82dfmr9784224plp.38.1714402114325; Mon, 29 Apr 2024 07:48:34 -0700 (PDT) MIME-Version: 1.0 References: <20240326190422.577028-1-luca.vizzarro@arm.com> <20240326190422.577028-6-luca.vizzarro@arm.com> In-Reply-To: From: Jeremy Spewock Date: Mon, 29 Apr 2024 10:48:23 -0400 Message-ID: Subject: Re: [PATCH 5/6] dts: add statefulness to InteractiveShell To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: Luca Vizzarro , dev@dpdk.org, Jack Bond-Preston , Honnappa Nagarahalli Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Apr 10, 2024 at 9:36=E2=80=AFAM Juraj Linke=C5=A1 wrote: > > On Wed, Apr 10, 2024 at 1:27=E2=80=AFPM Luca Vizzarro wrote: > > > > On 10/04/2024 07:53, Juraj Linke=C5=A1 wrote: > > > I have a general question. What are these changes for? Do you > > > anticipate us needing this in the future? Wouldn't it be better to ad= d > > > it only when we need it? > > > > It's been sometime since we raised this task internally. This patch and > > the next one arise from some survey done on old DTS test cases. > > Unfortunately, I can't pinpoint. > > > > Specifically for this patch though, the timeout bit is useful in > > conjunction with the related change in the next. Instead of giving an > > optional timeout argument to all the commands where we may want to > > change it, aren't we better off with providing a facility to temporaril= y > > change this for the current scope? > > > > This is a good question. If the scope is just one command, then no. If > it's more than one, then maybe yes. I don't know which is better. > > We should also consider that this would introduce a difference in API > between the interactive and non-interactive sessions. Do we want to do > this there as well? I believe there already is a difference in this case since the interactive shell doesn't support modification of timeout on a per-command basis. This is mainly because the way interactive shells handle timeouts is on a lower level than sending a command using fabric. Currently the interactive shells are modifying the timeout on the channel of the connection, whereas fabric supports a keyword argument that can modify timeouts on a per-command basis. Of course we could also change the interactive shell send_command to modify the timeout of the shell, but something else to note here is that changing the timeout of the channel of the connection is slightly different than giving a timeout for a command. This is because when you change the timeout of the channel you're setting the timeout for read/write operations on that channel. So, if you send a command and give a timeout of 5 seconds for example, as long as you are receiving output from the shell at least every 5 seconds, the command actually wouldn't ever timeout. If we want to make the interactive shell support passing a timeout per command, I would recommend we do it in a different way that is more representative of your *command* having a timeout instead of the shell's channel having a timeout. Waiting for the status of a link to be "up" in testpmd was an exception where I did allow you to give a specific timeout for the command and this is exactly for the reason above. I wanted to make sure that the user was able to specify how long they wanted to wait for this status to be what they expect as opposed to how long to wait for getting output from the channel. This is not supported in any other method of the interactive shell. > > Also, maybe set_timeout should be a property or we could just make > _timeout public. > And is_privileged should just be privileged, as it's a property (which > shouldn't contain a verb; if it was a method it would be a good name). > >