Add `NIOBSDSocket.ProtocolSubtype` #2317

dnadoba · 2022-11-16T11:15:45Z

Motivation:

We want to support custom IP protocols through POSIX Raw Socket API. To allow this, users will need to specify the IP sub protocol type they want to use.

Modifications:

Add NIOBSDSocket.ProtocolSubtype with a lot of well known protocols
Plumb NIOBSDSocket.ProtocolSubtype through DatagramChannel which we will reuse for the Raw Socket API

Result:

DatagramChannel is prepared to be used in a RawSocketBootstrap

Sources/NIOPosix/BSDSocketAPICommon.swift

Lukasa · 2022-11-16T11:40:38Z

Sources/NIOPosix/BSDSocketAPICommon.swift

+// }
+extension NIOBSDSocket.ProtocolSubtype {
+    /// internet protocol, pseudo protocol number
+    public static let ip = Self(rawValue: 0)


These protocol subtypes are not necessarily stable from OS release to OS release. As much as possible, can we use the symbolic names?

Where can I find the symbolic names?

Are you sure that they can change? I thought that these are the numbers used in the IP header protocol field and therefore should be and need to be OS independent.
258 (Divert pseudo-protocol [non IANA]) and 262 (MTCP) are outliers and I'm not quite sure how they fit into this picture as they exceed the 8-bit available in the IPv4 protocol (IPv6 nextHeader) field.

Some of these have that effect, but some do not. The protocol subtype field has uses outside of raw sockets.

In this case, these all match IPPROTO_* constants in the various libcs. I suspect netinet/in.h has most of them.

I have made NIOBSDSocket.ProtocolSubtype internal and moved all known protocol types to a new type called NIOIPProtocol which is public. Should I still use the IPPROTO_* constants for those as well?

I think if we can point to the IANA registry that's sufficient.

Not sure I can follow. What do you mean by pointing to the IANA registry?

IANA (the Internet Assigned Numbers Authority) holds registries for most extension points in IETF protocols. In the case of the IPv4 next-level protocol field or IPv6 next header field, that registry is this one. You can skip most of that registry: I'd limit myself to assignments with a reference to an RFC or IANA. But that's the authoritative reference for the numbers, so we should reference it as the source for the numbers we choose.

Done in 30e96bb

Make `ProtocolSubtype` internal

Lukasa · 2022-11-22T10:17:41Z

Sources/NIOCore/IPProtocol.swift

+/// called "Protocol" to identify the next level protocol.  This is an 8
+/// bit field.  In Internet Protocol version 6 (IPv6) [RFC8200], this field
+/// is called the "Next Header" field.
+public struct NIOIPProtocol: RawRepresentable, Hashable {


For the sake of convenience it might be nice to add a CustomStringConvertible implementation that switches over the named static lets and prints those names as well as the raw number.

done in 904c5fd

Lukasa · 2022-11-22T10:17:59Z

Sources/NIOCore/IPProtocol.swift

+    public var rawValue: RawValue
+
+    @inlinable
+    public init(rawValue: RawValue) {


Can we add an init(_ rawValue: Int) as well? It's useful to be able to construct this from a general Int.

Is it really that useful? I can't think of an instance where I would want to use that and NIOIPProtocol(rawValue: .init(someInt)) isn't too bad. This would also crash at runtime unless we make it an optional init.

I think so. It remains helpful for cases where you can easily get hold of an Int, and represents good practice in the Swift ecosystem (where Int is the currency numerical type). The crash at runtime is a red herring: if the user has an Int, we just force them to write the crash in their code instead of ours. The type very clearly communicates that it can only hold a UInt8, so there's no ambiguity, and we can safely save the user the hassle of writing that code.

Done in 35cb11f

Lukasa · 2022-11-22T10:18:44Z

Sources/NIOPosix/BSDSocketAPICommon.swift

+}
+
+extension NIOBSDSocket.ProtocolSubtype {
+    static let ip = Self(rawValue: 0)


Should we add an MPTCP static let here to avoid using the raw value elsewhere?

Done in fadd5c1

Lukasa · 2022-11-22T10:19:01Z

Sources/NIOPosix/BSDSocketAPIPosix.swift

@@ -212,10 +212,10 @@ extension NIOBSDSocket {

    // The protocol subtype for MPTCP.
    // Returns nil if mptcp is not supported.
-    static var mptcpProtocolSubtype: Int? {
+    static var mptcpProtocolSubtype: NIOBSDSocket.ProtocolSubtype? {


Should we remove this in favour of NIOBSDSocket.ProtocolSubtype.mptcp?

Done in fadd5c1

Lukasa · 2022-11-22T10:19:26Z

Sources/NIOCore/IPProtocol.swift

+    // Use for experimentation and testing (253) - [RFC3692]
+    public static let useForExperimentationAndTesting1 = Self(rawValue: 253)
+    // Use for experimentation and testing (254) - [RFC3692]
+    public static let useForExperimentationAndTesting2 = Self(rawValue: 254)


I'd skip these bottom two names: we can just leave them undefined.

Done in 904c5fd

Lukasa · 2022-11-22T10:20:07Z

Sources/NIOPosix/BSDSocketAPICommon.swift

+}
+
+extension NIOBSDSocket.ProtocolSubtype {
+    static let ip = Self(rawValue: 0)


I don't think 0 implies IP: I think it just implies "default".

Done in dddf452

and remove `NIOBSDSocket.mptcpProtocolSubtype`

Lukasa · 2022-11-22T11:13:57Z

@swift-server-bot test this please

Lukasa · 2022-11-22T11:45:48Z

Looks like the syscall wrapper test has broken. Probably this is the result of another dependency getting added to actually building that project (listed in IntegrationTests/tests_02_syscall_wrappers/defines.sh). You probably need to add IPProtocol.swift to that list.

dnadoba added the semver/minor Adds new public API. label Nov 16, 2022

dnadoba requested a review from Lukasa November 16, 2022 11:27

Lukasa requested changes Nov 16, 2022

View reviewed changes

dnadoba added 3 commits November 22, 2022 09:52

RawSocket prototype

111c78c

Conform ProtocolSubtype to Hashable

d702dee

Add public NIOIPProtocol type

60d0b87

Make `ProtocolSubtype` internal

dnadoba force-pushed the dn-protocol-subtype branch from e9fe575 to 60d0b87 Compare November 22, 2022 08:53

Lukasa reviewed Nov 22, 2022

View reviewed changes

Subset of IANA protocols with an RFC

6ee625c

dnadoba force-pushed the dn-protocol-subtype branch from 30e96bb to 6ee625c Compare November 22, 2022 10:22

dnadoba added 4 commits November 22, 2022 11:42

Add CustomStringConvertible to NIOIPProtocol

904c5fd

Add init(_ rawValue: Int)

35cb11f

Rename NIOBSDSocket.ProtocolSubtype.ip to .default

dddf452

Add NIOBSDSocket.ProtocolSubtype.mptcp

fadd5c1

and remove `NIOBSDSocket.mptcpProtocolSubtype`

Lukasa approved these changes Nov 22, 2022

View reviewed changes

Lukasa enabled auto-merge (squash) November 22, 2022 11:08

Add missing IPProtocol.swift file in integration tests

e0c4279

Lukasa merged commit 0b4edd8 into apple:main Nov 22, 2022

dnadoba deleted the dn-protocol-subtype branch November 22, 2022 14:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `NIOBSDSocket.ProtocolSubtype` #2317

Add `NIOBSDSocket.ProtocolSubtype` #2317

dnadoba commented Nov 16, 2022 •

edited

Loading

Lukasa Nov 16, 2022

dnadoba Nov 16, 2022

dnadoba Nov 16, 2022 •

edited

Loading

Lukasa Nov 16, 2022

dnadoba Nov 17, 2022

Lukasa Nov 17, 2022

dnadoba Nov 17, 2022

Lukasa Nov 17, 2022

dnadoba Nov 22, 2022

Lukasa Nov 22, 2022

dnadoba Nov 22, 2022

Lukasa Nov 22, 2022

dnadoba Nov 22, 2022

Lukasa Nov 22, 2022

dnadoba Nov 22, 2022

Lukasa Nov 22, 2022

dnadoba Nov 22, 2022

Lukasa Nov 22, 2022

dnadoba Nov 22, 2022

Lukasa Nov 22, 2022

dnadoba Nov 22, 2022

Lukasa Nov 22, 2022

dnadoba Nov 22, 2022

Lukasa commented Nov 22, 2022

Lukasa commented Nov 22, 2022

Add NIOBSDSocket.ProtocolSubtype #2317

Add NIOBSDSocket.ProtocolSubtype #2317

Conversation

dnadoba commented Nov 16, 2022 • edited Loading

Motivation:

Modifications:

Result:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnadoba Nov 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lukasa commented Nov 22, 2022

Lukasa commented Nov 22, 2022

Add `NIOBSDSocket.ProtocolSubtype` #2317

Add `NIOBSDSocket.ProtocolSubtype` #2317

dnadoba commented Nov 16, 2022 •

edited

Loading

dnadoba Nov 16, 2022 •

edited

Loading