Re: [PATCH net] net/smc: fix mismatch between CLC header and proposal extensions

["D. Wythe" <alibuda@linux.alibaba.com >]

On Mon, Nov 03, 2025 at 09:28:22AM +0100, Alexandra Winter wrote:
> 
> 
> On 31.10.25 04:18, D. Wythe wrote:
> > The current CLC proposal message construction uses a mix of
> > `ini->smc_type_v1/v2` and `pclc_base->hdr.typev1/v2` to decide whether
> > to include optional extensions (IPv6 prefix extension for v1, and v2
> > extension). This leads to a critical inconsistency: when
> > `smc_clc_prfx_set()` fails - for example, in IPv6-only environments with
> > only link-local addresses, or when the local IP address and the outgoing
> > interface’s network address are not in the same subnet.
> > 
> > As a result, the proposal message is assembled using the stale
> > `ini->smc_type_v1` value—causing the IPv6 prefix extension to be
> > included even though the header indicates v1 is not supported.
> > The peer then receives a malformed CLC proposal where the header type
> > does not match the payload, and immediately resets the connection.
> > 
> > Fix this by consistently using `pclc_base->hdr.typev1` and
> > `pclc_base->hdr.typev2`—the authoritative fields that reflect the
> > actual capabilities advertised in the CLC header—when deciding whether
> > to include optional extensions, as required by the SMC-R v2
> > specification ("V1 IP Subnet Extension and V2 Extension only present if
> > applicable").
> 
> 
> Just thinking out loud:
> It seems to me that the 'ini' structure exists once per socket and is used
> to pass information between many functions involved with the handshake.
> Did you consider updating ini->smc_type_v1/v2 when `smc_clc_prfx_set()` fails,
> and using ini as the authoritative source?
> With your patch, it seems to me `ini->smc_type_v1` still contains a stale value,
> which may lead to issues in other places or future code.

Based on my understanding, ini->smc_type_v1/v2 represents the local
device's inherent hardware capabilities. This value is a static property
and, from my perspective, should remain immutable, independent of
transient network conditions such as invalid IPv6 prefixes or GID
mismatches. Therefore, I believe modifying this field within
smc_clc_send_proposal() might not be the most appropriate approach.

In contrast, pclc_base->hdr.typev1/v2 reflects the actual capabilities
negotiated for a specific connection—what we might term "soft
capabilities." These can, and often do, dynamically adjust based on
current network conditions (e.g., in the event of a prefix validation
failure) and could potentially be restored if network conditions
improve.

Furthermore, once CLC negotiation is complete, the SMC protocol stack
relies exclusively on these negotiated results for all subsequent
operations. It no longer refers to the initial capability values stored
in ini. Consequently, maintaining ini->smc_type_v1/v2 in its original,
unaltered state appears to present no practical risks or functional
issues.