Skip to content

Switching Windows to use oobConn #4962

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
Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Switching Windows to use oobConn #4962

wants to merge 9 commits into from

Conversation

Samsonnyyeet
Copy link

@Samsonnyyeet Samsonnyyeet commented Feb 17, 2025 •
edited
Loading

Which issue does this address?

#4325

Purpose

This draft PR showcases the current progress on implementing oobConn support for Windows. Since this is my first time working on something like this, I recognize that the implementation is still a work in progress. There are several open issues and uncertainties, which I have marked with TO DO: comments.

Most of the code is largely based on the existing Linux implementation, with minimal changes to maintain clarity for existing contributors. I would sincerely appreciate any feedback or guidance on my approach, as well as suggestions for improvements.

Major Changes and Issues

  • Implemented cmsg support from scratch, as I couldn’t find existing support in the Go libraries.
  • Using a workaround for ReadBatch, as it is unsupported on Windows.
  • ECN support is still in progress; I am encountering several issues getting it to function correctly.
  • Unable to find a Windows equivalent for Linux’s forced receive and send buffer settings.
  • No isGSOError function implemented yet.
  • Current tests need significant improvement, both in structure and coverage.

Any feedback or guidance would be greatly appreciated!

Samsonnyyeet and others added 8 commits February 17, 2025 11:57
@Samsonnyyeet
port windows to use OOBCapablePacketConn
4f0a386 
need to figure out how to handle cmsg data on
windows with go. quinn and ws2def.h are calculating
certain values at compile time, an i'm yet to figure
out how to do this at compile time in go, or what is
the convention to doing this in go.
@Samsonnyyeet
Adding cmsg support
de82c82 
TO DO:
- fix TCLASS flag
- add parse ipv4 pkt info function
- fix readpacket function
@samarth-clone @Samsonnyyeet
Added ParseIPv4pktInfo
cd9e982
@Samsonnyyeet
fixed TCLASS flag, added documentation
3e8769e
@Samsonnyyeet
added writepacket
8b5991d
@Samsonnyyeet
implemented appendUPDDegmentSizeMsg
3c9e18c
@samarth-clone @Samsonnyyeet
Added testing for GSO, removed Readbatch which is unsupported on windows
e65e88e
@Samsonnyyeet
FINALLY fixed control message struct
c74ca80 
Added documentation
Fixed some socket options
Temporarily finished some missing functions

Pending:
- ECN support checking function
- Fixing append ecn ipv4 and ipv6 functions
- Writing unit tests for the rest of the functions
@google-cla Google CLA
Copy link

google-cla bot commented Feb 17, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Samsonnyyeet Samsonnyyeet marked this pull request as ready for review February 17, 2025 16:00
@Samsonnyyeet Samsonnyyeet marked this pull request as draft February 17, 2025 16:01
@Samsonnyyeet
Pretty much everything is fixed.
3c4e31c 
- Fixed Control Message Support
- Fixed ECN Support
- Removed ReadBatch
- Simplified oobConn
- Removed unnecessary comments
@Samsonnyyeet
Copy link
Author

So I have fixed a ton of the issues with the first iteration. Still need to work on tests.

I have simplified the oobConn structure and entirely removed batched reads. ECN support has been fixed as well (correct control messages are now being generated). There was an oversight with the Control Message struct, which has been rectified. Also fixed a lot of the wrong socket options.

Current issues/doubts:

  • Windows expects c_int as the data type for the body of the ecn control messages. Do I continue using int32 or do I import and use cgo.
  • WritePacket requires uint16 for gsoSize, which I am currently just converting to uint32 as expected by windows.
  • Which options do I use for forceSetReceiveBuffer and forceSetSendBuffer?
  • Still need to implement isECNEnabled (totally overlooked this 😭, will try to finish this asap)
  • What tests should I focus on?
  • Any other comments/advice please!

@Samsonnyyeet
Copy link
Author

Hey @marten-seemann !
Would love to get your opinion on the current structure of things.

@Samsonnyyeet Samsonnyyeet changed the title Switching Wndows to use oobConn Switching Windows to use oobConn Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Samsonnyyeet @samarth-clone