lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <686dd999-bee4-ecf8-8dc4-c85a098c4a92@iogearbox.net> Date: Fri, 6 Oct 2023 15:49:18 +0200 From: Daniel Borkmann <daniel@...earbox.net> To: Jakub Kicinski <kuba@...nel.org> Cc: Jamal Hadi Salim <jhs@...atatu.com>, Victor Nogueira <victor@...atatu.com>, xiyou.wangcong@...il.com, jiri@...nulli.us, davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com, paulb@...dia.com, netdev@...r.kernel.org, kernel@...atatu.com, martin.lau@...ux.dev, bpf@...r.kernel.org Subject: Re: [PATCH net-next 1/1] net/sched: Disambiguate verdict from return code On 10/6/23 3:32 PM, Jakub Kicinski wrote: > On Fri, 6 Oct 2023 13:18:40 +0200 Daniel Borkmann wrote: >> That should be possible with some work this way, agree. I've been toying a bit >> more on this issue, and actually there is an even better way which would cleanly >> solve all use cases and we likely would utilize it for bpf as well in future. >> I wasn't aware of it before, but the drop reason actually has per-subsystem infra >> already which so far only mac80211 and ovs makes use of. > > FWIW I'm not sure if leaning into the subsys specific error codes for > something as core as TC is a good direction. I'd think that what > matters to the user is was it an intentional policy drop or some form > of an error / exception. More detailed info can come from stats. > > Maybe I'm overly conservative because I don't care about debugging > mac80211 or ovs but do very much care about TC :) And 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. Very cool! > Which will no longer work with the "pack multiple values into > the reason" scheme of subsys-specific values :( Too bad, do you happen to know why it won't work? Given they went into the length of extending this for subsystems, they presumably would also like to benefit from above. :/ > What I'm saying is that there is a trade-off here between providing > as much info as possible vs basic user getting intelligible data.. Makes sense. I think we can drop that aspect for the subsys specific error codes. Fwiw, TCP has 22 drop codes in the core section alone, so this should be fine if you think it's better. The rest of the patch shown should still apply the same way. I can tweak it to use the core section for codes, and then it can be successively extended if that looks good to you - unless you are saying from above, that just one error code is better and then going via detailed stats for specific errors is preferred. Thanks, Daniel
Powered by blists - more mailing lists