less malloc on the hot path

The main goal of this commit is to reduce the amount of allocation
happening per-packet on the hot path (aka when we have a UFT hit).
However, achieving that goal led to refactoring several bits of OPTE.
I'm still not 100% happy with this code, but it does reduce
allocation, and I do think it's simpler and more robust in some
regards.

new parsing strategy
--------------------

One major refactor was to packet parsing. Up to this point OPTE used a
generic parser which allowed either a single frame or a Geneve
encapsulated frame. However, as it was part of the generic bits of
OPTE, this parser had no way to know the shape of the traffic expected
by the network implementation. For example, in the case of oxide-vpc,
all outbound traffic is coming from an HVM guest. For such traffic, we
only care about parsing one level of headers. The guest may be sending
encapsulated packets for all we know, but that's not our concern. Our
only concern is that the outermost headers map to a destination in the
guest's VPC (or some other reachable network), and so only one level
of headers should be parsed. In the old parser we would attempt to
parse two levels, which could easily lead to connection issues,
confusion, and maybe even some shenanigans.

The generic parser also allowed nonsensical combinations. For
example, one could pair TCP + Geneve in the outer header. The new
parser/packet type limits those combinations a bit further to reduce
the possibility of invalid combos.

The new parser is inspired (but not equal to) the approach taken in
P4: provide a custom parser tailored to your expected network traffic.
The new parser provides a generic two-level packet structure: either
you have a single frame with typical ARP/ICMP/ULP traffic, or you have
said frame encapsulated. That generic structure is filled out by a
network-implementation-specific parser. This is achieved by the
`NetworkImpl` and `NetworkParser` traits.

The `NetworkImpl` trait is meant as a callback mechanism in which the
generic bits of OPTE can pass control flow to the network
implementation to make decisions that are specific to it. The new
parser uses the associated `Parser` type which maps to a
`NetworkParser` implementation. This trait provides hooks for
inbound/outbound packet parsing. The network implementation uses
generic parsing routines provided by `Packet` and provides a
`PacketInfo` value describing how it decided to parse the packet.

out-of-band packet handling
---------------------------

OPTE is highly inspired by Microsoft's VFP. You could even say it's
basically a copy of VFP; and I wouldn't argue with you. That said, the
entire point of VFP is really two tings:

1. To deal in flows, not packets.

2. To establish a hot path for active flows by caching the header
   transformations that take place in either direction of an active
   flow.

At the end of the day, this really boils down to optimizing TCP/UDP
flows as much as possible, and doing it in a manner that is amenable
to pushing down into a more traditional MAT (match-action table).
That's the point of the UFT, and the point of phrasing traffic in
terms of flows.

However, not all traffic needs the absolute best latency, or has high
throughput demands. Attempting to shoehorn _all_ traffic into this
flow-based model can be quite quirky; along with making the hot-path
more complicated and brittle. For those reasons, I introduced a new
out-of-band processing action intended for traffic that should be
treated on a per-packet basis. This is the purpose of the
`NetworkImpl::handle_pkt()` callback. To use it, you set the `Rule`
action value to `Action::HandlePacket`. This immediately kicks control
flow to the `handle_pkt()` implementation, upon which it can either
allow (and modify) the packet, drop the packet, or produce a packet to
be hairpinned back in the source's direction.

Any rule that specifies this action does not create a flow, does not
create a UFT entry, and is handled on a packet-by-packet basis. This
has several benefits, including not using hot-path resources (limited
UFT entries) for traffic that doesn't need it, and allows more
flexible, deep inspection of the packet.

The `handle_pkt()` callback _does_ have read access to the UFT tables.
While the oxide-vpc implementation is not currently using those
arguments, they will become necessary when implementation solutions
for dealing with ICMP DU and TE messages, where one has to map a ICMP
body to an existing flow.

I migrated the ARP handling to this new method. I think this should be
taken a step further, and `DataPredicate` should be fully removed and
replaced by adding logic to `handle_pkt()`. That is, I think data
predicates were probably a mistake, as they break the flow model that
OPTE is based on.

new header/meta types
---------------------

