netdev - Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 25 Nov 2022 07:06:07 +0000
From:   <Arun.Ramadoss@...rochip.com>
To:     <olteanv@...il.com>
CC:     <andrew@...n.ch>, <linux-kernel@...r.kernel.org>,
        <UNGLinuxDriver@...rochip.com>, <vivien.didelot@...il.com>,
        <linux@...linux.org.uk>, <Tristram.Ha@...rochip.com>,
        <f.fainelli@...il.com>, <kuba@...nel.org>, <edumazet@...gle.com>,
        <pabeni@...hat.com>, <richardcochran@...il.com>,
        <netdev@...r.kernel.org>, <Woojung.Huh@...rochip.com>,
        <davem@...emloft.net>
Subject: Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware
 time stamping support

Hi Vladimir,

On Thu, 2022-11-24 at 16:14 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, Nov 24, 2022 at 10:52:46AM +0000, Arun.Ramadoss@...rochip.com
>  wrote:
> > Mistake here. It is carried forwarded from Christian Eggers patch.
> 
> Still taken from sja1105_hwtstamp_set(). Anyway, doesn't matter where
> it's taken from, as long as it has a justification for being there.
> 
> > > Why do you need to call hwtstamp_set_state anyway?
> > 
> > In tag_ksz.c, xmit function query this state, to determine whether
> > to
> > allocate the 4 PTP timestamp bytes in the skb_buffer or not. Using
> > this
> > tagger_data set state, ptp enable and disable is communicated
> > between
> > ksz_ptp.c and tag_ksz.c
> 
> Why do you need to query this state in particular, considering that
> the
> skb goes first through the port_txtstamp() dsa_switch_ops function?
> Can't you just check there if TX timestamping is enabled, and leave a
> mark in KSZ_SKB_CB()?
KSZ switches need a additional 4 bytes in tail tag if the PTP is
enabled in hardware. If the PTP is enabled and if we didn't add 4
additional bytes in the tail tag then packets are corrupted.

Tristram explained this in the patch conversation

https://lore.kernel.org/netdev/20201118203013.5077-1-ceggers@arri.de/T/#mb3eba4918bda351a405168e7a2140d29262f4c63

I did the follwing experiment today, 
* Removed the ptp time stamp check in tag_ksz.c. In the ksz_xmit
function, 4 additional bytes are added only if KSZ_SKB_CB->ts_en bit is
set.
* Setup the board, ping two boards. Ping is successful.
* Run the ptpl in the background
* Now if I run the ping, ping is not successful. And also in the ptp4l
log message it shows as bad message received.

We need a mechanism to inform tag_ksz.c to add 4 additional bytes in
tail_tag for all the packets if the ptp is enabled in the hardware.