• Daniel Borkmann's avatar
    net, sched: Make tc-related drop reason more flexible · 54a59aed
    Daniel Borkmann authored
    Currently, the kfree_skb_reason() in sch_handle_{ingress,egress}() can only
    express a basic SKB_DROP_REASON_TC_INGRESS or SKB_DROP_REASON_TC_EGRESS reason.
    
    Victor kicked-off an initial proposal to make this more flexible by disambiguating
    verdict from return code by moving the verdict into struct tcf_result and
    letting tcf_classify() return a negative error. If hit, then two new drop
    reasons were added in the proposal, that is SKB_DROP_REASON_TC_INGRESS_ERROR
    as well as SKB_DROP_REASON_TC_EGRESS_ERROR. Further analysis of the actual
    error codes would have required to attach to tcf_classify via kprobe/kretprobe
    to more deeply debug skb and the returned error.
    
    In order to make the kfree_skb_reason() in sch_handle_{ingress,egress}() more
    extensible, it can be addressed in a more straight forward way, that is: Instead
    of placing the verdict into struct tcf_result, we can just put the drop reason
    in there, which does not require changes throughout various classful schedulers
    given the existing verdict logic can stay as is.
    
    Then, SKB_DROP_REASON_TC_ERROR{,_*} can be added to the enum skb_drop_reason
    to disambiguate between an error or an intentional drop. New drop reason error
    codes can be added successively to the tc code base.
    
    For internal error locations which have not yet been annotated with a
    SKB_DROP_REASON_TC_ERROR{,_*}, the fallback is SKB_DROP_REASON_TC_INGRESS and
    SKB_DROP_REASON_TC_EGRESS, respectively. Generic errors could be marked with a
    SKB_DROP_REASON_TC_ERROR code until they are converted to more specific ones
    if it is found that they would be useful for troubleshooting.
    
    While drop reasons have infrastructure for subsystem specific error codes which
    are currently used by mac80211 and ovs, Jakub mentioned that it is preferred
    for tc to use the enum skb_drop_reason core codes given it is a better fit and
    currently the tooling support is better, too.
    
    With regards to the latter:
    
      [...] I think Alastair (bpftrace) is working on auto-prettifying enums when
      bpftrace outputs maps. So we can do something like:
    
      $ bpftrace -e 'tracepoint:skb:kfree_skb { @[args->reason] = count(); }'
      Attaching 1 probe...
      ^C
    
      @[SKB_DROP_REASON_TC_INGRESS]: 2
      @[SKB_CONSUMED]: 34
    
      ^^^^^^^^^^^^ names!!
    
      Auto-magically. [...]
    
    Add a small helper tcf_set_drop_reason() which can be used to set the drop reason
    into the tcf_result.
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Cc: Jamal Hadi Salim <jhs@mojatatu.com>
    Cc: Victor Nogueira <victor@mojatatu.com>
    Link: https://lore.kernel.org/netdev/20231006063233.74345d36@kernel.orgReviewed-by: default avatarJakub Kicinski <kuba@kernel.org>
    Link: https://lore.kernel.org/r/20231009092655.22025-1-daniel@iogearbox.netSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    54a59aed
pkt_cls.h 25.6 KB