linux-kernel - Re: [RFC Patch net-next v2 7/8] net: dsa: microchip: add the transmission tstamp logic

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:   Wed, 23 Nov 2022 08:49:49 +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 7/8] net: dsa: microchip: add the
 transmission tstamp logic

Hi Vladimir,
Thanks for the detailed explanation of race condition.

On Tue, 2022-11-22 at 00:51 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> Hi Arun,
> 
> On Mon, Nov 21, 2022 at 09:11:49PM +0530, Arun Ramadoss wrote:
> > This patch adds the routines for transmission of ptp packets. When
> > the
> > ptp packets(sync, pdelay_req, pdelay_rsp) to be transmitted, the
> > skb is
> > copied to global skb through port_txtstamp ioctl.
> > After the packet is transmitted, ISR is triggered. The time at
> > which
> > packet transmitted is recorded to separate register available for
> > each
> > message. This value is reconstructed to absolute time and posted to
> > the
> > user application through skb complete.
> 
> "skb complete" is not a thing. "socket error queue" is.

I will update as socket error queue.

> 
> > 
> > 
> diff --git a/drivers/net/dsa/microchip/ksz9477_ptp.c
> b/drivers/net/dsa/microchip/ksz9477_ptp.c
> index c646689cb71e..bc3f0283859a 100644
> --- a/drivers/net/dsa/microchip/ksz9477_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz9477_ptp.c
> @@ -1106,6 +1106,11 @@ irqreturn_t ksz9477_ptp_port_interrupt(struct
> ksz_device *dev, int port)
>         if (ret)
>                 return IRQ_NONE;
> 
> +       /* Clear interrupt(s) (W1C) */
> +       ret = ksz_write16(dev, addr, data);
> +       if (ret)
> +               return IRQ_NONE;
> +
>         if (data & PTP_PORT_XDELAY_REQ_INT) {
>                 /* Timestamp for Pdelay_Req / Delay_Req */
>                 struct ksz_device_ptp_shared *ptp_shared = &dev-
> >ptp_shared;
> @@ -1128,11 +1133,6 @@ irqreturn_t ksz9477_ptp_port_interrupt(struct
> ksz_device *dev, int port)
>                 complete(&prt->tstamp_completion);
>         }
> 
> -       /* Clear interrupt(s) (W1C) */
> -       ret = ksz_write16(dev, addr, data);
> -       if (ret)
> -               return IRQ_NONE;
> -
>         return IRQ_HANDLED;
>  }
> 
> 
> About the only difference seems to be that ACK-ing the interrupt is
> done
> at the end of ksz_ptp_irq_thread_fn(), while complete(&port-
> >tstamp_msg_comp)
> is called from ksz_ptp_msg_thread_fn() - which is called by
> handle_nested_irq()
> IIUC.

I will clear the interrupts before calling the handle_nested_irq().

> 
> > 
> > +/*