search

quic-interop / quic-network-simulator

ns3-based network simulator for QUIC testing
Other
151 stars 45 forks source link

upgrade ns3 and go versions #113

Closed jaikiran closed 9 months ago

jaikiran commented 9 months ago

This PR is a proposal to upgrade the ns3 version from the currently used 3.32 to the latest released 3.40.

I started this upgrade because I was running into intermittent failures in the quic-interop-runner tests, where on some occasions, the network communication between the client and server containers would never happen and the test would thus fail. Rerunning those same tests would then make them pass. I wanted to see if ns3 library (used in the sim container) was playing a role and decided to upgrade it to the latest version. Even after the upgrade to ns3 3.40 version, the same intermittent failures continue to happen. So I think at least the version of ns3 can be ruled out.

Since I had already worked on the upgrade, I decided to propose this as a PR to see if there's any interest in including this upstream in this repo. I've been using this upgraded version for the quic-interop-runner for at least a week or more now and haven't run into any regressions or other failures (like I noted, the intermittent failures which were happening before the upgrade continue to happen, so nothing changes on that front).

Since ns3 3.36 version, there have been changes to the way ns3 project is built. So the changes in this PR use a different command to build ns3 (as noted in their documentation). Additionally, a CMakeLists.txt has been introduced in the scenarios directory to build the quic specific scenarios. I think this should make it easier and cleaner to maintain the build of these "scenarios" instead of patching the ns3 project's build file.

I had to patch a couple of ns3 project's file to allow us to be able to build the executable files without including the version number of ns3. That way we still get to launch these executables in quic-interop-runner's sim container just by their name (for example: simple-p2p instead of ns3.40-simple-p2p).

While at it, this commit also upgrades the go version to 1.21.1.

I have run the entire set of tests in quic-interop-runner with quic-go as the server and the client and haven't seen any failures.

marten-seemann commented 9 months ago

Looks like we have two PRs now that do the same thing: #110 and this one. Which one should we take?

jaikiran commented 9 months ago

Hello Marten, I don't mind if the other one is accepted (I hadn't seen that PR when I started work on this). I don't have prior experience in this project so I don't know which of these 2 PRs would be preferable. Whichever makes it easier to maintain for you all, is fine with me. I'll keep a watch on the other PR too and if that one gets accepted, I will close mine.

marten-seemann commented 9 months ago

I fixed CI in #114. Can you rebase please?

jaikiran commented 9 months ago

Done. I've rebased this PR with the latest master branch.

jaikiran commented 9 months ago

Hello Marten, was this PR expected to trigger some github jobs? I see that it says the job workflow is waiting for an approval from a maintainer to trigger it.

jaikiran commented 9 months ago

I'm closing this in favour of https://github.com/quic-interop/quic-network-simulator/pull/110