From nobody Fri Jun 6 14:34:11 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4bDP2r2RY9z5xDMh; Fri, 06 Jun 2025 14:34:12 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4bDP2r1XQwz4B7M; Fri, 06 Jun 2025 14:34:12 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1749220452; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fx2AgCbvOr8DxMcMFOZEp2HatdjZ1/mw2Pi2d7d+XPE=; b=b6Fgnk3ynGh74ndFYMuwJPb3IXdvpqOHeg+pZZiUIesCdwbgTWVKye7N4Dq98V/lxIABnh rmwiM3xeBT3TCpZRqrbqqapRy3q3mIXnIiH6aFqidPmra3vemwYGt7Wn+JrV+WLyaAgCWS qOEeCAXeWJLJB+vLO6rlkBFZxnHsgMv9bm/+/KRHYdETVss0dkgGtHSdYTm8ToOnRGPLOY DeZOFMNEvyPkmLLQtTXp9uZyrbY+0iMGhaFkTBuEKDuBLWDJuvB16Kx2eHom8QohVLeyPG ItSlqYKENPi+S2OBOROuoUkYH4HUAR7ptifqDxGpktmAKGnmbqidpbqbj+C5qg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1749220452; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fx2AgCbvOr8DxMcMFOZEp2HatdjZ1/mw2Pi2d7d+XPE=; b=TuduQznquliVZgJNUdTk5/SJ0uCpXh9KvQ4+mfjM/AQfrGpfKT6eGk9KXOrdqLG/v3kgvi JCLzrGa0NQmwn2GR9n98Iw5+5fr6YeSzth7FfnqE8pbhED9eaOjY0NLxQaHKU43qpafD5o SqImxf5QCMXkSKWaQqCbR8kLou227rsdlr3h6eEPhPpTK5Q6GvwOhu5Tx5RrQYrDgh4Fhh UlTYeGuaViOF/BEGvyVpLbASEsDVZgb7aA6pk+Xcbqi51s79Wm9W+RvZh4e583xSUODROe 1Lyfjjg0gOimFJlyBllMQgOCj9XSk3VBdKJDlsR/1O/RWG/+9Q53/PKXlUGknQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1749220452; a=rsa-sha256; cv=none; b=i+5eQgviKwR86hcD6fuibge/azeQXpqVX4gIYOVPvuICXFK2DgDYgxUnJFO+XNLahimwtB WjXV1fgWIOKsEyBXEMpct+JKVs1n0BNbTV02mgarJnUOvXM7qq5nm9yIhVd+eLjqgsXDaM 5Wjq5kkE+3dKAAyE/qc/mCrCLlK3WoRCmImELSEsehGGeDqi5Plb+s2EFA0TqP9LYSlOGW dNfkyx3MpkRczQi2oYXzZkwTAMC2kLoWht3Lo1TpHiK1jnwiSxR4dSm/r5F7DxzoJbPZCm p085tm6a6ej8LX2FnaE4yABoYPPWQE0dQA168BiKqZ6yJhnW2zEuBDMu55bQOQ== Received: from [IPV6:2601:5c0:4200:b830:f:bebf:564e:f7bd] (unknown [IPv6:2601:5c0:4200:b830:f:bebf:564e:f7bd]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4bDP2q6b6BzP8h; Fri, 06 Jun 2025 14:34:11 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <346c0310-b9b6-4eb7-ac94-0575a83c3416@FreeBSD.org> Date: Fri, 6 Jun 2025 10:34:11 -0400 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: git: 7b3ee39e73af - main - libcam: Include nvme opcode and status code routines from nvme_util.c Content-Language: en-US To: Warner Losh Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202506060132.5561Wakc094185@gitrepo.freebsd.org> From: John Baldwin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 6/5/25 22:21, Warner Losh wrote: > On Thu, Jun 5, 2025 at 7:41 PM John Baldwin wrote: >> >> 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 >>> AuthorDate: 2025-06-06 01:28:38 +0000 >>> Commit: John Baldwin >>> 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//_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. > > No. We can't remove all that. They are used by camcontrol, ddcam and > others. We use the functions in *_all.h/c functions to populate the > CCBs to send down into the kernel. I think that we need a different > take on it. It is, after all, basically designed in FreeBSD 3 > timeframe when such considerations were the furthest thing from the > minds of the developers. Times have changed, though, and libcam hasn't > with it. We don't use a single routine from nvme_all.c outside of the kernel. libcam also doesn't include mmc_all.c. > We don't document it very well... Sometimes I've thought we should > make libcam a private library. But I don't know who all uses it in raw > mode. I think about a dozen ports use this for things like smart > reporting... I think a decent start would be to maybe be a bit more choosy in what things have to be exported. Only a few routines in ata_all.c and smp_all.c appear to be used in camcontrol for example. And it certainly seems like we could remove nvme_all.c from libcam entirely. Using a version map with symbol versioning might be the best way to enforce that (though more liberal use of #ifdef _KERNEL in *_all.c would be another route). We definitely will need to bump the SHLIB_MAJOR for libcam before branching 15 though. -- John Baldwin