Re: [tcpdump-workers] Request for a new LINKTYPE_/DLT_ type.
On Nov 27, 2018, at 4:08 AM, Dave Barach (dbarach) wrote: > Opaque[10] is the primary metadata. That's only 40 bytes. Do you mean that > /* Offset within data[] that we are currently processing. >If negative current header points into predata area. */ > i16 current_data; /**< signed offset in data[], pre_data[] > that we are currently processing. > If negative current header points into predata area. >*/ > u16 current_length; /**< Nbytes between current data and > the end of this buffer. > */ > u32 flags; /**< buffer flags */ > u32 flow_id; /**< Generic flow identifier */ > > > u32 next_buffer; /**< Next buffer for this linked-list of buffers. > Only valid if VLIB_BUFFER_NEXT_PRESENT flag is set. >*/ > > u32 current_config_index; /**< Used by feature subgraph arcs to > visit enabled feature nodes > */ > u16 error;/**< Error code for buffers to be enqueued > to error handler. > */ > u8 n_add_refs; /**< Number of additional references to this buffer. */ > > u8 buffer_pool_index; /**< index of buffer pool this buffer belongs. */ > > u32 opaque[10]; /**< Opaque data used by sub-graphs for their own purposes. > See above */ is the primary metadata? > Opaque2[12] is the secondary metadata. That's only 48 bytes; do you mean that > u32 trace_index; /**< Specifies index into trace buffer > if VLIB_PACKET_IS_TRACED flag is set. > */ > u32 recycle_count; /**< Used by L2 path recycle code */ > > u32 total_length_not_including_first_buffer; > /**< Only valid for first buffer in chain. Current length plus >total length given here give total number of bytes in buffer chain. > */ > u8 free_list_index; /** < only used if > > VLIB_BUFFER_NON_DEFAULT_FREELIST > flag is set */ > u8 align_pad[3]; /**< available */ > u32 opaque2[12]; /**< More opaque data, see ../vnet/vnet/buffer.h */ > > /* end of second cache line */ > u8 pre_data[VLIB_BUFFER_PRE_DATA_SIZE]; /**< Space for inserting data > before buffer start. > Packet rewrite string will be > rewritten backwards and may > extend > back before buffer->data[0]. > Must come directly before packet > data. > */ > is the secondary metadata? > BTW, I've decided to pull all of these opaque structure definitions out of > the WS dissector, and do most all of the formatting on the vpp side. Should > reduce maintenance to near-zero, eliminate endian swapping issues, and > generally make life less miserable for everyone involved. So does "doing most [or] all of the formatting on the vpp side" change what's in the header? Or does it just mean that most of the fields of the header should be marked as "opaque data"? ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] Request for a new LINKTYPE_/DLT_ type.
On Nov 27, 2018, at 1:50 PM, Dave Barach (dbarach) wrote:
> After thinking about some of your feedback, I decided to move most of the
> work back to the vpp side where it probably belonged in the first place.
...
> Anyhow, here's what I implemented. Take a look AYC and let me know what you
> think.
>
> VPP graph dispatch trace record description.
>
>0 1 2 3
>0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Major Version | Minor Version | NStrings | ProtoHint |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Buffer index (big endian FWIW)|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + VPP graph node name ... ... | NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Buffer Metadata ASCII string ... ... | NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Buffer Opaque ASCII string ... ...| NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Buffer Opaque 2 ASCII string ... ... | NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | VPP ASCII packet trace (if NStrings > 4) ... | NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Packet data (up to 16K) |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> Graph dispatch records comprise a version stamp, an indication of how many
> NULL-terminated strings will follow the record header, and a
> protocol hint.
>
> The buffer index allows downstream consumers of these data to easily
> filter/track single packets as they traverse the forwarding
> graph.
So does that mean that there might be multiple records in the file for the same
packet, with all records for the same packet having the same buffer index?
> FWIW, the 32-bit buffer index is stored in big endian format.
If it's only to be used for matching purposes, presumably that means that
1) its value has no numerical significance;
2) the only comparisons done on that value are equality comparisons;
so it could be treated as a 4-byte opaque value, in which case the byte order
doesn't matter.
> As of this writing, major version = 1, minor version = 0; Nstrings will be
> either 4 or 5.
So smaller or larger values MAY (and probably SHOULD) be treated as errors.
> Here is the current set of protocol hints:
>
> typedef enum
> {
>VLIB_NODE_PROTO_HINT_NONE = 0,
>VLIB_NODE_PROTO_HINT_ETHERNET,
>VLIB_NODE_PROTO_HINT_IP4,
>VLIB_NODE_PROTO_HINT_IP6,
>VLIB_NODE_PROTO_HINT_TCP,
>VLIB_NODE_PROTO_HINT_UDP,
>VLIB_NODE_N_PROTO_HINTS,
> } vlib_node_proto_hint_t;
So a dissector MAY use that to indicate what the next protocol is.
> Example: VLIB_NODE_PROTO_HINT_IP6 means that the first octet of packet data
> SHOULD be 0x60, and should begin an ipv6 packet header.
That's SHOULD, not MUST; is there anything else a dissector should do other
than use the protocol hint for handoff?
___
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] Request for a new LINKTYPE_/DLT_ type.
inline
-Original Message-
From: Guy Harris
Sent: Tuesday, November 27, 2018 5:58 PM
To: Dave Barach (dbarach)
Cc: tcpdump-workers
Subject: Re: [tcpdump-workers] Request for a new LINKTYPE_/DLT_ type.
On Nov 27, 2018, at 1:50 PM, Dave Barach (dbarach) wrote:
> After thinking about some of your feedback, I decided to move most of the
> work back to the vpp side where it probably belonged in the first place.
...
> Anyhow, here's what I implemented. Take a look AYC and let me know what you
> think.
>
> VPP graph dispatch trace record description.
>
>0 1 2 3
>0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Major Version | Minor Version | NStrings | ProtoHint |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Buffer index (big endian FWIW)|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + VPP graph node name ... ... | NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Buffer Metadata ASCII string ... ... | NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Buffer Opaque ASCII string ... ...| NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Buffer Opaque 2 ASCII string ... ... | NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | VPP ASCII packet trace (if NStrings > 4) ... | NULL octet|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Packet data (up to 16K) |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> Graph dispatch records comprise a version stamp, an indication of how
> many NULL-terminated strings will follow the record header, and a protocol
> hint.
>
> The buffer index allows downstream consumers of these data to easily
> filter/track single packets as they traverse the forwarding graph.
So does that mean that there might be multiple records in the file for the same
packet, with all records for the same packet having the same buffer index?
>>> There absolutely will be multiple records for every packet in the trace,
>>> unless e.g. ethernet-input has a horrible bug.
> FWIW, the 32-bit buffer index is stored in big endian format.
If it's only to be used for matching purposes, presumably that means that
1) its value has no numerical significance;
2) the only comparisons done on that value are equality comparisons;
so it could be treated as a 4-byte opaque value, in which case the byte order
doesn't matter.
>>> Exactly so. I wrote the vpp code so it will always show up in big endian
>>> order, but it truly doesn't matter.
> As of this writing, major version = 1, minor version = 0; Nstrings will be
> either 4 or 5.
So smaller or larger values MAY (and probably SHOULD) be treated as errors.
>>> Yes, that would probably mean that the capture file is damaged beyond
>>> repair.
> Here is the current set of protocol hints:
>
> typedef enum
> {
>VLIB_NODE_PROTO_HINT_NONE = 0,
>VLIB_NODE_PROTO_HINT_ETHERNET,
>VLIB_NODE_PROTO_HINT_IP4,
>VLIB_NODE_PROTO_HINT_IP6,
>VLIB_NODE_PROTO_HINT_TCP,
>VLIB_NODE_PROTO_HINT_UDP,
>VLIB_NODE_N_PROTO_HINTS,
> } vlib_node_proto_hint_t;
So a dissector MAY use that to indicate what the next protocol is.
>>> Yes.
> Example: VLIB_NODE_PROTO_HINT_IP6 means that the first octet of packet data
> SHOULD be 0x60, and should begin an ipv6 packet header.
That's SHOULD, not MUST; is there anything else a dissector should do other
than use the protocol hint for handoff?
>>> Right. If someone screws up on the vpp side, the hint might not match
>>> reality. Here's why my current dissector does with it:
/*
* Delegate the rest of the packet dissection as directed. If there wasn't
a hint,
* take a guess.
*/
if (protocol_hint >= array_length(next_dissectors)) {
ws_debug_printf ("protocol_hint %d out of range (max %d)",
(int) protocol_hint,
(int) array_length(next_dissectors));
protocol_hint = 0;
}
/* note: next_dissectors[0] = eth_maybefcs_dissector_handle... */
use_this_dissector = next_dissectors [protocol_hint];
if (protocol_hint == 0) {
maybe_protocol_id = tvb_get_guint8 (tvb, offset);
switch (maybe_protocol_id) {
case 0x45:
use_this_dissector = next_dissectors[VLIB_NODE_PROTO_HINT_IP4];
break;
case 0x60:
use_this_dissector = next_dissectors[VLIB_NODE_PROTO_HINT_IP6];
break;
default:
break;
}
}
call_di
Re: [tcpdump-workers] Request for a new LINKTYPE_/DLT_ type.
On Nov 27, 2018, at 3:11 PM, Dave Barach (dbarach) wrote: > On November 27, 2018, at 5:58 PM, Guy Harris wrote: > >> On Nov 27, 2018, at 1:50 PM, Dave Barach (dbarach) wrote: >> >>> The buffer index allows downstream consumers of these data to easily >>> filter/track single packets as they traverse the forwarding graph. >> >> So does that mean that there might be multiple records in the file for the >> same packet, with all records for the same packet having the same buffer >> index? > > There absolutely will be multiple records for every packet in the trace, > unless e.g. ethernet-input has a horrible bug. So this needs to be noted in the specification. Does that mean that you might see multiple instances of the same packet payload, e.g. more than one copy of a single request and more than one copy of the response to that request in some protocol? >>> FWIW, the 32-bit buffer index is stored in big endian format. >> >> If it's only to be used for matching purposes, presumably that means that >> >> 1) its value has no numerical significance; >> >> 2) the only comparisons done on that value are equality comparisons; >> >> so it could be treated as a 4-byte opaque value, in which case the byte >> order doesn't matter. > > Exactly so. I wrote the vpp code so it will always show up in big endian > order, but it truly doesn't matter. OK, so we'll just specify it as an opaque 32-bit cookie to use to match up multiple records for the same packets. >>> As of this writing, major version = 1, minor version = 0; Nstrings will be >>> either 4 or 5. >> >> So smaller or larger values MAY (and probably SHOULD) be treated as errors. > > Yes, that would probably mean that the capture file is damaged beyond repair. Or, to be a bit more robust, treat it as a count of strings; if it's less than 4, we display only the strings that are there and, if it's greater than 5, we just don't display the others, or display them as "unknown string". >>> Example: VLIB_NODE_PROTO_HINT_IP6 means that the first octet of packet data >>> SHOULD be 0x60, and should begin an ipv6 packet header. >> >> That's SHOULD, not MUST; is there anything else a dissector should do other >> than use the protocol hint for handoff? > > Right. If someone screws up on the vpp side, the hint might not match > reality. Here's why my current dissector does with it: That appears to assume that the hint is valid (although both Wireshark and tcpdump will, if handed a purportedly-IPv4 packet with a version of 6, or a purportedly-IPv6 packet with a version of 4, report it as an error). ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
