Re: [PATCH v1] dts: remove the OS UDP test suite

[Luca Vizzarro <Luca.Vizzarro@arm.com>]

On 19/04/2024 14:52, Juraj Linkeš wrote:
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> <snip>
> -    def configure_testbed_ipv4(self, restore: bool = False) -> None:
> -        """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 = True if restore else False
> -        enable = 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, delete
> -        )
> -        self.sut_node.configure_port_state(self._sut_port_egress, enable)
> -        self.sut_node.configure_port_ip_address(
> -            self._sut_ip_address_ingress, self._sut_port_ingress, delete
> -        )
> -        self.sut_node.configure_port_state(self._sut_port_ingress, enable)
> -        self.tg_node.configure_port_ip_address(
> -            self._tg_ip_address_ingress, self._tg_port_ingress, delete
> -        )
> -        self.tg_node.configure_port_state(self._tg_port_ingress, enable)
> -        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, enable)
> -
> -    def _configure_ipv4_forwarding(self, enable: bool) -> None:
> -        self.sut_node.configure_ipv4_forwarding(enable)
> -
> <snip>
> diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
> <snip>
> -    def configure_port_state(self, port: Port, enable: bool) -> None:
> -        """Overrides :meth:`~.os_session.OSSession.configure_port_state`."""
> -        state = "up" if enable else "down"
> -        self.send_command(f"ip link set dev {port.logical_name} {state}", privileged=True)
> -
> -    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 = "del" if delete else "add"
> -        self.send_command(
> -            f"ip address {command} {address} dev {port.logical_name}",
> -            privileged=True,
> -            verify=True,
> -        )
> -
 > <snip>
> @@ -205,8 +185,3 @@ def configure_port_mtu(self, mtu: int, port: Port) -> None:
 > <snip>
> -
> -    def configure_ipv4_forwarding(self, enable: bool) -> None:
> -        """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
> -        state = 1 if enable else 0
> -        self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
> <snip>
> -    def configure_port_state(self, port: Port, enable: bool = 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 = 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 the port instead of adding it.
> -        """
> -        self.main_session.configure_port_ip_address(address, port, delete)
> -
> <snip>
> diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
> <snip>
> -    @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, otherwise configure it.
> -        """
> -
> <snip>
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> <snip>
> -    def configure_ipv4_forwarding(self, enable: bool) -> None:
> -        """Enable/disable IPv4 forwarding on the node.
> -
> -        Args:
> -            enable: If :data:`True`, enable the forwarding, otherwise 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...