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 853D545602; Thu, 11 Jul 2024 21:34:13 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6E4C942E50; Thu, 11 Jul 2024 21:34:13 +0200 (CEST) Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by mails.dpdk.org (Postfix) with ESMTP id 2C99F402CC for ; Thu, 11 Jul 2024 21:34:12 +0200 (CEST) Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-2c9785517c0so1025068a91.0 for ; Thu, 11 Jul 2024 12:34:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1720726451; x=1721331251; 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=qt631+EeUnsebluGldb29g7sROrvq0Vx0aa+Qgt+y1c=; b=Monr0PcbO4a3P7R+gg2+OdcH1xtYV4a0iuQZOCOXosNiIbRYn5h55S2QNs4/w6wirx 4IJDWCBZs7NtWEQkaWvIiIjM409J03HQE3noAqoYw92FWSA9uPrj3U1pFft6IWXeZXme BJKT1q3TjDCCLlFf1BiQQ0vCxuMRZeagmwL0U= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720726451; x=1721331251; 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=qt631+EeUnsebluGldb29g7sROrvq0Vx0aa+Qgt+y1c=; b=LyGC5Kh+iBpPQ7KsVLQjC25mms8ke/qcJwQf0AOywjCfH374m77YDXulRrfAhQo+gh u3RYKUpjG0LVL5PlejZAfLc3PG068oSnl9x/6A6W/okrf5RtgyTH9fDFj2LgkaFj/Sct Z0lk2IccrIUBoK0B1lqeQ0/az834q6yftIEQeEfTxoB5gcX59m+EQ/HQf6tiV+0DeuRR 3NrWY1ddjjcn1sLMJoRqe1Z8bJLuQBJPoEuE+GSErM+uplAlqjGH+AC0aUOTL316XYsn 2t6ev0DQfl3DdbOxJOutk6LivR65DkFeKdtVoOkzk2dKwpJ0ZnglyoRObpm1TW+TDvP+ Fs+g== X-Forwarded-Encrypted: i=1; AJvYcCXA/xzs4VlM24FrRLLqw2jyBT5wQa2PNG893f2Z4mbpXuExx2PE6MD1z1mpV0N4tZVd8LpIBsmEyjkMs0M= X-Gm-Message-State: AOJu0YyEzqBZfbl7drFQApgtr3eD2QtyDIho/yojlSM8dTVYe1uySI5A MRL035wXR0n+1AwYNJWnlL4jyF9vcZPxJuZUpuSTfIldkq27/1oNsGuv77uCk3tEG17TpHAMlym aLVNVZBunrwVN7GZrh6Nh72DUzoVaXHM3DJMupYtibuFB0dDB X-Google-Smtp-Source: AGHT+IE6TIXUo2nqSq+ptR+8dKBWUq3PnV0Pi0yaLbjPax0wWK54FK2nmsLz6qhiBz4YM3b7EDNfI1rfgGWbv4p7sYA= X-Received: by 2002:a17:90a:f415:b0:2c8:a8e:c1cd with SMTP id 98e67ed59e1d1-2ca786c24f1mr5123313a91.11.1720726451151; Thu, 11 Jul 2024 12:34:11 -0700 (PDT) MIME-Version: 1.0 References: <20240702192422.2480-2-npratte@iol.unh.edu> <20240702192422.2480-5-npratte@iol.unh.edu> In-Reply-To: <20240702192422.2480-5-npratte@iol.unh.edu> From: Jeremy Spewock Date: Thu, 11 Jul 2024 15:34:00 -0400 Message-ID: Subject: Re: [PATCH v2 3/3] dts: mac filter test suite refactored for new dts To: Nicholas Pratte Cc: Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu, juraj.linkes@pantheon.tech, paul.szczepanek@arm.com, luca.vizzarro@arm.com, yoan.picchi@foss.arm.com, dmarx@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 Code all looked good to me, just a couple of documentation comments. On Tue, Jul 2, 2024 at 3:25=E2=80=AFPM Nicholas Pratte wrote: > > +class TestMacFilter(TestSuite): > + """Mac address allowlist filtering test suite. > + > + Configure mac address filtering on a given port, and test the port's= filtering behavior > + using both a given port's hardware address as well as dummy addresse= s. If a port accepts > + a packet that is not contained within its mac address allowlist, the= n a given test case > + fails. Alternatively, if a port drops a packet that is designated wi= thin its mac address > + allowlist, a given test case will fail. > + > + Moreover, a given port should demonstrate proper behavior when bound= to the Poll Mode > + Driver. A port should not have an mac address allowlist that exceeds= its designated size. I think this should just be " a mac address allowlist" rather than "an". > + A port's default hardware address should not be removed from its add= ress pool, and invalid > + addresses should not be included in the allowlist. If a port abides = by the above rules, the > + test case passes. > + """ > + > + def send_packet_and_verify( > + self, > + mac_address: str, > + add_vlan: bool =3D False, > + should_receive: bool =3D True, > + ) -> None: > + """Generate, send, and verify a packet based on specified parame= ters. > + > + Test cases within this suite utilize this method to create, send= , and verify > + packets based on criteria relating to the packet's destination m= ac address, > + vlan tag, and whether or not the packet should be received or no= t. Packets > + are verified using an inserted payload. If the list of received = packets > + contains this payload within any of its packets, the test case p= asses. Each It might be worth noting here that it passes assuming `should_recieve` is t= rue. > + call with this method sends exactly one packet. > + > + Args: > + mac_address: The destination mac address of the packet being= sent. > + add_vlan: Add a vlan tag to the packet being sent. :data:'2'= if the packet I think if this started with "Whether to add..." it would be more clear that it is a boolean. > + should be received, :data:'1' if the packet should not b= e received but > + requires a vlan tag, and None for any other condition. This tripped me up at first because I thought you were saying that add_vlan could be 2, 1, or None. It might be worth just adding to the start of that second sentence that "The tag will be...". Also, it might be more clear if you explain that the tag will be omitted in other cases rather than it being None. > + should_receive: If :data:'True', assert whether or not the s= ent packet > + has been received. If :data:'False', assert that the sen= d packet was not > + received. :data:'True' by default > + """ > + if add_vlan: > + packet =3D Ether() / Dot1Q(vlan=3D2 if should_receive else 1= ) / IP() / Raw(load=3D"X" * 22) > + else: > + packet =3D Ether() / IP() / Raw(load=3D"X" * 22) > + packet.dst =3D mac_address > + received_packets =3D [ > + packets > + for packets in self.send_packet_and_capture(packet, adjust_a= ddresses=3DFalse) > + if hasattr(packets, "load") and "X" * 22 in str(packets.load= ) > + ] > + if should_receive: > + self.verify(len(received_packets) =3D=3D 1, "Expected packet= not received") > + else: > + self.verify(len(received_packets) =3D=3D 0, "Expected packet= received") > + > + def test_add_remove_mac_addresses(self) -> None: > + """Assess basic mac addressing filtering functionalities. > + > + This test cases validates for proper behavior of mac address fil= tering with both Small typo, but I think this is meant to be "this test case validates...". > + a port's default, burned-in mac address, as well as additional m= ac addresses > + added to the PMD. Packets should either be received or not recei= ved depending on > + the properties applied to the PMD at any given time. > + > + Test: > + Start TestPMD with promiscuous mode. > + Send a packet with the port's default mac address. (Should r= eceive) > + Send a packet with fake mac address. (Should not receive) > + Add fake mac address to the PMD's address pool. > + Send a packet with the fake mac address to the PMD. (Should = receive) > + Remove the fake mac address from the PMD's address pool. > + Sent a packet with the fake mac address to the PMD. (Should = not receive) > + """ > + testpmd =3D TestPmdShell(self.sut_node) > + testpmd.set_promisc(0, on=3DFalse) > + testpmd.start() > + mac_address =3D self._sut_port_ingress.mac_address > + > + # Send a packet with NIC default mac address > + self.send_packet_and_verify(mac_address=3Dmac_address, should_re= ceive=3DTrue) > + # Send a packet with different mac address > + fake_address =3D "00:00:00:00:00:01" > + self.send_packet_and_verify(mac_address=3Dfake_address, should_r= eceive=3DFalse) > + > + # Add mac address to pool and rerun tests > + testpmd.set_mac_addr(0, mac_address=3Dfake_address, add=3DTrue) > + self.send_packet_and_verify(mac_address=3Dfake_address, should_r= eceive=3DTrue) > + testpmd.set_mac_addr(0, mac_address=3Dfake_address, add=3DFalse) > + self.send_packet_and_verify(mac_address=3Dfake_address, should_r= eceive=3DFalse) > + testpmd.close() > + sleep(6) > > -- > 2.44.0 >