linux-kernel - RE: [PATCH bpf-next v2 2/3] net: stmmac: Add txtime support to XDP ZC

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: <PH0PR11MB5830F08AC202C42501D986C0D887A@PH0PR11MB5830.namprd11.prod.outlook.com>
Date:   Sun, 3 Dec 2023 10:11:01 +0000
From:   "Song, Yoong Siang" <yoong.siang.song@...el.com>
To:     Jesper Dangaard Brouer <hawk@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        Bjorn Topel <bjorn@...nel.org>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        "Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        Stanislav Fomichev <sdf@...gle.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        Tariq Toukan <tariqt@...dia.com>,
        Willem de Bruijn <willemb@...gle.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        "Mykola Lysenko" <mykolal@...com>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>,
        Yonghong Song <yonghong.song@...ux.dev>,
        KP Singh <kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>,
        Jiri Olsa <jolsa@...nel.org>, Shuah Khan <shuah@...nel.org>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "xdp-hints@...-project.net" <xdp-hints@...-project.net>,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: RE: [PATCH bpf-next v2 2/3] net: stmmac: Add txtime support to XDP ZC

On Friday, December 1, 2023 11:02 PM, Jesper Dangaard Brouer wrote:
>On 12/1/23 07:24, Song Yoong Siang wrote:
>> This patch enables txtime support to XDP zero copy via XDP Tx
>> metadata framework.
>>
>> Signed-off-by: Song Yoong Siang<yoong.siang.song@...el.com>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac.h      |  2 ++
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +++++++++++++
>>   2 files changed, 15 insertions(+)
>
>I think we need to see other drivers using this new feature to evaluate
>if API is sane.
>
>I suggest implementing this for igc driver (chip i225) and also for igb
>(i210 chip) that both support this kind of LaunchTime feature in HW.
>
>The API and stmmac driver takes a u64 as time.
>I'm wondering how this applies to i210 that[1] have 25-bit for
>LaunchTime (with 32 nanosec granularity) limiting LaunchTime max 0.5
>second into the future.
>And i225 that [1] have 30-bit max 1 second into the future.
>
>
>[1]
>https://github.com/xdp-project/xdp-
>project/blob/master/areas/tsn/code01_follow_qdisc_TSN_offload.org

I am using u64 for launch time because existing EDT framework is using it.
Refer to struct sk_buff below. Both u64 and ktime_t can be used as launch time.
I choose u64 because ktime_t often requires additional type conversion and
we didn't expect negative value of time.

include/linux/skbuff.h-744- *   @tstamp: Time we arrived/left
include/linux/skbuff.h:745- *   @skb_mstamp_ns: (aka @tstamp) earliest departure time; start point
include/linux/skbuff.h-746- *           for retransmit timer
--
include/linux/skbuff.h-880-     union {
include/linux/skbuff.h-881-             ktime_t         tstamp;
include/linux/skbuff.h:882-             u64             skb_mstamp_ns; /* earliest departure time */
include/linux/skbuff.h-883-     };

tstamp/skb_mstamp_ns are used by various drivers for launch time support
on normal packet, so I think u64 should be "friendly" to all the drivers. For an
example, igc driver will take launch time from tstamp and recalculate it 
accordingly (i225 expect user to program "delta time" instead of "time" into
HW register).

drivers/net/ethernet/intel/igc/igc_main.c-1602- txtime = skb->tstamp;
drivers/net/ethernet/intel/igc/igc_main.c-1603- skb->tstamp = ktime_set(0, 0);
drivers/net/ethernet/intel/igc/igc_main.c:1604- launch_time = igc_tx_launchtime(tx_ring, txtime, &first_flag, &insert_empty);

Do you think this is enough to say the API is sane?