-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
x/net/ipv{4,6}: adopt net/netip address types #54883
Comments
CC @neild |
This proposal has been added to the active column of the proposals project |
What is the concrete API being proposed? |
The minimal change to support type Message struct {
Buffers [][]byte
OOB []byte
Addr net.Addr
AddrPort netip.AddrPort // + Specifies destination address when writing with WriteAddrPort functions. Contains source address after reading with ReadAddrPort functions.
N int
NN int
Flags int
}
func (c *PacketConn) ReadBatchAddrPort(ms []Message, flags int) (int, error)
func (c *PacketConn) WriteBatchAddrPort(ms []Message, flags int) (int, error)
func (c *RawConn) ReadBatchAddrPort(ms []Message, flags int) (int, error)
func (c *RawConn) WriteBatchAddrPort(ms []Message, flags int) (int, error) Note that for UDP, this almost identical to #45886. The remaining difference is the Additionally, we could extend the An alternative option could be to add a new package, e.g. |
If we pattern off of #45886, maybe we should define IPMsg matching the new UDPMsg Then at least people using the UDP version would know how to use the IP version and vice versa? |
@martin-sucha @database64128 you commented on #45886. Any thoughts about my previous comment, essentially applying the same API here? |
If we apply the #45886 API then we get
Do I have that right? Does that sound like a reasonable API? |
The comment I see that the there is no
Note that the C-API has a flags field in the message header and a flags parameter for the send/receive functions. The flags field in the message header is only used to return flags on received messages. Otherwise, this looks great to me, thanks for taking this up! |
Add flags bits to the argument list and removed the populated by callee comment. |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
The
x/net/ipv{4,6}
packages currently usenet.IP
andnet.Addr
types everywhere, with all the performance issues that the new types in thenet/netip
package were designed to resolve. In particular, the performance sensitive functionReadBatch
must allocate the returned source addresses on every call (see related #26838).In order to introduce this without breaking changes, parts of the API needs to be duplicated, analogous to, for example,
net.ReadFromUDPAddrPort
added in go-1.18.To avoid too much API bloat, it might be sufficient to implement this for performance sensitive functions like
Read/WriteBatch
.Losely related to proposal #45886; when a fast
net.Read/WriteUDPMsgs
functionality is available, we may simply no longer needx/net.Read/WriteBatch
for some use cases.The text was updated successfully, but these errors were encountered: