GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-109015: Add test.support.socket_helper.tcp_blackhole() by vstinner · Pull Request #109016 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Sep 6, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: test_asyncio, test_imaplib and test_socket fail with FreeBSD TCP blackhole #109015

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot mentioned this pull request Sep 6, 2023
test_asyncio, test_imaplib and test_socket fail with FreeBSD TCP blackhole #109015
Closed
@vstinner vstinner mentioned this pull request Sep 6, 2023
gh-91960: Add FreeBSD build and test using Cirrus-CI [DEBUG PR] #108972
Closed
@vstinner
vstinner force-pushed the tcp_blackhole branch from 8f3344f to 57f552a Compare September 6, 2023 16:29
@vstinner

vstinner commented Sep 6, 2023

Copy link
Copy Markdown
Member Author

My previous DEBUG PR: PR #108972.

See also issue #91960 "Add FreeBSD build and test using Cirrus-CI" and PR #91961 which the TCP blackhole was discussed.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner force-pushed the tcp_blackhole branch from 57f552a to e8e8711 Compare September 6, 2023 18:02
@vstinner

vstinner commented Sep 6, 2023

Copy link
Copy Markdown
Member Author

I validated my fix on Cirrus-CI with this patch:

diff --git a/.cirrus.yml b/.cirrus.yml
index 823b1f921d66d..2e087f1f57706 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -7,9 +7,9 @@ freebsd_task:
   # local local connections to fail with ETIMEDOUT instead of ECONNREFUSED.
   # For more information see https://reviews.freebsd.org/D41751 and
   # https://github.com/cirruslabs/cirrus-ci-docs/issues/483.
-  sysctl_script:
-    - sysctl net.inet.tcp.blackhole=0
-    - sysctl net.inet.udp.blackhole=0
+#  sysctl_script:
+#    - sysctl net.inet.tcp.blackhole=0
+#    - sysctl net.inet.udp.blackhole=0
   build_script:
     - mkdir build
     - cd build

test_asyncio failed at the first version of my PR, which proves that the Cirrus-CI did its job: test my fix :-) I had to skip 2 more tests in test_asyncio.

At the second version, it passed: Total test files: run=461/461 skipped=25 resource_denied=1 rerun=1: https://cirrus-ci.com/task/4512145228955648 (test.test_asyncio.test_subprocess had to be re-run, it's unrelated to this fix.)

cc @emaste
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Sep 6, 2023

Copy link
Copy Markdown
Member Author

My second patch adds skips to Lib/test/test_asyncio/test_events.py:

  • test_create_connection_local_addr_skip_different_family()
  • test_create_connection_local_addr_nomatch_family()
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner force-pushed the tcp_blackhole branch from e8e8711 to 2737a22 Compare September 6, 2023 18:46
@vstinner
vstinner marked this pull request as ready for review September 6, 2023 18:46
@vstinner
vstinner requested review from a team, 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners September 6, 2023 18:46
@bedevere-bot bedevere-bot added the awaiting core review label Sep 6, 2023
@vstinner

vstinner commented Sep 6, 2023

Copy link
Copy Markdown
Member Author

I reverted the Cirrus CI change.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
pythongh-109015: Add test.support.socket_helper.tcp_blackhole()
43499b7 
Skip test_asyncio, test_imaplib and test_socket tests if FreeBSD TCP
blackhole is enabled (net.inet.tcp.blackhole=2).
@vstinner
vstinner force-pushed the tcp_blackhole branch from 7b7469c to 43499b7 Compare September 6, 2023 18:56
emaste
emaste reviewed Sep 6, 2023
View reviewed changes
Comment thread Lib/test/support/socket_helper.py Outdated
except KeyError:
pass

cmd = ['sysctl', name]

@emaste emaste Sep 6, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

sysctl provides a -n arg to suppress the name:

$ sysctl -n net.inet.tcp.blackhole
0

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vstinner vstinner Sep 6, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Interesting, I didn't know it. It's also supported by Linux (even if right now, this function is only called on FreeBSD ;-)). I updated my PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gvanrossum
gvanrossum removed their request for review September 6, 2023 19:23
@vstinner
Use -n option suggested by Ed
410b2d3
@vstinner

vstinner commented Sep 6, 2023

Copy link
Copy Markdown
Member Author

I ran a quick test on my latest PR:

