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
Closed

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

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

Comments

@mikioh
Copy link
Contributor

mikioh 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
Copy link
Contributor Author

mikioh 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
Copy link
Contributor Author

mikioh 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
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 3:

Labels changed: added go1.1maybe.

@mikioh
Copy link
Contributor Author

mikioh commented Mar 6, 2013

Comment 4:

Status changed to Started.

@rsc
Copy link
Contributor

rsc commented Mar 12, 2013

Comment 5:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

@bradfitz
Copy link
Contributor

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
Copy link
Contributor Author

mikioh 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
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 8:

Labels changed: added go1.2maybe.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 9:

Labels changed: added feature.

@robpike
Copy link
Contributor

robpike commented Aug 16, 2013

Comment 10:

Mikio, are you planning to make the documentation change?

@mikioh
Copy link
Contributor Author

mikioh commented Aug 27, 2013

Comment 11:

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

@mikioh
Copy link
Contributor Author

mikioh commented Aug 29, 2013

Comment 12:

This issue was closed by revision a8b4a1e.

Status changed to Fixed.

@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.
Projects
None yet
Development

No branches or pull requests

5 participants