On Tue, Apr 30, 2019 at 3:39 PM Jan Tluka <jtluka@redhat.com> wrote:

Tue, Apr 30, 2019 at 02:13:12PM CEST, jurbanov@redhat.com wrote:
>From: Jozef Urbanovsky <jurbanov@redhat.com>
>
>IPv6 address assigment to an interface involves duplicate address
>detection using neighbor solicitation and neighbor advertisement
>messages. Duration of this process was not accounted for in the recipe,
>therefore ping6 was failing as IPv6 address was not yet assigned to
>interface.
>
>Signed-off-by: Jozef Urbanovsky <jurbanov@redhat.com>
>---
> recipes/regression_tests/phase3/simple_macsec.py | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/recipes/regression_tests/phase3/simple_macsec.py b/recipes/regression_tests/phase3/simple_macsec.py
>index ced140c..5cec47c 100644
>--- a/recipes/regression_tests/phase3/simple_macsec.py
>+++ b/recipes/regression_tests/phase3/simple_macsec.py
>@@ -94,6 +94,9 @@ def macsecSetup(encryption):
> m1.run("ip -6 addr add %s/64 dev %s" % (m1_tif_addr6, msec_tif_name))
> m2.run("ip -6 addr add %s/64 dev %s" % (m2_tif_addr6, msec_tif_name))
>
>+ if ipv in ['ipv6', 'both']:
>+ ctl.wait(5)
>+

Though it's not a big deal, I'd prefer adding this after _call_ of
the macsecSetup() instead of having it directly in the function.

It does not change anything from the code execution perspective, but IMO
the macsecSetup() should do the configuration of the MACSEC only and the
wait should be done separately (and more explicitly).

As we have discussed this face to face, it makes sense for macsecSetup() method to wait until the link is ready to be used.

Since we don't have macsec support in LNST, macsecSetup() method itself contains link creation, as well as address assignment and I presume that result of this call should be functional link.

In case, where you would want to call macsecSetup() method once again, you would have to put wait there yourself, as link might not be ready yet.

Still I'm ok to take the patch as is.

Acked-by: Jan Tluka <jtluka@redhat.com>

-Jan

> if netdev_cpupin:
> m1.run("service irqbalance stop")
> m2.run("service irqbalance stop")
>--
>2.20.1
>_______________________________________________
>LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org
>To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org
>Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>List Archives: https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedorahosted.org