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 9B98A45966; Thu, 12 Sep 2024 05:46:56 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2604E40612; Thu, 12 Sep 2024 05:46:56 +0200 (CEST) Received: from mail-oa1-f42.google.com (mail-oa1-f42.google.com [209.85.160.42]) by mails.dpdk.org (Postfix) with ESMTP id 3BE184027D for ; Thu, 12 Sep 2024 05:46:54 +0200 (CEST) Received: by mail-oa1-f42.google.com with SMTP id 586e51a60fabf-26ff21d82e4so214210fac.2 for ; Wed, 11 Sep 2024 20:46:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1726112813; x=1726717613; 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=aGE6eQgM5rzGDasiw7X4g6VUrDGaUMzgjFbZL7pSP9M=; b=VOefUaHu/aQUoCVPJjd6FlN7aFnd69igcYk/goIJSJ8k2QsfXtLNSRi41WvRetCeE7 hZX2X1kMa2KaTDdccHftm92XueXEv4arIhWeFQcr2ktBiOxcrFh5zI+7zNOgkzX3Jtqv fAgmX5PoRDPTn6RaUwmrYVCNC8SQGJW4IXPAg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726112813; x=1726717613; 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=aGE6eQgM5rzGDasiw7X4g6VUrDGaUMzgjFbZL7pSP9M=; b=ARmlqHpdCCmulZOPM0IsyFrzUgo1HbcneGurAfT6YkdleCb64CPQIBdF2V4pambJZi AUKFmE6dYSnVA719wCVlxqGMxkqf6QVUfDwNoskp0+lZAugQEcXCfCjQUU6jw+Xpp5DS 0mVf8fj0T2ifmnKfjVCDPVQSwXC/2W5uM+tditq9WcRiGrQ79m0TuBBGwLpXwGx1zp/D 6vJg+W+NuiZJLk3qhiFgTQHsHf2ax5DybdBPWA1jJRekfAfhavBCT6ohmFU0waULsDi6 APMfb/ayszWcRTqi0D4d6aAX5Wf7Oid3xww0iV4tC8RbNnnHdZ6lCon+46siDwO2Ukqs B4BA== X-Forwarded-Encrypted: i=1; AJvYcCWU+E19vfg0dtcQkmJzX9x9aQ56oavahI9rq1F8kGQUlgsUr3kpJ9bNPAeiu7PrVRHYH3A=@dpdk.org X-Gm-Message-State: AOJu0YxE2bWn5qLrc+gI7kpJSXm0PfsIYCi0iaaLeR7p3ACeDzmd/Uk3 yo0wNviHmPnZUPnKksx2padpr66DjMDGKEKDElYTFrwWFxAzlXIvVYQXiNlDbRZ1YpfuSysPXPQ x2IONSfuHVz518jK7mBlLAJhmibBROtaZLtDiFw== X-Google-Smtp-Source: AGHT+IHGW7ulruGkCECS7s4uH3s6PK0yh0EbMgO6ICWvpnqv8D8rUuRUyCA4ggizmWBcHr8Ee/xRFKXKhGTj/bjkWFc= X-Received: by 2002:a05:6871:822:b0:278:3de:c8de with SMTP id 586e51a60fabf-27c3f40d44cmr1206638fac.24.1726112813264; Wed, 11 Sep 2024 20:46:53 -0700 (PDT) MIME-Version: 1.0 References: <20240715195852.254033-1-jspewock@iol.unh.edu> <20240724171345.341495-1-jspewock@iol.unh.edu> <20240724171345.341495-2-jspewock@iol.unh.edu> In-Reply-To: <20240724171345.341495-2-jspewock@iol.unh.edu> From: Patrick Robb Date: Wed, 11 Sep 2024 23:46:02 -0400 Message-ID: Subject: Re: [PATCH v3 1/2] dts: add dual_vlan testing suite To: jspewock@iol.unh.edu Cc: yoan.picchi@foss.arm.com, paul.szczepanek@arm.com, juraj.linkes@pantheon.tech, Luca.Vizzarro@arm.com, Honnappa.Nagarahalli@arm.com, thomas@monjalon.net, npratte@iol.unh.edu, wathsala.vithanage@arm.com, 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 Looks good other than updating the testpmd_shell method names Reviewed-by: Patrick Robb On Wed, Jul 24, 2024 at 1:13=E2=80=AFPM wrote: > +class TestDualVlan(TestSuite): > + """DPDK Dual VLAN test suite. > + > + This suite tests the behavior of VLAN functions and properties in th= e presence of two VLAN > + headers. All VLAN functions which are tested in this suite are speci= fied using the inner class > + :class:`TestCaseOptions` and should have cases for configuring them = in > + :meth:`configure_testpmd` as well as cases for testing their behavio= r in > + :meth:`verify_vlan_functions`. Every combination of VLAN functions b= eing enabled should be > + tested. Additionally, attributes of VLAN headers, such as priority, = are tested to ensure they > + are not modified in the case of two VLAN headers. > + """ > + > + class TestCaseOptions(Flag): > + """Flag for specifying which VLAN functions to configure.""" > + > + #: > + VLAN_STRIP =3D auto() > + #: > + VLAN_FILTER_INNER =3D auto() > + #: > + VLAN_FILTER_OUTER =3D auto() I think this can now be dropped and we can import VLANOffloadFlag from testpmd_shell in its place. > + > + #: ID to set on inner VLAN tags. > + inner_vlan_tag: ClassVar[int] =3D 2 > + #: ID to set on outer VLAN tags. > + outer_vlan_tag: ClassVar[int] =3D 1 > + #: ID to use when inserting VLAN tags. > + vlan_insert_tag: ClassVar[int] =3D 3 > + #: > + rx_port: ClassVar[int] =3D 0 > + #: > + tx_port: ClassVar[int] =3D 1 > + > + def is_relevant_packet(self, pkt: Packet) -> bool: > + """Check if a packet was sent by functions in this suite. > + > + All functions in this test suite send packets with a payload tha= t is packed with 20 "X" > + characters. This method, therefore, can determine if the packet = was sent by this test suite > + by just checking to see if this payload exists on the received p= acket. > + > + Args: > + pkt: Packet to check for relevancy. > + > + Returns: > + :data:`True` if the packet contains the expected payload, :d= ata:`False` otherwise. > + """ > + return hasattr(pkt, "load") and "X" * 20 in str(pkt.load) > + > + def pkt_payload_contains_layers(self, pkt: Packet, *expected_layers:= Dot1Q) -> bool: > + """Verify that the payload of the packet matches `expected_layer= s`. > + > + The layers in the payload of `pkt` must match the type and the u= ser-defined fields of the > + layers in `expected_layers` in order. > + > + Args: > + pkt: Packet to check the payload of. > + *expected_layers: Layers expected to be in the payload of `p= kt`. > + > + Returns: > + :data:`True` if the payload of `pkt` matches the layers in `= expected_layers` in order, > + :data:`False` otherwise. > + """ > + current_pkt_layer =3D pkt.payload > + ret =3D True > + for layer in expected_layers: > + ret &=3D isinstance(current_pkt_layer, type(layer)) > + if not ret: > + break > + for field, val in layer.fields.items(): > + ret &=3D ( > + hasattr(current_pkt_layer, field) and getattr(curren= t_pkt_layer, field) =3D=3D val > + ) > + current_pkt_layer =3D current_pkt_layer.payload > + return ret > + > + def verify_vlan_functions(self, send_packet: Packet, options: TestCa= seOptions) -> None: > + """Send packet and verify the received packet has the expected s= tructure. > + > + Expected structure is defined by `options` according to the foll= owing table: > + +----------------------------------------------+----------------= -------+ > + | Configure setting | Result = | > + +=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D= =3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D=3D+=3D=3D= =3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D+=3D=3D=3D=3D=3D=3D=3D+ > + | Outer | Inner | Vlan | Vlan | Vlan | Pass/ | Outer |= Inner | > + | vlan | vlan | strip | filter | insert | Drop | vlan |= vlan | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + | 0x1 | 0x2 | no | no | no | pass | 0x1 |= 0x2 | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + | 0x1 | 0x2 | yes | no | no | pass | no |= 0x2 | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + | 0x1 | 0x2 | no | yes,0x1 | no | pass | 0x1 |= 0x2 | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + | 0x1 | 0x2 | no | yes,0x2 | no | pass | 0x1 |= 0x2 | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + | 0x1 | 0x2 | no | yes,0x1,0x2| no | pass | 0x1 |= 0x2 | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + | 0x1 | 0x2 | yes | yes,0x1 | no | pass | no |= 0x2 | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + | 0x1 | 0x2 | yes | yes,0x2 | no | pass | no |= 0x2 | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + | 0x1 | 0x2 | yes | yes,0x1,0x2| no | pass | no |= 0x2 | > + +-------+-------+--------+------------+--------+-------+-------+= -------+ > + > + Args: > + send_packet: Packet to send for testing. > + options: Flag which defines the currents configured settings= in testpmd. > + """ > + recv =3D self.send_packet_and_capture(send_packet) > + recv =3D list(filter(self.is_relevant_packet, recv)) > + expected_layers: list[Packet] =3D [] > + > + if self.TestCaseOptions.VLAN_STRIP not in options: > + expected_layers.append(Dot1Q(vlan=3Dself.outer_vlan_tag)) > + expected_layers.append(Dot1Q(vlan=3Dself.inner_vlan_tag)) > + > + self.verify( > + len(recv) > 0, > + f"Expected to receive packet with the payload {expected_laye= rs} but got nothing.", > + ) > + > + for pkt in recv: > + self.verify( > + self.pkt_payload_contains_layers(pkt, *expected_layers), > + f"Received packet ({pkt.summary()}) did not match the ex= pected sequence of layers " > + f"{expected_layers} with options {options}.", > + ) > + > + def configure_testpmd(self, shell: TestPmdShell, options: TestCaseOp= tions, add: bool) -> None: > + """Configure VLAN functions in testpmd based on `options`. > + > + Args: > + shell: Testpmd session to configure the settings on. Expecte= d to already be running > + with all ports stopped before being passed into this fun= ction. Instead of having this requirement that all ports be stopped for the testpmd session, we can just use testpmd_shell.requires_stopped_ports decorator now. > + options: Settings to modify in `shell`. > + add: If :data:`True`, turn the settings in `options` on, oth= erwise turn them off. > + """ > + if ( > + self.TestCaseOptions.VLAN_FILTER_INNER in options > + or self.TestCaseOptions.VLAN_FILTER_OUTER in options > + ): > + if add: > + # If we are adding a filter, filtering has to be enabled= first > + shell.vlan_filter_set(self.rx_port, True) > + > + if self.TestCaseOptions.VLAN_FILTER_INNER in options: > + shell.rx_vlan(self.inner_vlan_tag, self.rx_port, add) > + if self.TestCaseOptions.VLAN_FILTER_OUTER in options: > + shell.rx_vlan(self.outer_vlan_tag, self.rx_port, add) > + > + if not add: > + # If we are removing filters then we need to remove the = filters before we can > + # disable filtering. > + shell.vlan_filter_set(self.rx_port, False) > + if self.TestCaseOptions.VLAN_STRIP in options: > + shell.vlan_strip_set(self.rx_port, add) > + > + def test_insert_second_vlan(self) -> None: > + """Test that a packet with a single VLAN can have an additional = one inserted into it.""" > + with TestPmdShell(self.sut_node, forward_mode=3DSimpleForwarding= Modes.mac) as testpmd: > + testpmd.port_stop_all() Should be testpmd.stop_all_ports() now per luca's pktegen/testpmd patch. > + testpmd.tx_vlan_set(self.tx_port, self.vlan_insert_tag) > + testpmd.port_start_all() start_all_ports() > + testpmd.start() > + recv =3D self.send_packet_and_capture( > + Ether() / Dot1Q(vlan=3Dself.outer_vlan_tag) / Raw("X" * = 20) > + ) > + self.verify(len(recv) > 0, "Did not receive any packets when= testing VLAN insertion.") > + self.verify( > + any( > + self.is_relevant_packet(p) > + and self.pkt_payload_contains_layers( > + p, *[Dot1Q(vlan=3Dself.vlan_insert_tag), Dot1Q(v= lan=3Dself.outer_vlan_tag)] > + ) > + for p in recv > + ), > + "Packet was unable to insert a second VLAN tag.", > + ) > + > + def test_all_vlan_functions(self) -> None: > + """Test that all combinations of :class:`TestCaseOptions` behave= as expected. > + > + To test this, the base case is tested first, ensuring that a pac= ket with two VLANs is > + unchanged without the VLAN modification functions enabled. Then = the same Testpmd shell is > + modified to enable all necessary VLAN functions, followed by ver= ification that the > + functions work as expected, and finally the functions are disabl= ed to allow for a clean > + environment for the next test. > + """ > + send_pakt =3D ( > + Ether() > + / Dot1Q(vlan=3Dself.outer_vlan_tag) > + / Dot1Q(vlan=3Dself.inner_vlan_tag) > + / Raw("X" * 20) > + ) > + with TestPmdShell(self.sut_node, forward_mode=3DSimpleForwarding= Modes.mac) as testpmd: > + testpmd.start() > + recv =3D self.send_packet_and_capture(send_pakt) > + self.verify(len(recv) > 0, "Unmodified packet was not receiv= ed.") > + self.verify( > + any( > + self.is_relevant_packet(p) > + and self.pkt_payload_contains_layers( > + p, *[Dot1Q(vlan=3Dself.outer_vlan_tag), Dot1Q(vl= an=3Dself.inner_vlan_tag)] > + ) > + for p in recv > + ), > + "Packet was modified without any VLAN functions applied.= ", > + ) > + testpmd.stop() > + testpmd.port_stop_all() stop_all_ports(). I'll stop mentioning this below. :)