Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3653)

Issue 5677086: code review 5677086: net: make Dial and Listen behavior consistent across ov... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 1 month ago by mikio
Modified:
12 years ago
Reviewers:
CC:
rsc, minux1, golang-dev
Visibility:
Public.

Description

net: make Dial and Listen behavior consistent across over platforms This CL changes the behavior of Dial and Listen API family. Previous Dial and Listen allow a combo of "tcp6" and IPv4 or IPv6 IPv4-mapped address as its argument, but it also makes slightly different behaviors between Linux and other platforms. This CL fixes such differences across over platforms by tweaking IP-level socket option IPV6_V6ONLY. Consequently new Dial and Listen API family will reject arguments consists of "tcp6" and IPv4 or IPv6 IPv4-mapped address. This CL also adds a bit clarified unicast listener tests. Fixes issue 2581.

Patch Set 1 : diff -r 5df6a5e63bb9 https://go.googlecode.com/hg/ #

Patch Set 2 : diff -r 5338874648c7 https://go.googlecode.com/hg/ #

Total comments: 6

Patch Set 3 : diff -r be26a03f6be9 https://go.googlecode.com/hg/ #

Total comments: 9

Patch Set 4 : diff -r ffde178ab122 https://go.googlecode.com/hg/ #

Total comments: 14

Patch Set 5 : diff -r 50adb6a9e76c https://go.googlecode.com/hg/ #

Total comments: 2

Patch Set 6 : diff -r ea5cd2b9ca6c https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+596 lines, -113 lines) Patch
M src/pkg/net/file_test.go View 1 2 3 4 2 chunks +2 lines, -9 lines 0 comments Download
M src/pkg/net/iprawsock_posix.go View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M src/pkg/net/ipsock_posix.go View 1 2 3 4 5 4 chunks +50 lines, -32 lines 0 comments Download
M src/pkg/net/net_test.go View 1 2 3 4 1 chunk +8 lines, -0 lines 0 comments Download
M src/pkg/net/server_test.go View 1 2 3 4 2 chunks +2 lines, -7 lines 0 comments Download
M src/pkg/net/sock.go View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/net/sockopt_bsd.go View 1 2 3 4 1 chunk +9 lines, -4 lines 0 comments Download
M src/pkg/net/sockopt_linux.go View 1 2 3 4 1 chunk +9 lines, -4 lines 0 comments Download
M src/pkg/net/sockopt_windows.go View 1 2 3 4 1 chunk +9 lines, -4 lines 0 comments Download
M src/pkg/net/tcpsock_posix.go View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M src/pkg/net/udpsock_posix.go View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M src/pkg/net/unicast_test.go View 1 2 3 4 2 chunks +483 lines, -50 lines 0 comments Download
M src/pkg/net/unixsock_posix.go View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 25
mikio
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
12 years, 1 month ago (2012-02-18 13:27:44 UTC) #1
mikio
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), Please take another look.
12 years, 1 month ago (2012-02-19 03:09:25 UTC) #2
rsc1
I'll look more carefully at the tests on Monday. There's a lot there. http://codereview.appspot.com/5677086/diff/8/src/pkg/net/multicast_test.go File ...
12 years, 1 month ago (2012-02-19 05:08:14 UTC) #3
mikio
Sure, me too, thx. Have a nice weekend. -- Mikio On Feb 19, 2012, at ...
12 years, 1 month ago (2012-02-19 05:12:44 UTC) #4
mikio
http://codereview.appspot.com/5677086/diff/8/src/pkg/net/multicast_test.go File src/pkg/net/multicast_test.go (right): http://codereview.appspot.com/5677086/diff/8/src/pkg/net/multicast_test.go#newcode183 src/pkg/net/multicast_test.go:183: goto done On 2012/02/19 05:08:14, rsc1 wrote: > return ...
12 years, 1 month ago (2012-02-21 12:40:43 UTC) #5
mikio
Hello golang-dev@googlegroups.com, rsc@google.com (cc: golang-dev@googlegroups.com), Please take another look.
12 years, 1 month ago (2012-02-21 12:41:03 UTC) #6
rsc
Please move unicast_test.go into its own CL. The rest are very close. Thanks. http://codereview.appspot.com/5677086/diff/10008/src/pkg/net/lookup_test.go File ...
12 years, 1 month ago (2012-02-22 19:56:33 UTC) #7
mikio
On Thu, Feb 23, 2012 at 4:56 AM, <rsc@golang.org> wrote: > Please move unicast_test.go into ...
12 years, 1 month ago (2012-02-23 03:44:18 UTC) #8
rsc
On Wed, Feb 22, 2012 at 22:44, Mikio Hara <mikioh.mikioh@gmail.com> wrote: > Unfortunately no, perhaps. ...
12 years, 1 month ago (2012-02-23 03:50:10 UTC) #9
mikio
On Thu, Feb 23, 2012 at 12:50 PM, Russ Cox <rsc@golang.org> wrote: > See ipsock_posix.go. ...
12 years, 1 month ago (2012-02-23 04:56:54 UTC) #10
rsc
On Wed, Feb 22, 2012 at 23:56, Mikio Hara <mikioh.mikioh@gmail.com> wrote: > 1) A wild-wild ...
12 years, 1 month ago (2012-02-23 05:03:41 UTC) #11
mikio
Hello golang-dev@googlegroups.com, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
12 years, 1 month ago (2012-02-26 05:57:12 UTC) #12
mikio
Would someone mind doing "go test -v net -external" w/ this CL on windows? On ...
12 years, 1 month ago (2012-02-27 02:11:27 UTC) #13
mikio
ping
12 years, 1 month ago (2012-02-28 08:57:37 UTC) #14
minux1
On 2012/02/27 02:11:27, mikioh wrote: > Would someone mind doing "go test -v net -external" ...
12 years, 1 month ago (2012-02-28 11:24:45 UTC) #15
mikio
On Tue, Feb 28, 2012 at 8:24 PM, <minux.ma@gmail.com> wrote: >> Would someone mind doing ...
12 years, 1 month ago (2012-02-28 12:16:45 UTC) #16
rsc
Thanks for working on this. I know it's no fun. http://codereview.appspot.com/5677086/diff/16065/src/pkg/net/ipsock_posix.go File src/pkg/net/ipsock_posix.go (right): http://codereview.appspot.com/5677086/diff/16065/src/pkg/net/ipsock_posix.go#newcode166 ...
12 years, 1 month ago (2012-02-28 17:47:46 UTC) #17
mikio
On Wed, Feb 29, 2012 at 2:47 AM, <rsc@golang.org> wrote: > http://codereview.appspot.com/5677086/diff/16065/src/pkg/net/unicast_test.go#newcode53 > src/pkg/net/unicast_test.go:53: if ...
12 years, 1 month ago (2012-02-28 23:00:37 UTC) #18
mikio
On Wed, Feb 29, 2012 at 2:47 AM, <rsc@golang.org> wrote: > src/pkg/net/unicast_test.go:211: // "tcp4" "" ...
12 years, 1 month ago (2012-02-29 05:51:53 UTC) #19
mikio
Correction. On Wed, Feb 29, 2012 at 2:51 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote: > Of ...
12 years, 1 month ago (2012-02-29 07:21:49 UTC) #20
mikio
Oops, correction again. There's a side effect that a combo of "tcp6" + "wildcard address" ...
12 years, 1 month ago (2012-02-29 07:55:07 UTC) #21
mikio
PTAL. Also will send new server, file tests after this CL landed. http://codereview.appspot.com/5677086/diff/16065/src/pkg/net/ipsock_posix.go File src/pkg/net/ipsock_posix.go ...
12 years, 1 month ago (2012-03-01 03:40:06 UTC) #22
mikio
Hello rsc@golang.org, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
12 years ago (2012-03-02 08:24:29 UTC) #23
rsc
LGTM http://codereview.appspot.com/5677086/diff/40001/src/pkg/net/ipsock_posix.go File src/pkg/net/ipsock_posix.go (right): http://codereview.appspot.com/5677086/diff/40001/src/pkg/net/ipsock_posix.go#newcode80 src/pkg/net/ipsock_posix.go:80: // We prefer an IPv4 wildcard address listen ...
12 years ago (2012-03-05 14:58:17 UTC) #24
mikio
12 years ago (2012-03-05 15:13:15 UTC) #25
*** Submitted as http://code.google.com/p/go/source/detail?r=e28f1aac1223 ***

