----- Original Message -----
From: "Ondrej Lichtner" <olichtne(a)redhat.com>
To: "Christos Sfakianakis" <csfakian(a)redhat.com>
Cc: lnst-developers(a)lists.fedorahosted.org
Sent: Wednesday, July 24, 2019 1:34:52 PM
Subject: Re: [PATCH-next v2 8/8] lnst.Recipes.ENRT: rework recipes to adopt to
334e2c..7bde5c
On Wed, Jul 24, 2019 at 07:29:08AM -0400, Christos Sfakianakis wrote:
>
>
> ----- Original Message -----
> > From: "Jan Tluka" <jtluka(a)redhat.com>
> > To: csfakian(a)redhat.com
> > Cc: lnst-developers(a)lists.fedorahosted.org
> > Sent: Wednesday, July 24, 2019 12:05:08 PM
> > Subject: Re: [PATCH-next v2 8/8] lnst.Recipes.ENRT: rework recipes to
> > adopt to 334e2c..7bde5c
> >
> > Tue, Jul 23, 2019 at 06:19:26PM CEST, csfakian(a)redhat.com wrote:
> > >From: Christos Sfakianakis <csfakian(a)redhat.com>
> > >
> > >Rewrite recipes to be compatible with the above patches.
> > >Rename SimplePerfRecipe to SimpleNetworkRecipe.
> > >
> > >Signed-off-by: Christos Sfakianakis <csfakian(a)redhat.com>
> > >---
> > > lnst/Recipes/ENRT/BondRecipe.py | 137 +++++---
> > > lnst/Recipes/ENRT/DoubleBondRecipe.py | 140 +++++---
> > > lnst/Recipes/ENRT/DoubleTeamRecipe.py | 159 ++++++----
> > > lnst/Recipes/ENRT/IpsecEspAeadRecipe.py | 280 ++++++++--------
> > > lnst/Recipes/ENRT/IpsecEspAhCompRecipe.py | 297 ++++++++---------
> > > lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py | 130 +++++---
> > > lnst/Recipes/ENRT/PingFloodRecipe.py | 21 +-
> > > .../ENRT/ShortLivedConnectionsRecipe.py | 87 ++---
> > > lnst/Recipes/ENRT/SimpleMacsecRecipe.py | 252 +++++++--------
> > > ...lePerfRecipe.py => SimpleNetworkRecipe.py} | 41 +--
> > > lnst/Recipes/ENRT/TeamRecipe.py | 134 +++++---
> > > lnst/Recipes/ENRT/TeamVsBondRecipe.py | 162 ++++++----
> > > lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py | 230 +++++++++++---
> > > .../VirtualBridgeVlanInGuestMirroredRecipe.py | 191 ++++++-----
> > > .../ENRT/VirtualBridgeVlanInGuestRecipe.py | 167 ++++++----
> > > .../VirtualBridgeVlanInHostMirroredRecipe.py | 178 +++++++----
> > > .../ENRT/VirtualBridgeVlanInHostRecipe.py | 155 ++++++---
> > > .../ENRT/VirtualBridgeVlansOverBondRecipe.py | 298 +++++++++++++-----
> > > ...rtualOvsBridgeVlanInGuestMirroredRecipe.py | 193 +++++++-----
> > > .../ENRT/VirtualOvsBridgeVlanInGuestRecipe.py | 164 ++++++----
> > > ...irtualOvsBridgeVlanInHostMirroredRecipe.py | 155 +++++----
> > > .../ENRT/VirtualOvsBridgeVlanInHostRecipe.py | 144 ++++++---
> > > .../VirtualOvsBridgeVlansOverBondRecipe.py | 277 ++++++++++------
> > > lnst/Recipes/ENRT/VlansOverBondRecipe.py | 267 +++++++++++-----
> > > lnst/Recipes/ENRT/VlansOverTeamRecipe.py | 269 +++++++++++-----
> > > lnst/Recipes/ENRT/VlansRecipe.py | 230 ++++++++++----
> > > lnst/Recipes/ENRT/VxlanMulticastRecipe.py | 155 ++++++---
> > > lnst/Recipes/ENRT/VxlanRemoteRecipe.py | 115 +++++--
> > > lnst/Recipes/ENRT/__init__.py | 2 +-
> > > 29 files changed, 3195 insertions(+), 1835 deletions(-)
> > > rename lnst/Recipes/ENRT/{SimplePerfRecipe.py =>
> > > SimpleNetworkRecipe.py}
> > > (73%)
> >
> > A note on the commit description ($subj). When we push the changes
> > the commit ids will be different or am I wrong? I'd remove them or
> > replace with 'to adopt previous changes'.
> >
> Ok, will remove these hashes from the subject line
> > I have one more comment. I asked Ondrej why following pattern is in
> > SimpleNetworkRecipe:
> >
> > + @property
> > + def offload_nics(self):
> > + return [self.matched.host1.bond0, self.matched.host2.bond0]
> > +
> > + @property
> > + def mtu_hw_config_dev_list(self):
> > + return self.offload_nics
> > +
> >
> > I meant why we use offload_nics for mtu_hwconfig_dev_list. Ondrej said
> > that the idea is to avoid copy-paste errors. I'm fine with that.
> >
> > But when I checked the rest of the code, there's a mixture of approach
> > there.
> >
> > E.g. following recipes does not follow this pattern:
> > lnst/Recipes/ENRT/IpsecEspAeadRecipe.py
> > lnst/Recipes/ENRT/IpsecEspAhCompRecipe.py
> > lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
> > lnst/Recipes/ENRT/SimpleMacsecRecipe.py
> >
> > I'd like to have this consistent.
> Ok, I will avoid referencing one property via another to avoid confusion
> and
> misleads.
> >
> > Additionally:
> > lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py
> > this recipe does not contain baremetal NICs in offload_nics list
> > note that old master does not even contain offloading, but maybe
> > that's ok
> >
> Regarding the offload values, the default case is 'all-on' (unless
> overwritten in
> the specific recipe). If you think it makes sense to avoid touching
> offloads completely
> for phase 3 (like for the old code), I can redefine the offload_nics
> property to return
> an empty list.
You can also just not inherit from the OffloadSubconfigMixin class and
remove the offload_nics property. That would make it a bit more in line
with the older recipes since they didn't support the feature at all.
Even better, thanks.
> -Ondrej
> _______________________________________________
> 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...
>