netdev - Re: [PATCH net-next v4 01/12] inet: homa: define user-visible API for Homa

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAGXJAmyW2Mnz1hwvTo7PKsXLVJO6dy_TK-ZtDW1E-Lrds6o+WA@mail.gmail.com>
Date: Fri, 20 Dec 2024 09:59:53 -0800
From: John Ousterhout <ouster@...stanford.edu>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, edumazet@...gle.com, 
	horms@...nel.org
Subject: Re: [PATCH net-next v4 01/12] inet: homa: define user-visible API for Homa

On Thu, Dec 19, 2024 at 5:41 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> > Any suggestions on how to make the header file work with C++ files
> > without the #ifdef __cplusplus?
>
> With the little C++ understanding I have, I _think_ the include site
> can wrap:
>
> extern "C" {
> #include "<linux/homa.h>"
> }

I have done this now. I was hesitant to do that because it seemed like
it was creating unnecessary extra work for anyone who uses homa.h in a
C++ program, but since this seems to be the convention I have
conformed to it.

> > > > +/** define SO_HOMA_RCVBUF - setsockopt option for specifying buffer region. */
> > > > +#define SO_HOMA_RCVBUF 10
> > > > +
> > > > +/** struct homa_rcvbuf_args - setsockopt argument for SO_HOMA_RCVBUF. */
> > > > +struct homa_rcvbuf_args {
> > > > +     /** @start: First byte of buffer region. */
> > > > +     void *start;
> > >
> > > I'm not sure if pointers are legal in uAPI.
> > > I *think* we are supposed to use __aligned_u64, because pointers
> > > will be different size for 32b binaries running in compat mode
> > > on 64b kernels, or some such.
> >
> > I see that "void *" is used in the declaration for struct msghdr
> > (along with some other pointer types as well) and struct msghdr is
> > part of several uAPI interfaces, no?
>
> Off the top off my head this use is a source of major pain, grep around
> for compat_msghdr.

How should I go about confirming that this __aligned_u64 is indeed the
expected convention (sounds like you aren't certain)? Also, any idea
why it needs to be aligned rather than just __u64?

> My recommendation is to use normal comments where kdoc just repeats
> obvious stuff. All these warnings sooner or later will result in some
> semi-automated and often poor quality patch submissions to "fix" it.
> Which is just work for maintainers to deal with :(

I have done this now. I had assumed that kdoc would also complain if
there was a declaration without official kdoc documentation, but now
that I see that it won't, I'll use that approach for places where kdoc
style is awkward. Personally I'm agnostic about whether to use kdoc; I
assumed that I should use it since it exists.

-John-