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 37BAD43F13; Fri, 26 Apr 2024 10:55:18 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B1EFD43AE0; Fri, 26 Apr 2024 10:55:17 +0200 (CEST) Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by mails.dpdk.org (Postfix) with ESMTP id 78886402EF for ; Fri, 26 Apr 2024 10:55:16 +0200 (CEST) Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-a5894c1d954so104854166b.1 for ; Fri, 26 Apr 2024 01:55:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1714121716; x=1714726516; 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=Uf3PimUZieEp0BJijtWIthLPipcrPUWNPuL1g6qKoZg=; b=TOcB/rdZFu2ZwnpGqyf0Ycj4RBk726wbev46nIJfGZ/TcAblfweT18yXuuMavspND/ FpruH/0MV6F1cz6F47/XovdVO3PyVW/uvlSYB1auEmTzD9TsvSPOjKBwJkUUU9wGAXG5 IHk7sZBuc3iIlTzV6FhTnoWOg4Km/tiOKUXLHmfxk4VlLB/1dS8X2+4wo2Wfh1Z7r6w6 IQcOD6co0xR0zvqaHQVOtQd+erRkmXqcu8geCA/TqUae7N0VLPF/TL517NpRDYD/ptam APrt2nHqnpVth0oRwTUlR1RWQJQc7670gD/qpe98p0y3dM61CZ0iB05f99KLTDofknMU aXfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714121716; x=1714726516; 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=Uf3PimUZieEp0BJijtWIthLPipcrPUWNPuL1g6qKoZg=; b=AvNEVi19e1S7VK89pe4RsgF8FBFFdYrq31MsNqck/8JLaeL9ZpG88Z8U81vt0v0yEp Tn/EcvER1Apaq0DloBwSSksZP18M3oQpFhb+NAD/sb5o5vDd27+OMBDICjh9DNNDqjpD skQvHP75/tcM4EuQPsODCT0j5xNG9d+RXwIwqgytILTwCj/rPhUYrZtSJ2s+wCTLJuFd YBvOqpDWi4aipAJZpB0bbf/OK4h47iyco5Ke6K8YfFyBA/SaZ9s+6hYicEVm7A8CPsvH aidkbnUhhSCFof16lEwql6LKaxXPCm6g3Lj1WTx5jp0IklfdNF//S++Y+kcMKMia7txw vjMg== X-Forwarded-Encrypted: i=1; AJvYcCU4ecsNGl3Pbhzx5MRVvY2q0bQSrQj/VuUYHsloKGST0dRbip8NdyWuLFDmolruar+eAxdDfS3klQrV6dE= X-Gm-Message-State: AOJu0Yx94rKVVFirgwxDyWH60HkCkxKYxrkqtKc2i3BCOYQDH+N0MNjS XVaEZoLR8Ca1ojUd6b25lnENnGtfX3683aLzsFC+0m9vaYmi8e/ChyXxWeZIC8udf6ncankQ3Y2 PMk24xepijuRAdolK+ceYquZjYt2Ga6vZwJUNcw== X-Google-Smtp-Source: AGHT+IHEScziBF5ivG560Qeuc6oeDpe1FPx25QYLU+YekR/sVQ6xzhgoxc+m0HfgDTNLbHshchyrllnJAQeR9s4qMnM= X-Received: by 2002:a50:bb2a:0:b0:56b:829a:38e3 with SMTP id y39-20020a50bb2a000000b0056b829a38e3mr1675237ede.16.1714121716041; Fri, 26 Apr 2024 01:55:16 -0700 (PDT) MIME-Version: 1.0 References: <20240419135216.106445-1-juraj.linkes@pantheon.tech> In-Reply-To: From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Fri, 26 Apr 2024 10:55:05 +0200 Message-ID: Subject: Re: [PATCH v1] dts: remove the OS UDP test suite To: Luca Vizzarro Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com, npratte@iol.unh.edu, dev@dpdk.org 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 Tue, Apr 23, 2024 at 2:00=E2=80=AFPM Juraj Linke=C5=A1 wrote: > > On Tue, Apr 23, 2024 at 12:39=E2=80=AFPM Luca Vizzarro wrote: > > > > On 19/04/2024 14:52, Juraj Linke=C5=A1 wrote: > > > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.p= y > > > > > > - def configure_testbed_ipv4(self, restore: bool =3D False) -> Non= e: > > > - """Configure IPv4 addresses on all testbed ports. > > > - > > > - The configured ports are: > > > - > > > - * SUT ingress port, > > > - * SUT egress port, > > > - * TG ingress port, > > > - * TG egress port. > > > - > > > - Args: > > > - restore: If :data:`True`, will remove the configuration = instead. > > > - """ > > > - delete =3D True if restore else False > > > - enable =3D False if restore else True > > > - self._configure_ipv4_forwarding(enable) > > > - self.sut_node.configure_port_ip_address( > > > - self._sut_ip_address_egress, self._sut_port_egress, dele= te > > > - ) > > > - self.sut_node.configure_port_state(self._sut_port_egress, en= able) > > > - self.sut_node.configure_port_ip_address( > > > - self._sut_ip_address_ingress, self._sut_port_ingress, de= lete > > > - ) > > > - self.sut_node.configure_port_state(self._sut_port_ingress, e= nable) > > > - self.tg_node.configure_port_ip_address( > > > - self._tg_ip_address_ingress, self._tg_port_ingress, dele= te > > > - ) > > > - self.tg_node.configure_port_state(self._tg_port_ingress, ena= ble) > > > - self.tg_node.configure_port_ip_address( > > > - self._tg_ip_address_egress, self._tg_port_egress, delete > > > - ) > > > - self.tg_node.configure_port_state(self._tg_port_egress, enab= le) > > > - > > > - def _configure_ipv4_forwarding(self, enable: bool) -> None: > > > - self.sut_node.configure_ipv4_forwarding(enable) > > > - > > > > > > diff --git a/dts/framework/testbed_model/linux_session.py b/dts/frame= work/testbed_model/linux_session.py > > > > > > - def configure_port_state(self, port: Port, enable: bool) -> None= : > > > - """Overrides :meth:`~.os_session.OSSession.configure_port_st= ate`.""" > > > - state =3D "up" if enable else "down" > > > - self.send_command(f"ip link set dev {port.logical_name} {sta= te}", privileged=3DTrue) > > > - > > > - def configure_port_ip_address( > > > - self, > > > - address: Union[IPv4Interface, IPv6Interface], > > > - port: Port, > > > - delete: bool, > > > - ) -> None: > > > - """Overrides :meth:`~.os_session.OSSession.configure_port_ip= _address`.""" > > > - command =3D "del" if delete else "add" > > > - self.send_command( > > > - f"ip address {command} {address} dev {port.logical_name}= ", > > > - privileged=3DTrue, > > > - verify=3DTrue, > > > - ) > > > - > > > > > > @@ -205,8 +185,3 @@ def configure_port_mtu(self, mtu: int, port: Port= ) -> None: > > > > > > - > > > - def configure_ipv4_forwarding(self, enable: bool) -> None: > > > - """Overrides :meth:`~.os_session.OSSession.configure_ipv4_fo= rwarding`.""" > > > - state =3D 1 if enable else 0 > > > - self.send_command(f"sysctl -w net.ipv4.ip_forward=3D{state}"= , privileged=3DTrue) > > > diff --git a/dts/framework/testbed_model/node.py b/dts/framework/test= bed_model/node.py > > > > > > - def configure_port_state(self, port: Port, enable: bool =3D True= ) -> None: > > > - """Enable/disable `port`. > > > - > > > - Args: > > > - port: The port to enable/disable. > > > - enable: :data:`True` to enable, :data:`False` to disable= . > > > - """ > > > - self.main_session.configure_port_state(port, enable) > > > - > > > - def configure_port_ip_address( > > > - self, > > > - address: Union[IPv4Interface, IPv6Interface], > > > - port: Port, > > > - delete: bool =3D False, > > > - ) -> None: > > > - """Add an IP address to `port` on this node. > > > - > > > - Args: > > > - address: The IP address with mask in CIDR format. Can be= either IPv4 or IPv6. > > > - port: The port to which to add the address. > > > - delete: If :data:`True`, will delete the address from th= e port instead of adding it. > > > - """ > > > - self.main_session.configure_port_ip_address(address, port, d= elete) > > > - > > > > > > diff --git a/dts/framework/testbed_model/os_session.py b/dts/framewor= k/testbed_model/os_session.py > > > > > > - @abstractmethod > > > - def configure_port_state(self, port: Port, enable: bool) -> None= : > > > - """Enable/disable `port` in the operating system. > > > - > > > - Args: > > > - port: The port to configure. > > > - enable: If :data:`True`, enable the port, otherwise shut= it down. > > > - """ > > > - > > > - @abstractmethod > > > - def configure_port_ip_address( > > > - self, > > > - address: Union[IPv4Interface, IPv6Interface], > > > - port: Port, > > > - delete: bool, > > > - ) -> None: > > > - """Configure an IP address on `port` in the operating system= . > > > - > > > - Args: > > > - address: The address to configure. > > > - port: The port to configure. > > > - delete: If :data:`True`, remove the IP address, otherwis= e configure it. > > > - """ > > > - > > > > > > diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/= testbed_model/sut_node.py > > > > > > - def configure_ipv4_forwarding(self, enable: bool) -> None: > > > - """Enable/disable IPv4 forwarding on the node. > > > - > > > - Args: > > > - enable: If :data:`True`, enable the forwarding, otherwis= e disable it. > > > - """ > > > - self.main_session.configure_ipv4_forwarding(enable) > > > - > > > > While I agree to remove unused code, especially code that will never be > > used again. Is it worthwhile removing already implemented features? > > Unless we don't foresee using them ever again... > > My thinking was that we don't need these for DPDK apps, but maybe we > need some of them for Scapy (or any other tg using the OS networking > stack). I'll verify this just to make sure we're not removing > something that Scapy needs. >From what I can tell, we don't need any of these for Scapy, but we should test this in other environments as well (my virtual environment doesn't play nice with port rebinding and running testpmd - I have to start Scapy after starting testpmd to get it to work. This may be a nightmare to debug.). Can you, Luca and Patrick, run the scatter test suite with this patch to verify this?