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 DE4B246074; Mon, 13 Jan 2025 18:01:06 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AB60A40669; Mon, 13 Jan 2025 18:01:06 +0100 (CET) Received: from mail-yb1-f178.google.com (mail-yb1-f178.google.com [209.85.219.178]) by mails.dpdk.org (Postfix) with ESMTP id 74BC840653 for ; Mon, 13 Jan 2025 18:01:05 +0100 (CET) Received: by mail-yb1-f178.google.com with SMTP id 3f1490d57ef6-e53ef7462b6so7569059276.3 for ; Mon, 13 Jan 2025 09:01:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1736787665; x=1737392465; 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=Pn95TayfVjz9CzBxCQ63SDEmg44+HnjqYqjPimqKjAM=; b=EqLX4iW0Cog5SZ/mgcQWS8cqW3yrrO+z99irgNLq++sw8w3Zpl9ZQIz9Fqz9qrspeZ xePTgL8yzN9Wi6VSj5zfhZR1x0mX9IWbkDw76pokfkJDUN7WPXGRZ0/NyAJJaiu3odH7 Uu6HU2bT9UFfG3d2xwRSeNqW3Q6w4rRBNb4Tk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736787665; x=1737392465; 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=Pn95TayfVjz9CzBxCQ63SDEmg44+HnjqYqjPimqKjAM=; b=LcVmtstn2OlOcnYwGWz2zYshhm91PzVxMUBFdaMQD7/JFWh0bJ9wgwZ5WckRUoxVkr 6pfR2vkODfk76uCZ/NoRqJfaJbMxFtBW0oa8Tzb/gM3zCB4IqSavv3N0j00Xk7PAMF3m cC2CNOzk/IDyGLnoI4u+1XzUfo2raBdCq2Rk0bHhfwIMQngtqfYdIfb3NQb2ZI/gQq9b oS5rMlbJiV5vp+2Rpn/MS8sh+4nUq4gKUvRljC1b7HsT0kwomX6Zn3HOw9pHIsR7+oMU svCKossK3MUpaBdQFtzu7Ii/YR6BtN9R7zCQ+XyKdKLdAtrt4+bh1coH1716cCPOjcpu 49ZA== X-Forwarded-Encrypted: i=1; AJvYcCUSieEKx9bAlgXIw5KqyfLrmo+IqqgNcLLkcjcq38LnsWs0IrzPdeR5XvfF0avokIpak10=@dpdk.org X-Gm-Message-State: AOJu0Yy44jut7ZflTfNR0mBeIyuqQi1aPlb/k43gUpeQtnk8jrUYRTO/ 45AjQ5l+U8ezrJ+hLWNlJPxaGLRSqwaDw509LMGZvLtJVCHdSKetmz8debWs4sSNgVWVLdun5Jp +TjPfCAuZgOgn+oJtHi1VS7A70BXHZmTEv8D62g== X-Gm-Gg: ASbGncudMk34Tvbb12Tum+W0S1WJG5NEvqEsZ+T3lTkN3femjuvbLpWdKSFNf8V7RqO IlvVZ8jEauAxlKxd29OHJAyIojn9UjuIgkbXwu1KF6gvDiLdd3rBx8U0hQzC1BuuIoD7vOxuh X-Google-Smtp-Source: AGHT+IFmnW7Vyoxme07MKKgbG6DrkINJTDBJTfvBJjxiIGzoiSevueaYnfQMP0VtU/D2aF5RB+T4ZHkNJpRzswl+fBs= X-Received: by 2002:a05:690c:744a:b0:6f4:3b8f:8e3f with SMTP id 00721157ae682-6f53131b3c2mr165689947b3.38.1736787664782; Mon, 13 Jan 2025 09:01:04 -0800 (PST) MIME-Version: 1.0 References: <20241223183901.20107-1-dmarx@iol.unh.edu> <20250106203747.22489-1-dmarx@iol.unh.edu> In-Reply-To: From: Dean Marx Date: Mon, 13 Jan 2025 12:00:56 -0500 X-Gm-Features: AbW1kvZNEmlwY8fWgCNwQOLeExMz3UwtJ6IygRtcT9ofUixiC1KOwEPpwng_9VY Message-ID: Subject: Re: [PATCH v14] dts: port over queue start/stop suite To: Patrick Robb Cc: npratte@iol.unh.edu, luca.vizzarro@arm.com, yoan.picchi@foss.arm.com, Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com, dev@dpdk.org, Jeremy Spewock , Stephen Hemminger 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 > To me this does not read like it is properly validating deferred_start, s= o I want to double check this before we merge. > > From the link below, "All device queues (except form deferred start queue= s) status should be RTE_ETH_QUEUE_STATE_STARTED after start." > https://doc.dpdk.org/api/rte__ethdev_8h.html#afdc834c1c52e9fb512301990468= ca7c2 > > So, I assume that we want to validate: > 1. When deferred_start is set for a queue, the queue does not erroneously= start on testpmd.start() > 2. You can successfully start the queue after the DPDK application is sta= rted and send packets. > > In order to do this. I would write the testcase like: > > with TestPmdShell(node=3Dself.sut_node) as testpmd: > testpmd.set_forward_mode(SimpleForwardingModes.mac) > testpmd.stop_all_ports() > testpmd.set_queue_deferred_start(0, 0, True, True) > testpmd.start() > self.send_packet_and_verify(should_receive=3DFalse) > testpmd.start_port_queue(0, 0, True) > self.send_packet_and_verify(should_receive=3DTrue) > > What do you think? I see what you're saying, I can modify the deferred start cases to add this functionality. > Thanks Dean. This mostly looks good to me, but I have a couple questions/= comments. > > 1. I see that you are calling self.verify() twice per testsuite, once thr= ough send_packet_and_verify(), and once by collecting testpmd port stats. S= o, we basically have two means of verifying that the ports are behaving as = expected. But, are they essentially verifying the same thing, and are thus = overlapping in responsibilities? If this is the case then we may be adding = complexity for no gain. > > Taking the test_tx_queue_deferred_start testcase as an example, instead o= f using send_packet_and_verify(), can you just, directly in the testcase: > > packet =3D Ether() / IP() / Raw(load=3D"xxxxx") > self.send_packet_and_capture(packet) > > And then check port stats, with that being the sole testcase assertion? O= r, we can do the opposite and drop the port stats assertion, and rely solel= y on send_packet_and_capture() and reading the RAW layer, which seems equal= ly as good. Anyhow perhaps there is some added value from doing both assert= ions, and I realize they did that in the legacy DTS testcase, but I figured= I'd ask for your thoughts. Let me know if you agree/disagree. Yes I agree, I was mostly just doing it since the legacy DTS suite did it like you mentioned. I'd be fine with removing the stats check portion though, and just checking whether testpmd receives/doesn't receive the packet. > 2. The way the testcase was written in the legacy framework involved stop= ping the tx queue, sending a packet, asserting none was received on that po= rt, then restarting the queue, then sending a packet and asserting it is re= ceived on the queue. You are not doing the second half of this process in y= our testcases - is there a reason why not? Do you think it would be good to= add this? See this blurb from the old test plan: > > #. Run "port 0 txq 0 stop" to stop txq 0 in port 0 > #. Start packet generator to transmit and check tester port not receive p= ackets > and in testpmd it not has "port 0/queue 0: received 1 packets" print > > #. Run "port 0 txq 0 start" to start txq 0 in port 0 > #. Start packet generator to transmit and check tester port receive packe= ts > and in testpmd it has "port 0/queue 0: received 1 packets" print I honestly don't remember why I took that out, I can definitely add that back it seems like it would be useful. I'll send out a new version ASAP so it's ready to merge by tomorrow, thanks Patrick.