| Message ID | 20231201230011.2925305-4-victor@mojatatu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: sched: Make tc-related drop reason more flexible for remaining qdiscs | expand |
On Fri, Dec 1, 2023 at 6:00 PM Victor Nogueira <victor@mojatatu.com> wrote: > > Continue expanding Daniel's patch by adding new skb drop reasons that > are idiosyncratic to TC. > > More specifically: > > - SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up using > ext, but was not found. > > - SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was looked up using cookie > and either was not found or different from expected. > > - SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed. > > - SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop > iterations > > Signed-off-by: Victor Nogueira <victor@mojatatu.com> > --- > include/net/dropreason-core.h | 30 +++++++++++++++++++++++++++--- > net/sched/act_api.c | 3 ++- > net/sched/cls_api.c | 22 ++++++++++++++-------- > 3 files changed, 43 insertions(+), 12 deletions(-) > > diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h > index 3c70ad53a49c..fa6ace8f1611 100644 > --- a/include/net/dropreason-core.h > +++ b/include/net/dropreason-core.h > @@ -85,7 +85,11 @@ > FN(IPV6_NDISC_BAD_OPTIONS) \ > FN(IPV6_NDISC_NS_OTHERHOST) \ > FN(QUEUE_PURGE) \ > - FN(TC_ERROR) \ > + FN(TC_EXT_COOKIE_NOTFOUND) \ > + FN(TC_COOKIE_EXT_MISMATCH) \ > + FN(TC_COOKIE_MISMATCH) \ > + FN(TC_CHAIN_NOTFOUND) \ > + FN(TC_RECLASSIFY_LOOP) \ > FNe(MAX) > > /** > @@ -376,8 +380,28 @@ enum skb_drop_reason { > SKB_DROP_REASON_IPV6_NDISC_NS_OTHERHOST, > /** @SKB_DROP_REASON_QUEUE_PURGE: bulk free. */ > SKB_DROP_REASON_QUEUE_PURGE, > - /** @SKB_DROP_REASON_TC_ERROR: generic internal tc error. */ > - SKB_DROP_REASON_TC_ERROR, > + /** > + * @SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up > + * using ext, but was not found. > + */ > + SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND, > + /** > + * @SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was lookup using > + * cookie and either was not found or different from expected. > + */ > + SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH, > + /** > + * @SKB_DROP_REASON_TC_COOKIE_MISMATCH: tc cookie available but was > + * unable to match to filter. > + */ > + SKB_DROP_REASON_TC_COOKIE_MISMATCH, > + /** @SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed. */ > + SKB_DROP_REASON_TC_CHAIN_NOTFOUND, > + /** > + * @SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop > + * iterations. > + */ > + SKB_DROP_REASON_TC_RECLASSIFY_LOOP, > /** > * @SKB_DROP_REASON_MAX: the maximum of core drop reasons, which > * shouldn't be used as a real 'reason' - only for tracing code gen > diff --git a/net/sched/act_api.c b/net/sched/act_api.c > index 12ac05857045..429cb232e17b 100644 > --- a/net/sched/act_api.c > +++ b/net/sched/act_api.c > @@ -1098,7 +1098,8 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions, > } > } else if (TC_ACT_EXT_CMP(ret, TC_ACT_GOTO_CHAIN)) { > if (unlikely(!rcu_access_pointer(a->goto_chain))) { > - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); > + tcf_set_drop_reason(skb, > + SKB_DROP_REASON_TC_CHAIN_NOTFOUND); > return TC_ACT_SHOT; > } > tcf_action_goto_chain_exec(a, res); > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c > index 32457a236d77..5012fc0a24a1 100644 > --- a/net/sched/cls_api.c > +++ b/net/sched/cls_api.c > @@ -1682,13 +1682,15 @@ static inline int __tcf_classify(struct sk_buff *skb, > */ > if (unlikely(n->tp != tp || n->tp->chain != n->chain || > !tp->ops->get_exts)) { > - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); > + tcf_set_drop_reason(skb, > + SKB_DROP_REASON_TC_COOKIE_MISMATCH); > return TC_ACT_SHOT; > } > > exts = tp->ops->get_exts(tp, n->handle); > if (unlikely(!exts || n->exts != exts)) { > - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); > + tcf_set_drop_reason(skb, > + SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH); > return TC_ACT_SHOT; > } > > @@ -1717,7 +1719,8 @@ static inline int __tcf_classify(struct sk_buff *skb, > } > > if (unlikely(n)) { > - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); > + tcf_set_drop_reason(skb, > + SKB_DROP_REASON_TC_COOKIE_MISMATCH); > return TC_ACT_SHOT; > } > > @@ -1729,7 +1732,8 @@ static inline int __tcf_classify(struct sk_buff *skb, > tp->chain->block->index, > tp->prio & 0xffff, > ntohs(tp->protocol)); > - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); > + tcf_set_drop_reason(skb, > + SKB_DROP_REASON_TC_RECLASSIFY_LOOP); > return TC_ACT_SHOT; > } > > @@ -1767,7 +1771,8 @@ int tcf_classify(struct sk_buff *skb, > n = tcf_exts_miss_cookie_lookup(ext->act_miss_cookie, > &act_index); > if (!n) { > - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); > + tcf_set_drop_reason(skb, > + SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND); > return TC_ACT_SHOT; > } > > @@ -1778,7 +1783,9 @@ int tcf_classify(struct sk_buff *skb, > > fchain = tcf_chain_lookup_rcu(block, chain); > if (!fchain) { > - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); > + tcf_set_drop_reason(skb, > + SKB_DROP_REASON_TC_CHAIN_NOTFOUND); > + > return TC_ACT_SHOT; > } > > @@ -1800,10 +1807,9 @@ int tcf_classify(struct sk_buff *skb, > > ext = tc_skb_ext_alloc(skb); > if (WARN_ON_ONCE(!ext)) { > - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); > + tcf_set_drop_reason(skb, SKB_DROP_REASON_NOMEM); > return TC_ACT_SHOT; > } > - > ext->chain = last_executed_chain; > ext->mru = cb->mru; > ext->post_ct = cb->post_ct; > -- > 2.25.1 > > I have been meaning to get around to adding QDISC_DROP_REASON_{CONGEST,OVERFLOW,FLOOD,SPIKE} to cake/fq_codel/red/etc for some time now. Would this be the right facility to leverage (or something more direct?) I discussed the why at netdevconf: https://docs.google.com/document/d/1tTYBPeaRdCO9AGTGQCpoiuLORQzN_bG3TAkEolJPh28/edit Other names welcomed. Otherwise I suppose I will wait for this stuff to land: Acked-By: Dave Taht <dave.taht@gmail.com> -- :( My old R&D campus is up for sale: https://tinyurl.com/yurtlab Dave Täht CSO, LibreQos
On 05/12/2023 19:28, Dave Taht wrote: > On Fri, Dec 1, 2023 at 6:00 PM Victor Nogueira <victor@mojatatu.com> wrote: >> >> Continue expanding Daniel's patch by adding new skb drop reasons that >> are idiosyncratic to TC. >> >> More specifically: >> >> - SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up using >> ext, but was not found. >> >> - SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was looked up using cookie >> and either was not found or different from expected. >> >> - SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed. >> >> - SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop >> iterations >> >> Signed-off-by: Victor Nogueira <victor@mojatatu.com> >> --- >> >> [...] > > I have been meaning to get around to adding > QDISC_DROP_REASON_{CONGEST,OVERFLOW,FLOOD,SPIKE} to > cake/fq_codel/red/etc for some time now. Would this be the right > facility to leverage (or something more direct?) I discussed the why > at netdevconf: > > https://docs.google.com/document/d/1tTYBPeaRdCO9AGTGQCpoiuLORQzN_bG3TAkEolJPh28/edit Yes, I think here will be the place to add these new drop reasons. After this patch lands, we will add more, but feel free to also propose others. cheers, Victor
diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h index 3c70ad53a49c..fa6ace8f1611 100644 --- a/include/net/dropreason-core.h +++ b/include/net/dropreason-core.h @@ -85,7 +85,11 @@ FN(IPV6_NDISC_BAD_OPTIONS) \ FN(IPV6_NDISC_NS_OTHERHOST) \ FN(QUEUE_PURGE) \ - FN(TC_ERROR) \ + FN(TC_EXT_COOKIE_NOTFOUND) \ + FN(TC_COOKIE_EXT_MISMATCH) \ + FN(TC_COOKIE_MISMATCH) \ + FN(TC_CHAIN_NOTFOUND) \ + FN(TC_RECLASSIFY_LOOP) \ FNe(MAX) /** @@ -376,8 +380,28 @@ enum skb_drop_reason { SKB_DROP_REASON_IPV6_NDISC_NS_OTHERHOST, /** @SKB_DROP_REASON_QUEUE_PURGE: bulk free. */ SKB_DROP_REASON_QUEUE_PURGE, - /** @SKB_DROP_REASON_TC_ERROR: generic internal tc error. */ - SKB_DROP_REASON_TC_ERROR, + /** + * @SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up + * using ext, but was not found. + */ + SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND, + /** + * @SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was lookup using + * cookie and either was not found or different from expected. + */ + SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH, + /** + * @SKB_DROP_REASON_TC_COOKIE_MISMATCH: tc cookie available but was + * unable to match to filter. + */ + SKB_DROP_REASON_TC_COOKIE_MISMATCH, + /** @SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed. */ + SKB_DROP_REASON_TC_CHAIN_NOTFOUND, + /** + * @SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop + * iterations. + */ + SKB_DROP_REASON_TC_RECLASSIFY_LOOP, /** * @SKB_DROP_REASON_MAX: the maximum of core drop reasons, which * shouldn't be used as a real 'reason' - only for tracing code gen diff --git a/net/sched/act_api.c b/net/sched/act_api.c index 12ac05857045..429cb232e17b 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -1098,7 +1098,8 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions, } } else if (TC_ACT_EXT_CMP(ret, TC_ACT_GOTO_CHAIN)) { if (unlikely(!rcu_access_pointer(a->goto_chain))) { - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); + tcf_set_drop_reason(skb, + SKB_DROP_REASON_TC_CHAIN_NOTFOUND); return TC_ACT_SHOT; } tcf_action_goto_chain_exec(a, res); diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 32457a236d77..5012fc0a24a1 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -1682,13 +1682,15 @@ static inline int __tcf_classify(struct sk_buff *skb, */ if (unlikely(n->tp != tp || n->tp->chain != n->chain || !tp->ops->get_exts)) { - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); + tcf_set_drop_reason(skb, + SKB_DROP_REASON_TC_COOKIE_MISMATCH); return TC_ACT_SHOT; } exts = tp->ops->get_exts(tp, n->handle); if (unlikely(!exts || n->exts != exts)) { - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); + tcf_set_drop_reason(skb, + SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH); return TC_ACT_SHOT; } @@ -1717,7 +1719,8 @@ static inline int __tcf_classify(struct sk_buff *skb, } if (unlikely(n)) { - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); + tcf_set_drop_reason(skb, + SKB_DROP_REASON_TC_COOKIE_MISMATCH); return TC_ACT_SHOT; } @@ -1729,7 +1732,8 @@ static inline int __tcf_classify(struct sk_buff *skb, tp->chain->block->index, tp->prio & 0xffff, ntohs(tp->protocol)); - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); + tcf_set_drop_reason(skb, + SKB_DROP_REASON_TC_RECLASSIFY_LOOP); return TC_ACT_SHOT; } @@ -1767,7 +1771,8 @@ int tcf_classify(struct sk_buff *skb, n = tcf_exts_miss_cookie_lookup(ext->act_miss_cookie, &act_index); if (!n) { - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); + tcf_set_drop_reason(skb, + SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND); return TC_ACT_SHOT; } @@ -1778,7 +1783,9 @@ int tcf_classify(struct sk_buff *skb, fchain = tcf_chain_lookup_rcu(block, chain); if (!fchain) { - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); + tcf_set_drop_reason(skb, + SKB_DROP_REASON_TC_CHAIN_NOTFOUND); + return TC_ACT_SHOT; } @@ -1800,10 +1807,9 @@ int tcf_classify(struct sk_buff *skb, ext = tc_skb_ext_alloc(skb); if (WARN_ON_ONCE(!ext)) { - tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR); + tcf_set_drop_reason(skb, SKB_DROP_REASON_NOMEM); return TC_ACT_SHOT; } - ext->chain = last_executed_chain; ext->mru = cb->mru; ext->post_ct = cb->post_ct;
Continue expanding Daniel's patch by adding new skb drop reasons that are idiosyncratic to TC. More specifically: - SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up using ext, but was not found. - SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was looked up using cookie and either was not found or different from expected. - SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed. - SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop iterations Signed-off-by: Victor Nogueira <victor@mojatatu.com> --- include/net/dropreason-core.h | 30 +++++++++++++++++++++++++++--- net/sched/act_api.c | 3 ++- net/sched/cls_api.c | 22 ++++++++++++++-------- 3 files changed, 43 insertions(+), 12 deletions(-)