Re: [PATCH v2 net-next 0/2] net: snmp: tracepoint support for snmp

On 11/18/21 8:45 PM, Menglong Dong wrote:

Hello~

On Thu, Nov 18, 2021 at 11:36 PM David Ahern [off-list ref] wrote:

quoted

[...]

quoted

there is already good infrastructure around kfree_skb - e.g., drop watch
monitor. Why not extend that in a way that other drop points can benefit
over time?

Thanks for your advice.

In fact, I don't think that this is a perfect idea. This way may have benefit
of reuse the existing kfree_skb event, but this will do plentiful modification
to the current code. For example, in tcp_v4_rcv(), you need to introduce the
new variate 'int free_reason' and record the drop reason in it, and pass
it to 'kfree_skb_with_reason()' in 'discard_it:'. Many places need this kind
modification. What's more, some statistics don't use 'kfree_skb()'.

However, with the tracepoint for snmp, we just need to pass 'skb' to
'UDP_INC_STATS()/TCP_INC_STATS()', the reason is already included.
This way, the modification is more simple and easier to maintain.

But it integrates into existing tooling which is a big win.

Ido gave the references for his work:
https://github.com/nhorman/dropwatch/pull/11
https://github.com/nhorman/dropwatch/commit/199440959a288dd97e3b7ae701d4e78968cddab7

And the Wireshark dissector is also upstream:
https://github.com/wireshark/wireshark/commit/a94a860c0644ec3b8a129fd243674a2e376ce1c8

i.e., the skb is already pushed to userspace for packet analysis. You
would just be augmenting more metadata along with it and not reinventing
all of this for just snmp counter based drops.