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 B3A1843E32; Wed, 10 Apr 2024 09:41:43 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4B39E402C5; Wed, 10 Apr 2024 09:41:43 +0200 (CEST) Received: from mail-ed1-f54.google.com (mail-ed1-f54.google.com [209.85.208.54]) by mails.dpdk.org (Postfix) with ESMTP id 6BE8D4029E for ; Wed, 10 Apr 2024 09:41:41 +0200 (CEST) Received: by mail-ed1-f54.google.com with SMTP id 4fb4d7f45d1cf-56e136cbcecso8884726a12.3 for ; Wed, 10 Apr 2024 00:41:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1712734901; x=1713339701; 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=9ZVzfpdi0Ogtt1IZvu6UG8I96BCVGNEArJpJXifQ6ko=; b=rpOpdK0ssu4g9GjbjurIm1qEwgNWbL0WjmXTIXF90emdUHqNlxbq/Sfz//nOt3Xce2 EY/1zDW8/QUbd8pwnL69lQiOkKzbPVjBa/iGJRbebBteKu+ovnEJk7+Z5tnAANSvZDnc cSVZXzWCQIxrTjMRNeZQM0uzzwjrvrHXtDOXp9qoAD8tT8QqrkQKTPAqfJlfmEmUF57l cr58TwkJRqtIGXMR95sEaBZWbL4U78LmkFQCCdbDsGLopqc3aghBQrz1OeSDjstjmXpK 118d19azf5nCvoXDRAoiwXeGyyZ4u+ah/oJc8xnvJLh920qHPPfTaFNwOMqWQRcZNpM5 OHww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712734901; x=1713339701; 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=9ZVzfpdi0Ogtt1IZvu6UG8I96BCVGNEArJpJXifQ6ko=; b=qdeotK3eZs6bxXSDiloTHTN23UEAhJ2zdbQtsXd9IO/xep60YOceIPtfi+T7Uh/VWp J/GFE7aHVFAuicp11Gz3aXglYOHOUNQQUXeAXFtoIeNq08q2pIWfCQjtibgk0mtbJ8ni 7OYLh7b0FqBuYz1czunTFouLhuXhnz8iCISVXCaQX3vBh+TSEfcaLYCGeEpxLIA4Oc3j WOTJo6bGE+rKln7OWpkD0WugWgj+XLO3x4BjR9GtVf0Oimv4D35Z4GM9KeSfXiX+pHTQ cfjK97RSUITILGMzGN7mnHUdUwqFWgCoFDbJIzplrDgxzgaat7qMrVJ42qUSLYI7PJ6o BTIw== X-Forwarded-Encrypted: i=1; AJvYcCWNWOmFolUHWg6Gq2zwo9513/0bmqsPDLK89uuGyE5rY0dmw8fp+rtXuiXwmmezwc/v0tODWUIQEXX7CP8= X-Gm-Message-State: AOJu0Yz3nRe2XV8L99o/ll60B3wJx5qwXyX/FOA8Nw4XMQINlTx3e1Kv upAVHhUd6fCLLS+uXsgJ1dW65ryP9QguinxH836pBXcn9LKLihSr0KLh5c9jnDAsb7TiW0+D2U3 xR5dlJfk7Ur4wCtUID6nKutN5IJ/nE4rpViawQg== X-Google-Smtp-Source: AGHT+IFEhFOqAhM4KLiDUtkcHXQ2QjDx/G2VZa/d7SAJlqABVTPk+0hhbXecwEOYDXXy5kUm5q3InbD/wSnjAzBSanQ= X-Received: by 2002:a17:907:7f88:b0:a51:9449:6fa1 with SMTP id qk8-20020a1709077f8800b00a5194496fa1mr1542238ejc.15.1712734900975; Wed, 10 Apr 2024 00:41:40 -0700 (PDT) MIME-Version: 1.0 References: <20240326190422.577028-1-luca.vizzarro@arm.com> <20240326190422.577028-7-luca.vizzarro@arm.com> In-Reply-To: From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Wed, 10 Apr 2024 09:41:29 +0200 Message-ID: Subject: Re: [PATCH 6/6] dts: add statefulness to TestPmdShell To: Jeremy Spewock 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 Thu, Mar 28, 2024 at 5:49=E2=80=AFPM Jeremy Spewock wrote: > > On Tue, Mar 26, 2024 at 3:04=E2=80=AFPM Luca Vizzarro wrote: > > > > This commit provides a state container for TestPmdShell. It currently > > only indicates whether the packet forwarding has started > > or not, and the number of ports which were given to the shell. > > > > This also fixes the behaviour of `wait_link_status_up` to use the > > command timeout as inherited from InteractiveShell. > > > > Signed-off-by: Luca Vizzarro > > Reviewed-by: Jack Bond-Preston > > Reviewed-by: Honnappa Nagarahalli > > --- > > > @@ -723,7 +731,13 @@ def _start_application(self, get_privileged_comman= d: Callable[[str], str] | None > > if self._app_args.app_params is None: > > self._app_args.app_params =3D TestPmdParameters() > > > > - self.number_of_ports =3D len(self._app_args.ports) if self._ap= p_args.ports is not None else 0 > > + assert isinstance(self._app_args.app_params, TestPmdParameters= ) > > + > > This is tricky because ideally we wouldn't have the assertion here, > but I understand why it is needed because Eal parameters have app args > which can be any instance of params. I'm not sure of the best way to > solve this, because making testpmd parameters extend from eal would > break the general scheme that you have in place, and having an > extension of EalParameters that enforces this app_args is > TestPmdParameters would solve the issues, but might be a little > clunky. Is there a way we can use a generic to get python to just > understand that, in this case, this will always be TestPmdParameters? > If not I might prefer making a private class where this is > TestPmdParameters, just because there aren't really any other > assertions that we use elsewhere and an unexpected exception from this > (even though I don't think that can happen) could cause people some > issues. > > It might be the case that an assertion is the easiest way to deal with > it though, what do you think? > We could change the signature (just the type of app_args) of the init method - I think we should be able to create a type that's EalParameters with .app_params being TestPmdParameters or None. The init method would just call super(). Something like the above is basically necessary with inheritance where subclasses are all extensions (not just implementations) of the superclass (having differences in API). > > + if self._app_args.app_params.auto_start: > > + self.state.packet_forwarding_started =3D True > > + > > + if self._app_args.ports is not None: > > + self.state.number_of_ports =3D len(self._app_args.ports) > > > > super()._start_application(get_privileged_command) > > > > > 2.34.1 > >