Re: [RFC Patch v1 4/5] dts: add trex traffic generator to dts framework

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

>> Implement the TREX traffic generator for use in the DTS framework. The
>> provided implementation leverages TREX's stateless API automation
>> library, via use of a Python shell. As such, version control of TREX may
>> be needed. The DTS context has been modified to include a performance
>> traffic generator in addition to a functional traffic generator.
>
>
> I think the statement is that only certain versions are confirmed to work off of your implementation? I think usage of the term version control is confusing since we are just talking about using particular versions of TREX, and not using a "version control" tool like git. This is more about listing approved versions of the dependency for the TG.

I can fix that!
>>
>>
>>
>>          dpdk_build_env = DPDKBuildEnvironment(config.dpdk.build, sut_node)
>>          dpdk_runtime_env = DPDKRuntimeEnvironment(config.dpdk, sut_node, dpdk_build_env)
>> -        traffic_generator = create_traffic_generator(config.traffic_generator, tg_node)
>> +        # There is definitely a better way to do this.
>> +        tg_dpdk_runtime_env = None
>> +        if (
>> +            config.perf_traffic_generator.type == TrafficGeneratorType.TREX
>> +            or config.func_traffic_generator.type == TrafficGeneratorType.TREX
>> +        ):
>
>
> We have this from the testrun config
>
> perf: false # disable performance testing
> func: true # enable functional testing
>
> So, use if testrunconfig.perf == true?

I see what you're saying, I can make that change for the time being.
My reason for explicitly checking traffic generators types was that
any future implementations may not need a DPDK runtime environment.

>>
>>
>> +
>> +@dataclass
>> +class TrexPerformanceStats(PerformanceTrafficStats):
>> +    """Data structure to store performance statistics for a given test run.
>> +
>> +    Attributes:
>> +        packet: The packet that was sent in the test run.
>> +        frame_size: The total length of the frame. (L2 downward)
>> +        tx_expected_bps: The expected bits per second on a given NIC.
>> +        tx_expected_cps: ...
>
>
> "The expected connections per second" ? I think you just missed this one.

This was ultimately removed from the final version, incidentally. But
the way this was implemented does allow us to check this data.

>
>>
>> +
>> +        Attributes:
>> +            ports: Related ports utilized in TG instance.
>> +        """
>> +        super().setup(ports)
>> +        # Start TREX server process.
>> +        try:
>> +            self._logger.info("Starting TREX server process: sending 45 second sleep.")
>> +            privileged_command = self._os_session._get_privileged_command(
>> +                f"""
>> +                    cd /opt/v3.03/; {self._tg_config.remote_path}/t-rex-64
>> +                     --cfg {self._tg_config.config} -i
>> +                """
>
>
> I know this was just a work in progress for RFC with some hardcoding, but you already have tg_config.remote_path, right? So, the leading hardcoded cd does not do anything, and even if you had to cd there you could use the tg_config.remote_path?

Yes, that was hard coded as a temporary measure. The cd is needed to
ensure that the TREX executable actually runs properly. I must have
just forgot to switch that back as I was working with haste.