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 6905E4577A; Fri, 9 Aug 2024 17:10:29 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 36B7442D78; Fri, 9 Aug 2024 17:10:29 +0200 (CEST) Received: from mail-pg1-f175.google.com (mail-pg1-f175.google.com [209.85.215.175]) by mails.dpdk.org (Postfix) with ESMTP id C37054025F for ; Fri, 9 Aug 2024 17:10:28 +0200 (CEST) Received: by mail-pg1-f175.google.com with SMTP id 41be03b00d2f7-7a0e8b76813so1525630a12.3 for ; Fri, 09 Aug 2024 08:10:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1723216228; x=1723821028; 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=rTuDSXPLITxGXjjIzLPJ/uuBug8V8WKHa1JlSrZ+BGk=; b=K/JwZ4lDxqIZ/rdtuTiQkTvwv57T7csIzoTgpw+YMS2rNED9vpw4XY2Ac9QUo7Q0Vr 1MnGRd+WrIHA1yfg3/BwEM9R96kfEfl/7AW4o78KFQrZRZguc5WDlzDr9RWKHf981QoG +gX4AbEx9/36Fxx9TXY6OdXzvMU+zAO76PWFM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723216228; x=1723821028; 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=rTuDSXPLITxGXjjIzLPJ/uuBug8V8WKHa1JlSrZ+BGk=; b=emgNNWcu7IyvvItNXuW5kBW9qkAbS1zqFByYk/8uxCdUsx4euh/jvEoqForGgUatXC jf4c38p0lbMto/dzSCSDxnTlHEuzC7ec1Ec4okXXrkgbTTFB4uTGLjHOzglf/dnF1coc ohy1AT4OzxvG+/k7vss1h0xi2kbOPfwdbuGWJ+DcwR98nOgaLhb69ESNWUx+fklGmQc6 xmhvg/5k3ReOZ5CohJYHFT8otrmMlaL7JVlzUB6ItJ2mNWKdODtJ6topcxXXSSOngoQG FU/vNTINfUM4td8py7aFG227mQrQ3XHGkGiN1SeTG3ZUdqsg/rQ2Pp300c3Pi0uL6EYc 6mPQ== X-Gm-Message-State: AOJu0YzfZBpgCGvm0fsMI+OgXzw+FlWI6fS2hZsstPMBN3ZZvakxqAc+ 0TjKB0p/i3eQG1bCLEL/BIhNiQn4RV9Fh1sQa2337MKYW1C1kkaSzpEpqd8mJ8lP6d4ehvBk0uB B+50pMTXGHVswL2YT1x4KBFBXRaRcfG56EnvwHkauoROw1cyTnyxSFg== X-Google-Smtp-Source: AGHT+IEXb+slQ74TcRSdRpEgktdgATQNS0HevvFnj3yjnDKbXlJV5wMj591UE1b3JEARUvU7VKya4oZSdfHrzArK4IE= X-Received: by 2002:a17:90b:503:b0:2c7:700e:e2b7 with SMTP id 98e67ed59e1d1-2d1e8066c66mr2049221a91.39.1723216227699; Fri, 09 Aug 2024 08:10:27 -0700 (PDT) MIME-Version: 1.0 References: <20240806121417.2567708-1-Luca.Vizzarro@arm.com> <20240806124642.2580828-1-luca.vizzarro@arm.com> <20240806124642.2580828-2-luca.vizzarro@arm.com> In-Reply-To: <20240806124642.2580828-2-luca.vizzarro@arm.com> From: Jeremy Spewock Date: Fri, 9 Aug 2024 11:10:16 -0400 Message-ID: Subject: Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets To: Luca Vizzarro Cc: dev@dpdk.org, =?UTF-8?Q?Juraj_Linke=C5=A1?= , Honnappa Nagarahalli , Paul Szczepanek , Alex Chapman 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 Hey Luca, Thanks for the patch! I think this change makes a lot of sense in general, and I like the idea of removing the duplication between capturing traffic generators and the test suite. I just had one thought that I left below, but otherwise: Reviewed-by: Jeremy Spewock On Tue, Aug 6, 2024 at 8:49=E2=80=AFAM Luca Vizzarro wrote: > > The framework allows only to send one packet at once via Scapy. This > change adds the ability to send multiple packets, and also introduces a > new fast way to verify if we received several expected packets. > > Moreover, it reduces code duplication by keeping a single packet sending > method only at the test suite level. > > Signed-off-by: Luca Vizzarro > Reviewed-by: Paul Szczepanek > Reviewed-by: Alex Chapman > --- > + def match_all_packets( > + self, expected_packets: list[Packet], received_packets: list[Pac= ket] > + ) -> None: This is a very interesting approach to comparing what you expect to what you received. I hadn't seen counters used before but they seem useful for this purpose. This method reminds me a lot of the filtering process that was discussed in bugzilla ticket 1438 (https://bugs.dpdk.org/show_bug.cgi?id=3D1438) where there was some talk about how to handle the noise vs what you received. This is different in a few ways though of course in that it allows you to specify multiple different criteria that are acceptable rather than just getting the payload and really only concerns itself with matching lists instead of doing any filtering. The main reason I mention this is it brought up a question for me: Do you think, in the future when the payload filtering method is implemented, these two methods will co-exist or it would make more sense to just let test suites get their noise-free list of packets and then check that what they care about is present? I think a method like this might be useful in some more niche cases where you have multiple packet structures to look for, so I don't doubt there are some reasons to have both, I was just wondering if you had thought about it or had an opinion. > + """Matches all the expected packets against the received ones. > + > + Matching is performed by counting down the occurrences in a dict= ionary which keys are the > + raw packet bytes. No deep packet comparison is performed. All th= e unexpected packets (noise) > + are automatically ignored. > + > + Args: > + expected_packets: The packets we are expecting to receive. > + received_packets: All the packets that were received. > + > + Raises: > + TestCaseVerifyError: if and not all the `expected_packets` w= ere found in > + `received_packets`. > + """ > + expected_packets_counters =3D Counter(map(raw, expected_packets)= ) > + received_packets_counters =3D Counter(map(raw, received_packets)= ) > + # The number of expected packets is subtracted by the number of = received packets, ignoring > + # any unexpected packets and capping at zero. > + missing_packets_counters =3D expected_packets_counters - receive= d_packets_counters > + missing_packets_count =3D missing_packets_counters.total() > + self._logger.debug( > + f"match_all_packets: expected {len(expected_packets)}, " > + f"received {len(received_packets)}, missing {missing_packets= _count}" > + ) > + > + if missing_packets_count !=3D 0: > + self._fail_test_case_verify( > + f"Not all packets were received, expected {len(expected_= packets)} " > + f"but {missing_packets_count} were missing." > + ) > + > -- > 2.34.1 >