Messages in this thread | | | Date | Tue, 15 Mar 2022 20:08:47 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH net-next 1/3] net: gre_demux: add skb drop reasons to gre_rcv() |
| |
On Mon, 14 Mar 2022 21:33:10 +0800 menglong8.dong@gmail.com wrote: > + reason = SKB_DROP_REASON_NOT_SPECIFIED; > if (!pskb_may_pull(skb, 12)) > goto drop;
REASON_HDR_TRUNC ?
> 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..
> goto drop; > + } > > rcu_read_lock(); > proto = rcu_dereference(gre_proto[ver]); > - if (!proto || !proto->handler) > + if (!proto || !proto->handler) { > + reason = SKB_DROP_REASON_GRE_NOHANDLER;
I think the ->handler check is defensive programming, there's no protocol upstream which would leave handler NULL.
This is akin to SKB_DROP_REASON_PTYPE_ABSENT, we can reuse that or add a new reason, but I'd think the phrasing should be kept similar.
> goto drop_unlock; > + } > ret = proto->handler(skb);
| |