On 23. 9. 2024 20:42, jspew...@iol.unh.edu wrote:
From: Jeremy Spewock <jspew...@iol.unh.edu>

In DPDK applications virtual functions are treated the same as ports,
but within the framework there are benefits to differentiating the two
in order to add more metadata to VFs about where they originate from.
For this reason this patch adds a new class for handling virtual
functions that extends the Port class with some additional information
about the VF.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspew...@iol.unh.edu>
---
  dts/framework/testbed_model/port.py | 37 ++++++++++++++++++++++++++++-
  1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/port.py 
b/dts/framework/testbed_model/port.py
index 817405bea4..c1d85fec2b 100644
--- a/dts/framework/testbed_model/port.py
+++ b/dts/framework/testbed_model/port.py
@@ -27,7 +27,7 @@ class PortIdentifier:
      pci: str
-@dataclass(slots=True)
+@dataclass

This should be explained in the commit message.

  class Port:
      """Physical port on a node.
@@ -80,6 +80,41 @@ def pci(self) -> str:
          return self.identifier.pci
+@dataclass
+class VirtualFunction(Port):
+    """Virtual Function (VF) on a port.
+
+    DPDK applications often treat VFs the same as they do the physical ports 
(PFs) on the host.
+    For this reason VFs are represented in the framework as a type of port 
with some additional
+    metadata that allows the framework to more easily identify which device 
the VF belongs to as
+    well as where the VF originated from.
+
+    Attributes:
+        created_by_framework: :data:`True` if this VF represents one that the 
DTS framework created
+            on the node, :data:`False` otherwise.

I had to go look in the other patches to understand why this is here. The patch split should be redone along logical lines (one patch should contain related logic, now the related logic is in basically all patches), not files (that doesn't help with review and it's also not going to result in better git history).

But I figured out this is here because of cleanup. It makes more sense to me for framework to remember whether it created the port or not as opposed to port remembering it, especially when the framework is doing the cleanup and not the ports.

+        pf_port: The PF that this VF was created on/gathered from.

Maybe it would make more sense to store the VF ports in the PF port. If we need to use VF ports, we can just refer to the PF port which has the benefit making it easier to use the proper link between ports.

And the framework can store which PF ports needs VF cleanup instead of storing which VFs needs cleaning.

+    """
+
+    created_by_framework: bool = False
+    pf_port: Port | None = None
+
+    def __init__(
+        self, node_name: str, config: PortConfig, created_by_framework: bool, 
pf_port: Port
+    ) -> None:
+        """Extends :meth:`Port.__init__` with VF specific metadata.
+
+        Args:
+            node_name: The name of the node the VF resides on.
+            config: Configuration information about the VF.
+            created_by_framework: :data:`True` if DTS created this VF, 
otherwise :data:`False` if
+                this class represents a VF that was preexisting on the node.
+            pf_port: The PF that this VF was created on/gathered from.
+        """
+        super().__init__(node_name, config)
+        self.created_by_framework = created_by_framework
+        self.pf_port = pf_port
+
+
  @dataclass(slots=True, frozen=True)
  class PortLink:
      """The physical, cabled connection between the ports.

Reply via email to