On Tue, Nov 04, 2025 at 09:51:09AM +0100, Alexandra Winter wrote:
On 04.11.25 08:08, D. Wythe wrote:
quoted
On Mon, Nov 03, 2025 at 09:28:22AM +0100, Alexandra Winter wrote:
quoted
On 31.10.25 04:18, D. Wythe wrote:
quoted
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.
'ini' is allocated in __smc_connect() and in smc_listen_work().
So it seems to me the purpose of 'ini' is to store information about the
current connection, not device's inherent hardware capabilities.
Fields like ini->smc_type_v1/v2 and ini->smcd/r_version are adjusted in
multiple places during the handshake.
I must say that the usage of these fields is confusing and looks somehow
redundant to me.
But looking at pclc_base->hdr.typev1/v2, as yet another source of
information doesn't make things cleaner IMO.
That’s definitely a reasonable way to look at it as well. If the community
prefers this interpretation as more natural, I’m fully open to it.
I’d like to do some testing first, as I have concerns about
possible side effects from directly modifying ini and if nothing
problematic shows up, I’ll send the updated version with this change.
Best wishes,
D. Wythe