Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
MIME-Version: 1.0
References: <20231120220549.cvsz2ni3wj7mcukh@skbuf> <20231121183114.727fb6d7@kmaincent-XPS-13-7390>
 <20231121094354.635ee8cd@kernel.org> <20231122144453.5eb0382f@kmaincent-XPS-13-7390>
 <20231122140850.li2mvf6tpo3f2fhh@skbuf> <20231122143618.cqyb45po7bon2xzg@skbuf>
 <20231122085459.1601141e@kernel.org> <20231122165955.tujcadked5bgqjet@skbuf>
 <20231122095525.1438eaa3@kernel.org> <CA+FuTSe+SOFciGf+d+e=Co22yZ56gGGkJ0WBbvfT-2P0+Ug8DQ@mail.gmail.com>
 <20231124172754.tneftor7uobrul5f@skbuf>
In-Reply-To: <20231124172754.tneftor7uobrul5f@skbuf>
From:   Willem de Bruijn <willemb@google.com>
Date:   Fri, 24 Nov 2023 14:45:46 -0500
Message-ID: <CA+FuTSc7aEokNcY2sJeDZQe3_iV9ARC_1hov=9-wTzdMAkcayA@mail.gmail.com>
Subject: Re: [PATCH net-next v7 15/16] net: ethtool: ts: Let the active time
 stamping layer be selectable
To:     Vladimir Oltean <vladimir.oltean@nxp.com>
Cc:     Jakub Kicinski <kuba@kernel.org>,
        =?UTF-8?Q?K=C3=B6ry_Maincent?= <kory.maincent@bootlin.com>,
        Florian Fainelli <florian.fainelli@broadcom.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Richard Cochran <richardcochran@gmail.com>,
        Radu Pirea <radu-nicolae.pirea@oss.nxp.com>,
        Jay Vosburgh <j.vosburgh@gmail.com>,
        Andy Gospodarek <andy@greyhouse.net>,
        Nicolas Ferre <nicolas.ferre@microchip.com>,
        Claudiu Beznea <claudiu.beznea@tuxon.dev>,
        Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Horatiu Vultur <horatiu.vultur@microchip.com>,
        UNGLinuxDriver@microchip.com, Simon Horman <horms@kernel.org>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org,
        Maxime Chevallier <maxime.chevallier@bootlin.com>,
        Mahesh Bandewar <maheshb@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Nov 24, 2023 at 12:28=E2=80=AFPM Vladimir Oltean
<vladimir.oltean@nxp.com> wrote:
>
> Hi Willem,
>
> On Wed, Nov 22, 2023 at 01:11:02PM -0500, Willem de Bruijn wrote:
> > There already is a disconnect between configuring hardware timestamp
> > generation. Through the ioctl, which is a global admin-only interface.
> > And requesting timestamps with SO_TIMESTAMPING.
> >
> > Today the user of ptp4l already has to know that the admin has
> > configured the right RX and TX filters. That is no different if
> > multiple filters can be installed? (PHY for PTP, DMA for everything
> > else).
>
> Are you saying that ptp4l doesn't configure the RX and TX filters by
> itself, just the admin had to do that? Because it does.
> https://github.com/richardcochran/linuxptp/blob/master/sk.c#L59
>
> I'm not seeing the disconnect. SO_TIMESTAMPING is for the socket,
> SIOCSHWTSTAMP is for the configuration at the device level.
>
> It _is_ different if multiple filters can be installed, because either
> we let things be (and ptp4l issues the same ioctl which affects the
> default hwtstamp provider, which may or may not coincide with what we
> intend), or we teach ptp4l to deal with the multitude of providers that
> a port may have.

I see. By disconnect, I meant that the socket option is unprivileged and
can be set by many processes, while the ioctl is a global privileged
setting, so must be under control of a single admin.

But I did not know that ptp4l can take on both those roles for PTP.

Perhaps multiple SIOCSHWTSTAMP rules can coexist, up to
one per level:

HWTSTAMP_FILTER_PTP_V2_EVENT, level=3DPHY
HWTSTAMP_FILTER_ALL, level, level=3DDMA

Then ptp4l can manage all levels except the DMA level. And DMA
timestamps can be configured independently by another admin.

If only one timestamp can be communicated to the host, the earliest
match must takes precedence. Jakub pointed out how one device
handles this by having a separate queue for PHY  timestamped
packets.

This does not address the issue that packets with different
precision skb_shinfo(skb)->hwtstamps->hwtstamp may now exist
in the system. All packets reaching ptp4l sockets must have a high
resolution source, but there is no explicit annotation to ensure or
check this. This is fully based on trusting the HWSTAMP_FILTER.
Expanding the skb infra and cmsg might be follow-on work.