pimvanpelt
commented
1 month ago Worth mentioning: the code we have checks for flags & VLIB_BUFFER_IS_TRACED, and the subsequent vlib_add_trace() doesn't add a trace, it adds a message to a growing list of information gathered for that one packet for which tracing was selected. If tracing is not selected, nothing happens. We could remove the trace code, although it'll predict false and be dormant for all packets for which tracing is not turned on (and by default, tracing is not turned on unless the operator asks for it).
It is customary for each node in the feature arc to mark its contribution to the packet. Currently, all we're doing is setting a simple 'hello, we saw the packet', and I think that's what is expected.
I do not expect additional performance by removing the code, but since you mentioned it: adding a note in format_sflow_trace() in case a packet was selected (and if it was added to the FIFO, or dropped).
Calling vlib_add_trace() for every packet that goes through the sflow node seems redundant, since the exact same packets could be traced in the node before or the node after.
There might be an argument for tracing only the packets that are sampled? Although making that easy to do elsewhere is kind of the whole point of sFlow, so could we remove all the tracing code? It's over 100 lines.