A main alloc culprit revolved around my previous (read poor) decision
around header parsing and emitting. In the old code we would use "raw"
headers to essentially cast a sequence of bytes into a raw definition
of the base header. We would then use this raw header to create a
logical `Header` value. These logical values would live with the
packet inside `HeaderGroup`. Along with these logical headers, we
created header metadata types (e.g. Ipv4Meta), that provided a limited
view into the header values. It's this data that is used to express
header transformations throughout the Layer pipeline. At the end of
processing the logical header values would be "unified" with the
metadata values, and then emit new header bytes based on them. This
final step was done via the `as_bytes()` method, which did it's job by
creating a `Vec<u8>`. These bytes would then be copied into the new
header segment. That's a lot of waste.

The main fix here is to modify the emit stage to provide a slice of
mutable bytes for the logical header type to write its raw bytes to.
This eliminates any need to create a `Vec<u8>`. However, I went a bit
beyond this and made a large refactor around how headers are dealt
with. I'm not sure I'm 100% happy with how it turned out, and in some
sense I just kind of shifted things around, but I did also managed to
eliminate the unnecessary allocs. That said, it felt more important to
land this commit than continue to noodle with this stuff. Here's what
I did.

First, I replaced the notion of the logical `Header` type with the
metadata type. For example, `TcpHdr` is now subsumed by `TcpMeta`. The
metadata type is derived from a raw version of the base header:
`TcpHdrRaw`. Any options are stored as an array in the `TcpMeta` type.
Port/Layer processing was modified to no longer act directly on this
metadata, and instead now acts via `PushAction`/`ModifyAction` traits.
E.g., TCP defines the `TcpPush` and `TcpMod` types, which allow the
processing pipeline to create/modify the source/destination ports, but
all other parts of the header are off limits.

So during processing, the metadata types are modified based on the
limited view of the `HeaderAction` type. At the end of processing, we
now have metadata types that represent the logical values of the
headers and we need to emit these to raw bytes. That is now done with
`PacketSegWriter` and the `emit()` method on each metadata type. For
each defined metadata type in the `PacketMeta` type, we attempt to get
a mutable slice of the required length from the `PacketSegWriter`, and
then call emit passing it that slice.

Now, one might wonder: what's the point of creating all these logical
values in the metadata type, which represent the various fields in the
header type: e.g., why have things like IPv4 TTL or TCP window_size in
their respective metadata types? Why not just reuse the bytes that are
already in the mblk? We could, but it requires more complicated code.
Technically, the network implementation could decide to
drop/push/modify headers, and those changes can change the offsets of
the headers in the packet. If the offsets don't change, then we can
effectively just do a zero-copy form of modification on them, but we
need a way to know they haven't shifted. If they have shifted, then we
essentially have to copy anyways, and so perhaps doing it up front
isn't the worst cost.

reuse header mblk when possible
-------------------------------

The other culprit of superfluous allocation was the invariable
creation of a new mblk for the purpose of emitting the new headers.
This commit adds the `hdr_seg()` function, which creates a new header
segment only as needed. For oxide-vpc specifically, this avoids the
additional allocation on ingress, as we are decaping and modifying,
thus there is plenty of room to write the new headers in the existing
segment. For the outbound case we still need to allocate, as we need
to encapsulate.

I think the best way to solve this issue is to have some way for a
client of OPTE/xde to negotiate a prefix length. That is, a given
network implementation could specify that it would like a certain
amount of prefix margin for each outbound packet. The client, if it
agrees, would then make sure that each header mblk contains said
margin. With this contract negotiated, there would always be enough
space in the first segment for writing the new headers. This type of
capability would require both mac and viona changes.

remove useless dbg calls
------------------------

Another source of wasteful allocation were various
`opte::engine::dbg()` calls. These calls were useful when first
developing the "external IP hack", but are effectively useless today
(especially since the hack is going away). This situation was
especially bad because even when `opte_debug` was set to zero, we still
payed the allocation cost of `format!`.

reduced cost of some SDT probes
--------------------------------

Another issue we currently have is around SDT probes: we still do the
work even if the probe is not enabled (AFAIK there is no "is enabled"
parlance for SDT probes). This is especially painful for any probe
site that creates a new CString on the fly (even more so because
often it first allocates a String, and then allocates a new CString
based on that). This issue is documented more in opte#259.

