Propagate IP_RECVERR/IP_RECVERR_RFC4884 and
IPV6_RECVERR/IPV6_RECVERR_RFC4884 from the MPTCP socket to
existing and future subflows.
Apply the matching sockopt according to the subflow family so mixed-
family subflows stay aligned with the parent socket configuration,
including disable-time errqueue purge semantics.
Signed-off-by: David Carlier <devnexen@gmail.com>
Assisted-by: Codex:gpt-5
Signed-off-by: David Carlier <devnexen@gmail.com>
---
net/mptcp/sockopt.c | 129 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 79db15903e7a..acb0ca330e44 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -8,6 +8,8 @@
#include <linux/kernel.h>
#include <linux/module.h>
+#include <net/ip.h>
+#include <net/ipv6.h>
#include <net/sock.h>
#include <net/protocol.h>
#include <net/tcp.h>
@@ -384,6 +386,72 @@ static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int optname,
return -EOPNOTSUPP;
}
+static bool mptcp_recverr_enabled(const struct sock *sk, bool rfc4884)
+{
+ bool enabled;
+
+ enabled = rfc4884 ? inet_test_bit(RECVERR_RFC4884, sk) :
+ inet_test_bit(RECVERR, sk);
+
+#if IS_ENABLED(CONFIG_IPV6)
+ if (sk->sk_family == AF_INET6)
+ enabled |= rfc4884 ? inet6_test_bit(RECVERR6_RFC4884, sk) :
+ inet6_test_bit(RECVERR6, sk);
+#endif
+
+ return enabled;
+}
+
+static int mptcp_subflow_set_recverr(struct sock *sk, struct sock *ssk,
+ bool rfc4884)
+{
+ int level, optname, val;
+
+#if IS_ENABLED(CONFIG_IPV6)
+ if (ssk->sk_family == AF_INET6) {
+ level = SOL_IPV6;
+ optname = rfc4884 ? IPV6_RECVERR_RFC4884 : IPV6_RECVERR;
+ } else
+#endif
+ {
+ level = SOL_IP;
+ optname = rfc4884 ? IP_RECVERR_RFC4884 : IP_RECVERR;
+ }
+
+ val = mptcp_recverr_enabled(sk, rfc4884);
+ return tcp_setsockopt(ssk, level, optname, KERNEL_SOCKPTR(&val),
+ sizeof(val));
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static int mptcp_setsockopt_v6_recverr(struct mptcp_sock *msk, int optname,
+ sockptr_t optval, unsigned int optlen)
+{
+ struct mptcp_subflow_context *subflow;
+ struct sock *sk = (struct sock *)msk;
+ int ret;
+
+ ret = ipv6_setsockopt(sk, SOL_IPV6, optname, optval, optlen);
+ if (ret)
+ return ret;
+
+ lock_sock(sk);
+ sockopt_seq_inc(msk);
+ mptcp_for_each_subflow(msk, subflow) {
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+ bool rfc4884 = optname == IPV6_RECVERR_RFC4884;
+
+ ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
+ if (ret)
+ break;
+ subflow->setsockopt_seq = msk->setsockopt_seq;
+ }
+ release_sock(sk);
+
+ return ret;
+}
+#endif
+
static int mptcp_setsockopt_v6(struct mptcp_sock *msk, int optname,
sockptr_t optval, unsigned int optlen)
{
@@ -426,6 +494,12 @@ static int mptcp_setsockopt_v6(struct mptcp_sock *msk, int optname,
release_sock(sk);
break;
+#if IS_ENABLED(CONFIG_IPV6)
+ case IPV6_RECVERR:
+ case IPV6_RECVERR_RFC4884:
+ ret = mptcp_setsockopt_v6_recverr(msk, optname, optval, optlen);
+ break;
+#endif
}
return ret;
@@ -760,6 +834,33 @@ static int mptcp_setsockopt_v4_set_tos(struct mptcp_sock *msk, int optname,
return 0;
}
+static int mptcp_setsockopt_v4_recverr(struct mptcp_sock *msk, int optname,
+ sockptr_t optval, unsigned int optlen)
+{
+ struct mptcp_subflow_context *subflow;
+ struct sock *sk = (struct sock *)msk;
+ int err;
+
+ err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);
+ if (err)
+ return err;
+
+ lock_sock(sk);
+ sockopt_seq_inc(msk);
+ mptcp_for_each_subflow(msk, subflow) {
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+ bool rfc4884 = optname == IP_RECVERR_RFC4884;
+
+ err = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
+ if (err)
+ break;
+ subflow->setsockopt_seq = msk->setsockopt_seq;
+ }
+ release_sock(sk);
+
+ return err;
+}
+
static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname,
sockptr_t optval, unsigned int optlen)
{
@@ -771,6 +872,9 @@ static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname,
return mptcp_setsockopt_sol_ip_set(msk, optname, optval, optlen);
case IP_TOS:
return mptcp_setsockopt_v4_set_tos(msk, optname, optval, optlen);
+ case IP_RECVERR:
+ case IP_RECVERR_RFC4884:
+ return mptcp_setsockopt_v4_recverr(msk, optname, optval, optlen);
}
return -EOPNOTSUPP;
@@ -1459,6 +1563,12 @@ static int mptcp_getsockopt_v4(struct mptcp_sock *msk, int optname,
case IP_LOCAL_PORT_RANGE:
return mptcp_put_int_option(msk, optval, optlen,
READ_ONCE(inet_sk(sk)->local_port_range));
+ case IP_RECVERR:
+ return mptcp_put_int_option(msk, optval, optlen,
+ inet_test_bit(RECVERR, sk));
+ case IP_RECVERR_RFC4884:
+ return mptcp_put_int_option(msk, optval, optlen,
+ inet_test_bit(RECVERR_RFC4884, sk));
}
return -EOPNOTSUPP;
@@ -1479,6 +1589,12 @@ static int mptcp_getsockopt_v6(struct mptcp_sock *msk, int optname,
case IPV6_FREEBIND:
return mptcp_put_int_option(msk, optval, optlen,
inet_test_bit(FREEBIND, sk));
+ case IPV6_RECVERR:
+ return mptcp_put_int_option(msk, optval, optlen,
+ inet6_test_bit(RECVERR6, sk));
+ case IPV6_RECVERR_RFC4884:
+ return mptcp_put_int_option(msk, optval, optlen,
+ inet6_test_bit(RECVERR6_RFC4884, sk));
}
return -EOPNOTSUPP;
@@ -1536,6 +1652,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
{
static const unsigned int tx_rx_locks = SOCK_RCVBUF_LOCK | SOCK_SNDBUF_LOCK;
struct sock *sk = (struct sock *)msk;
+ bool recverr, recverr_rfc4884;
bool keep_open;
keep_open = sock_flag(sk, SOCK_KEEPOPEN);
@@ -1586,6 +1703,18 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
inet_assign_bit(FREEBIND, ssk, inet_test_bit(FREEBIND, sk));
inet_assign_bit(BIND_ADDRESS_NO_PORT, ssk, inet_test_bit(BIND_ADDRESS_NO_PORT, sk));
WRITE_ONCE(inet_sk(ssk)->local_port_range, READ_ONCE(inet_sk(sk)->local_port_range));
+ recverr = mptcp_recverr_enabled(sk, false);
+ recverr_rfc4884 = mptcp_recverr_enabled(sk, true);
+#if IS_ENABLED(CONFIG_IPV6)
+ if (ssk->sk_family == AF_INET6) {
+ inet6_assign_bit(RECVERR6, ssk, recverr);
+ inet6_assign_bit(RECVERR6_RFC4884, ssk, recverr_rfc4884);
+ } else
+#endif
+ {
+ inet_assign_bit(RECVERR, ssk, recverr);
+ inet_assign_bit(RECVERR_RFC4884, ssk, recverr_rfc4884);
+ }
}
void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock *ssk)
--
2.53.0On 4/22/26 12:33 AM, David Carlier wrote:
> Propagate IP_RECVERR/IP_RECVERR_RFC4884 and
> IPV6_RECVERR/IPV6_RECVERR_RFC4884 from the MPTCP socket to
> existing and future subflows.
>
> Apply the matching sockopt according to the subflow family so mixed-
> family subflows stay aligned with the parent socket configuration,
> including disable-time errqueue purge semantics.
>
> Signed-off-by: David Carlier <devnexen@gmail.com>
You should drop this line, only a single SoB tag is needed.
[...]
> +#if IS_ENABLED(CONFIG_IPV6)
Is this compiler guard strictly needed?
> +static int mptcp_setsockopt_v6_recverr(struct mptcp_sock *msk, int optname,
> + sockptr_t optval, unsigned int optlen)
> +{
> + struct mptcp_subflow_context *subflow;
> + struct sock *sk = (struct sock *)msk;
> + int ret;
> +
> + ret = ipv6_setsockopt(sk, SOL_IPV6, optname, optval, optlen);
> + if (ret)
> + return ret;
> +
> + lock_sock(sk);
> + sockopt_seq_inc(msk);
> + mptcp_for_each_subflow(msk, subflow) {
> + struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> + bool rfc4884 = optname == IPV6_RECVERR_RFC4884;
> +
> + ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
The above looks a bit overcomplicated?!? It looks like you could
leverage mptcp_setsockopt_all_sf() in and drop the
mptcp_subflow_set_recverr() and mptcp_recverr_enabled() helpers.
@Mattbe: it's not clear to me why mptcp_setsockopt_all_sf() does not
call sockopt_seq_inc(msk) or sync the setsockopt_seq, possibly an
unrelated bug?
[...]
> @@ -760,6 +834,33 @@ static int mptcp_setsockopt_v4_set_tos(struct mptcp_sock *msk, int optname,
> return 0;
> }
>
> +static int mptcp_setsockopt_v4_recverr(struct mptcp_sock *msk, int optname,
> + sockptr_t optval, unsigned int optlen)
> +{
> + struct mptcp_subflow_context *subflow;
> + struct sock *sk = (struct sock *)msk;
> + int err;
> +
> + err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);
> + if (err)
> + return err;
> +
> + lock_sock(sk);
> + sockopt_seq_inc(msk);
> + mptcp_for_each_subflow(msk, subflow) {
> + struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> + bool rfc4884 = optname == IP_RECVERR_RFC4884;
> +
> + err = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
> + if (err)
> + break;
Same note here WRT possible mptcp_setsockopt_all_sf() usage.
> + subflow->setsockopt_seq = msk->setsockopt_seq;
> + }
> + release_sock(sk);
> +
> + return err;
> +}
> +
> static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname,
> sockptr_t optval, unsigned int optlen)
> {
> @@ -771,6 +872,9 @@ static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname,
> return mptcp_setsockopt_sol_ip_set(msk, optname, optval, optlen);
> case IP_TOS:
> return mptcp_setsockopt_v4_set_tos(msk, optname, optval, optlen);
> + case IP_RECVERR:
> + case IP_RECVERR_RFC4884:
> + return mptcp_setsockopt_v4_recverr(msk, optname, optval, optlen);
> }
>
> return -EOPNOTSUPP;
> @@ -1459,6 +1563,12 @@ static int mptcp_getsockopt_v4(struct mptcp_sock *msk, int optname,
> case IP_LOCAL_PORT_RANGE:
> return mptcp_put_int_option(msk, optval, optlen,
> READ_ONCE(inet_sk(sk)->local_port_range));
> + case IP_RECVERR:
> + return mptcp_put_int_option(msk, optval, optlen,
> + inet_test_bit(RECVERR, sk));
> + case IP_RECVERR_RFC4884:
> + return mptcp_put_int_option(msk, optval, optlen,
> + inet_test_bit(RECVERR_RFC4884, sk));
> }
>
> return -EOPNOTSUPP;
> @@ -1479,6 +1589,12 @@ static int mptcp_getsockopt_v6(struct mptcp_sock *msk, int optname,
> case IPV6_FREEBIND:
> return mptcp_put_int_option(msk, optval, optlen,
> inet_test_bit(FREEBIND, sk));
> + case IPV6_RECVERR:
> + return mptcp_put_int_option(msk, optval, optlen,
> + inet6_test_bit(RECVERR6, sk));
> + case IPV6_RECVERR_RFC4884:
> + return mptcp_put_int_option(msk, optval, optlen,
> + inet6_test_bit(RECVERR6_RFC4884, sk));
> }
>
> return -EOPNOTSUPP;
> @@ -1536,6 +1652,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
> {
> static const unsigned int tx_rx_locks = SOCK_RCVBUF_LOCK | SOCK_SNDBUF_LOCK;
> struct sock *sk = (struct sock *)msk;
> + bool recverr, recverr_rfc4884;
> bool keep_open;
>
> keep_open = sock_flag(sk, SOCK_KEEPOPEN);
> @@ -1586,6 +1703,18 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
> inet_assign_bit(FREEBIND, ssk, inet_test_bit(FREEBIND, sk));
> inet_assign_bit(BIND_ADDRESS_NO_PORT, ssk, inet_test_bit(BIND_ADDRESS_NO_PORT, sk));
> WRITE_ONCE(inet_sk(ssk)->local_port_range, READ_ONCE(inet_sk(sk)->local_port_range));
> + recverr = mptcp_recverr_enabled(sk, false);
> + recverr_rfc4884 = mptcp_recverr_enabled(sk, true);
> +#if IS_ENABLED(CONFIG_IPV6)
> + if (ssk->sk_family == AF_INET6) {
> + inet6_assign_bit(RECVERR6, ssk, recverr);
> + inet6_assign_bit(RECVERR6_RFC4884, ssk, recverr_rfc4884);
> + } else
> +#endif
> + {
> + inet_assign_bit(RECVERR, ssk, recverr);
> + inet_assign_bit(RECVERR_RFC4884, ssk, recverr_rfc4884);
> + }
Instead of the above you could add a pre req patch converting the
existing inet_assign_bit() to something alike:
#define MPTCP_INET_FLAGS_MASK (INET_FLAGS_TRANSPARENT |
INET_FLAGS_FREEBIND /* ... */ )
inet_sk(ssk)->inet_flags = inet_sk(sk)->inet_flags & MPTCP_INET_FLAGS_MASK;
and just expand the MPTCP_INET_FLAGS_MASK.
/P
On 4/22/26 10:05 AM, Paolo Abeni wrote: > You should drop this line, only a single SoB tag is needed. Ack. >> +#if IS_ENABLED(CONFIG_IPV6) > > Is this compiler guard strictly needed? For this one yes — it wraps mptcp_setsockopt_v6_recverr(), which calls ipv6_setsockopt() (no !CONFIG_IPV6 stub, link error otherwise). But the helper goes away once I move to mptcp_setsockopt_all_sf() + MPTCP_INET_FLAGS_MASK as you suggest below, so the guard drops with it. >> + ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884); > > The above looks a bit overcomplicated?!? It looks like you could > leverage mptcp_setsockopt_all_sf() [...] Will do, rebased on Matt's sockopt_seq_inc() fix. > Instead of the above you could add a pre req patch converting the > existing inet_assign_bit() to something alike [...] Ack, will add as prereq (and mirror for inet6_sk()->flags). Cheers,
Hi David, On 22/04/2026 23:51, David CARLIER wrote: > On 4/22/26 10:05 AM, Paolo Abeni wrote: > > You should drop this line, only a single SoB tag is needed. > > Ack. (I don't know why there is an indentation in your replies, but I guess most email readers will not interpret this properly, even lore [1]: check your messages vs. the other ones) https://lore.kernel.org/mptcp/CA+XhMqw4xRGdJbtX02++9eS8u_bWOzbPimM1Mk_dDNed5SX94w@mail.gmail.com/T/#mf03244a0e5af6ca0fdd80dfb1bff9f2f2d769e0b https://docs.kernel.org/process/email-clients.html > >> + ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884); > > > > The above looks a bit overcomplicated?!? It looks like you could > > leverage mptcp_setsockopt_all_sf() [...] > > Will do, rebased on Matt's sockopt_seq_inc() fix. FYI, I sent my patch there: https://lore.kernel.org/mptcp/20260424-mptcp-pm-sockopt-set-all-sf-v1-1-38e7023822f8@kernel.org/ I don't think you need to rebase your series on it, except if not having this patch breaks some tests. If that's the case, feel free to add this line in the cover-letter: Based-on: <20260424-mptcp-pm-sockopt-set-all-sf-v1-1-38e7023822f8@kernel.org> See this for more details: https://github.com/multipath-tcp/mptcp_net-next/wiki/CI Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Paolo,
Thank you for the review!
On 22/04/2026 10:05, Paolo Abeni wrote:
> On 4/22/26 12:33 AM, David Carlier wrote:
>> Propagate IP_RECVERR/IP_RECVERR_RFC4884 and
>> IPV6_RECVERR/IPV6_RECVERR_RFC4884 from the MPTCP socket to
>> existing and future subflows.
>>
>> Apply the matching sockopt according to the subflow family so mixed-
>> family subflows stay aligned with the parent socket configuration,
>> including disable-time errqueue purge semantics.
(...)
>> +static int mptcp_setsockopt_v6_recverr(struct mptcp_sock *msk, int optname,
>> + sockptr_t optval, unsigned int optlen)
>> +{
>> + struct mptcp_subflow_context *subflow;
>> + struct sock *sk = (struct sock *)msk;
>> + int ret;
>> +
>> + ret = ipv6_setsockopt(sk, SOL_IPV6, optname, optval, optlen);
>> + if (ret)
>> + return ret;
>> +
>> + lock_sock(sk);
>> + sockopt_seq_inc(msk);
>> + mptcp_for_each_subflow(msk, subflow) {
>> + struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>> + bool rfc4884 = optname == IPV6_RECVERR_RFC4884;
>> +
>> + ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
>
> The above looks a bit overcomplicated?!? It looks like you could
> leverage mptcp_setsockopt_all_sf() in and drop the
> mptcp_subflow_set_recverr() and mptcp_recverr_enabled() helpers.
>
> @Mattbe: it's not clear to me why mptcp_setsockopt_all_sf() does not
> call sockopt_seq_inc(msk) or sync the setsockopt_seq, possibly an
> unrelated bug?
Oh yes, good catch! And a lock of the sk (msk) before iterating the
subflows, no? Same in __mptcp_setsockopt_set_val()?
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
On 22/04/2026 10:32, Matthieu Baerts wrote:
> Hi Paolo,
>
> Thank you for the review!
>
> On 22/04/2026 10:05, Paolo Abeni wrote:
>> On 4/22/26 12:33 AM, David Carlier wrote:
>>> Propagate IP_RECVERR/IP_RECVERR_RFC4884 and
>>> IPV6_RECVERR/IPV6_RECVERR_RFC4884 from the MPTCP socket to
>>> existing and future subflows.
>>>
>>> Apply the matching sockopt according to the subflow family so mixed-
>>> family subflows stay aligned with the parent socket configuration,
>>> including disable-time errqueue purge semantics.
>
> (...)
>
>>> +static int mptcp_setsockopt_v6_recverr(struct mptcp_sock *msk, int optname,
>>> + sockptr_t optval, unsigned int optlen)
>>> +{
>>> + struct mptcp_subflow_context *subflow;
>>> + struct sock *sk = (struct sock *)msk;
>>> + int ret;
>>> +
>>> + ret = ipv6_setsockopt(sk, SOL_IPV6, optname, optval, optlen);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + lock_sock(sk);
>>> + sockopt_seq_inc(msk);
>>> + mptcp_for_each_subflow(msk, subflow) {
>>> + struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>>> + bool rfc4884 = optname == IPV6_RECVERR_RFC4884;
>>> +
>>> + ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
>>
>> The above looks a bit overcomplicated?!? It looks like you could
>> leverage mptcp_setsockopt_all_sf() in and drop the
>> mptcp_subflow_set_recverr() and mptcp_recverr_enabled() helpers.
>>
>> @Mattbe: it's not clear to me why mptcp_setsockopt_all_sf() does not
>> call sockopt_seq_inc(msk) or sync the setsockopt_seq, possibly an
>> unrelated bug?
>
> Oh yes, good catch! And a lock of the sk (msk) before iterating the
> subflows, no? Same in __mptcp_setsockopt_set_val()?
(Not the same in __mptcp_setsockopt_set_val(): the sk is already locked)
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
On 4/22/26 10:35 AM, Matthieu Baerts wrote:
> On 22/04/2026 10:32, Matthieu Baerts wrote:
>> On 22/04/2026 10:05, Paolo Abeni wrote:
>>> On 4/22/26 12:33 AM, David Carlier wrote:
>>>> Propagate IP_RECVERR/IP_RECVERR_RFC4884 and
>>>> IPV6_RECVERR/IPV6_RECVERR_RFC4884 from the MPTCP socket to
>>>> existing and future subflows.
>>>>
>>>> Apply the matching sockopt according to the subflow family so mixed-
>>>> family subflows stay aligned with the parent socket configuration,
>>>> including disable-time errqueue purge semantics.
>>
>> (...)
>>
>>>> +static int mptcp_setsockopt_v6_recverr(struct mptcp_sock *msk, int optname,
>>>> + sockptr_t optval, unsigned int optlen)
>>>> +{
>>>> + struct mptcp_subflow_context *subflow;
>>>> + struct sock *sk = (struct sock *)msk;
>>>> + int ret;
>>>> +
>>>> + ret = ipv6_setsockopt(sk, SOL_IPV6, optname, optval, optlen);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + lock_sock(sk);
>>>> + sockopt_seq_inc(msk);
>>>> + mptcp_for_each_subflow(msk, subflow) {
>>>> + struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>>>> + bool rfc4884 = optname == IPV6_RECVERR_RFC4884;
>>>> +
>>>> + ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
>>>
>>> The above looks a bit overcomplicated?!? It looks like you could
>>> leverage mptcp_setsockopt_all_sf() in and drop the
>>> mptcp_subflow_set_recverr() and mptcp_recverr_enabled() helpers.
>>>
>>> @Mattbe: it's not clear to me why mptcp_setsockopt_all_sf() does not
>>> call sockopt_seq_inc(msk) or sync the setsockopt_seq, possibly an
>>> unrelated bug?
>>
>> Oh yes, good catch! And a lock of the sk (msk) before iterating the
>> subflows, no? Same in __mptcp_setsockopt_set_val()?
>
> (Not the same in __mptcp_setsockopt_set_val(): the sk is already locked)
AFAICS for mptcp_setsockopt_all_sf() the msk lock is acquired by the
caller. Same for __mptcp_setsockopt_set_val().
Side note; AFAICS a few other paths apparently lack the setsockopt_seq;
that is minor problem/not a real issue: worst case it will cause a
redundant mptcp_sockopt_sync_locked() call at finish_join time.
The missing sockopt_seq_inc() instead looks really bogus, as it will
caused missing synchronization for later subflows.
/P
On 22/04/2026 10:48, Paolo Abeni wrote:
> On 4/22/26 10:35 AM, Matthieu Baerts wrote:
>> On 22/04/2026 10:32, Matthieu Baerts wrote:
>>> On 22/04/2026 10:05, Paolo Abeni wrote:
>>>> On 4/22/26 12:33 AM, David Carlier wrote:
>>>>> Propagate IP_RECVERR/IP_RECVERR_RFC4884 and
>>>>> IPV6_RECVERR/IPV6_RECVERR_RFC4884 from the MPTCP socket to
>>>>> existing and future subflows.
>>>>>
>>>>> Apply the matching sockopt according to the subflow family so mixed-
>>>>> family subflows stay aligned with the parent socket configuration,
>>>>> including disable-time errqueue purge semantics.
>>>
>>> (...)
>>>
>>>>> +static int mptcp_setsockopt_v6_recverr(struct mptcp_sock *msk, int optname,
>>>>> + sockptr_t optval, unsigned int optlen)
>>>>> +{
>>>>> + struct mptcp_subflow_context *subflow;
>>>>> + struct sock *sk = (struct sock *)msk;
>>>>> + int ret;
>>>>> +
>>>>> + ret = ipv6_setsockopt(sk, SOL_IPV6, optname, optval, optlen);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + lock_sock(sk);
>>>>> + sockopt_seq_inc(msk);
>>>>> + mptcp_for_each_subflow(msk, subflow) {
>>>>> + struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>>>>> + bool rfc4884 = optname == IPV6_RECVERR_RFC4884;
>>>>> +
>>>>> + ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
>>>>
>>>> The above looks a bit overcomplicated?!? It looks like you could
>>>> leverage mptcp_setsockopt_all_sf() in and drop the
>>>> mptcp_subflow_set_recverr() and mptcp_recverr_enabled() helpers.
>>>>
>>>> @Mattbe: it's not clear to me why mptcp_setsockopt_all_sf() does not
>>>> call sockopt_seq_inc(msk) or sync the setsockopt_seq, possibly an
>>>> unrelated bug?
>>>
>>> Oh yes, good catch! And a lock of the sk (msk) before iterating the
>>> subflows, no? Same in __mptcp_setsockopt_set_val()?
>>
>> (Not the same in __mptcp_setsockopt_set_val(): the sk is already locked)
>
> AFAICS for mptcp_setsockopt_all_sf() the msk lock is acquired by the
> caller. Same for __mptcp_setsockopt_set_val().
>
> Side note; AFAICS a few other paths apparently lack the setsockopt_seq;
> that is minor problem/not a real issue: worst case it will cause a
> redundant mptcp_sockopt_sync_locked() call at finish_join time.
>
> The missing sockopt_seq_inc() instead looks really bogus, as it will
> caused missing synchronization for later subflows.
Good point! Do you want me to send a patch, or do you already have one?
(or do you plan to look at it)
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
On 4/22/26 10:50 AM, Matthieu Baerts wrote: > On 22/04/2026 10:48, Paolo Abeni wrote: >> AFAICS for mptcp_setsockopt_all_sf() the msk lock is acquired by the >> caller. Same for __mptcp_setsockopt_set_val(). >> >> Side note; AFAICS a few other paths apparently lack the setsockopt_seq; >> that is minor problem/not a real issue: worst case it will cause a >> redundant mptcp_sockopt_sync_locked() call at finish_join time. >> >> The missing sockopt_seq_inc() instead looks really bogus, as it will >> caused missing synchronization for later subflows. > > Good point! Do you want me to send a patch, or do you already have one? > (or do you plan to look at it) Please go ahead, I'm lagging behind by far, sorry. /P
On 22/04/2026 10:35, Matthieu Baerts wrote:
> On 22/04/2026 10:32, Matthieu Baerts wrote:
>> Hi Paolo,
>>
>> Thank you for the review!
>>
>> On 22/04/2026 10:05, Paolo Abeni wrote:
>>> On 4/22/26 12:33 AM, David Carlier wrote:
>>>> Propagate IP_RECVERR/IP_RECVERR_RFC4884 and
>>>> IPV6_RECVERR/IPV6_RECVERR_RFC4884 from the MPTCP socket to
>>>> existing and future subflows.
>>>>
>>>> Apply the matching sockopt according to the subflow family so mixed-
>>>> family subflows stay aligned with the parent socket configuration,
>>>> including disable-time errqueue purge semantics.
>>
>> (...)
>>
>>>> +static int mptcp_setsockopt_v6_recverr(struct mptcp_sock *msk, int optname,
>>>> + sockptr_t optval, unsigned int optlen)
>>>> +{
>>>> + struct mptcp_subflow_context *subflow;
>>>> + struct sock *sk = (struct sock *)msk;
>>>> + int ret;
>>>> +
>>>> + ret = ipv6_setsockopt(sk, SOL_IPV6, optname, optval, optlen);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + lock_sock(sk);
>>>> + sockopt_seq_inc(msk);
>>>> + mptcp_for_each_subflow(msk, subflow) {
>>>> + struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>>>> + bool rfc4884 = optname == IPV6_RECVERR_RFC4884;
>>>> +
>>>> + ret = mptcp_subflow_set_recverr(sk, ssk, rfc4884);
>>>
>>> The above looks a bit overcomplicated?!? It looks like you could
>>> leverage mptcp_setsockopt_all_sf() in and drop the
>>> mptcp_subflow_set_recverr() and mptcp_recverr_enabled() helpers.
>>>
>>> @Mattbe: it's not clear to me why mptcp_setsockopt_all_sf() does not
>>> call sockopt_seq_inc(msk) or sync the setsockopt_seq, possibly an
>>> unrelated bug?
>>
>> Oh yes, good catch! And a lock of the sk (msk) before iterating the
>> subflows, no? Same in __mptcp_setsockopt_set_val()?
>
> (Not the same in __mptcp_setsockopt_set_val(): the sk is already locked)
... same when calling mptcp_setsockopt_all_sf() for the moment. Maybe I
should have prefixed with "__".
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
© 2016 - 2026 Red Hat, Inc.