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

Issue 6482044: code review 6482044: go.net/ipv4: new package (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 3 months ago by mikio
Modified:
10 years, 2 months ago
Reviewers:
CC:
rsc, dfc, brainman, gobot, golang-dev
Visibility:
Public.

Description

go.net/ipv4: new package Package ipv4 implements IP-level socket options for the Internet Protocol version 4. It also provides raw IP socket access methods including IPv4 header manipulation. Fixes issue 3684. Fixes issue 3820. This CL requires CL 6426047; net: add read, write message methods to IPConn, UDPConn

Patch Set 1 : diff -r 147c4a6dca9b https://code.google.com/p/go.net #

Patch Set 2 : diff -r 147c4a6dca9b https://code.google.com/p/go.net #

Total comments: 22

Patch Set 3 : diff -r 147c4a6dca9b https://code.google.com/p/go.net #

Total comments: 114

Patch Set 4 : diff -r 2513e9008213 https://code.google.com/p/go.net #

Total comments: 44

Patch Set 5 : diff -r 2513e9008213 https://code.google.com/p/go.net #

Patch Set 6 : diff -r 2513e9008213 https://code.google.com/p/go.net #

Patch Set 7 : diff -r 2513e9008213 https://code.google.com/p/go.net #

Patch Set 8 : diff -r 2513e9008213 https://code.google.com/p/go.net #

Patch Set 9 : diff -r 2513e9008213 https://code.google.com/p/go.net #

Patch Set 10 : diff -r 2513e9008213 https://code.google.com/p/go.net #

Total comments: 13

Patch Set 11 : diff -r 2513e9008213 https://code.google.com/p/go.net #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3421 lines, -0 lines) Patch
A ipv4/control.go View 1 2 3 4 1 chunk +47 lines, -0 lines 0 comments Download
A ipv4/control_bsd.go View 1 2 3 1 chunk +112 lines, -0 lines 0 comments Download
A ipv4/control_linux.go View 1 2 3 4 1 chunk +116 lines, -0 lines 0 comments Download
A ipv4/control_plan9.go View 1 2 3 1 chunk +29 lines, -0 lines 0 comments Download
A ipv4/control_windows.go View 1 2 3 1 chunk +29 lines, -0 lines 0 comments Download
A ipv4/dgramopt_plan9.go View 1 2 3 1 chunk +50 lines, -0 lines 0 comments Download
A ipv4/dgramopt_posix.go View 1 2 3 4 1 chunk +125 lines, -0 lines 0 comments Download
A ipv4/doc.go View 1 2 3 4 5 6 7 8 9 10 1 chunk +213 lines, -0 lines 0 comments Download
A ipv4/endpoint.go View 1 2 3 4 1 chunk +181 lines, -0 lines 0 comments Download
A ipv4/example_test.go View 1 2 3 4 5 6 7 8 9 1 chunk +252 lines, -0 lines 0 comments Download
A ipv4/genericopt_plan9.go View 1 2 3 1 chunk +29 lines, -0 lines 0 comments Download
A ipv4/genericopt_posix.go View 1 2 1 chunk +61 lines, -0 lines 0 comments Download
A ipv4/header.go View 1 2 3 4 1 chunk +194 lines, -0 lines 0 comments Download
A ipv4/header_test.go View 1 2 3 4 5 6 7 8 9 1 chunk +99 lines, -0 lines 0 comments Download
A ipv4/helper.go View 1 2 3 1 chunk +85 lines, -0 lines 0 comments Download
A ipv4/helper_plan9.go View 1 2 3 1 chunk +29 lines, -0 lines 0 comments Download
A ipv4/helper_posix.go View 1 2 3 1 chunk +42 lines, -0 lines 0 comments Download
A ipv4/helper_unix.go View 1 2 3 4 1 chunk +50 lines, -0 lines 0 comments Download
A ipv4/helper_windows.go View 1 2 1 chunk +49 lines, -0 lines 0 comments Download
A ipv4/mockicmp_test.go View 1 2 3 4 5 1 chunk +47 lines, -0 lines 0 comments Download
A ipv4/mocktransponder_test.go View 1 2 3 4 5 6 7 8 9 1 chunk +133 lines, -0 lines 0 comments Download
A ipv4/multicast_test.go View 1 2 3 4 5 6 7 1 chunk +128 lines, -0 lines 0 comments Download
A ipv4/multicastlistener_test.go View 1 2 3 4 5 6 7 8 9 10 1 chunk +244 lines, -0 lines 0 comments Download
A ipv4/multicastsockopt_test.go View 1 2 3 4 5 6 7 8 9 1 chunk +129 lines, -0 lines 0 comments Download
A ipv4/packet.go View 1 2 3 4 1 chunk +97 lines, -0 lines 0 comments Download
A ipv4/payload.go View 1 2 3 1 chunk +81 lines, -0 lines 0 comments Download
A ipv4/sockopt_bsd.go View 1 2 3 1 chunk +123 lines, -0 lines 0 comments Download
A ipv4/sockopt_freebsd.go View 1 2 1 chunk +26 lines, -0 lines 0 comments Download
A ipv4/sockopt_linux.go View 1 2 3 4 5 6 1 chunk +122 lines, -0 lines 0 comments Download
A ipv4/sockopt_plan9.go View 1 2 3 1 chunk +19 lines, -0 lines 0 comments Download
A ipv4/sockopt_unix.go View 1 2 1 chunk +76 lines, -0 lines 0 comments Download
A ipv4/sockopt_windows.go View 1 2 3 4 5 6 7 8 9 1 chunk +179 lines, -0 lines 0 comments Download
A ipv4/unicast_test.go View 1 2 3 4 5 6 7 1 chunk +79 lines, -0 lines 0 comments Download
A ipv4/unicastsockopt_test.go View 1 2 3 4 5 6 7 8 9 1 chunk +146 lines, -0 lines 0 comments Download

Messages

Total messages: 29
dfc
nit: please change the description to go.net/ipv4 as rietveld doesn't make it obvious which repo ...
10 years, 3 months ago (2012-08-23 10:54:44 UTC) #1
mikio
done.
10 years, 3 months ago (2012-08-23 11:41:24 UTC) #2
mikio
Hello dave@cheney.net (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.net
10 years, 3 months ago (2012-08-28 21:55:12 UTC) #3
gobot
R=rsc (assigned by rsc)
10 years, 3 months ago (2012-09-01 14:40:48 UTC) #4
rsc
[codereview just ate my last reply, which was quite long; this one will be shorter] ...
10 years, 3 months ago (2012-09-01 14:59:26 UTC) #5
dfc
Thanks Mikio and rsc. I haven't finished reviewing this fully, but I know you have ...
10 years, 2 months ago (2012-09-03 11:04:47 UTC) #6
mikio
On 2012/09/01 14:59:26, rsc wrote: > [codereview just ate my last reply, which was quite ...
10 years, 2 months ago (2012-09-05 03:32:53 UTC) #7
mikio
Thanks Dave. http://codereview.appspot.com/6482044/diff/25001/ipv4/control.go File ipv4/control.go (right): http://codereview.appspot.com/6482044/diff/25001/ipv4/control.go#newcode14 ipv4/control.go:14: mu sync.Mutex for now I'd keep them ...
10 years, 2 months ago (2012-09-05 03:50:55 UTC) #8
mikio
Hello rsc@golang.org, dave@cheney.net (cc: gobot@golang.org, golang-dev@googlegroups.com), Please take another look.
10 years, 2 months ago (2012-09-05 03:51:55 UTC) #9
dfc
This package is looking really good. I think one more pass and it should be ...
10 years, 2 months ago (2012-09-05 06:27:15 UTC) #10
mikio
Hello rsc@golang.org, dave@cheney.net (cc: gobot@golang.org, golang-dev@googlegroups.com), Please take another look.
10 years, 2 months ago (2012-09-06 10:43:31 UTC) #11
mikio
http://codereview.appspot.com/6482044/diff/21002/ipv4/control_bsd.go File ipv4/control_bsd.go (right): http://codereview.appspot.com/6482044/diff/21002/ipv4/control_bsd.go#newcode109 ipv4/control_bsd.go:109: func marshalControlMessage(cm *ControlMessage) []byte { On 2012/09/05 06:27:16, dfc ...
10 years, 2 months ago (2012-09-06 10:43:47 UTC) #12
mikio
Hi Dave, On Wed, Sep 5, 2012 at 3:27 PM, <dave@cheney.net> wrote: > There are ...
10 years, 2 months ago (2012-09-06 10:50:50 UTC) #13
dfc
LGTM modulo the minor tweaks to the comments below. I have tested on linux/amd64 and ...
10 years, 2 months ago (2012-09-06 12:40:36 UTC) #14
mikio
On Thu, Sep 6, 2012 at 9:40 PM, <dave@cheney.net> wrote: > I have tested on ...
10 years, 2 months ago (2012-09-06 13:12:35 UTC) #15
mikio
all done, thanks for your review, Dave. will back to the stdlib tree for preparing ...
10 years, 2 months ago (2012-09-06 15:09:16 UTC) #16
brainman
It fails to build for windows: # GOOS=windows go test -c # code.google.com/p/go.net/ipv4 ./packet.go:28: c.c.ReadMsgIP ...
10 years, 2 months ago (2012-09-07 02:04:40 UTC) #17
mikio
On Fri, Sep 7, 2012 at 11:04 AM, <alex.brainman@gmail.com> wrote: > It fails to build ...
10 years, 2 months ago (2012-09-07 03:28:11 UTC) #18
brainman
On 2012/09/07 03:28:11, mikio wrote: > On Fri, Sep 7, 2012 at 11:04 AM, <mailto:alex.brainman@gmail.com> ...
10 years, 2 months ago (2012-09-07 03:53:42 UTC) #19
mikio
Hello rsc@golang.org, dave@cheney.net, alex.brainman@gmail.com (cc: gobot@golang.org, golang-dev@googlegroups.com), Please take another look.
10 years, 2 months ago (2012-09-07 08:01:04 UTC) #20
mikio
On Fri, Sep 7, 2012 at 5:01 PM, <mikioh.mikioh@gmail.com> wrote: > Please take another look. ...
10 years, 2 months ago (2012-09-07 08:03:08 UTC) #21
brainman
On 2012/09/07 08:03:08, mikio wrote: > > replumbing to windows done. C:\go\path\src\code.google.com\p\go.net\ipv4>go test -v === ...
10 years, 2 months ago (2012-09-10 04:14:32 UTC) #22
mikio
On 2012/09/10 04:14:32, brainman wrote: > === RUN TestTCPUnicastSockopt > --- FAIL: TestTCPUnicastSockopt (0.00 seconds) ...
10 years, 2 months ago (2012-09-10 05:56:24 UTC) #23
mikio
Hello rsc@golang.org, dave@cheney.net, alex.brainman@gmail.com (cc: gobot@golang.org, golang-dev@googlegroups.com), Please take another look.
10 years, 2 months ago (2012-09-24 22:00:33 UTC) #24
dfc
Hello, Here are some final tweaks. I think this CL as it stands now represents ...
10 years, 2 months ago (2012-09-25 13:10:32 UTC) #25
brainman
LGTM All tests PASS on windows. Thank you. Alex
10 years, 2 months ago (2012-09-26 01:36:00 UTC) #26
mikio
On Tue, Sep 25, 2012 at 10:10 PM, <dave@cheney.net> wrote: > Here are some final ...
10 years, 2 months ago (2012-09-26 12:00:53 UTC) #27
mikio
On Wed, Sep 26, 2012 at 10:36 AM, <alex.brainman@gmail.com> wrote: > All tests PASS on ...
10 years, 2 months ago (2012-09-26 12:02:05 UTC) #28
mikio
10 years, 2 months ago (2012-09-26 12:03:23 UTC) #29
*** Submitted as
http://code.google.com/p/go/source/detail?r=7336440da745&repo=net ***

go.net/ipv4: new package

Package ipv4 implements IP-level socket options for the Internet
Protocol version 4. It also provides raw IP socket access methods
including IPv4 header manipulation.

Fixes issue 3684.
Fixes issue 3820.

This CL requires CL 6426047;
net: add read, write message methods to IPConn, UDPConn

R=rsc, dave, alex.brainman
CC=gobot, golang-dev
http://codereview.appspot.com/6482044

http://codereview.appspot.com/6482044/diff/41001/ipv4/doc.go
File ipv4/doc.go (right):

http://codereview.appspot.com/6482044/diff/41001/ipv4/doc.go#newcode8
ipv4/doc.go:8: // The package provides IP-level socket options that allow
manipulaton
On 2012/09/25 13:10:33, dfc wrote:
> s/manipulaton/manipulation/

Done.

http://codereview.appspot.com/6482044/diff/41001/ipv4/doc.go#newcode12
ipv4/doc.go:12: // field on the DiffServ, differentiated services environment.
On 2012/09/25 13:10:33, dfc wrote:
> maybe s/on the/in a/, but i'm not sure, i've never used such a thing.

will take "in a", thx.

http://codereview.appspot.com/6482044/diff/41001/ipv4/doc.go#newcode12
ipv4/doc.go:12: // field on the DiffServ, differentiated services environment.
On 2012/09/25 13:10:33, dfc wrote:
> maybe s/on the/in a/, but i'm not sure, i've never used such a thing.

Done.

http://codereview.appspot.com/6482044/diff/41001/ipv4/doc.go#newcode82
ipv4/doc.go:82: // this operation because joining groups affects only network
and link
On 2012/09/25 13:10:33, dfc wrote:
> s/because/as

Done.

http://codereview.appspot.com/6482044/diff/41001/ipv4/doc.go#newcode152
ipv4/doc.go:152: // 1024 might join two diffrent groups across over two
different
On 2012/09/25 13:10:33, dfc wrote:
> s/diffrent/different/

Done.

http://codereview.appspot.com/6482044/diff/41001/ipv4/doc.go#newcode174
ipv4/doc.go:174: // It is possible for multiple UDP listeners that listen to the
same
On 2012/09/25 13:10:33, dfc wrote:
> s/listen to/listen on/ -- maybe ?

Done.

http://codereview.appspot.com/6482044/diff/41001/ipv4/doc.go#newcode176
ipv4/doc.go:176: // provide a socket that listens to a wildcard address with
resuable
On 2012/09/25 13:10:33, dfc wrote:
> s/resuable/reusable/

Done.
Sign in to reply to this message.

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