Reviewed 1 of 8 files at r1.
Reviewable status: 1 of 8 files reviewed, 3 unresolved discussions (waiting on @oncilla and @sgmonroy)

go/border/io.go, line 238 at r1 (raw file):

				}
				// Before dropping the packets, we have to return them to the freelist.
				releaseAllPkts(epkts[:toWrite])

releasePkts would be clearer to me. All sounds like it's not getting a subset of packets.

go/border/io.go, line 239 at r1 (raw file):

				// Before dropping the packets, we have to return them to the freelist.
				releaseAllPkts(epkts[:toWrite])
				// Drop packets attempted to write.

"Drop packets from the failed write attempt" maybe.

go/border/io.go, line 322 at r1 (raw file):

}

// releaseAllPkts releases all pkts in the entry list.

I'd put this before shiftUnwrittenPkts as it should be called before it.

Reviewable status: 1 of 8 files reviewed, 3 unresolved discussions (waiting on @kormat and @sgmonroy)

go/border/io.go, line 238 at r1 (raw file):

Done.

go/border/io.go, line 239 at r1 (raw file):

Done.

go/border/io.go, line 322 at r1 (raw file):

Done.

Reviewed 1 of 8 files at r1, 1 of 1 files at r2.
Reviewable status: 2 of 8 files reviewed, all discussions resolved (waiting on @sgmonroy)

Reviewed 6 of 8 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @oncilla)

go/border/io_test.go, line 116 at r2 (raw file):

}

func testSuccessfulWrite(done chan<- struct{}) func(msgsI interface{}) (int, error) {

Any reason for the interface{} argument? Gomock's reflection allows for explicit types in there, and will work correctly with conn.Messages.

go/border/io_test.go, line 127 at r2 (raw file):

}

func testConn(mctrl *gomock.Controller) *mock_conn.MockConn {

I'd prefer newTestConn, it feels more Go-ish for object construction.

go/border/io_test.go, line 136 at r2 (raw file):

}

func testPktList(t *testing.T, l int) (ringbuf.EntryList, func(expected int)) {

Rename l, it's one of the most awkward one letter vars because it looks too much like I or 1 or | in too many fonts, even the good console ones. Use n or c instead.

go/border/io_test.go, line 163 at r2 (raw file):

func testSock(r *Router, ringSize int, mconn conn.Conn) *rctx.Sock {
	return rctx.NewSock(ringbuf.New(ringSize, nil, "locOut",
		prometheus.Labels{"ringId": "posixOutput"}), mconn, 0, 12,

So many concrete dependencies in test code that has nothing to do with them makes me sad :(. But there's no easy way around this.

Reviewable status: 7 of 8 files reviewed, 3 unresolved discussions (waiting on @scrye)

go/border/io_test.go, line 116 at r2 (raw file):

I never realized.
Done.

go/border/io_test.go, line 127 at r2 (raw file):

Done.

go/border/io_test.go, line 136 at r2 (raw file):

Done.

go/border/io_test.go, line 163 at r2 (raw file):

I feel your pain, believe me!

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved