From: Willem de Bruijn <willemdebruijn.kernel@gmail•com>
To: Jakub Kicinski <kuba@kernel•org>, davem@davemloft•net
Cc: netdev@vger•kernel.org, edumazet@google•com, pabeni@redhat•com,
andrew+netdev@lunn•ch, horms@kernel•org,
Jakub Kicinski <kuba@kernel•org>,
richardcochran@gmail•com, yangbo.lu@nxp•com,
kuniyu@google•com, willemb@google•com, matttbe@kernel•org
Subject: Re: [PATCH net] ethtool: vclock: propagate errors from ptp_get_vclocks_index()
Date: Tue, 26 May 2026 14:46:20 -0400 [thread overview]
Message-ID: <willemdebruijn.kernel.178a41a53d041@gmail.com> (raw)
In-Reply-To: <20260526153650.2779821-1-kuba@kernel.org>
Jakub Kicinski wrote:
> ptp_get_vclocks_index() was written with sock_timestamping_bind_phc()
> in mind. Any return <= 0 will get converted to EINVAL by the virtue
> of sock_timestamping_bind_phc() not finding a matching vclock.
>
> ethtool's phc_vclocks_prepare_data(), however, should not be silently
> returning empty messages on allocation errors or when PHC does not
> exist at all.
>
> Commit c156174a6707 ("ethtool: add a new command for getting PHC virtual
> clocks") added phc_vclocks_prepare_data(), but let's not treat this
> as a fix in case some user space app now depends on the failures being
> silent.
>
> The check in sock_timestamping_bind_phc() is not strickly necessary,
strictly
> but include it just for clarity.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel•org>
Reviewed-by: Willem de Bruijn <willemb@google•com>
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index b37b664b6eb9..9e33e979a283 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -870,6 +870,8 @@ static int sock_timestamping_bind_phc(struct sock *sk, int phc_index)
>
> num = ethtool_get_phc_vclocks(dev, &vclock_index);
> dev_put(dev);
> + if (num < 0)
> + return num;
>
> for (i = 0; i < num; i++) {
> if (*(vclock_index + i) == phc_index) {
Not for this patch:
This is called by sk_setsockopt and by MPTCP for every subflow.
Apparently MPTCP does not check the return value. Should it?
next prev parent reply other threads:[~2026-05-26 18:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-26 15:36 [PATCH net] ethtool: vclock: propagate errors from ptp_get_vclocks_index() Jakub Kicinski
2026-05-26 18:46 ` Willem de Bruijn [this message]
2026-05-26 23:34 ` Matthieu Baerts
2026-05-28 0:44 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=willemdebruijn.kernel.178a41a53d041@gmail.com \
--to=willemdebruijn.kernel@gmail$(echo .)com \
--cc=andrew+netdev@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=horms@kernel$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=kuniyu@google$(echo .)com \
--cc=matttbe@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=richardcochran@gmail$(echo .)com \
--cc=willemb@google$(echo .)com \
--cc=yangbo.lu@nxp$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox