Initial version

[FelixMcFelix]

This PR contains the first implementation of this library, which is designed as a faster (zero-copy when possible) parsing and metadata backend for OPTE.

At a high level, this includes:

• A proc macro which generates, for an owned packet header definition:
  • a packed version of that header, and methods to parse these headers from buffers.
  • traits to read and write fields consistently, regardless of whether a packet is owned/borrowed.
• A proc macro which generates choices between several headers, conditioned on a common 'choice' type.
• A proc macro which generates a parser for a chain of headers (with optional control actions insterspersed).
  • This is designed to account mainly for platforms which store packets as chains of buffers, and do not straddle individual headers across these buffers (i.e., illumos).
• Where possible, additional traits to emit packets into byte buffers and convert to/from owned versions.

Testing notes

• Macros (choice, Ingot, Parse) and base types (ingot_types) should be quite thoroughly documented on their intended use, at least via cargo +nightly doc.
  • Individual packets and newtypes for actual protocols, less so. There isn't yet logic to forward docattrs or build up docstrings for generated fields/methods/items.
• cargo +nightly miri test is an essential part of ensuring that all operations here are sane -- my understanding is that the test suite exercises all elements of packet parsing, field access/setting, and packet emit that use unsafe.
• cargo expand -p ingot > full.rs is my main sledgehammer for verifying that generated methods are behaving as expected. This is probably most useful for examining how, e.g., packet parsing and bitfields are implemented.
• cargo bench includes some ballparks for certain operations: individual header parse, full packet parse in a single buffer/across multiple buffers, full packet parse returning purely borrowed vs. hybrid contents.

Rough TODOs:

• Use a separate Parse Error type on full packets, including the name of the affected layer as a &'static str and/or &'static CStr.
• Fully document macros.
• Forward doc-attrs to generated structs.

[FelixMcFelix]

Sanity check within OPTE for the recent error handling changes:

kyle@farme:~/gits/opte/dtrace$ pfexec ./opte-trace opte-bad-packet.d
PORT         DIR MBLK               MSG+DATA

unknown      IN  0xfffffe69f9c1fa20 Parse->IngotError->outer_encap->TooSmall[0, 0]
unknown      IN  0xfffffe69e52a9a00 Parse->IngotError->outer_v6->Unwanted[0, 0]
unknown      IN  0xfffffe69f9da2160 Parse->IngotError->outer_v6->Unwanted[0, 0]
unknown      IN  0xfffffe69e2e871a0 Parse->IngotError->outer_udp->Unwanted[0, 0]
unknown      IN  0xfffffe69f9d42e40 Parse->IngotError->outer_udp->Unwanted[0, 0]
unknown      IN  0xfffffe69fa0c4e80 Parse->IngotError->outer_udp->Unwanted[0, 0]
unknown      IN  0xfffffe69e64bae80 Parse->IngotError->outer_udp->Unwanted[0, 0]
unknown      IN  0xfffffe69fe1ece00 Parse->IngotError->outer_encap->TooSmall[0, 0]

I think this is in a good spot now. Quick round of self review then I shall loop folks in while I push on getting the OPTE side cleaned up.

[zeeshanlakhani]

General Q: Is packet length automatically set when I extend a V6 packet, for example, with an additional extension header?

[FelixMcFelix]

It is, yep. Header::packet_length is derived to include the current wire lengths of all variable-length and parsed fields:

    impl<V: ::ingot::types::ByteSlice> ::ingot::types::Header for ValidIpv6<V> {
        const MINIMUM_LENGTH: usize = 40usize;
        #[inline]
        fn packet_length(&self) -> usize {
            Self::MINIMUM_LENGTH + self.1.packet_length()
        }
    }
    impl ::ingot::types::Header for Ipv6 {
        const MINIMUM_LENGTH: usize = 40usize;
        #[inline]
        fn packet_length(&self) -> usize {
            Self::MINIMUM_LENGTH + self.v6ext.packet_length()
        }
    }

In the borrowed case, this just returns the length of the underlying buffer for said options. In the owned case (for Repeated anyhow), it sums up the packet_length of all elements in the inner vec.

[pfmooney]

I'm still working my way through this, but here's an initial set of comments/questions.

[pfmooney]

If I'm reading this (and other bits) correctly, the expectations of the parsing logic is that the buffer from which the item (header, or component) is being parsed must be large enough to contain the entire thing? This would be at odds with the statement in the readme:

Ingot allows packet parsing over split buffers

Am I missing how a packet with its headers split across several mblk_ts would be parsed?

[FelixMcFelix]

