Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Jun 2025 21:41:23 -0400
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   Re: git: 7b3ee39e73af - main - libcam: Include nvme opcode and status code routines from nvme_util.c
Message-ID:  <eea5514d-2b3b-4491-bc2a-9bbed5bdacbc@FreeBSD.org>
In-Reply-To: <202506060132.5561Wakc094185@gitrepo.freebsd.org>
References:  <202506060132.5561Wakc094185@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 6/5/25 21:32, John Baldwin wrote:
> The branch main has been updated by jhb:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=7b3ee39e73af36f49f471f7900baeb98ac3504d0
> 
> commit 7b3ee39e73af36f49f471f7900baeb98ac3504d0
> Author:     John Baldwin <jhb@FreeBSD.org>
> AuthorDate: 2025-06-06 01:28:38 +0000
> Commit:     John Baldwin <jhb@FreeBSD.org>
> CommitDate: 2025-06-06 01:28:38 +0000
> 
>      libcam: Include nvme opcode and status code routines from nvme_util.c
>      
>      libcam in userspace also includes nvme_all.c which now depends on
>      nvme_util.c, so add nvme_util.c to libcam's sources.  This requires
>      exporting the opcode and status code routines in nvme_util.c to
>      userspace as well as the kernel.  In turn, this means nvmecontrol now
>      depends on libsbuf (which is already present in /lib).
>      
>      Reported by:    viswhin, Jenkins
>      Fixes:          60159a98a837 ("nvme: Move opcode and status code tables from base CAM to nvme_util.c")
>      Sponsored by:   Chelsio Communications

This fixes the build for now (and sorry for breaking it).  However, this
raises a few questions for me at least.  Why does libcam include nvme_all.c
at all?  We don't document any of the nvme_* functions in cam(3), nor any
of the functions from scsi_all.c, smp_all.c, etc.  This seems really odd,
and it also means that we can add (and remove!) symbols from libcam without
realizing it by changing sys/cam/<proto>/<proto>_all.h which is not very
obvious.

It seems to me that we should be more intentional about which symbols we
export from libcam.  Switching to symbol versioning (which implicitly
enforces hidden visibility on all symbols not explicitly exported) would
keep us from leaking symbols into the ABI of libcam that we don't intend
to export.

I also wonder if we can remove some of the *_all.c files from libcam
entirely?  None of the nvme_* ones are used outside of the kernel in the
base system for example.

-- 
John Baldwin




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?eea5514d-2b3b-4491-bc2a-9bbed5bdacbc>