Re: [PATCH v1 1/2] dts: add qinq strip and VLAN extend to testpmd shell

[Luca Vizzarro <Luca.Vizzarro@arm.com>]

Looks good to me, just some minor comments.

On 17/07/2025 21:57, Dean Marx wrote:
> +    def set_vlan_extend(self, port: int, enable: bool, verify: bool = True) -> None:
> <snip>
> +            if enable ^ (vlan_settings is not None and VLANOffloadFlag.EXTEND in vlan_settings):
> +                self._logger.debug(
> +                    f"""Failed to {"enable" if enable else "disable"}
> +                                   extend on port {port}: \n{extend_cmd_output}"""
> +                )
> +                raise InteractiveCommandExecutionError(
> +                    f"""Failed to {"enable" if enable else "disable"} extend on port {port}"""
> +                )
Please don't use triple quotes if you need to break a line, or if you 
don't need a reason like for InteractiveCommandExecutionError.

Breaking a line with triple quotes will result in all the whitespace 
being considered as part of the string. You can just as easily split by 
writing multiple quoted text next to each other:

   self._logger.debug(
       f"Failed to {...} extend "
       f"on port {port}..."
   )

> +    def set_qinq_strip(self, port: int, enable: bool, verify: bool = True) -> None:
> <snip>
> +            if enable ^ (vlan_settings is not None and VLANOffloadFlag.QINQ_STRIP in vlan_settings):
> +                self._logger.debug(
> +                    f"""Failed to {"enable" if enable else "disable"}
> +                                   QinQ strip on port {port}: \n{qinq_cmd_output}"""
> +                )
> +                raise InteractiveCommandExecutionError(
> +                    f"""Failed to {"enable" if enable else "disable"} QinQ strip on port {port}"""
> +                )
Same problem as above.