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.

Sign up for GitHub

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

Jump to bottom

Socket configuration #3979

Merged
merged 9 commits into from
Sep 1, 2022
Merged

Socket configuration #3979

merged 9 commits into from
Sep 1, 2022
+312 −217

Conversation

coot
Copy link
Contributor

@coot coot commented Aug 28, 2022 •
edited
Loading

Description

Fixes #3977

  • Updated network dependency (Export StructLinger haskell/network#491 was merged)
  • snocket: various stylistic changes
  • snocket: do not configure sockets in bind method
  • connection-manager: add a method to configure connection sockets
  • socket: added methods to configure sockets
  • Increment zfs copy fail counter

Checklist

  • Branch
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • New tests are added if needed and existing tests are updated
    • If this branch changes Consensus and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@coot coot requested review from bolt12 and njd42 August 28, 2022 11:07
@coot coot force-pushed the coot/systemd-socket-activation-support branch 2 times, most recently from fe5cd72 to 2d4cee8 Compare August 28, 2022 16:04
@coot coot marked this pull request as ready for review August 29, 2022 07:13
@coot coot force-pushed the coot/systemd-socket-activation-support branch from 2d4cee8 to 789366d Compare August 29, 2022 07:15
@coot coot requested a review from nfrisby as a code owner August 29, 2022 07:15
bolt12
bolt12 approved these changes Aug 29, 2022
View reviewed changes
Copy link
Contributor

@bolt12 bolt12 left a comment

Choose a reason for hiding this comment

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

LGTM!

@coot coot force-pushed the coot/systemd-socket-activation-support branch 3 times, most recently from 5c6ee07 to 7b6a25b Compare August 29, 2022 14:52
@coot
Updated network dependency
74598cc 
network-3.1.2.2 package exposes `StructLinger`, see
haskell/network#491
@coot
snocket: various stylistic changes
2830297
@coot
snocket: do not configure sockets in bind method
0bbef16 
Configuring sockets in `Ouroboros.Network.Snocket.bind` method has the
disadvantage that systemd sockets, or outbound sockets (for which we do
not use bind method) are left not configured.

This patch removes sockets configuration in `bind` method, in a later
commit it will be added as a diffusion level callbacks which configure
the listening  sockets  (and thus accept sockets) as well as connection
sockets.
@coot
connection-manager: add a method to configure connection sockets
251fcb5
@coot coot force-pushed the coot/systemd-socket-activation-support branch 3 times, most recently from bad958e to 9c5cc64 Compare August 29, 2022 16:24
@coot coot requested a review from dnadales as a code owner August 29, 2022 16:24
@coot coot force-pushed the coot/systemd-socket-activation-support branch 7 times, most recently from 928b52c to 5375cde Compare August 30, 2022 10:23
@coot
Copy link
Contributor Author

coot commented Aug 30, 2022

bors merge

iohk-bors bot added a commit that referenced this pull request Aug 30, 2022
@iohk-bors @coot
Merge #3979
02f55b7 
3979: Socket configuration r=coot a=coot

# Description

Fixes #3977
         
- Updated network dependency (haskell/network#491 was merged)  
- snocket: various stylistic changes       
- snocket: do not configure sockets in bind method 
- connection-manager: add a method to configure connection sockets 
- socket: added methods to configure sockets 
- Increment zfs copy fail counter
     



Co-authored-by: Marcin Szamotulski <coot@coot.me>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 30, 2022

Timed out.

@coot coot force-pushed the coot/systemd-socket-activation-support branch from 5375cde to 22ecc40 Compare August 30, 2022 14:54
@coot
Copy link
Contributor Author

coot commented Aug 31, 2022

bors merge

iohk-bors bot added a commit that referenced this pull request Aug 31, 2022
@iohk-bors @coot
Merge #3979
7497a25 
3979: Socket configuration r=coot a=coot

# Description

Fixes #3977
         
- Updated network dependency (haskell/network#491 was merged)  
- snocket: various stylistic changes       
- snocket: do not configure sockets in bind method 
- connection-manager: add a method to configure connection sockets 
- socket: added methods to configure sockets 
- Increment zfs copy fail counter
     



Co-authored-by: Marcin Szamotulski <coot@coot.me>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 31, 2022

Timed out.

@coot
Copy link
Contributor Author

coot commented Aug 31, 2022 •
edited
Loading

There is a strange test failure on Windows:

configuration bind getSocketName error
yes yes yes getSocketName: failed (Invalid argument (WSAEINVAL)) unexpected
no yes yes bind: failed (Address already in use (WSAEADDRINUSE)) expected
yes no yes no error (test finishes successfully)
yes yes no bind: failed (Permission denied (WSAEACCESS)) unexpected

configuration options for connection socket: ReuseAddr, NoDelay, Linger; Sequence: configuration -> bind -> connect -> getSocketName.

@coot
socket: added methods to configure sockets
bf91434 
* `configureSocket`: used by listening and connection sockets by the
  connection manager
* `configureSystemdSocket`: used by listening sockets passed through
  systemd socket activation
* `configureOutboundSocket`: sued by connection sockets created by
  `connectTo`

These methods are only used to configure `node-to-node` sockets; We
don't do anything special to unix sockets or named pipes.
@coot
subscription: exception handling
65dfc9e 
Report exceptions thrown when configuring the socket.
@coot
subscription: configure sockets
ccc6377 
Use `Ouroboros.Network.Socket.configureSocket` to configure outbound
sockets.
@coot
Increment zfs copy fail counters
8e11ee2
@coot coot force-pushed the coot/systemd-socket-activation-support branch from 22ecc40 to 8e11ee2 Compare August 31, 2022 13:50
@coot
Copy link
Contributor Author

coot commented Aug 31, 2022

It turned out that the issue was caused by first calling bind then listen then configuring the socket; if done in the right order (configure, bind, listen) things should work 🤞

@coot
Copy link
Contributor Author

coot commented Sep 1, 2022

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 1, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit a0e1c2b into master Sep 1, 2022
@iohk-bors iohk-bors bot deleted the coot/systemd-socket-activation-support branch September 1, 2022 08:44
@CarlosLopezDeLara CarlosLopezDeLara mentioned this pull request Sep 21, 2022
Open
40 tasks
This was referenced Oct 27, 2022
Closed
Merged
iohk-bors bot added a commit that referenced this pull request Nov 9, 2022
@iohk-bors @mtullsen @coot
Merge #4120
e933084 
4120: Cherry picked network changes for cardano-node-1.35.5 release r=coot a=coot

This cherry-picked patches from the following PRs:

* #3794
* #3844
* #3785
* #3904
* #3915
* #3852
* #3970
* #3979
* #4015
* #4067
* #4004
* #4086
* #4113
* #4106
* #4127
* #4103

Also cherry-picked almost all the commits which modify GitHub actions:
* 18c5244 Run GitHub Actions on pull requests   
* 3adf5a9 Use newer version of io-sim           
* ee9b7a6 Fix GH Actions Windows CI: switch from pkgconf to pkg-config 
* e6cf074 github-actions: use `ubuntu-latest`   
* 9a8b959 Updated versions of github actions    
* fc8f8f0 Fix GH Actions Windows CI caching     
* 7f07c40 Windows Github Actions now use MSYS2  
* b21a7ce Fix chocolatey CI error
* #4134               

TODO:

* [x] bump versions of packages
* [x] input-output-hk/cardano-haskell-packages#84

Co-authored-by: Mark Tullsen <tullsen@galois.com>
Co-authored-by: Marcin Szamotulski <coot@coot.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bolt12 bolt12 bolt12 approved these changes

@njd42 njd42 Awaiting requested review from njd42

@nfrisby nfrisby Awaiting requested review from nfrisby

@dnadales dnadales Awaiting requested review from dnadales

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Configure sockets passed through systemd socket activation
2 participants
@coot @bolt12