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.

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

net: inconsistent behavior between ReadFrom and Read on IPConn for IPv4 #3944

Closed
mikioh opened this issue Aug 11, 2012 · 12 comments

Comments

Projects
None yet
5 participants
@mikioh
Copy link
Contributor

commented Aug 11, 2012

What steps will reproduce the problem?
1. run pingxbind and pingxdial
2.
3.

What is the expected output? What do you see instead?
both report done but got MISMATCH from pingxdial

Please use labels and text to provide additional information.

Attachments:

  1. pingxbind.go (2462 bytes)
  2. pingxdial.go (2379 bytes)
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Aug 11, 2012

Comment 1:

Try to demistify SOCK_RAW with AF_INET/AF_INET6 world...
[SOCK_RAW with AF_INET]
at syscall:
- both Send,  SendTo prepend an IPv4 header automatically by default.
  sockopt IP_HDRINCL disables IPv4 header prepending.
- both Recv, Recvfrom return an incoming packet with an IPv4 header and options intact.
at net:
- both Write,  WriteTo prepend an IPv4 header automatically by default.
  sockopt IP_HDRINCL disables IPv4 header prepending.
- Read returns an incoming packet with an IPv4 header and options intact.
- ReadFrom returns an incoming packet without an IPv4 header and options.
[SOCK_RAW with AF_INET6]
at syscall:
- both Send,  SendTo prepend an IPv6 header automatically.
- both Recv, Recvfrom return an incoming packet without an IPv6 header and extension
headers.
at net:
- both Write,  WriteTo prepend an IPv6 header automatically.
- both Read, RedFrom return an incoming packet without an IPv6 header and extension
headers.
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Aug 11, 2012

Comment 2:

Looks like removing an IPv4 header in net.ReadFrom might be pointless for 
the applications that use SOCK_RAW with AF_INET.
For example, an app such as OSPF requires:
- IP multicast packet exchange for neighboring,
- IP unicast packet exchange btw DR/BDR and DROTHER nodes.
That means that the app needs to identify a pair of source and destination 
IPv4 addresses for OSPF message processing but net.ReadFrom bothers us 
by dropping IPv4 header.
In the case of OSPF for IPv6 we usually use recvmsg/sendmsg with ancillary 
data instead.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2012

Comment 3:

Labels changed: added go1.1maybe.

@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2013

Comment 4:

Status changed to Started.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2013

Comment 5:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

@bradfitz

This comment has been minimized.

Copy link
Member

commented May 14, 2013

Comment 6:

Since we didn't accept the change in https://golang.org/cl/6445105/ as being
too risky (or an API change), what about an alternate fix:
Can there be an API addition to let users opt-in to the correct ("new") behavior
per-IPConn? Some new method on IPConn that returns a new IPConn with a "wantGoodBehavior
bool" set true?
Gross, but I don't see how else to fix this safely, without breaking old users.
Or are old users completely busted anyway?  That's what nobody understood before, and
why this didn't go into 1.1.
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2013

Comment 7:

> Or are old users completely busted anyway?
How did you read my mind? ;) Yup, will take Plan B Russ suggested; mark something like
"we would not recommend to use ReadFrom, ReadFromIP of IPConn because of this bug,
breaking traditional, portable old BSD school API behavior, ouch. Please use ReadMsgIP
instead until Go 2." at BUG section.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2013

Comment 8:

Labels changed: added go1.2maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2013

Comment 9:

Labels changed: added feature.

@robpike

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2013

Comment 10:

Mikio, are you planning to make the documentation change?
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2013

Comment 11:

Yes, just sent: https://golang.org/cl/13263043/
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2013

Comment 12:

This issue was closed by revision a8b4a1e.

Status changed to Fixed.

@mikioh mikioh added fixed labels Aug 29, 2013

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015

@rsc rsc removed the go1.2maybe label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.