Yes, individual headers do still need to reside in one contiguous buffer. Packets (#[derive(Parse)]) can contain headers in different buffers, however, using the Read trait and the generated parse_read methods. It could be possible in future to extend that functionality all the way down for extension headers or convert a straddled read to the owned version of a header, but we don't quite need that today.

This is the same model we have in OPTE -- a header can't straddle several mblk_ts, but zones do send (ether, ip, icmp), (ether) -> (ip, icmp) and similar permutations so we need to handle those.

[pfmooney]

Ok. I think this constraint on operation should be called out pretty clearly. While I'm not saying that Ingot and OPTE need to handle parsing of individual items across mblk_t boundaries immediately, that is something that many other things in mac do not take for granted today. (mac_ether_offload_info() does not even assume it can read a u16 without crossing an mblk_t boundary)

Yes, it's true that zones are well behaved, and viona transmissions should be too. I have not yet looked into cxgbe to see if the buffer sizes which are chosen there will guarantee that an incoming packet does not violate the restrictions set forth here.

Have you asked @rmustacc about his position on this?

[FelixMcFelix]

Ok. I think this constraint on operation should be called out pretty clearly.

Agreed. I wasn't aware that illumos pessimised so heavily on this front.

Have you asked @rmustacc about his position on this?

Not yet, I'll reach out.

[rmustacc]

One thing I'd like to confirm is that my understanding is that say the entire IP or TCP header (including any extensions) must be in a single buffer. Each distinct header can be in a different single buffer. Multiple headers may be in same single contiguous buffer. Is that correct?

Ok. I think this constraint on operation should be called out pretty clearly.

Agreed. I wasn't aware that illumos pessimised so heavily on this front.

I was pessimistic in mac_ether_offload_info() that I didn't want to think about single mblk and calling msgpullup which is what a lot of folks did. That said, I think I am probably more pessimistic then required. I wrote that function from the most paranoid position I could be in. Basically I'd rather do what I did than ever call msgpullup(). I know that @pfmooney has told me many times how we could be much smarter in that code.

Have you asked @rmustacc about his position on this?

In general the way that most device drivers and hardware works is that they'll basically fill data into descriptors linearly. That is, if they get given mixed size buffers they'll fill them that way (though most hardware has constraint on buffer sizes). The only feature that some hardware has (but we don't use) is that such hardware allows for splitting the header and data into different descriptors.

I think there are a lot of places that do end up doing different checks here. But effectively the whole IP fast path relies on the headers being contiguous in the first mblk and other things through the stack.

Pretty much all driver buffer sizes are going to be sized in a way that I don't think we're going to trip over this given a reasonable upper bound on header size (e.g. 256 bytes). I think the current constraint is probably something we can reasonably live with, with the following two constraints:

1. It must not cause a safety issue if it is violated. I believe this is already the case.
2. We need counters and a way to be notified or make this visible if it does happen so that this isn't just silent dropping.

[FelixMcFelix]

One thing I'd like to confirm is that my understanding is that say the entire IP or TCP header (including any extensions) must be in a single buffer. Each distinct header can be in a different single buffer. Multiple headers may be in same single contiguous buffer. Is that correct?

That is correct. The Packet macro is basically saying, for a set of headers:

• Each header is backed by a contiguous byteslice (including its extensions/options).
• If there are still bytes in the current segment, then try to parse the next header from that remainder (size permitting). If we fail, then we bail on the whole packet.
• If the current segment is empty, then access the next and keep parsing.

So headers can all be in the same buffer, or all different buffers, or a middleground in the same b_cont chain. They just can't straddle, or you won't have enough bytes left to parse out a whole header -- we're not doing any pullup.

Pretty much all driver buffer sizes are going to be sized in a way that I don't think we're going to trip over this given a reasonable upper bound on header size (e.g. 256 bytes). I think the current constraint is probably something we can reasonably live with, with the following two constraints:

1. It must not cause a safety issue if it is violated. I believe this is already the case.

2. We need counters and a way to be notified or make this visible if it does happen so that this isn't just silent dropping.

1 is the case today, since everything is length-checked. On 2, we do have visibility via OPTE's bad-packet SDT, but the error in case of a straddle is a little cryptic (ParseError::TooSmall -- we can clarify this one). We don't have any KStats outside of ports today, but I think adding one wouldn't be too hard.

EDIT: This is now its own error class as of 1cd3484.

[FelixMcFelix]

From out of band conversations, we're in a good place with regard to stress testing (lengthy iperf sessions, fuzzing via OPTE's test suite), and I think the best move forward is to get these bits onto dogfood to surface any further issues from there. From review in OPTE, we also have the right level of baseline support in Ingot to safely handle body transforms / packet pullups in a way we hadn't accounted for before. There are still some feature gaps around e.g. validity checks that it would be nice to tie to the packet definition itself, but I'll fill those in as issues shortly.