On Mon, Jan 03, 2022 at 07:01:30PM -0700, David Ahern wrote:

On 1/3/22 6:47 PM, Cong Wang wrote:

quoted

On Thu, Dec 30, 2021 at 05:32:37PM +0800, menglong8.dong@gmail.com wrote:

quoted

From: Menglong Dong <redacted>

In this series patch, the interface kfree_skb_with_reason() is
introduced(), which is used to collect skb drop reason, and pass
it to 'kfree_skb' tracepoint. Therefor, 'drop_monitor' or eBPF is
able to monitor abnormal skb with detail reason.

We already something close, __dev_kfree_skb_any(). Can't we unify
all of these?

Specifically?

The 'reason' passed around by those is either SKB_REASON_CONSUMED or
SKB_REASON_DROPPED and is used to call kfree_skb vs consume_skb. i.e.,
this is unrelated to this patch set and goal.

What prevents you extending it?

quoted

quoted

In fact, this series patches are out of the intelligence of David
and Steve, I'm just a truck man :/

I think there was another discussion before yours, which I got involved
as well.

quoted

Previous discussion is here:

https://lore.kernel.org/netdev/20211118105752.1d46e990@gandalf.local.home/ (local)
https://lore.kernel.org/netdev/67b36bd8-2477-88ac-83a0-35a1eeaf40c9@gmail.com/ (local)

In the first patch, kfree_skb_with_reason() is introduced and
the 'reason' field is added to 'kfree_skb' tracepoint. In the
second patch, 'kfree_skb()' in replaced with 'kfree_skb_with_reason()'
in tcp_v4_rcv(). In the third patch, 'kfree_skb_with_reason()' is
used in __udp4_lib_rcv().

I don't follow all the discussions here, but IIRC it would be nice
if we can provide the SNMP stat code (for instance, TCP_MIB_CSUMERRORS) to
user-space, because those stats are already exposed to user-space, so
you don't have to invent new ones.

Those SNMP macros are not unique and can not be fed into a generic
kfree_skb_reason function.

Sure, you also have the skb itself, particularly skb protocol, with
these combined, it should be unique.

Thanks.