code review 21260043: go.net/ipv4: make sure whether the interface under test...

go.net/ipv4: make sure whether the interface under test is routed

Some multicast tests depend on proper IP routing because multicasting
requires an unicast source address for its delivery. We call a network
interface that can route IP traffic to neighbors a routed interface
conventionally. This CL makes sure that the interface under test is a
routed interface to avoid using non-routed network interfaces for IP
routing.

Also removes unnecessary build tag.

Fixes issue 6709.

[mikio, 2013-11-03 07:30:37]

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

[dave_cheney.net, 2013-11-03 07:46:49]

LGTM with minor comments.

Please add a short description saying why a routed interface is necessary and
what routed means.

https://codereview.appspot.com/21260043/diff/2/ipv4/mocktransponder_test.go
File ipv4/mocktransponder_test.go (right):

https://codereview.appspot.com/21260043/diff/2/ipv4/mocktransponder_test.go#n...
ipv4/mocktransponder_test.go:87: for _, ifi := range ift {
please add a short comment here

// skip SOMETHING interfaces if they are SOMETHING

or update the comment for loopbackInterface() to say something like

// the interface chosen will be random but SOMETHING interfaces will be ignored
as they are SOMETHING

[mikio, 2013-11-03 08:28:05]

On Sun, Nov 3, 2013 at 4:46 PM,  <dave@cheney.net> wrote:

> ipv4/mocktransponder_test.go:87: for _, ifi := range ift {
> please add a short comment here
>
> // skip SOMETHING interfaces if they are SOMETHING
>
> or update the comment for loopbackInterface() to say something like
>
> // the interface chosen will be random but SOMETHING interfaces will be
> ignored as they are SOMETHING

Well, I don't understand what do you mean by "will be random"?
It just returns a usable loopback interface when the node under test
supports it, no-more, no-less.

[dave_cheney.net, 2013-11-03 08:33:46]

does `a useable` interface mean `the first` ?

On Sun, Nov 3, 2013 at 7:28 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
> On Sun, Nov 3, 2013 at 4:46 PM,  <dave@cheney.net> wrote:
>
>> ipv4/mocktransponder_test.go:87: for _, ifi := range ift {
>> please add a short comment here
>>
>> // skip SOMETHING interfaces if they are SOMETHING
>>
>> or update the comment for loopbackInterface() to say something like
>>
>> // the interface chosen will be random but SOMETHING interfaces will be
>> ignored as they are SOMETHING
>
> Well, I don't understand what do you mean by "will be random"?
> It just returns a usable loopback interface when the node under test
> supports it, no-more, no-less.

[mikio, 2013-11-03 08:59:58]

On Sun, Nov 3, 2013 at 5:33 PM, Dave Cheney <dave@cheney.net> wrote:

> does `a useable` interface mean `the first` ?

I don't get it. Why do you need some comments on the following line?

> // skip SOMETHING interfaces if they are SOMETHING
if ifi.Flags&net.FlagLoopback == 0 || ifi.Flags&net.FlagUp == 0 {}

[dave_cheney.net, 2013-11-03 09:05:10]

Lets drop this, it's not important.

On Sun, Nov 3, 2013 at 7:59 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
> On Sun, Nov 3, 2013 at 5:33 PM, Dave Cheney <dave@cheney.net> wrote:
>
>> does `a useable` interface mean `the first` ?
>
> I don't get it. Why do you need some comments on the following line?
>
>> // skip SOMETHING interfaces if they are SOMETHING
> if ifi.Flags&net.FlagLoopback == 0 || ifi.Flags&net.FlagUp == 0 {}

[mikio, 2013-11-03 09:08:39]

On Sun, Nov 3, 2013 at 6:05 PM, Dave Cheney <dave@cheney.net> wrote:

> Lets drop this, it's not important.

Yup, thanks.

[mikio, 2013-11-03 11:39:44]

*** Submitted as
https://code.google.com/p/go/source/detail?r=b42777b97fe7&repo=net ***

go.net/ipv4: make sure whether the interface under test is routed

Some multicast tests depend on proper IP routing because multicasting
requires an unicast source address for its delivery. We call a network
interface that can route IP traffic to neighbors a routed interface
conventionally. This CL makes sure that the interface under test is a
routed interface to avoid using non-routed network interfaces for IP
routing.

Also removes unnecessary build tag.

Fixes issue 6709.

R=dave
CC=golang-dev
https://codereview.appspot.com/21260043