net: make Dial and Listen behavior consistent across over platforms

This CL changes the behavior of Dial and Listen API family.

Previous Dial and Listen allow a combo of "tcp6" and IPv4 or IPv6
IPv4-mapped address as its argument, but it also makes slightly
different behaviors between Linux and other platforms. This CL fixes
such differences across over platforms by tweaking IP-level socket
option IPV6_V6ONLY. Consequently new Dial and Listen API family will
reject arguments consists of "tcp6" and IPv4 or IPv6 IPv4-mapped
address.

This CL also adds a bit clarified unicast listener tests.

Fixes issue 2581.

R=rsc, minux.ma
CC=golang-dev
http://codereview.appspot.com/5677086

http://codereview.appspot.com/5677086/diff/40001/src/pkg/net/ipsock_posix.go
File src/pkg/net/ipsock_posix.go (right):

http://codereview.appspot.com/5677086/diff/40001/src/pkg/net/ipsock_posix.go#...
src/pkg/net/ipsock_posix.go:80: //	We prefer an IPv4 wildcard address listen
over an AF_INET
On 2012/03/05 14:58:17, rsc wrote:
> Why does this say over?  Isn't an AF_INET socket exactly what we prefer?
> I would write this as
> 
> We use an IPv4 (AF_INET) wildcard address listen.
> 
> and then the next one as
> 
> We use an IPv6 (AF_INET6, IPV6_V6ONLY=1) wildcard address listen.
> 

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b