Wed, May 13, 2020 at 11:51:45AM CEST, olichtne(a)redhat.com wrote:
On Thu, May 07, 2020 at 11:05:07AM +0200, Jan Tluka wrote:
> The mixin now allows to configure any pause frames configuration
> instead of a single option to turn them off.
>
> For example, to turn the pause frames off:
>
> from lnst.Recipes.ENRT import SimpleNetworkRecipe
> recipe = SimpleNetworkRecipe(
> rx_pause_frames=False,
> tx_pause_frames=False,
> )
>
> Signed-off-by: Jan Tluka <jtluka(a)redhat.com>
> ---
> lnst/Recipes/ENRT/BondRecipe.py | 2 +-
> .../ConfigMixins/PauseFramesHWConfigMixin.py | 41 ++++++++++---------
> lnst/Recipes/ENRT/SimpleNetworkRecipe.py | 2 +-
> 3 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/lnst/Recipes/ENRT/BondRecipe.py b/lnst/Recipes/ENRT/BondRecipe.py
> index 16b80e90..d5c48a8b 100644
> --- a/lnst/Recipes/ENRT/BondRecipe.py
> +++ b/lnst/Recipes/ENRT/BondRecipe.py
> @@ -113,6 +113,6 @@ class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin,
> self.matched.host2.eth0]
>
> @property
> - def no_pause_frames_dev_list(self):
> + def pause_frames_dev_list(self):
> return [self.matched.host1.eth0, self.matched.host1.eth1,
> self.matched.host2.eth0]
> diff --git a/lnst/Recipes/ENRT/ConfigMixins/PauseFramesHWConfigMixin.py
b/lnst/Recipes/ENRT/ConfigMixins/PauseFramesHWConfigMixin.py
> index 74a1d348..d72e2463 100644
> --- a/lnst/Recipes/ENRT/ConfigMixins/PauseFramesHWConfigMixin.py
> +++ b/lnst/Recipes/ENRT/ConfigMixins/PauseFramesHWConfigMixin.py
> @@ -1,41 +1,44 @@
> from time import sleep
> +from lnst.Common.Parameters import BoolParam
> from lnst.Recipes.ENRT.ConfigMixins.BaseHWConfigMixin import BaseHWConfigMixin
>
>
> class PauseFramesHWConfigMixin(BaseHWConfigMixin):
> """
> - This class is an extension to the :any:`BaseEnrtRecipe` class to turn off
> + This class is an extension to the :any:`BaseEnrtRecipe` class to configure
> the Ethernet pause frames on the devices defined by
> - the :attr:`no_pause_frames_dev_list` property.
> + the :attr:`pause_frames_dev_list` property.
> """
>
> + rx_pause_frames = BoolParam(mandatory=False)
> + tx_pause_frames = BoolParam(mandatory=False)
> +
> @property
> - def no_pause_frames_dev_list(self):
> + def pause_frames_dev_list(self):
> """
> The value of this property is a list of devices for which the pause
> - frames should be disabled. It has to be defined by a derived class.
> + frames should be configured. It has to be defined by a derived class.
> """
> return []
>
> def hw_config(self, config):
> super().hw_config(config)
>
> - for dev in self.no_pause_frames_dev_list:
> - dev.host.run("ethtool -A {} rx off tx off".format(dev.name))
> - sleep(1)
> - dev.host.run("ethtool -a {}".format(dev.name))
> -
> - def hw_deconfig(self, config):
> - for dev in self.no_pause_frames_dev_list:
> - dev.host.run("ethtool -A {} rx on tx on".format(dev.name))
> -
> - super().hw_deconfig(config)
I'm not sure about removing this method. I assume you removed it because
the previous patch implements pause frame configuration in the Device
class such that it will be deconfigured with the Device deconfiguration?
But, this is a SubConfig mixin class which means that theoretically you
could have multiple sub config loop iterations where each could define
it's own configuration for pause frames. Even though the value will
always be overwritten I think it may make more sense to have it in the
"comple" form of running both configuration and deconfiguration for this
setting.
What do you think?
I think it should not be removed. I can't remember why I'd want to
remove this. I'll think about this more, but likely I will send a v3
patchset.
> + for param in ["rx_pause_frames",
"tx_pause_frames"]:
> + param_value = getattr(self.params, param, None)
> + if param_value is not None:
> + self._configure_dev_attribute(
> + config,
> + self.pause_frames_dev_list,
> + param,
> + param_value
> + )
>
> def describe_hw_config(self, config):
> desc = super().describe_hw_config(config)
> - desc += [
> - "Pause frames disabled for: {}".format(
> - self.no_pause_frames_dev_list
> - )
> - ]
> + for param in ["rx_pause_frames", "tx_pause_frames"]:
> + desc.extend(
> + self._describe_dev_attribute(config, param)
> + )
> +
> return desc
> diff --git a/lnst/Recipes/ENRT/SimpleNetworkRecipe.py
b/lnst/Recipes/ENRT/SimpleNetworkRecipe.py
> index 508afdc0..506cbdaa 100644
> --- a/lnst/Recipes/ENRT/SimpleNetworkRecipe.py
> +++ b/lnst/Recipes/ENRT/SimpleNetworkRecipe.py
> @@ -112,7 +112,7 @@ class SimpleNetworkRecipe(
> return [(self.matched.host1.eth0, self.matched.host2.eth0)]
>
> @property
> - def no_pause_frames_dev_list(self):
> + def pause_frames_dev_list(self):
> return [self.matched.host1.eth0, self.matched.host2.eth0]
>
> @property
> --
> 2.21.1
> _______________________________________________
> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...