vstinner@freebsd$ sudo sysctl net.inet.tcp.blackhole=0
net.inet.tcp.blackhole: 0 -> 0
vstinner@freebsd$ ./python -m test -v test_socket -m test_connect -v
(...)
test_connect (test.test_socket.NetworkConnectionNoServer.test_connect) ... ok
(...)
Result: SUCCESS

and:

vstinner@freebsd$ sudo sysctl net.inet.tcp.blackhole=2
net.inet.tcp.blackhole: 0 -> 2

vstinner@freebsd$ ./python -m test -v test_socket -m test_connect -v
(...)
test_connect (test.test_socket.NetworkConnectionNoServer.test_connect) ... skipped 'TCP blackhole is enabled (sysctl 
(...)
Result: SUCCESS
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Sep 6, 2023

Copy link
Copy Markdown
Member Author

@emaste: Would you mind to review the updated PR?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
emaste
emaste approved these changes Sep 6, 2023
View reviewed changes
Comment thread Lib/test/support/socket_helper.py Outdated
return None
output = proc.stdout

# Parse 'net.inet.tcp.blackhole: 0\n' to get '0'

@emaste emaste Sep 6, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think this comment is leftover

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
Update comment
4eea5ef
emaste
emaste approved these changes Sep 6, 2023
View reviewed changes
@vstinner
vstinner enabled auto-merge (squash) September 6, 2023 23:28
@vstinner vstinner added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Sep 6, 2023
@vstinner
vstinner disabled auto-merge September 6, 2023 23:57
@vstinner
vstinner merged commit a52a350 into python:main Sep 6, 2023
@vstinner
vstinner deleted the tcp_blackhole branch September 6, 2023 23:58
@miss-islington

miss-islington commented Sep 6, 2023

Copy link
Copy Markdown
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting core review label Sep 6, 2023
@miss-islington

miss-islington commented Sep 6, 2023

Copy link
Copy Markdown
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker a52a3509770f29f940cda9307704908949912276 3.11
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned vstinner Sep 6, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 6, 2023
@vstinner @miss-islington
pythongh-109015: Add test.support.socket_helper.tcp_blackhole() (pyth…
a766a68 
…onGH-109016)

Skip test_asyncio, test_imaplib and test_socket tests if FreeBSD TCP
blackhole is enabled (net.inet.tcp.blackhole=2).
(cherry picked from commit a52a350)

Co-authored-by: Victor Stinner <[email protected]>
@bedevere-bot

bedevere-bot commented Sep 6, 2023

Copy link
Copy Markdown

GH-109041 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Sep 6, 2023
vstinner added a commit to vstinner/cpython that referenced this pull request Sep 7, 2023
@vstinner
pythongh-109015: Add test.support.socket_helper.tcp_blackhole() (pyth…
bc1579e 
…on#109016)

Skip test_asyncio, test_imaplib and test_socket tests if FreeBSD TCP
blackhole is enabled (net.inet.tcp.blackhole=2).

(cherry picked from commit a52a350)
@bedevere-bot

bedevere-bot commented Sep 7, 2023

Copy link
Copy Markdown

GH-109042 is a backport of this pull request to the 3.11 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Sep 7, 2023
vstinner added a commit that referenced this pull request Sep 7, 2023
@vstinner
[3.11] gh-109015: Add test.support.socket_helper.tcp_blackhole() (#10…
1ebe014 
…9016) (#109042)

gh-109015: Add test.support.socket_helper.tcp_blackhole() (#109016)

Skip test_asyncio, test_imaplib and test_socket tests if FreeBSD TCP
blackhole is enabled (net.inet.tcp.blackhole=2).

(cherry picked from commit a52a350)
Yhg1s pushed a commit that referenced this pull request Sep 8, 2023
@miss-islington @vstinner
[3.12] gh-109015: Add test.support.socket_helper.tcp_blackhole() (GH-…
c9dc2bc 
…109016) (#109041)

gh-109015: Add test.support.socket_helper.tcp_blackhole() (GH-109016)

Skip test_asyncio, test_imaplib and test_socket tests if FreeBSD TCP
blackhole is enabled (net.inet.tcp.blackhole=2).
(cherry picked from commit a52a350)

Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner
1 more reviewer
@emaste emaste emaste approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@vstinner vstinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@vstinner @miss-islington @bedevere-bot @emaste
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.