I made the following changes to reduce the cost of SDT probes in the
hot path.

* Use an integer instead of string to pass `Direction` value, saving
  an alloc for each probe site.

* Changed `Layer` names to `&'static str` to avoid unnecessary alloc
  when returning error.

* Add `Layer.ft_cstr` and `Layer.rt_cstr` to avoid unnecessary alloc.

* Removed the `ht_probe()` call from the processing hot-path (UFT
  hit). It was a bit redundant as you get the same info via the
  port-process probes; and it means less work in the hot path.

replace Vec seg list with heapless Vec
--------------------------------------

I replaced the `PacketSeg` vec with a heapless vec, eliminating an
allocation per packet. At the moment this list is limited to 4
segments and there is some `unwrap()` that could cause crashes if this
turns out not to be enough.

misc. changes
-------------

* Sprinkle various `#[inline]` attributes on hot functions based on
  some DTrace investigation. Some of these took, some didn't.

* Drop the `state` field from `PacketReader`, it no longer serves any
  purpose.

* Update `Protocol` and `EtherType` to not use `TryFrom`. Instead,
  they now represent either a protocol or ether type OPTE is
  interested in, or it's deemed "unknown" and the raw value is stored
  instead.

* Rework the guest-loopback code in regards to the ext-ip-hack in
  order to save some redundant work.

* Modified `RawHeader` trait to just have `new_mut()`: aka a
  zero-copy, mutable type over the underlying header bytes.

* Replaced use of `EtherAddr` with `MacAddr` in some places. The goal
  is to eventually get rid of `EtherAddr`.

* Modified `GeneveHdrRaw` to include it's associated UDP header as well.

* Eliminated a bunch of unused macros.

Author: rzezeski

dtrace/common.h

@@ -48,3 +48,10 @@
 	svar = strjoin(svar, substr(lltostr(evar[4], 16), 2));	\
 	svar = strjoin(svar, ":");				\
 	svar = strjoin(svar, substr(lltostr(evar[5], 16), 2));
+
+/* Direction
+ *
+ * 1 = Inbound
+ * 2 = Outbound
+ */
+#define DIR_STR(dir)	((dir) == 1 ? "IN" : "OUT")

dtrace/lib/common.d

