[v2,1/3] dts: add boolean to adjust addresses

Message ID 20240702192422.2480-3-npratte@iol.unh.edu (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series Mac Filter Port to New DTS |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Nicholas Pratte July 2, 2024, 7:24 p.m. UTC
  Various test cases in the mac filter test suite called for granular
manipulation of destination mac addresses to properly test mac address
filtering functionality. To compensate, there is now an
adjust_addresses boolean which the user can toggle if they wish to send
their own addressing; the boolean is true by default.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/test_suite.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
  

Comments

Jeremy Spewock July 11, 2024, 7:31 p.m. UTC | #1
I think this change makes sense, and I mentioned this on Dean's patch
that has the same change in it but I think we should have more
discussion about which route to take with this addressing problem. It
is definitely something that we have to address since it is required
for a suite like this or any suite that operates with multiple queues
to ensure traffic is handled by the different queues. If we end up
going the boolean parameter route however, then I think this solution
looks great and it passes by my standards.

There was one comment that I had about adding something super small to
the doc-string, but other than that the code all looked good to me.

On Tue, Jul 2, 2024 at 3:25 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Various test cases in the mac filter test suite called for granular
> manipulation of destination mac addresses to properly test mac address
> filtering functionality. To compensate, there is now an
> adjust_addresses boolean which the user can toggle if they wish to send
> their own addressing; the boolean is true by default.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
>  dts/framework/test_suite.py | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index 694b2eba65..551a587525 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -185,6 +185,7 @@ def send_packet_and_capture(
>          packet: Packet,
>          filter_config: PacketFilteringConfig = PacketFilteringConfig(),
>          duration: float = 1,
> +        adjust_addresses: bool = True,
>      ) -> list[Packet]:
>          """Send and receive `packet` using the associated TG.
>
> @@ -195,11 +196,15 @@ def send_packet_and_capture(
>              packet: The packet to send.
>              filter_config: The filter to use when capturing packets.
>              duration: Capture traffic for this amount of time after sending `packet`.
> +            adjust_addresses: If :data:'True', adjust addresses of the egressing packet with
> +                a default addressing scheme. If :data:'False', do not adjust the addresses of
> +                egressing packet.

It might also be worth naming what the default is in the doc-string
(by which I mean that the parameter defaults to True, not the default
address scheme).

>
>          Returns:
>              A list of received packets.
>          """
> -        packet = self._adjust_addresses(packet)
> +        if adjust_addresses:
> +            packet = self._adjust_addresses(packet)
>          return self.tg_node.send_packet_and_capture(
>              packet,
>              self._tg_port_egress,
> --
> 2.44.0
>
  
Nicholas Pratte July 17, 2024, 5:19 p.m. UTC | #2
<snip>
> It might also be worth naming what the default is in the doc-string
> (by which I mean that the parameter defaults to True, not the default
> address scheme).

I can fix that.
  
Jeremy Spewock July 19, 2024, 3:37 p.m. UTC | #3
On Thu, Jul 11, 2024 at 3:31 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> I think this change makes sense, and I mentioned this on Dean's patch
> that has the same change in it but I think we should have more
> discussion about which route to take with this addressing problem. It
> is definitely something that we have to address since it is required
> for a suite like this or any suite that operates with multiple queues
> to ensure traffic is handled by the different queues. If we end up
> going the boolean parameter route however, then I think this solution
> looks great and it passes by my standards.
>
> There was one comment that I had about adding something super small to
> the doc-string, but other than that the code all looked good to me.
>

We did end up having a discussion about this on slack and it seemed
there was a slight favor towards using the method of only changing it
if the user hasn't set it themselves. If you have any other thoughts
on it though definitely feel free to share them!
  

Patch

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..551a587525 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,7 @@  def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        adjust_addresses: bool = True,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -195,11 +196,15 @@  def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            adjust_addresses: If :data:'True', adjust addresses of the egressing packet with
+                a default addressing scheme. If :data:'False', do not adjust the addresses of
+                egressing packet.
 
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
+        if adjust_addresses:
+            packet = self._adjust_addresses(packet)
         return self.tg_node.send_packet_and_capture(
             packet,
             self._tg_port_egress,