[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJwJo6bnYm9cuJ=-xXw4n5dscq+CG4mpbtOc_vLMFmWKD2GD1g@mail.gmail.com>
Date: Thu, 7 Nov 2024 17:35:53 +0000
From: Dmitry Safonov <0x7f454c46@...il.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: devnull+0x7f454c46.gmail.com@...nel.org, borisp@...dia.com,
colona@...sta.com, davem@...emloft.net, dsahern@...nel.org,
edumazet@...gle.com, geliang@...nel.org, horms@...nel.org,
john.fastabend@...il.com, kuba@...nel.org, linux-kernel@...r.kernel.org,
martineau@...nel.org, matttbe@...nel.org, mptcp@...ts.linux.dev,
netdev@...r.kernel.org, pabeni@...hat.com
Subject: Re: [PATCH net 4/6] net/diag: Always pre-allocate tcp_ulp info
Hi Kuniyuki,
thanks for your review,
On Thu, 7 Nov 2024 at 00:21, Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
>
> From: Dmitry Safonov via B4 Relay <devnull+0x7f454c46.gmail.com@...nel.org>
> Date: Wed, 06 Nov 2024 18:10:17 +0000
> > From: Dmitry Safonov <0x7f454c46@...il.com>
> >
> > Currently there is a theoretical race between netlink one-socket dump
> > and allocating icsk->icsk_ulp_ops.
> >
> > Simplify the expectations by always allocating maximum tcp_ulp-info.
> > With the previous patch the typical netlink message allocation was
> > decreased for kernel replies on requests without idiag_ext flags,
> > so let's use it.
> >
>
> I think Fixes tag is needed.
Yeah, probably, wasn't sure if it's -stable material as I didn't
attempt to create a reproducer for this.
[..]
> > @@ -97,6 +97,53 @@ void inet_diag_msg_common_fill(struct inet_diag_msg *r, struct sock *sk)
> > }
> > EXPORT_SYMBOL_GPL(inet_diag_msg_common_fill);
> >
> > +static size_t tls_get_info_size(void)
> > +{
> > + size_t size = 0;
> > +
> > +#ifdef CONFIG_TLS
> > + size += nla_total_size(0) + /* INET_ULP_INFO_TLS */
>
> It seems '\t' after '+' was converted to '\s' by copy-and-paste.
Thanks, will correct
[..]
> > +static size_t subflow_get_info_size(void)
> > +{
> > + size_t size = 0;
> > +
> > +#ifdef CONFIG_MPTCP
> > + size += nla_total_size(0) + /* INET_ULP_INFO_MPTCP */
> > + nla_total_size(4) + /* MPTCP_SUBFLOW_ATTR_TOKEN_REM */
> > + nla_total_size(4) + /* MPTCP_SUBFLOW_ATTR_TOKEN_LOC */
> > + nla_total_size(4) + /* MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ */
> > + nla_total_size_64bit(8) + /* MPTCP_SUBFLOW_ATTR_MAP_SEQ */
>
> While at it, let's adjust tabs to match with MPTCP_SUBFLOW_ATTR_MAP_SEQ.
Sure
[..]
> > +static size_t tcp_ulp_ops_size(void)
> > +{
> > + size_t size = max(tls_get_info_size(), subflow_get_info_size());
> > +
> > + return size + nla_total_size(0) + nla_total_size(TCP_ULP_NAME_MAX);
>
> Is nla_total_size(0) for INET_DIAG_ULP_INFO ?
>
> It would be better to break them down in the same format with comment
> like tls_get_info_size() and subflow_get_info_size().
Good idea! Will do in v2.
[..]
Thanks,
Dmitry
Powered by blists - more mailing lists