| Message ID | 20220303174707.40431-2-imagedong@tencent.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dev: add skb drop reasons to net/core/dev.c | expand |
On Fri, 4 Mar 2022 01:47:01 +0800 menglong8.dong@gmail.com wrote: > Replace kfree_skb() used in sch_handle_egress() with kfree_skb_reason(). > The drop reason SKB_DROP_REASON_QDISC_EGRESS is introduced. Considering > the code path of qdisc egerss, we make it distinct with the drop reason > of SKB_DROP_REASON_QDISC_DROP in the next commit. I don't think this has much to do with Qdiscs, this is the TC egress hook, it's more for filtering. Classful Qdisc like HTB will run its own classification. I think. Maybe TC_EGRESS?
On Fri, Mar 4, 2022 at 12:25 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Fri, 4 Mar 2022 01:47:01 +0800 menglong8.dong@gmail.com wrote: > > Replace kfree_skb() used in sch_handle_egress() with kfree_skb_reason(). > > The drop reason SKB_DROP_REASON_QDISC_EGRESS is introduced. Considering > > the code path of qdisc egerss, we make it distinct with the drop reason > > of SKB_DROP_REASON_QDISC_DROP in the next commit. > > I don't think this has much to do with Qdiscs, this is the TC > egress hook, it's more for filtering. Classful Qdisc like HTB > will run its own classification. I think. > > Maybe TC_EGRESS? You are right, I think I misunderstanded the concept of qdisc and tc before. and seems all 'QDISC' here should be 'TC'? which means: QDISC_EGRESS -> TC_EGRESS QDISC_DROP -> TC_DROP QDISC_INGRESS -> TC_INGRESS
On Fri, 4 Mar 2022 12:56:40 +0800 Menglong Dong wrote: > You are right, I think I misunderstanded the concept of qdisc and tc before. > and seems all 'QDISC' here should be 'TC'? which means: > > QDISC_EGRESS -> TC_EGRESS > QDISC_DROP -> TC_DROP For this one QDISC is good, I think, it will mostly catch packets which went thru qdisc_drop(), right? > QDISC_INGRESS -> TC_INGRESS
On Fri, Mar 4, 2022 at 1:05 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Fri, 4 Mar 2022 12:56:40 +0800 Menglong Dong wrote: > > You are right, I think I misunderstanded the concept of qdisc and tc before. > > and seems all 'QDISC' here should be 'TC'? which means: > > > > QDISC_EGRESS -> TC_EGRESS > > QDISC_DROP -> TC_DROP > > For this one QDISC is good, I think, it will mostly catch packets > which went thru qdisc_drop(), right? > Yeah, you are right. > > QDISC_INGRESS -> TC_INGRESS
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index d67941f78b92..ebd18850b63e 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -394,6 +394,10 @@ enum skb_drop_reason { * entry is full */ SKB_DROP_REASON_NEIGH_DEAD, /* neigh entry is dead */ + SKB_DROP_REASON_QDISC_EGRESS, /* qdisc of type egress check + * failed (mapbe an eBPF program + * is tricking?) + */ SKB_DROP_REASON_MAX, }; diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h index 1977f301260d..3cadf13448c6 100644 --- a/include/trace/events/skb.h +++ b/include/trace/events/skb.h @@ -45,6 +45,7 @@ EM(SKB_DROP_REASON_NEIGH_FAILED, NEIGH_FAILED) \ EM(SKB_DROP_REASON_NEIGH_QUEUEFULL, NEIGH_QUEUEFULL) \ EM(SKB_DROP_REASON_NEIGH_DEAD, NEIGH_DEAD) \ + EM(SKB_DROP_REASON_QDISC_EGRESS, QDISC_EGRESS) \ EMe(SKB_DROP_REASON_MAX, MAX) #undef EM diff --git a/net/core/dev.c b/net/core/dev.c index 2d6771075720..828946d74a19 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3860,7 +3860,7 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev) case TC_ACT_SHOT: mini_qdisc_qstats_cpu_drop(miniq); *ret = NET_XMIT_DROP; - kfree_skb(skb); + kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_EGRESS); return NULL; case TC_ACT_STOLEN: case TC_ACT_QUEUED: