netdev - Re: [PATCH net-next v9 03/15] net: homa: create shared Homa header files

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: <CAGXJAmx61NFmvwDeUqi1x+sSetZwtNF_EvbYCgBoNDODXEp7jg@mail.gmail.com>
Date: Fri, 13 Jun 2025 11:36:18 -0700
From: John Ousterhout <ouster@...stanford.edu>
To: Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, edumazet@...gle.com, 
	kuba@...nel.org
Subject: Re: [PATCH net-next v9 03/15] net: homa: create shared Homa header files

On Fri, Jun 13, 2025 at 7:41 AM Simon Horman <horms@...nel.org> wrote:
>
> > +#ifdef __CHECKER__
> > +#define __context__(x, y, z) __attribute__((context(x, y, z)))
> > +#else
> > +#define __context__(...)
> > +#endif /* __CHECKER__ */
>
> I am unclear on the intent of this. But it does seem to be an
> unusual approach within the Kernel (I couldn't find any other similar
> code in-tree. And, with other patches in this series, it does seem
> to lead to Sparse and Smatch flagging the following (and other similar
> warnings):
>
>  .../rhashtable.h:411:9: error: macro "__context__" requires 3 arguments, but only 2 given
>  .../rhashtable.h:411:27: error: Expected ( after __context__ statement
>  .../rhashtable.h:411:27: error: got ;
>
> I suspect it's best to remove the above.

If I remove those lines then Homa doesn't compile. I have struggled to
figure out how to use sparse in a meaningful way, so I'm probably
doing things wrong;  if you have suggestions I'd be delighted to hear
them.

The code above is required because I use __context__ for Homa's RPC
locks. I'm not sure this was the right thing to do, but I did it for
two reasons:

* The locks for homa_rpc objects are kept outside the objects (in the
hash table buckets used to look them up). Sometimes they are accessed
via the rpc object (rpc->bucket->lock) and sometimes directly from the
bucket (bucket->lock). I was concerned that sparse would not be able
to figure out that these are actually the same lock.

* Functions such as homa_rpc_find_server in homa_rpc.c return an RPC
in locked state, so they need an __acquires annotation (actually,
__cond_acquires for homa_rpc_find_server). But I couldn't figure out
how to specify the acquired lock, since there is no variable in the
API that represents the returned RPC.

Defining an "rpc_bucket_lock" context for the lock (see homa_sock.h)
seemed to provide the expressive power to solve both these problems,
but it led to compilation errors without the additional code above.

Is there a different and better way I should be doing things?

-John-