Re: [PATCH 01/23] net: hpe: ss1: add the Cassini headers

[Doug Ledford]

On 10/31/26 6:58 AM, Andrew Lunn wrote:
> On Fri, Oct 30, 2026 at 01:58:57PM -0700, Doug Ledford wrote:
>> Start the Cassini core driver (cxi-ss1) with its headers: the register
>> and counter definitions, error definitions, telemetry item tables, and
>> the CXI interface (cxi.h, cxi_prov_hw.h, libcassini.h) through which
>> client drivers use the NIC.
>>
>> The cassini_* headers are generated from the hardware description and
>> are not meant to be edited by hand, so they carry the generator's
>> formatting.  cassini_user_defs.h is shared byte-for-byte with the user
>> space libraries and is therefore under include/uapi/ethernet.
> 
> You want to keep the UAPI as small as possible, because once this is
> merged, it can never change in ways that break backwards
> compatibility. 
> 
> We also strongly discourage APIs that allow user space drivers, which,
> at first glance, such definitions would allow.

The fact that we have headers in uapi/ethernet/* is a holdover from
internal issues and for all files except cassini_user_defs.h, which is
legitimately shared with the user space cxiverbs driver, can be moved to
the linux/hpe/* include directories.  That will be part of the V2
patchset.  The cassini_user_defs.h file could go into uapi/rdma instead,
but I figured that wouldn't fly with the netdev community as the
Ethernet driver legitimately needs that header to compile in the kernel,
and the sole current user space consumer is the user space cxiverbs
provider we have added to the rdma-core user space package (we have our
own libcxi and pycxi that will possibly use it in the future...they use
the one from the internal repos today).  So in that sense, it's an rdma
needed header, but it's needed in the Ethernet kernel driver, so we
didn't think you would want it in the uapi/rdma directory.

The rdma kernel portion of the driver went upstream yesterday FWIW:
RDMA/cxi: add the HPE Cassini verbs driver
https://lore.kernel.org/linux-rdma/20261030-cxi-for-next-v1-0-bc6e49e07037@xxxxxxx/T/#t

And the user space rdma-core portion of the driver went upstream today:
cxiverbs: add the HPE Cassini verbs provider
https://lore.kernel.org/linux-rdma/20261031-cxi-for-next-v1-0-93ce01907ce2@xxxxxxx/T/#t

> 
>> It
>> provides inline accessors that need errno.h, string.h and size_t,
>> which a standalone UAPI header cannot assume, so this patch excludes
>> it from the UAPI header self-test.
> 
> Does any other driver do this? If you are doing something which no
> other driver does, it is probably broken.

You need to be more specific.  Do you mean have a uapi at all for an
Ethernet device (I think I explained this part above well enough), or
have a uapi header file with embedded items that necessitate an
exclusion from the hdr test?  If it's the latter, that is now flagged as
a V2 fix.  The header file is a generated file and we need to fix the
generator.  Once fixed, the header is still needed by user space code
and hence still needs to be in the uapi structure somewhere, but will be
able to pass the hdr test in the kernel.

>> checkpatch --strict (100 columns) still reports the following in this
>> patch.
> 
> netdev prefers 80.

Is 80 mandated, or just preferred?  I can make the change if it's a
must, but prefer not to make that much code churn if I don't have to, as
I expect that formatting only change would be a huge number of lines.

>> They need coordination with the internal repositories this code
>> is maintained in and will be fixed in V2:
> 
> Your internal repos don't matter. Once this is merged it has a life of
> its own.

While I don't expect upstream to care about our internal repos, that
they matter to us should be no surprise.  This driver is 6 years old, so
obviously a mature driver, and it powers the top 3 US supercomputers (it
used to be the top 3 period, but a new one came online recently in China
that took the top spot).  We have contractual obligations to support
those systems, and I don't see them jumping from our provided driver
stack to the upstream kernel on a whim.  So our development plan (mostly
done, but still a bit to go) is simply to bring our internal driver and
the driver we are submitting to the upstream kernel into enough
similarity that we can use github actions and copybara to keep the repos
mostly in sync in an automated fashion and only requiring human
intervention for things that have true conflicts.  We are well aware of
the fact that once this driver lands in upstream, we won't be the only
ones making changes to it.  Our intent is to have a two way sync,
keeping our own development efforts up to date in both repos, and
pulling in upstream changes made to our code into our internal repo as
well.  My comment was not to say that I expected upstream to care about
our internal repos or some such, just explaining that it is taking us
some time to adapt everything and we are still in that process.

Which brings up a point that I guess people on this list wouldn't be
aware of.  We are coordinating our work with the Ultra Ethernet
Consortium's work.  As you are no doubt aware, code with no consumer is
not allowed to be merged into the kernel.  The UET group finished their
patches and submitted them to the linux-rdma community about two weeks
ago.  Those patches define a new queue pair type, but there are no
consumers/users of those new elements.  Our rdma kernel submission is
dependent on their patchset and on this patchset and is the first
consumer of the new queue pair type.  Between sashiko findings,
checkpatch findings, internal coordination, etc. we simply didn't get
everything done in the two week window.  But I felt it was necessary to
go ahead and get our V1 patch in front of people since the UET patch set
was waiting on us as their first consumer and as actual hardware using
their code.  Plus the next development window is opening next week too
and I wanted to get our V1 out by when it opened.

-- 
Doug Ledford <doug.ledford@xxxxxxx>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature