netdev - Re: [PATCH 1/5] net: fec: properly support external PTP PHY for hardware time stamping

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]

Message-ID: <87zh8cu0rs.fsf@osv.gnss.ru>
Date:   Mon, 06 Jul 2020 18:21:59 +0300
From:   Sergey Organov <sorganov@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     richardcochran@...il.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Fugang Duan <fugang.duan@....com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, andrew@...n.ch
Subject: Re: [PATCH  1/5] net: fec: properly support external PTP PHY for
 hardware time stamping

Vladimir Oltean <olteanv@...il.com> writes:

> Hi Sergey,
>
> On Mon, Jul 06, 2020 at 05:26:12PM +0300, Sergey Organov wrote:
>> When external PTP-aware PHY is in use, it's that PHY that is to time
>> stamp network packets, and it's that PHY where configuration requests
>> of time stamping features are to be routed.
>> 
>> To achieve these goals:
>> 
>> 1. Make sure we don't time stamp packets when external PTP PHY is in use
>> 
>> 2. Make sure we redirect ioctl() related to time stamping of Ethernet
>>    packets to connected PTP PHY rather than handle them ourselves

[...]

>> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
>> index 2d0d313..995ea2e 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -1298,7 +1298,11 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id)
>>  			ndev->stats.tx_bytes += skb->len;
>>  		}
>>  
>> +		/* It could be external PHY that had set SKBTX_IN_PROGRESS, so
>> +		 * we still need to check it's we who are to time stamp
>> +		 */
>>  		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) &&
>> +		    unlikely(fep->hwts_tx_en) &&
>
> I think this could qualify as a pretty significant fix in its own right,
> that should go to stable trees. Right now, this patch appears pretty
> easy to overlook.
>
> Is this the same situation as what is being described here for the
> gianfar driver?
>
> https://patchwork.ozlabs.org/project/netdev/patch/20191227004435.21692-2-olteanv@gmail.com/

Yes, it sounds exactly like that!

However, I'd insist that the second part of the patch is as important.
Please refer to my original post for the description of nasty confusion
the second part of the patch fixes:

https://lore.kernel.org/netdev/87r1uqtybr.fsf@osv.gnss.ru/

Basically, you get PHY response when you ask for capabilities, but then
MAC executes ioctl() request for corresponding configuration!

[...]

Thanks,
-- Sergey