@@ -15,22 +15,22 @@ typedef struct flow_id_sdt_arg {
 typedef struct rule_match_sdt_arg {
 	char			*port;
 	char			*layer;
-	char			*dir;
+	uintptr_t		dir;
 	flow_id_sdt_arg_t	*flow;
 	char			*rule_type;
 } rule_match_sdt_arg_t;
 
 typedef struct rule_no_match_sdt_arg {
 	char			*port;
 	char			*layer;
-	char			*dir;
+	uintptr_t		dir;
 	flow_id_sdt_arg_t	*flow;
 } rule_no_match_sdt_arg_t;
 
 typedef struct ht_run_sdt_arg {
 	char			*port;
 	char			*loc;
-	char			*dir;
+	uintptr_t		dir;
 	flow_id_sdt_arg_t	*flow_before;
 	flow_id_sdt_arg_t	*flow_after;
 } ht_run_sdt_arg_t;

dtrace/opte-bad-packet.d

@@ -15,7 +15,7 @@ BEGIN {
 
 bad-packet {
 	this->port = stringof(arg0);
-	this->dir = stringof(arg1);
+	this->dir = DIR_STR(arg1);
 	this->mblk = arg2;
 	this->msg = stringof(arg3);
 

dtrace/opte-gen-desc-fail.d

@@ -16,7 +16,7 @@ BEGIN {
 gen-desc-fail {
 	this->port = stringof(arg0);
 	this->layer = stringof(arg1);
-	this->dir = stringof(arg2);
+	this->dir = DIR_STR(arg2);
 	this->flow = (flow_id_sdt_arg_t *)arg3;
 	this->msg = stringof(arg4);
 	this->af = this->flow->af;

dtrace/opte-gen-ht-fail.d

@@ -17,7 +17,7 @@ BEGIN {
 gen-ht-fail {
 	this->port = stringof(arg0);
 	this->layer = stringof(arg1);
-	this->dir = stringof(arg2);
+	this->dir = DIR_STR(arg2);
 	this->flow = (flow_id_sdt_arg_t *)arg3;
 	this->msg = stringof(arg4);
 	this->af = this->flow->af;

dtrace/opte-ht.d

@@ -1,6 +1,10 @@
 /*
  * Track Header Transformations as they happen.
  *
+ * This only applies to header transformations which occur as part of
+ * layer/rule processing. If you are interested in flow modification
+ * in the hot path, see the opte-port-process script.
+ *
  * dtrace -L ./lib -I . -Cqs ./opte-ht.d
  */
 #include "common.h"
@@ -15,7 +19,7 @@ BEGIN {
 
 ht-run {
 	this->ht = (ht_run_sdt_arg_t*)arg0;
-	this->dir = stringof(this->ht->dir);
+	this->dir = DIR_STR(this->ht->dir);
 	this->port = stringof(this->ht->port);
 	this->loc = stringof(this->ht->loc);
 	this->before = this->ht->flow_before;

dtrace/opte-layer-process.d

@@ -18,7 +18,7 @@ BEGIN {
 }
 
 layer-process-return {
-	this->dir = stringof(arg0);
+	this->dir = DIR_STR(arg0);
 	this->port = stringof(arg1);
 	this->layer = stringof(arg2);
 	this->flow_before = (flow_id_sdt_arg_t *)arg3;

dtrace/opte-port-process.d

@@ -16,7 +16,7 @@ BEGIN {
 }
 
 port-process-return {
-	this->dir = stringof(arg0);
+	this->dir = DIR_STR(arg0);
 	this->name = stringof(arg1);
 	this->flow_before = (flow_id_sdt_arg_t *)arg2;
 	this->flow_after = (flow_id_sdt_arg_t *)arg3;

dtrace/opte-rule-match.d

@@ -18,7 +18,7 @@ rule-match {
 	this->port = stringof(this->match->port);
 	this->layer = stringof(this->match->layer);
 	this->flow = this->match->flow;
-	this->dir = stringof(this->match->dir);
+	this->dir = DIR_STR(this->match->dir);
 	this->af = this->flow->af;
 	num++;
 
@@ -44,7 +44,7 @@ rule-match /this->af == AF_INET6/ {
 rule-no-match {
 	this->no_match = (rule_no_match_sdt_arg_t *)arg0;
 	this->flow = this->no_match->flow;
-	this->dir = stringof(this->no_match->dir);
+	this->dir = DIR_STR(this->no_match->dir);
 	this->layer = stringof(this->no_match->layer);
 	this->af = this->flow->af;
 	num++;

dtrace/opte-uft-invaildate.d

@@ -19,7 +19,7 @@ BEGIN {
 }
 
 flow-entry-invalidated {
-	this->dir = stringof(arg0);
+	this->dir = DIR_STR(arg0);
 	this->port = stringof(arg1);
 	this->flow = (flow_id_sdt_arg_t *)arg2;
 	this->epoch = arg3;

illumos-sys-hdrs/src/lib.rs

@@ -117,12 +117,14 @@ pub union KStatNamedValue {
 }
 
 impl core::ops::AddAssign<u64> for KStatNamedValue {
+    #[inline]
     fn add_assign(&mut self, other: u64) {
         unsafe { self._u64 += other };
     }
 }
 
 impl core::ops::SubAssign<u64> for KStatNamedValue {
+    #[inline]
     fn sub_assign(&mut self, other: u64) {
         unsafe { self._u64 -= other };
     }
@@ -179,8 +181,8 @@ pub struct kmutex_t {
 #[derive(Debug)]
 pub struct dblk_t {
     pub db_frtnp: *const c_void, // imprecise
-    pub db_base: *const c_uchar,
-    pub db_lim: *const c_uchar,
+    pub db_base: *mut c_uchar,
+    pub db_lim: *mut c_uchar,
     pub db_ref: c_uchar,
     pub db_type: c_uchar,
     pub db_flags: c_uchar,
@@ -202,8 +204,8 @@ impl Default for dblk_t {
     fn default() -> Self {
         dblk_t {
             db_frtnp: ptr::null(),
-            db_base: ptr::null(),
-            db_lim: ptr::null(),
+            db_base: ptr::null_mut(),
+            db_lim: ptr::null_mut(),
             db_ref: 0,
             db_type: 0,
             db_flags: 0,

opte-api/src/encap.rs

@@ -33,6 +33,12 @@ pub struct Vni {
     inner: [u8; 3],
 }
 
+impl Default for Vni {
+    fn default() -> Self {
+        Vni::new(0u32).unwrap()
+    }
+}
+
 impl From<Vni> for u32 {
     fn from(vni: Vni) -> u32 {
         let bytes = vni.inner;

opte-api/src/ip.rs

@@ -222,74 +222,76 @@ impl SubnetRouterPair {
 }
 
 /// An IP protocol value.
-///
-/// TODO repr(u8)?
-#[repr(C)]
+#[repr(u8)]
 #[derive(
     Clone, Copy, Debug, Deserialize, Eq, Ord, PartialEq, PartialOrd, Serialize,
 )]
 pub enum Protocol {
-    ICMP = 0x1,
-    IGMP = 0x2,
-    TCP = 0x6,
-    UDP = 0x11,
-    ICMPv6 = 0x3A,
-    Reserved = 0xFF,
+    ICMP,
+    IGMP,
+    TCP,
+    UDP,
+    ICMPv6,
+    Unknown(u8),
 }
 
+pub const PROTO_ICMP: u8 = 0x1;
+pub const PROTO_IGMP: u8 = 0x2;
+pub const PROTO_TCP: u8 = 0x6;
+pub const PROTO_UDP: u8 = 0x11;
+pub const PROTO_ICMPV6: u8 = 0x3A;
+
 impl Default for Protocol {
     fn default() -> Self {
-        Protocol::Reserved
+        Self::Unknown(255)
     }
 }
 
 impl Display for Protocol {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            Protocol::ICMP => write!(f, "ICMP"),
-            Protocol::IGMP => write!(f, "IGMP"),
-            Protocol::TCP => write!(f, "TCP"),
-            Protocol::UDP => write!(f, "UDP"),
-            Protocol::ICMPv6 => write!(f, "ICMPv6"),
-            Protocol::Reserved => write!(f, "Reserved"),
+            Self::ICMP => write!(f, "ICMP"),
+            Self::IGMP => write!(f, "IGMP"),
+            Self::TCP => write!(f, "TCP"),
+            Self::UDP => write!(f, "UDP"),
+            Self::ICMPv6 => write!(f, "ICMPv6"),
+            Self::Unknown(_) => write!(f, "Unknown"),
         }
     }
 }
 
-impl TryFrom<u8> for Protocol {
-    type Error = String;
-
-    fn try_from(proto: u8) -> core::result::Result<Self, Self::Error> {
+impl From<u8> for Protocol {
+    fn from(proto: u8) -> Self {
         match proto {
-            0x1 => Ok(Protocol::ICMP),
-            0x2 => Ok(Protocol::IGMP),
-            0x6 => Ok(Protocol::TCP),
-            0x11 => Ok(Protocol::UDP),
-            0x3A => Ok(Protocol::ICMPv6),
-            proto => Err(format!("unhandled IP protocol: 0x{:X}", proto)),
+            PROTO_ICMP => Self::ICMP,
+            PROTO_IGMP => Self::IGMP,
+            PROTO_TCP => Self::TCP,
+            PROTO_UDP => Self::UDP,
+            PROTO_ICMPV6 => Self::ICMPv6,
+            _ => Self::Unknown(proto),
         }
     }
 }
 
-impl TryFrom<smoltcp::wire::IpProtocol> for Protocol {
-    type Error = String;
-
-    fn try_from(
-        proto: smoltcp::wire::IpProtocol,
-    ) -> core::result::Result<Self, Self::Error> {
-        use smoltcp::wire::IpProtocol::*;
+impl From<Protocol> for u8 {
+    fn from(proto: Protocol) -> u8 {
         match proto {
-            Icmp => Ok(Protocol::ICMP),
-            Igmp => Ok(Protocol::IGMP),
-            Tcp => Ok(Protocol::TCP),
-            Udp => Ok(Protocol::UDP),
-            Icmpv6 => Ok(Protocol::ICMPv6),
-            Unknown(x) if x == 0xFF => Ok(Protocol::Reserved),
-            _ => Err(format!("unhandled IP protocol: 0x{:X}", u8::from(proto))),
+            Protocol::ICMP => PROTO_ICMP,
+            Protocol::IGMP => PROTO_IGMP,
+            Protocol::TCP => PROTO_TCP,
+            Protocol::UDP => PROTO_UDP,
+            Protocol::ICMPv6 => PROTO_ICMPV6,
+            Protocol::Unknown(v) => v,
         }
     }
 }
 
+impl From<smoltcp::wire::IpProtocol> for Protocol {
+    fn from(proto: smoltcp::wire::IpProtocol) -> Self {
+        Self::from(u8::from(proto))
+    }
+}
+
 impl From<Protocol> for smoltcp::wire::IpProtocol {
     fn from(proto: Protocol) -> smoltcp::wire::IpProtocol {
         use smoltcp::wire::IpProtocol::*;
@@ -299,7 +301,7 @@ impl From<Protocol> for smoltcp::wire::IpProtocol {
             Protocol::TCP => Tcp,
             Protocol::UDP => Udp,
             Protocol::ICMPv6 => Icmpv6,
-            Protocol::Reserved => Unknown(0xFF),
+            Protocol::Unknown(proto) => Unknown(proto),
         }
     }
 }
@@ -366,10 +368,15 @@ impl Ipv4Addr {
     pub const LOCAL_BCAST: Self = Self { inner: [255; 4] };
 
     /// Return the bytes of the address.
+    #[inline]
     pub fn bytes(&self) -> [u8; 4] {
         self.inner
     }
 
+    pub const fn from_const(bytes: [u8; 4]) -> Self {
+        Self { inner: bytes }
+    }
+
     /// Return the address after applying the network mask.
     pub fn mask(mut self, mask: u8) -> Result<Self, String> {
         if mask > 32 {
@@ -486,6 +493,18 @@ impl AsRef<[u8]> for Ipv4Addr {
     }
 }
 
+impl AsRef<[u8; 4]> for Ipv4Addr {
+    fn as_ref(&self) -> &[u8; 4] {
+        &self.inner
+    }
+}
+
+impl From<Ipv4Addr> for [u8; 4] {
+    fn from(ip: Ipv4Addr) -> [u8; 4] {
+        ip.inner
+    }
+}
+
 impl Deref for Ipv4Addr {
     type Target = [u8];
     fn deref(&self) -> &Self::Target {
@@ -495,7 +514,16 @@ impl Deref for Ipv4Addr {
 
 /// An IPv6 address.
 #[derive(
-    Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Serialize, Deserialize,
+    Clone,
+    Copy,
+    Debug,
+    Default,
+    Eq,
+    Ord,
+    PartialEq,
+    PartialOrd,
+    Serialize,
+    Deserialize,
 )]
 pub struct Ipv6Addr {
     inner: [u8; 16],

opte-api/src/lib.rs

@@ -56,12 +56,12 @@ pub use ulp::*;
 ///
 /// We rely on CI and the check-api-version.sh script to verify that
 /// this number is incremented anytime the oxide-api code changes.
-pub const API_VERSION: u64 = 18;
+pub const API_VERSION: u64 = 19;
 
 #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)]
 pub enum Direction {
-    In,
-    Out,
+    In = 1,
+    Out = 2,
 }
 
 impl core::str::FromStr for Direction {
@@ -87,24 +87,6 @@ impl Display for Direction {
     }
 }
 
-impl From<Direction> for illumos_sys_hdrs::uintptr_t {
-    fn from(dir: Direction) -> Self {
-        match dir {
-            Direction::In => 0,
-            Direction::Out => 1,
-        }
-    }
-}
-
-impl Direction {
-    pub fn cstr_raw(&self) -> *const illumos_sys_hdrs::c_char {
-        match self {
-            Self::In => b"in\0".as_ptr() as *const illumos_sys_hdrs::c_char,
-            Self::Out => b"out\0".as_ptr() as *const illumos_sys_hdrs::c_char,
-        }
-    }
-}
-
 /// Set the underlay devices used by the xde kernel module
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct SetXdeUnderlayReq {