-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
UDP: Ensure propagation of control message to pool.Message #510
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #510 +/- ##
==========================================
+ Coverage 72.52% 73.85% +1.32%
==========================================
Files 70 71 +1
Lines 5504 5729 +225
==========================================
+ Hits 3992 4231 +239
+ Misses 1115 1112 -3
+ Partials 397 386 -11 ☔ View full report in Codecov by Sentry. |
03ea641
to
9719626
Compare
net/connUDP.go
Outdated
@@ -24,10 +24,36 @@ type UDPConn struct { | |||
} | |||
|
|||
type ControlMessage struct { | |||
Dst net.IP // destination address, receiving only | |||
Src net.IP // source address, specifying only |
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.
Source-Address is also valid when receiving packet, same address as the "remote" parameter you get from the read
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.
According to doc the https://pkg.go.dev/golang.org/x/net@v0.18.0/ipv4#ControlMessage it seems that Src
cannot be set for write.
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.
doc says "specifying only" for source (as in "write only).
but I am quite sure (just tested it on Debian) that you ALSO get the source set when receiving.
net/connUDP.go
Outdated
@@ -24,10 +24,36 @@ type UDPConn struct { | |||
} | |||
|
|||
type ControlMessage struct { | |||
Dst net.IP // destination address, receiving only | |||
Src net.IP // source address, specifying only | |||
IfIndex int // interface index, must be 1 <= value when specifying |
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.
Interface-Index 0 is defined as "any interface", so I think the "must be 1<=" part of the comment can be removed.
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.
Fixed, thx :)
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 just checked out the latest commit of this branch and I successfully got src/dst-IP address and Interface Index. I am looking forward to see this merged.
13d6653
to
9b60602
Compare
Just re-tested with the current commit, still works fine. |
53f734c
to
2249b85
Compare
d4bd6f1
to
848a6cf
Compare
This commit enhances the UDP functionality, ensuring proper dissemination of control messages to pool.Message for improved network coordination and responsiveness
3ac36df
to
7e6b2bb
Compare
@HRogge Pls this is a final state. Could you check if it works as it is expected ? |
Kudos, SonarCloud Quality Gate passed! |
It seems that multicast transmission is broken... not sure why, still trying to understand. I get neither a compiler nor a runtime error when using WriteMulticast() on net.UDPConn |
I also get a broken Date for the current commit when I look at it with 'git log':
|
It seems my multicast now ignores the specified interface and goes along the default route (into my local management interface). |
facepalm okay, got the problem resolved... stupidity on my side. |
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.
Code works fine with the multicast application I have been using for testing. Looking forward to see it merged.
This commit enhances the UDP functionality, ensuring proper dissemination of control messages to pool.Message for improved network coordination and responsiveness