Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race condition for parallel PTF nanomsg tests by using network namespaces. #4042

Merged
merged 3 commits into from
Jun 21, 2023

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Jun 20, 2023

The P4Tools PTF tests using nanomsg are failing because the device ID and TCP ports are conflicting during parallel execution.

Use network namespaces as a bandaid to avoid the spurious CI failures until I find a better solution.

@fruffy fruffy force-pushed the fruffy/fix_ptf_nn_racecondition branch from 746a9d9 to 73eb528 Compare June 20, 2023 17:03
@fruffy fruffy marked this pull request as ready for review June 20, 2023 18:33
@fruffy fruffy changed the title Fix race condition for parallel PTF nanomsg tests. Fix race condition for parallel PTF nanomsg tests by using network namespaces. Jun 20, 2023
@mihaibudiu
Copy link
Contributor

I am not the right person to review ptf programs, you may ask contributors to the ebpf backend, they wrote many

@fruffy
Copy link
Collaborator Author

fruffy commented Jun 20, 2023

I am not the right person to review ptf programs, you may ask contributors to the ebpf backend, they wrote many

This is a problem I introduced with #3951 unrelated to the PTF programs in the eBPF back end. The issue is that race conditions can occur when two tests bind to the same port and the existing solution of checking available ports does not seem work. Since the tests are not contained in namespaces one of the programs will fail.

This PR just hotfixes this regression so that other PRs can pass.

"---------------------- Creating a namespace ----------------------",
)
random.seed(datetime.now().timestamp())
bridge = Bridge(str(random.randint(0, sys.maxsize)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't a guid be safer?

@fruffy fruffy merged commit 07924a6 into main Jun 21, 2023
@fruffy fruffy deleted the fruffy/fix_ptf_nn_racecondition branch June 21, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants