Reuse VMs and snabbnfv-traffic instance in lib/nfv/selftest.sh

[eugeneia]

It works. 👍

This branch includes commits by Javier which fix bugs in the Intel driver related to on-line reconfiguration.

[lukego]

@eugeneia SnabbBot has failed this and all other open pull requests. How can we resolve this and get the working changes merged?

[eugeneia]

@lukego Javier thought of what slipped by me yesterday: The cards were all bound to the ixgbe driver after reboots so we had intel test failures everywhere.... which put me off quite a bit. WIll rerun the CI today.

[lukego]

NB: We already have a function lib.hardware.pci.unbind_device_from_linux() that will free a device from the ixgbe driver. We should call that from some appropriate place so that it happens for the Intel app.

[eugeneia]

@lukego There is still some room for false positives by SnabbBot, I ran the intel selftest 60 times before rerunning SnabbBot again and it had one failure on the card SnabbBot uses... See my snabb-devel post on this dilemma.

[eugeneia]

@lukego Regarding lib.hardware.pci.unbind_device_from_linux(): Should intel_app do the unbinding? @javierguerragiraldez stated that we did that before but stopped doing that. Why?

If not the documentation of intel_app should probably state that the NIC needs to be unbound. snabb-nfv-traffic/lib.nfv.config could be the second best place to unbind the NICs before they are used.

[lukego]

@eugeneia Yes, I think the intel_app should do the unbinding, if not the pci library. I don't think we removed this for any very good reason. Could have been collateral damage when I removed the VFIO code.

[lukego]

This is great work! both the code itself and the Git-fu to do a multi-person topic branch :-)

The only problem I have is that this pull request adds a bunch of lines of commented-out code without explanation. Can we fix that somehow e.g. remove those lines?

[eugeneia]

@javierguerragiraldez As per luke's request I removed the newly commented-out code (in 86e85b4). Is that sensible?

[javierguerragiraldez]

@javierguerragiraldez As per luke's request I removed the newly commented-out code (in 86e85b4). Is that sensible?

No objection from me. I didn't delete it originally because this whole thing passed through a very long phase of swinging code around and not knowing what made the failure rate rise or fall :-)

Still, I think this would change a lot when the straightline branch gets merged...

[lukego]

To me there should be an additional step after swinging code around with trial and error, etc, which is to say "okay, now that it works, what are the minimum changes needed to keep it working?". Then we work out what is essential to the solution and should be kept vs. what is incidental and should be removed (or taken separately).

I don't want to feel like I am pulling code that is just the contents of the working directory at the time something started working. Then when something else stops working it will be really hard to diagnose from the history.