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 4155443347; Thu, 16 Nov 2023 19:34:37 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 072E8402E0; Thu, 16 Nov 2023 19:34:37 +0100 (CET) Received: from mail-ed1-f43.google.com (mail-ed1-f43.google.com [209.85.208.43]) by mails.dpdk.org (Postfix) with ESMTP id A5E97402CB for ; Thu, 16 Nov 2023 19:34:35 +0100 (CET) Received: by mail-ed1-f43.google.com with SMTP id 4fb4d7f45d1cf-543456dbd7bso4501356a12.1 for ; Thu, 16 Nov 2023 10:34:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1700159675; x=1700764475; 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=B1HUNrn3b1Xuh2cV02BqcGSSWl4IGzrh82VXqsEiurc=; b=e/isi9NXMReXkP4mIgnoBw6mPgEVBBcRfdXJKUqEpgcmg5VOAGUMdzwkhBzOL7LZuU E5eKq08VpQtgERKPUO2ee4yw7fiqQywn9MtY1jjx2lRUcKxPU+/JNQsl7+aE68U4H27X KR+juY93qWe67TVtgxmnRuyjs4Orn+k5kck6bMlrGKE2Sm3XgkMESarfsj34uu8xyKvr c7tS8QMeq7h4Lh8x8RpVuhZZ9O1lQixIJNAgYabKgZpoxOxy3wWye5JC1BmG0oH3qJb+ TRniM2aZj6FQHk0KQ/HPFZCRmfTazkM8lenHGpVi19r89HO1epmIKk4hCW6/0ttCPY/j 25aA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700159675; x=1700764475; 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=B1HUNrn3b1Xuh2cV02BqcGSSWl4IGzrh82VXqsEiurc=; b=mLI7oIyOVQaMrncT1l9veuPs6bm1VrTEypqz26cd8jG0SNG3QU7hiZ4m1oRprD4pmv 6aNIyRvOgwyLYOBFoQXhlpDoTWIupxV/ND4s7YzEPf3yeD8cLkjcU9fdxPkMe1p4eJu6 7tZKCpz+RTPC80OIzkgpUc2X8jTOYpmMbKF4HiB4KBtmH/NueSizyRh5Zc0OgCGDPiRp xMrrmnVml4HL/wgZxaJ6qJJLVsB3es8StzRQWAIm5y45Q6UoHGfgj21PwDlrfzLL74J8 awdyLSg1afUbPDxnUNdx5oQ4V/k2vnM/g2cIyEl1RHUfJM3J4OMxW6dYb2bTrRKoJDQz AwHQ== X-Gm-Message-State: AOJu0YzeFCUk1A4AU6NomtkZl0ELARCRWTyHYmlqtIFyAWzS5rE08q0x 6CkvNt1JedF5HayKKUj7KLEjHqYaeAk37ErjlYxgkA== X-Google-Smtp-Source: AGHT+IFMfYgmm+PPdDXQ+yVt0c97zpvgAp9dal/K9U7t2+fv9aRqaw2fihnUhHir8OjGxpOFQLKYOm85sNVp0RT3Gl0= X-Received: by 2002:a17:906:f1c8:b0:9e2:b1a5:1d2 with SMTP id gx8-20020a170906f1c800b009e2b1a501d2mr2387943ejb.27.1700159675283; Thu, 16 Nov 2023 10:34:35 -0800 (PST) MIME-Version: 1.0 References: <20231113202833.12900-1-jspewock@iol.unh.edu> <20231113202833.12900-6-jspewock@iol.unh.edu> In-Reply-To: <20231113202833.12900-6-jspewock@iol.unh.edu> From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Thu, 16 Nov 2023 19:34:24 +0100 Message-ID: Subject: Re: [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer To: jspewock@iol.unh.edu Cc: Honnappa.Nagarahalli@arm.com, thomas@monjalon.net, wathsala.vithanage@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com, ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru, 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 As I'm thinking about the filtering, it's not a trivial manner. For now, I'd like to pass a class instead of multiple parameters, as that will be easier to extend if we need to add to the filter. The class could be a dataclass holding the various booleans. Then the capturing traffic generators would need to implement a method that would translate the dataclass into a TG-specific filters. I'm not sure we want to introduce an abstract method for this, as the return value may differ for different TGs - this needs more thought. On Mon, Nov 13, 2023 at 9:28=E2=80=AFPM wrote: > > From: Jeremy Spewock > > Added the options to filter out LLDP and ARP packets when > sniffing for packets with scapy. This was done using BPF filters to > ensure that the noise these packets provide does not interfere with test > cases. > > Signed-off-by: Jeremy Spewock > --- > dts/framework/test_suite.py | 13 +++++++++++-- > .../testbed_model/capturing_traffic_generator.py | 12 +++++++++++- > dts/framework/testbed_model/scapy.py | 11 ++++++++++- > dts/framework/testbed_model/tg_node.py | 4 +++- > 4 files changed, 35 insertions(+), 5 deletions(-) > > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py > index 3b890c0451..3222f172f3 100644 > --- a/dts/framework/test_suite.py > +++ b/dts/framework/test_suite.py > @@ -152,7 +152,11 @@ def _configure_ipv4_forwarding(self, enable: bool) -= > None: > self.sut_node.configure_ipv4_forwarding(enable) > > def send_packet_and_capture( > - self, packet: Packet, duration: float =3D 1 > + self, > + packet: Packet, > + duration: float =3D 1, > + no_lldp: bool =3D True, > + no_arp: bool =3D True, The parameters of this method are not documented in the docstring, but we should add these new parameters to the docstring. The same goes for the other methods (and other parameters of other methods) if they're not overriding an abstract method. The default should be False if these are meant to be optional, but I like these defaulting to True, so I'd change the wording from optional to configurable. > ) -> list[Packet]: > """ > Send a packet through the appropriate interface and > @@ -162,7 +166,12 @@ def send_packet_and_capture( > """ > packet =3D self._adjust_addresses(packet) > return self.tg_node.send_packet_and_capture( > - packet, self._tg_port_egress, self._tg_port_ingress, duratio= n > + packet, > + self._tg_port_egress, > + self._tg_port_ingress, > + duration, > + no_lldp, > + no_arp, > ) > > def get_expected_packet(self, packet: Packet) -> Packet: > diff --git a/dts/framework/testbed_model/capturing_traffic_generator.py b= /dts/framework/testbed_model/capturing_traffic_generator.py > index ab98987f8e..0a0d0f7d0d 100644 > --- a/dts/framework/testbed_model/capturing_traffic_generator.py > +++ b/dts/framework/testbed_model/capturing_traffic_generator.py > @@ -52,6 +52,8 @@ def send_packet_and_capture( > send_port: Port, > receive_port: Port, > duration: float, > + no_lldp: bool, > + no_arp: bool, > capture_name: str =3D _get_default_capture_name(), > ) -> list[Packet]: > """Send a packet, return received traffic. > @@ -71,7 +73,7 @@ def send_packet_and_capture( > A list of received packets. May be empty if no packets are = captured. > """ > return self.send_packets_and_capture( > - [packet], send_port, receive_port, duration, capture_name > + [packet], send_port, receive_port, duration, no_lldp, no_arp= , capture_name > ) > > def send_packets_and_capture( > @@ -80,6 +82,8 @@ def send_packets_and_capture( > send_port: Port, > receive_port: Port, > duration: float, > + no_lldp: bool, > + no_arp: bool, > capture_name: str =3D _get_default_capture_name(), > ) -> list[Packet]: > """Send packets, return received traffic. > @@ -93,6 +97,8 @@ def send_packets_and_capture( > send_port: The egress port on the TG node. > receive_port: The ingress port in the TG node. > duration: Capture traffic for this amount of time after send= ing the packets. > + no_lldp: Option to disable capturing LLDP packets > + no_arp: Option to disable capturing ARP packets > capture_name: The name of the .pcap file where to store the = capture. > > Returns: > @@ -108,6 +114,8 @@ def send_packets_and_capture( > send_port, > receive_port, > duration, > + no_lldp, > + no_arp, > ) > > self._logger.debug( > @@ -123,6 +131,8 @@ def _send_packets_and_capture( > send_port: Port, > receive_port: Port, > duration: float, > + no_lldp: bool, > + no_arp: bool, > ) -> list[Packet]: > """ > The extended classes must implement this method which > diff --git a/dts/framework/testbed_model/scapy.py b/dts/framework/testbed= _model/scapy.py > index af0d4dbb25..58f01af21a 100644 > --- a/dts/framework/testbed_model/scapy.py > +++ b/dts/framework/testbed_model/scapy.py > @@ -69,6 +69,7 @@ def scapy_send_packets_and_capture( > send_iface: str, > recv_iface: str, > duration: float, > + sniff_filter: str, > ) -> list[bytes]: > """RPC function to send and capture packets. > > @@ -90,6 +91,7 @@ def scapy_send_packets_and_capture( > iface=3Drecv_iface, > store=3DTrue, > started_callback=3Dlambda *args: scapy.all.sendp(scapy_packets, = iface=3Dsend_iface), > + filter=3Dsniff_filter, > ) > sniffer.start() > time.sleep(duration) > @@ -264,10 +266,16 @@ def _send_packets_and_capture( > send_port: Port, > receive_port: Port, > duration: float, > + no_lldp: bool, > + no_arp: bool, > capture_name: str =3D _get_default_capture_name(), > ) -> list[Packet]: > binary_packets =3D [packet.build() for packet in packets] > - > + sniff_filter =3D [] > + if no_lldp: > + sniff_filter.append("ether[12:2] !=3D 0x88cc") > + if no_arp: > + sniff_filter.append("ether[12:2] !=3D 0x0806") > xmlrpc_packets: list[ > xmlrpc.client.Binary > ] =3D self.rpc_server_proxy.scapy_send_packets_and_capture( > @@ -275,6 +283,7 @@ def _send_packets_and_capture( > send_port.logical_name, > receive_port.logical_name, > duration, > + " && ".join(sniff_filter), > ) # type: ignore[assignment] > > scapy_packets =3D [Ether(packet.data) for packet in xmlrpc_packe= ts] > diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testb= ed_model/tg_node.py > index 27025cfa31..98e55b7831 100644 > --- a/dts/framework/testbed_model/tg_node.py > +++ b/dts/framework/testbed_model/tg_node.py > @@ -56,6 +56,8 @@ def send_packet_and_capture( > send_port: Port, > receive_port: Port, > duration: float =3D 1, > + no_lldp: bool =3D True, > + no_arp: bool =3D True, > ) -> list[Packet]: > """Send a packet, return received traffic. > > @@ -73,7 +75,7 @@ def send_packet_and_capture( > A list of received packets. May be empty if no packets are = captured. > """ > return self.traffic_generator.send_packet_and_capture( > - packet, send_port, receive_port, duration > + packet, send_port, receive_port, duration, no_lldp, no_arp > ) > > def close(self) -> None: > -- > 2.42.0 >