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

windows net: replacement for /etc/protocols #2215

Closed
nerdatmath opened this issue Sep 1, 2011 · 9 comments
Closed

windows net: replacement for /etc/protocols #2215

nerdatmath opened this issue Sep 1, 2011 · 9 comments

Comments

@nerdatmath
Copy link
Contributor

Before filing a bug, please check whether it has been fixed since
the latest release: run "hg pull", "hg update default", rebuild, and
retry
what you did to
reproduce the problem.  Thanks.

What steps will reproduce the problem?
1. Attempt to open an IP raw socket with ListenIP or DialIP, with a netProto argument of
"ip4:icmp"
2. Send packets out using this socket
3. dump the packets with Wireshark or equivalent

What is the expected output?
The protocol on the packets is expected to be 1, meaning "icmp"

What do you see instead?
The protocol on the packets is 0.

Which compiler are you using (5g, 6g, 8g, gccgo)?
8g

Which operating system are you using?
Windows XP SP2

Which revision are you using?  (hg identify)
gomingw, release "gowin32_release.r59"

Please provide any additional information below.

In src/pkg/net/iprawsock.go, function readProtocols(), a platform specific assumption is
made, namely that the protocols file is named "/etc/protocols". On Windows XP,
it is named "C:\WINDOWS\system32\drivers\etc\protocol". Note that not only is
the directory different, but the word "protocol" is singular instead of plural.

My suggested fix would be any of the following:

1. Switch to using the C library function "getprotobyname", which exists on
both Windows and UNIX/POSIX and probably OS/X and plan9. This is in keeping with the way
services and hosts are looked up, in lookup_windows.go and lookup_unix.go.

2. Hard code a set of well-known IP protocols and use it if /etc/protocols is not found
on the system.

3. Choose which file to read based on the platform.
@alexbrainman
Copy link
Member

Comment 1:

You are correct. readProtocols will not work on windows. I think, getprotobyname is a
good alternative. Send a fix, if you like.

Labels changed: added packagebug, os-windows.

Owner changed to @alexbrainman.

Status changed to Accepted.

@nerdatmath
Copy link
Contributor Author

Comment 2:

Thanks. I will be submitting a patch for issues 2215 and 2216. This will
also present a new public net.LookupProtocol() function, which will invoke
getprotobyname. I prefer to use the reentrant version on Unix -
getprotobyname_r, however this is technically not POSIX. It works on Linux
and I suspect it will work on the other Unixes supported by go, since it was
in the original BSD, however I don't have easy access to those platforms to
test on. Please let me know if you think that using getprotobyname_r would
get the patch rejected.
I will also add the relevant tests. I fear the patchset may look a little
large but I want to do it right.

@alexbrainman
Copy link
Member

Comment 3:

Without looking at your code, it is hard to tell. Please, do not send big changes first
up. Try to do something small first. This should make your first submission quicker and
easier for you. It is always easier second time round.

@nerdatmath
Copy link
Contributor Author

Comment 4:

I appreciate your advice. Does this seem like too much to you?
$ hg diff --stat -b
 src/pkg/net/Makefile                      |   3 +++
 src/pkg/net/cgo_stub.go                   |   4 ++++
 src/pkg/net/cgo_unix.go                   |  16 ++++++++++++++++
 src/pkg/net/ipraw_test.go                 |  20 +++++++++++++++-----
 src/pkg/net/iprawsock.go                  |  34
+---------------------------------
 src/pkg/net/lookup_unix.go                |   9 +++++++++
 src/pkg/net/lookup_windows.go             |  11 +++++++++++
 src/pkg/net/proto.go                      |  55
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/pkg/net/proto_test.go                 |  50
++++++++++++++++++++++++++++++++++++++++++++++++++
 src/pkg/syscall/syscall_windows.go        |   1 +
 src/pkg/syscall/zsyscall_windows_386.go   |  16 ++++++++++++++++
 src/pkg/syscall/zsyscall_windows_amd64.go |  16 ++++++++++++++++
 src/pkg/syscall/ztypes_windows.go         |   7 +++++++
 13 files changed, 204 insertions(+), 38 deletions(-)
Bear in mind that I need to implement the protocol lookup three times: the C
based version in cgo_unix.go, the fallback to a direct read of
/etc/protocols in proto.go (modeled after port.go), and the Windows version.
All the new code is very similar to the existing port (service) lookup code.
I also implemented a test for LookupProtocol, modeled after the existing one
in port_test.go and updated ipraw_test.go so that it works on Windows when
run by an administrator. It continues to be skipped if the user doesn't have
permission to open a raw socket.
Also zsyscall_windows_*.go are auto generated because of the one line added
to syscall_windows.go.
If it is too much, can you suggest ways I could break it up into smaller
chunks?

@alexbrainman
Copy link
Member

Comment 5:

It is hard to say without looking at your code. Just send a CL and someone will review.
Thank you.

@nerdatmath
Copy link
Contributor Author

Comment 6:

I'm very sorry for having wasted your time. I had missed the need for a
corporate contributor license agreement. I should have realized this would
be required. I highly doubt that my employer has signed any such agreement.

@alexbrainman
Copy link
Member

Comment 7:

No harm done. Someone will fix it. Thanks for report anyway.

@rsc
Copy link
Contributor

rsc commented Oct 6, 2011

Comment 8:

Status changed to HelpWanted.

@alexbrainman
Copy link
Member

Comment 10:

This issue was closed by revision 059c68b.

Status changed to Fixed.

@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

4 participants