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

net: IPMask docs aren't great #28957

Open
bradfitz opened this Issue Nov 26, 2018 · 6 comments

Comments

Projects
None yet
6 participants
@bradfitz
Copy link
Member

bradfitz commented Nov 26, 2018

The docs for net.IPMask say:

https://golang.org/pkg/net/#IPMask

An IP mask is an IP address.

type IPMask []byte

That's, uh, pretty brief.

We should say more because as-is my first reaction would be, "Well, then why don't I juse use an IP address?"

For Go 1.13 at least, but Go 1.12 would be fine too.

/cc @ianlancetaylor @mikioh

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Nov 26, 2018

Honestly I'm not sure why we need IPMask in the net package at all. If you don't happen to be writing a router IP masks are uninteresting.

Not that we shouldn't write better docs as long as we have it.

@viswesr

This comment has been minimized.

Copy link

viswesr commented Nov 27, 2018

Is the following doc OK for IP Mask ?

IPMask
IP mask is a bit-mask corresponding to an IP address (to support subnets), a slice of bytes. Functions in this package accept either 4-byte or 16-byte slices as input

I can send a CL if the above doc is fine. Thanks.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Nov 27, 2018

@viswesr Thanks, but I think we can do better. The docs should be brief and to the point. What is the point of an IPMask? Why would someone want a value of that type?

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Nov 28, 2018

Change https://golang.org/cl/151479 mentions this issue: net: improve IPMask docs

@mikioh

This comment has been minimized.

Copy link
Contributor

mikioh commented Nov 28, 2018

@ianlancetaylor,

why we need IPMask in the net package

Yup, please ask and report back the original author the reason why? :) I can guess that initially he/she wanted some functionality that may interact with external (e.g., IP address prefixes via network attachments; now the package implements it as network and address identification API), but gave up working for more important issues.

We probably should revisit the necessity of IPMask in #18804. Basically, we need only two properties for forming the graph called IP network; address prefixes assigned to each edge and addresses assigned to the edge mount points on each vertex. Having more than two types might be overkill.

@jdreystadt

This comment has been minimized.

Copy link

jdreystadt commented Jan 12, 2019

Looking through the net package, IPMask is the return type for DefaultMask which is a function on the IP type. As a total newbie to Go, but an serious old-timer with TCP/IP, the concept of default masks goes all the way back to the early days of IP, when address allocation was being done with the concept of Class A, Class B and Class C address ranges. This is seriously out of date. But I am sure that this is what this function is about. Consider:

package main

import "fmt"
import "net"

func main() {
classAIP := net.IP([]byte{40, 40, 1, 1})
fmt.Println("IP Address ", classAIP.String(), " has default mask of ", classAIP.DefaultMask())
classBIP := net.IP([]byte{188, 40, 1, 1})
fmt.Println("IP Address ", classBIP.String(), " has default mask of ", classBIP.DefaultMask())
classCIP := net.IP([]byte{242, 40, 1, 1})
fmt.Println("IP Address ", classCIP.String(), " has default mask of ", classCIP.DefaultMask())
}
Result is:
IP Address 40.40.1.1 has default mask of ff000000
IP Address 188.40.1.1 has default mask of ffff0000
IP Address 242.40.1.1 has default mask of ffffff00

I agree that this should be removed, both the type IPMask and the function DefaultMask. This was never useful and refers to concepts no longer in use.

As to documentation for IPMask in the near term, how about

An IPMask is used to determine if a remote IP address is on the same subnet as a local IP address. This is done by doing an XOR of the two IP addresses and then masking the result with the IPMask. If the result after the masking is non-zero, then the two IP addresses are in different subnets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.