On Fri, Oct 24, 2025 at 2:16 PM Dean Marx wrote: > + packets = [ > + Ether(dst="00:11:22:33:44:55", src="66:77:88:99:aa:bb") > + / Dot1AD(vlan=100) > + / Dot1Q(vlan=200) > + / IP(dst="192.0.2.1", src="198.51.100.1") > + / UDP(dport=1234, sport=5678), + Ether(dst="00:11:22:33:44:55", src="66:77:88:99:aa:bb") > + / Dot1AD(vlan=101) > + / Dot1Q(vlan=200) > + / IP(dst="192.0.2.1", src="198.51.100.1") > + / UDP(dport=1234, sport=5678), > + ] > Add / Raw(b"xxxxx"), to both packets above if we keep this testcase. > + with TestPmd() as testpmd: > + testpmd.set_vlan_filter(0, True) > + testpmd.set_vlan_extend(0, True) > + testpmd.rx_vlan(100, 0, True) > + self._send_packet_and_verify(packets[0], testpmd, > should_receive=True) > + self._send_packet_and_verify(packets[1], testpmd, > should_receive=False) > + > The testcase does look good except that form what I'm seeing, vlan_filter is not supported for 88a8 + 8100 packets (it won't read the 88a8 tag). I sent Bruce and Morten an email about it. But, I think the most reasonable thing is to remove this testcase for the 25.11 release since the expected behavior is either not well defined, or vlan_filter is not supposed to be able to read 88a8 tags. > + @func_test > + def test_qinq_forwarding(self) -> None: > + """QinQ Rx filter test case. > + > + Steps: > + Launch testpmd with mac forwarding mode. > + Disable VLAN filter mode on port 0. > + Send test packet and capture verbose output. > + > + Verify: > + Check that the received packet has two separate VLAN layers > in proper QinQ fashion. > + Check that the received packet outer and inner VLAN layer has > the appropriate ID. > + """ > + test_packet = ( > + Ether(dst="ff:ff:ff:ff:ff:ff") > + / Dot1AD(vlan=100) > + / Dot1Q(vlan=200) > + / IP(dst="1.2.3.4") > + / UDP(dport=1234, sport=4321) > + / Raw(load="xxxxx") > + ) > + with TestPmd() as testpmd: > + testpmd.set_vlan_filter(0, False) > + testpmd.start() > + received_packets = send_packet_and_capture(test_packet) > + packet = self._get_relevant_packet(received_packets) > + > + verify(packet is not None, "Packet was dropped when it should > have been received.") > + > + if packet is not None: > + verify(bool(packet.haslayer(Dot1AD)), "QinQ layer not > found in packet") > I guess you can also verify that packet haslayer(Dot1Q) > + > + if outer_vlan := packet.getlayer(Dot1AD): > + outer_vlan_id = outer_vlan.vlan > + verify( > + outer_vlan_id == 100, > + f"Outer VLAN ID was {outer_vlan_id} when it > should have been 100.", > + ) > + else: > + verify(False, "VLAN layer not found in received > packet.") > + > + if outer_vlan and (inner_vlan := > outer_vlan.getlayer(Dot1Q)): > + inner_vlan_id = inner_vlan.vlan > + verify( > + inner_vlan_id == 200, > + f"Inner VLAN ID was {inner_vlan_id} when it > should have been 200", > + ) > + > + @requires_nic_capability(NicCapability.RX_OFFLOAD_QINQ_STRIP) > + @func_test > + def test_qinq_strip(self) -> None: > + """Test combinations of VLAN/QinQ strip settings with various > QinQ packets. > + > + Steps: > + Launch testpmd with QinQ and VLAN strip enabled. > + Send four VLAN/QinQ related test packets. > + > + Verify: > + Check received packets have the expected VLAN/QinQ > layers/tags. > + """ > + test_packets = [ > + Ether() / Dot1Q() / IP() / UDP(dport=1234, sport=4321) / > Raw(load="xxxxx"), > + Ether() > + / Dot1Q(vlan=100) > + / Dot1Q(vlan=200) > + / IP() > + / UDP(dport=1234, sport=4321) > + / Raw(load="xxxxx"), > + Ether() / Dot1AD() / IP() / UDP(dport=1234, sport=4321) / > Raw(load="xxxxx"), > + Ether() / Dot1AD() / Dot1Q() / IP() / UDP(dport=1234, > sport=4321) / Raw(load="xxxxx"), > + ] > + with TestPmd() as testpmd: > + testpmd.set_qinq_strip(0, True) > + testpmd.set_vlan_strip(0, True) > + testpmd.start() > + > + received_packets1 = send_packet_and_capture(test_packets[0]) > + packet1 = self._get_relevant_packet(received_packets1) > + received_packets2 = send_packet_and_capture(test_packets[1]) > + packet2 = self._get_relevant_packet(received_packets2) > + received_packets3 = send_packet_and_capture(test_packets[2]) > + packet3 = self._get_relevant_packet(received_packets3) > + received_packets4 = send_packet_and_capture(test_packets[3]) > + packet4 = self._get_relevant_packet(received_packets4) > rename to make them more descriptive? I.e. vlan_packet, 2_vlan_packet, qinq_packet, qinq_vlan_packet? Or similar. > + > + testpmd.stop() > + > + tests = [ > + ("Single 8100 tag", self._strip_verify(packet1, False, > "Single 8100 tag")), > + ( > + "Double 8100 tag", > + self._strip_verify(packet2, True, "Double 8100 tag"), > + ), > + ("Single 88a8 tag", self._strip_verify(packet3, False, > "Single 88a8 tag")), > + ( > + "QinQ (88a8 and 8100 tags)", > + self._strip_verify(packet4, False, "QinQ (88a8 and > 8100 tags)"), > + ), > + ] > this is good for testing behavior when we have both vlan_strip and qinq_strip enabled. What about when we have only vlan_strip enabled or only qinq_strip enabled? I.e. when we have only vlan_strip enabled, and we send an 88a8 only packet, nothing should get stripped at all. It probably makes sense to have different testcases for vlan strip, qinq_strip, and vlan_strip + qinq_strip. > + > + failed = [ctx for ctx, result in tests if not result] > + > + verify( > + not failed, > + f"The following packets were not stripped correctly: {', > '.join(failed)}", > + ) > -- > 2.51.0 > > Reviewed-by: Patrick Robb