Re: [PATCH v4 1/4] nvmet-tcp: unify sockopt with do_sock_setsockopt

[Geliang Tang <geliang@kernel.org>]

Hi,

Sorry for the late reply. In another patch "mptcp: sockopt: implement
IPV6_TCLASS" [1] on the MPTCP mailing list, I implemented the
__ip6_sock_set_tclass() helper and hoped it would be upstreamed first,
but it is still pending.

Thank you all very much for your review comments, but I was caught in a
dilemma after receiving different suggestions.

On Wed, 2026-08-19 at 11:04 +0200, Christoph Hellwig wrote:
> On Tue, Aug 18, 2026 at 02:03:00PM +0800, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > This patch consolidates socket option settings in nvmet-tcp by
> > utilizing
> > the generic do_sock_setsockopt() helper for options including
> > SO_LINGER,
> > SO_PRIORITY, SO_REUSEADDR, TCP_NODELAY, and IP_TOS. This change
> > eliminates
> > the need to export and use specialized helpers for each individual
> > socket
> > option.
> 
> Hmm.  I see that do_sock_setsockopt is exported, but it really should
> not be.  It's really just an internal helper exposed for io_uring
> and no modular code should be using it (never mind the non-GPL
> export).

You are right, my current version has several problems:

1. do_sock_setsockopt() should be exported with EXPORT_SYMBOL_GPL(),
not EXPORT_SYMBOL().

2. The "compat" parameter of do_sock_setsockopt() should be "true"
instead of "false" to skip BPF cgroup setsockopt programs, matching the
behavior of the original nvme-tcp code.

3. The helpers should return "void" instead of "int", to preserve the
original behavior in nvme-tcp where these socket options are set on a
best-effort basis.

4. Error logging (pr_err) should be inside the helpers to minimize
changes to nvme-tcp code.

5. The helpers should not be duplicated between host and target, they
should be consolidated into a shared header, e.g., nvme/common/tcp-
sockopt.h.

I wonder if we can move forward with this approach once these issues
are fixed?

> 
> > A key benefit of this refactoring is that it decouples the socket
> > option
> > configuration from the underlying transport protocol. This makes it
> > easier to extend nvmet-tcp to support other protocols, such as
> > MPTCP, in
> > the future, as do_sock_setsockopt() abstracts away protocol-
> > specific
> > differences without requiring per-option protocol-specific
> > wrappers.
> 
> We really should have generic helpers in the networking code for this
> and not duplicate them in driver using socket options.

However, I still need to use do_sock_setsockopt() for a fundamental
reason: the upcoming NVMe MPTCP support series requires protocol-
independent socket option handling.

When NVMe creates MPTCP sockets instead of TCP sockets, calling
protocol-specific helpers like tcp_sock_set_nodelay() or
ip_sock_set_tos() directly will not work correctly on MPTCP sockets.
MPTCP needs to set options not only on the MPTCP socket itself but also
synchronize them to all subflow sockets.

In the previous NVMe MPTCP support implementation [2], I exported
protocol-specific sockopt helpers separately on the TCP side and MPTCP
side, then had NVMe switch between them based on the protocol. This is
not as clean as using do_sock_setsockopt() for protocol-independent
calls.

For MPTCP, we could use sock_common_setsockopt() which always
dispatches to the protocol's .setsockopt for all options. However, for
TCP, this doesn't work correctly for SOL_SOCKET level options
(SO_LINGER, SO_PRIORITY, SO_REUSEADDR, SO_BINDTODEVICE) which should be
handled by sock_setsockopt() directly. That's why do_sock_setsockopt()
is used here - it intelligently chooses between sock_setsockopt() for
SOL_SOCKET options and sock->ops->setsockopt() for protocol-specific
options, making it work correctly for both TCP and MPTCP without
requiring nvme-tcp to know which protocol it is dealing with. This is
the key to enabling NVMe MPTCP support without adding MPTCP-specific
exports or protocol dispatch logic in nvme-tcp. A more detailed
discussion can be found in [3].

I would appreciate any further suggestions you may have.

Thanks,
-Geliang

[1]
https://patchwork.kernel.org/project/mptcp/patch/c17925172fd53c98f0ec5c0095c86e6e28cbebf4.1786938892.git.tanggeliang@kylinos.cn/
[2]
https://patchwork.kernel.org/project/linux-nvme/cover/cover.1779934709.git.tanggeliang@kylinos.cn/
[3]
https://patchwork.kernel.org/project/linux-nvme/patch/a1fa5dd3ebb0cafe1bf7948bf2a96436e9452e19.1779934709.git.tanggeliang@kylinos.cn/