Re: [RFC Patch v1 2/5] dts: rework traffic generator inheritance structure.

[Nicholas Pratte <npratte@iol.unh.edu>]

Hi Patick, see below!

>> +
>> +from framework.testbed_model.traffic_generator.traffic_generator import TrafficGenerator
>
>
> I think this can become:
>
> from .traffic_generator import TrafficGenerator

Ack.
>
>>
>> +
>> +
>> +@dataclass(slots=True)
>> +class PerformanceTrafficStats(ABC):
>> +    """Data structure for stats offered by a given traffic generator."""
>> +
>> +    frame_size: int
>
>
> Do we need to add an optional number of packet descriptors attribute? I realize that is a SUT testpmd param, not a TREX param, but presumably when we gather stats, we want to store the descriptor count with it.'

It could be done by passing another parameter when the function is
called. Maybe adding these parameters is helpful, maybe it isn't. It
might just be easier to strip the frame size and descriptors away
entirely from the statistics data structure, under the assumption that
these fields would be managed in the test suites themselves. I think
either/or would be fine.


>
>>
>> +
>> +
>> +class PerformanceTrafficGenerator(TrafficGenerator):
>> +    """An Abstract Base Class for all performance-oriented traffic generators.
>> +
>> +    Provides an intermediary interface for performance-based traffic generator.
>> +    """
>> +
>> +    _test_stats: list[PerformanceTrafficStats]
>> +
>> +    @property
>> +    def is_capturing(self) -> bool:
>> +        """Used for synchronization."""
>> +        return False
>> +
>> +    @property
>> +    def last_results(self) -> PerformanceTrafficStats | None:
>> +        """Get the latest set of results from TG instance.
>> +
>> +        Returns:
>> +            The most recent set of traffic statistics.
>> +        """
>> +        return self._test_stats.pop(0)
>> +
>> +    def generate_traffic_and_stats(
>> +        self,
>> +        packet: Packet,
>> +        duration: float,  # Default of 60 (in seconds).
>
>
> Should it be float = 60, so the default is coming from the function, and not a "default" which is coming from the testsuite? If not, I guess the "default" comment belongs in the testsuite?

I would say the comment can be removed. Right now, as you mentioned,
it is set up so that the default value is asserted at the test suite
level. It might just make sense to do away with this default value and
just insist the user explicitly provide a duration. That would make
sense in its own right.