netdev - Re: [PATCH net-next v3 2/6] tools/net/ynl: Report netlink errors without stacktrace

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZetYdY7DXcElIKwa@gmail.com>
Date: Fri, 8 Mar 2024 10:27:01 -0800
From: Breno Leitao <leitao@...ian.org>
To: Jakub Kicinski <kuba@...nel.org>, michel@...hel-slm.name
Cc: Donald Hunter <donald.hunter@...il.com>, netdev@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Jacob Keller <jacob.e.keller@...el.com>,
	Jiri Pirko <jiri@...nulli.us>, Stanislav Fomichev <sdf@...gle.com>,
	donald.hunter@...hat.com
Subject: Re: [PATCH net-next v3 2/6] tools/net/ynl: Report netlink errors
 without stacktrace

On Thu, Mar 07, 2024 at 07:58:15AM -0800, Jakub Kicinski wrote:
> On Thu, 7 Mar 2024 11:56:59 +0000 Donald Hunter wrote:
> > > Basically this is just hidding the stack, which may make it harder for
> > > someone not used to the code to find the problem.
> > >
> > > Usually fatal exception is handled to make the error more meaningful,
> > > i.e, better than just the exception message + stack. Hidding the stack
> > > and exitting may make the error less meaningful.  
> > 
> > NlError is used to report a usage error reported by netlink as opposed
> > to a fatal exception. My thinking here is that it is better UX to
> > report netlink error responses without the stack trace precisely
> > because they are not exceptional. An NlError is not ynl program
> > breakage or subsystem breakage, it's e.g. nlctrl telling you that you
> > requested an op that does not exist.
> 
> Right, I think the YNL library should still throw, but since this is
> a case of "kernel gave us this specific error in response" the stack
> trace adds relatively little for the CLI.
> 
> > > On a different topic, I am wondering if we want to add type hitting for
> > > these python program. They make the review process easier, and the
> > > development a bit more structured. (Maybe that is what we expect from
> > > upcoming new python code in netdev?!)  
> > 
> > It's a good suggestion. I have never used python type hints so I'll
> > need to learn about them. I defer to the netdev maintainers about
> > whether this is something they want.
> 
> I'm far from a Python expert, so up to you :)
> I used type hints a couple of times in the past, they are somewhat
> useful, but didn't feel useful enough to bother. Happy for someone
> else to do the work, tho :)

I am a big fan of type hitting, since it help in reviewing code, as also
with tooling that help you to find problems, since the function returns
and arguments now have a type.

What are the top 3 python scripts we have in network today? I can try to
find some time to help.

> FWIW I reckon that trying to get the CLI ready for distro packaging 
> may be higher prio. Apart from basic requirements to packaging python
> code (I have no idea what they are), we should probably extend the
> script to search some system paths? My thinking is that if someone
> installs the CLI as an RPM, they should be able to use it like this:
> 
>  $ ynl-cli --family nlctrl \
> 	--do getfamily --json '{"family-name": "nlctrl"}'
> 
> the --family would be used instead of --spec and look for the exact
> spec file in /usr/share/.../specs/ and probably also imply --no-schema,
> since hopefully the schema is already validated during development,
> and no point wasting time validating it on every user invocation.
> 
> WDYT?

This is a good idea. I've had a chat with Michel, and he can help with
the distro part. Adding him in the CC.