Truncate and sample

[petebristow]

2 very basic apps useful for monitoring rather than forwarding traffic.

[eugeneia]

Please don't use the packet fields directly and use the public functions packet.data and packet.length instead. It's a bit more verbose, but as of now the fields are not really part of the public API.

[wingo]

Can you point to documentation for this @eugeneia? To me it is useful to know that a packet is just a data and a length, and part of the core Snabb value proposition. I haven't seen a PR that goes by that changes this, though I could have missed something :)

[eugeneia]

http://snabbco.github.io/#packet-core.packet describes the core.packet API. It doesn't mention the actual packet structure which is why I am uneasy about using the fields directly instead of going through the API functions.

[wingo]

Yeah I know what you mean. Still, Luke has given several presentations with a lovely slide that's just

// The maximum amount of payload in any given packet.
enum { PACKET_PAYLOAD_SIZE = 10*1024 };

// Packet of network data, with associated metadata.
struct packet {
    unsigned char data[PACKET_PAYLOAD_SIZE];
    uint16_t length;           // data payload length
};

and so I dunno, it's part of the public messaging. There are quite a lot of uses out there already fwiw, and many of them introduced since you made the data/length functions in January last year.

[lukego]

I am in favor of documenting our ABI i.e. the memory layout of these structures.

These basic data structures like packet and link need to be usable by e.g. assembler code and this does not seem compatible with defining them via an abstract Lua API.

[eugeneia]

I think documenting the ABI is a separate issue, since that would not define the FFI field names (data/length) if I understand correctly(?).

One advantage (imho) of the API functions is that they are read-only. I am fine with documenting the field names as well and encouraging their use, or even getting rid of the API functions. Either way, I would like that API to be defined before I recommend its use.

I have encountered direct uses of the fields before as well, but considered them as semi-bugs until now.

[petebristow]

A quick and dirty (so take with a pinch of salt) grep of the source code shows ~10 uses of packet.data() and ~250 uses of p.data. The methods haven't had much impact.
I'm also in favor of documenting the structure, is there consensus enough to put through a PR to update the readme?
Should the API functions remain? They seem slightly redundant if the structure is documented.

[eugeneia]

I think the best way to find out is to create that PR. I suspect that the PR would be quite involved though as it opens up a can of safety considerations. I also see overlap with: #774 #740 #734 #789

[wingo]

I don't see any significant safety consideration, for what it's worth, by using well-known names for fields of a struct with a stable, documented ABI, especially considering that we need to access the packet data directly without any out-of-bounds-checking machinery. YMMV of course :)

[eugeneia]

To avoid stalling this PR I will merge it as is and open a separate PR to discuss the issue at hand.