Re: [PATCH net-next 1/3] net: gre_demux: add skb drop reasons to gre_rcv()

[David Ahern <dsahern@kernel.org>]

On 3/15/22 10:55 PM, Jakub Kicinski wrote:
> On Tue, 15 Mar 2022 21:49:01 -0600 David Ahern wrote:
>>>>  	ver = skb->data[1]&0x7f;
>>>> -	if (ver >= GREPROTO_MAX)
>>>> +	if (ver >= GREPROTO_MAX) {
>>>> +		reason = SKB_DROP_REASON_GRE_VERSION;  
>>>
>>> TBH I'm still not sure what level of granularity we should be shooting
>>> for with the reasons. I'd throw all unexpected header values into one 
>>> bucket, not go for a reason per field, per protocol. But as I'm said
>>> I'm not sure myself, so we can keep what you have..  
>>
>> I have stated before I do not believe every single drop point in the
>> kernel needs a unique reason code. This is overkill. The reason augments
>> information we already have -- the IP from kfree_skb tracepoint.
> 
> That's certainly true. I wonder if there is a systematic way of
> approaching these additions that'd help us picking the points were 
> we add reasons less of a judgment call.

In my head it's split between OS housekeeping and user visible data.
Housekeeping side of it is more the technical failure points like skb
manipulations - maybe interesting to a user collecting stats about how a
node is performing, but more than likely not. IMHO, those are ignored
for now (NOT_SPECIFIED).

The immediate big win is for packets from a network where an analysis
can show code location (instruction pointer), user focused reason (csum
failure, 'otherhost', no socket open, no socket buffer space, ...) and
traceable to a specific host (headers in skb data).