RouteGetWithOptions: Add source address option #591
Conversation
taktv6
changed the title
| if family == FAMILY_V4 { | ||
| srcAddr = options.SrcAddr.To4() | ||
| } else { | ||
| srcAddr = options.SrcAddr.To16() |
There was a problem hiding this comment.
To16 converts the IP address ip to a 16-byte representation
https://golang.org/pkg/net/#IP.To16
Is this what you want here?
There was a problem hiding this comment.
When family is != FAMILY_v4 it is v6 and v6 is 128 bits aka 16 bytes. Why shouldn't I want that?
There was a problem hiding this comment.
Given type IP []byte and var srcAddr []byte
When family != FAMILY_V4, srcAddr = options.SrcAddr.To16() is as good as srcAddr = options.SrcAddr
There was a problem hiding this comment.
That assumption can be wrong though when a user specifies an IPv4 source address when doing a route lookup for an IPv6 target.
Sure, in the end it would of course just not work. But at least the Netlink message is correct this way.
There was a problem hiding this comment.
That assumption can be wrong though when a user specifies an IPv4 source
As far as I know a func IPv4(a, b, c, d byte) IP and func ParseIP(<an IPv4 string here") IP will generate the same IPv4-in-IPv6 mapped 16 byte slice.
You need to try a bit harder, but yes you can create a 4 byte slice net.IP, by type converting a 4 byte slice:
b := []byte{10,10,10,1}
ip := net.IP(b)
although this is not the idiomatic way to create an IP in go.
If instead the caller passes a .To4() returned net.IP, then I'd say he intentionally wants things to fail.
This net.IP handling thing which internally stores a IPv4 address into a 16 byte slice net.IP variable, but then it also gives you a To4() method which will return a 4 byte slice net.IP variable is confusing. Looks like other people think the same golang/go#18804
We just need to be careful to not use the a.To16() != nil check as we use the a.To4() != nil check: The latter will tell whether a is a IPv4 address, while the former will not always tell you that ais a IPv6 address.
Anyhow, I see this unnecessary .To16() in several other places in this library.
|
LGTM |
No description provided.