UDP: Ensure propagation of control message to pool.Message #510

jkralik · 2023-11-12T10:39:28Z

This commit enhances the UDP functionality, ensuring proper dissemination of control messages to pool.Message for improved network coordination and responsiveness

codecov-commenter · 2023-11-12T10:41:57Z

Codecov Report

Attention: 47 lines in your changes are missing coverage. Please review.

Comparison is base (d63d5c8) 72.52% compared to head (7e6b2bb) 73.85%.

Files	Patch %	Lines
net/connUDP.go	77.83%	40 Missing and 5 partials ⚠️
message/pool/message.go	80.00%	1 Missing and 1 partial ⚠️

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.
📢 Have feedback on the report? Share it here.

HRogge · 2023-11-13T08:11:58Z

net/connUDP.go

@@ -24,10 +24,36 @@ type UDPConn struct {
 }

 type ControlMessage struct {
+	Dst     net.IP // destination address, receiving only
 	Src     net.IP // source address, specifying only


Source-Address is also valid when receiving packet, same address as the "remote" parameter you get from the read

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.

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.

HRogge · 2023-11-13T08:12:48Z

net/connUDP.go

@@ -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


Interface-Index 0 is defined as "any interface", so I think the "must be 1<=" part of the comment can be removed.

Fixed, thx :)

HRogge

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.

HRogge · 2023-11-13T13:08:22Z

Just re-tested with the current commit, still works fine.

.github/workflows/test.yml

net/connUDP.go

This commit enhances the UDP functionality, ensuring proper dissemination of control messages to pool.Message for improved network coordination and responsiveness

jkralik · 2023-11-15T07:33:39Z

@HRogge Pls this is a final state. Could you check if it works as it is expected ?

sonarcloud · 2023-11-15T07:34:15Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

82.1% Coverage
0.0% Duplication

HRogge · 2023-11-15T08:10:46Z

@HRogge Pls this is a final state. Could you check if it works as it is expected ?

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

HRogge · 2023-11-15T08:12:20Z

I also get a broken Date for the current commit when I look at it with 'git log':

commit 7e6b2bbd34f0051d0ff09d06af2d453450a30706 (HEAD -> jkralik/feature/udp-propagate-interface, github-upstream/jkralik/feature/udp-propagate-interface)
Author: Jozef Kralik <jojo.lwin@gmail.com>
Date:   Wed May 10 15:11:10 2023 +0000

    Update UDP: Ensure propagation of control message to pool.Message
    
    This commit enhances the UDP functionality, ensuring proper dissemination
    of control messages to pool.Message for improved network coordination
    and responsiveness

HRogge · 2023-11-15T08:27:20Z

@HRogge Pls this is a final state. Could you check if it works as it is expected ?

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

It seems my multicast now ignores the specified interface and goes along the default route (into my local management interface).

HRogge · 2023-11-15T08:31:42Z

@HRogge Pls this is a final state. Could you check if it works as it is expected ?

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

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.

HRogge

Code works fine with the multicast application I have been using for testing. Looking forward to see it merged.

jkralik requested review from HRogge and Danielius1922 November 12, 2023 10:39

jkralik force-pushed the jkralik/feature/udp-propagate-interface branch 3 times, most recently from 03ea641 to 9719626 Compare November 13, 2023 08:05

HRogge reviewed Nov 13, 2023

View reviewed changes

HRogge approved these changes Nov 13, 2023

View reviewed changes

jkralik force-pushed the jkralik/feature/udp-propagate-interface branch 6 times, most recently from 13d6653 to 9b60602 Compare November 13, 2023 13:07

jkralik force-pushed the jkralik/feature/udp-propagate-interface branch 5 times, most recently from 53f734c to 2249b85 Compare November 13, 2023 16:36

jkralik marked this pull request as ready for review November 13, 2023 16:36

jkralik force-pushed the jkralik/feature/udp-propagate-interface branch 2 times, most recently from d4bd6f1 to 848a6cf Compare November 14, 2023 06:57

Danielius1922 reviewed Nov 14, 2023

View reviewed changes

.github/workflows/test.yml Show resolved Hide resolved

net/connUDP.go Outdated Show resolved Hide resolved

Danielius1922 approved these changes Nov 14, 2023

View reviewed changes

jkralik added 2 commits November 15, 2023 07:30

calculate coverage and report also from the other platforms

e7a885a

Update UDP: Ensure propagation of control message to pool.Message

7e6b2bb

This commit enhances the UDP functionality, ensuring proper dissemination of control messages to pool.Message for improved network coordination and responsiveness

jkralik force-pushed the jkralik/feature/udp-propagate-interface branch from 3ac36df to 7e6b2bb Compare November 15, 2023 07:30

jkralik requested a review from HRogge November 15, 2023 07:31

HRogge approved these changes Nov 15, 2023

View reviewed changes

jkralik changed the title ~~Update UDP: Ensure propagation of control message to pool.Message~~ UDP: Ensure propagation of control message to pool.Message Nov 15, 2023

jkralik merged commit ce37a93 into master Nov 15, 2023
13 checks passed

jkralik deleted the jkralik/feature/udp-propagate-interface branch November 15, 2023 10:01

jkralik mentioned this pull request Nov 15, 2023

Incoming interface #493

Closed

jlevesy mentioned this pull request Sep 16, 2024

sendmsg invalid argument on udp connections with go-coap version >= 3.2.0 #581

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UDP: Ensure propagation of control message to pool.Message #510

UDP: Ensure propagation of control message to pool.Message #510

jkralik commented Nov 12, 2023

codecov-commenter commented Nov 12, 2023 •

edited

Loading

HRogge Nov 13, 2023

jkralik Nov 13, 2023

HRogge Nov 13, 2023

HRogge Nov 13, 2023

jkralik Nov 14, 2023

HRogge left a comment

HRogge commented Nov 13, 2023

jkralik commented Nov 15, 2023 •

edited

Loading

sonarcloud bot commented Nov 15, 2023

HRogge commented Nov 15, 2023

HRogge commented Nov 15, 2023

HRogge commented Nov 15, 2023

HRogge commented Nov 15, 2023

HRogge left a comment

UDP: Ensure propagation of control message to pool.Message #510

UDP: Ensure propagation of control message to pool.Message #510

Conversation

jkralik commented Nov 12, 2023

codecov-commenter commented Nov 12, 2023 • edited Loading

Codecov Report

HRogge Nov 13, 2023

Choose a reason for hiding this comment

jkralik Nov 13, 2023

Choose a reason for hiding this comment

HRogge Nov 13, 2023

Choose a reason for hiding this comment

HRogge Nov 13, 2023

Choose a reason for hiding this comment

jkralik Nov 14, 2023

Choose a reason for hiding this comment

HRogge left a comment

Choose a reason for hiding this comment

HRogge commented Nov 13, 2023

jkralik commented Nov 15, 2023 • edited Loading

sonarcloud bot commented Nov 15, 2023

HRogge commented Nov 15, 2023

HRogge commented Nov 15, 2023

HRogge commented Nov 15, 2023

HRogge commented Nov 15, 2023

HRogge left a comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 12, 2023 •

edited

Loading

jkralik commented Nov 15, 2023 •

edited

Loading