From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by mail.toke.dk (Postfix) with ESMTPS id D6958A43B78 for ; Tue, 5 Dec 2023 20:39:31 +0100 (CET) Authentication-Results: mail.toke.dk; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20230601 header.b=qo2R4yVO Received: by mail-pg1-x54a.google.com with SMTP id 41be03b00d2f7-5c680110ea9so2824214a12.2 for ; Tue, 05 Dec 2023 11:39:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1701805170; x=1702409970; darn=xdp-project.net; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=xfq67iLmriDqfduTRY0M8dju8QhNwul2zcs5mKn+Qdk=; b=qo2R4yVOHBFuKxmGM58vrGZZVKB0wtIjpAkcMEWOTI7rUu6eyZ5AyyAyXqkBdjr+JX Na4jRGqc3CTAt9YOA9zMBAmSxLzShVS6djDYnC6U0f0I9kRdli1OdtGhVpCFeidEAcF4 4oVL3iktcAYGdkdjFhhQbDl3faulzK7EoosZXe/HqCPZYuQ+xx+BdLpeVeG0ScvVCjTJ wheEXIN2ST87egJhMyvdx6P0lY9CmskLBRa/aYPt+aqQpDhi0nalgk5mocCf5YMAwlN3 Rp3ThE7S/OEwlO2fDc2zVet5tyGbg6PUWDSpd0gVo9pfCmRGgZeyshfkY21zW39p7FfI fXgg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701805170; x=1702409970; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=xfq67iLmriDqfduTRY0M8dju8QhNwul2zcs5mKn+Qdk=; b=exuhunYR0npuMX83NZItuGg9k/1ZaIcK65QMKpiHTZQQySRBCfJ/7xgubpewBNX33w 6UDzY9owcS0fRaMy+fPWOiO2ltay4fEuzdM9uz4T6V6W5N9ENxjWxe+yNN9+qZymtmSU +/W+3PAgTmUaGzcbDSPRtuvmF7szyA3lrT4qtphiLYDNYpBP5wgf91cCYumUnbesBpah hzj9RC8NnND1/ZjZQD0LYQ7JEDrCWU3L7LSmYLqjCXajRR5OF855KJWWnon/KKG+Ism4 033QII2sVtuWBFiR3vDStYzi7GvW8oHYj5L5VJbPMjUcWTis7gTsjdc5pH5R+q0glrla 69iQ== X-Gm-Message-State: AOJu0YyKDxSNYfFEMs9qgGZowPpZBbdy7WMEv6sYq04Qw3voOaBNZAll AgUjEaoT7ugVss1XcbJQBpVa6lw= X-Google-Smtp-Source: AGHT+IFIztxj2eUKXZ2GS1Ca6Xtk1G+ZaZVM3GaQJJGqcaBfxYyvfWdWIYuJqHVvsmWQlJgRFVSGrCQ= X-Received: from sdf.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5935]) (user=sdf job=sendgmr) by 2002:a63:1220:0:b0:5c2:2f9:c374 with SMTP id h32-20020a631220000000b005c202f9c374mr4433763pgl.9.1701805169961; Tue, 05 Dec 2023 11:39:29 -0800 (PST) Date: Tue, 5 Dec 2023 11:39:28 -0800 In-Reply-To: <656f66023f7bd_3dd6422942a@willemb.c.googlers.com.notmuch> Mime-Version: 1.0 References: <20231203165129.1740512-1-yoong.siang.song@intel.com> <20231203165129.1740512-3-yoong.siang.song@intel.com> <43b01013-e78b-417e-b169-91909c7309b1@kernel.org> <656de830e8d70_2e983e294ca@willemb.c.googlers.com.notmuch> <5a0faf8cc9ec3ab0d5082c66b909c582c8f1eae6.camel@siemens.com> <656f66023f7bd_3dd6422942a@willemb.c.googlers.com.notmuch> Message-ID: From: Stanislav Fomichev To: Willem de Bruijn Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: RJTRLJQYSQHBYZQVZ2JWZ6H6XB2KJLMZ X-Message-ID-Hash: RJTRLJQYSQHBYZQVZ2JWZ6H6XB2KJLMZ X-MailFrom: 3cXxvZQMKCVoK578GG8D6.4GEP5H-9AFLKP5H-HJGB64L.F6L@flex--sdf.bounces.google.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Florian Bezdeka , yoong.siang.song@intel.com, Jesper Dangaard Brouer , davem@davemloft.net, Eric Dumazet , Jakub Kicinski , Paolo Abeni , Jonathan Corbet , Bjorn Topel , magnus.karlsson@intel.com, maciej.fijalkowski@intel.com, Jonathan Lemon , Alexei Starovoitov , Daniel Borkmann , John Fastabend , Lorenzo Bianconi , Tariq Toukan , Willem de Bruijn , Maxime Coquelin , Andrii Nakryiko , Mykola Lysenko , Martin KaFai Lau , Song Liu , Yonghong Song , KP Singh , Hao Luo , Jiri Olsa , Shuah Khan , Alexandre Torgue , Jose Abreu , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-doc@vger.kernel.org" , "bpf@vger.kernel.org" , "xdp-hints@xdp-project.net" , "linux-stm32@st-md-mailman.stormreply.com" , "linux-arm-kernel@lists.infradead.org" , "linux-kselftest@vger.kernel.org" X-Mailman-Version: 3.3.8 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next v3 2/3] net: stmmac: add Launch Time support to XDP ZC List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 12/05, Willem de Bruijn wrote: > Stanislav Fomichev wrote: > > On Tue, Dec 5, 2023 at 7:34=E2=80=AFAM Florian Bezdeka > > wrote: > > > > > > On Tue, 2023-12-05 at 15:25 +0000, Song, Yoong Siang wrote: > > > > On Monday, December 4, 2023 10:55 PM, Willem de Bruijn wrote: > > > > > Jesper Dangaard Brouer wrote: > > > > > > > > > > > > > > > > > > On 12/3/23 17:51, Song Yoong Siang wrote: > > > > > > > This patch enables Launch Time (Time-Based Scheduling) suppor= t to XDP zero > > > > > > > copy via XDP Tx metadata framework. > > > > > > > > > > > > > > Signed-off-by: Song Yoong Siang > > > > > > > --- > > > > > > > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 ++ > > > > > > > > > > > > As requested before, I think we need to see another driver impl= ementing > > > > > > this. > > > > > > > > > > > > I propose driver igc and chip i225. > > > > > > > > Sure. I will include igc patches in next version. > > > > > > > > > > > > > > > > The interesting thing for me is to see how the LaunchTime max 1= second > > > > > > into the future[1] is handled code wise. One suggestion is to a= dd a > > > > > > section to Documentation/networking/xsk-tx-metadata.rst per dri= ver that > > > > > > mentions/documents these different hardware limitations. It is= natural > > > > > > that different types of hardware have limitations. This is a c= lose-to > > > > > > hardware-level abstraction/API, and IMHO as long as we document= the > > > > > > limitations we can expose this API without too many limitations= for more > > > > > > capable hardware. > > > > > > > > Sure. I will try to add hardware limitations in documentation. > > > > > > > > > > > > > > I would assume that the kfunc will fail when a value is passed th= at > > > > > cannot be programmed. > > > > > > > > > > > > > In current design, the xsk_tx_metadata_request() dint got return va= lue. > > > > So user won't know if their request is fail. > > > > It is complex to inform user which request is failing. > > > > Therefore, IMHO, it is good that we let driver handle the error sil= ently. > > > > > > > > > > If the programmed value is invalid, the packet will be "dropped" / wi= ll > > > never make it to the wire, right? >=20 > Programmable behavior is to either drop or cap to some boundary > value, such as the farthest programmable time in the future: the > horizon. In fq: >=20 > /* Check if packet timestamp is too far in the future. */ > if (fq_packet_beyond_horizon(skb, q, now)) { > if (q->horizon_drop) { > q->stat_horizon_drops++; > return qdisc_drop(skb, sch, to_fr= ee); > } > q->stat_horizon_caps++; > skb->tstamp =3D now + q->horizon; > } > fq_skb_cb(skb)->time_to_send =3D skb->tstamp; >=20 > Drop is the more obviously correct mode. >=20 > Programming with a clock source that the driver does not support will > then be a persistent failure. >=20 > Preferably, this driver capability can be queried beforehand (rather > than only through reading error counters afterwards). >=20 > Perhaps it should not be a driver task to convert from possibly > multiple clock sources to the device native clock. Right now, we do > use per-device timecounters for this, implemented in the driver. >=20 > As for which clocks are relevant. For PTP, I suppose the device PHC, > converted to nsec. For pacing offload, TCP uses CLOCK_MONOTONIC. Do we need to expose some generic netdev netlink apis to query/adjust nic clock sources (or maybe there is something existing already)? Then the userspace can be responsible for syncing/converting the timestamps to the internal nic clocks. +1 to trying to avoid doing this in the drivers. > > > That is clearly a situation that the user should be informed about. F= or > > > RT systems this normally means that something is really wrong regardi= ng > > > timing / cycle overflow. Such systems have to react on that situation= . > >=20 > > In general, af_xdp is a bit lacking in this 'notify the user that they > > somehow messed up' area :-( > > For example, pushing a tx descriptor with a wrong addr/len in zc mode > > will not give any visible signal back (besides driver potentially > > spilling something into dmesg as it was in the mlx case). > > We can probably start with having some counters for these events? >=20 > This is because the AF_XDP completion queue descriptor format is only > a u64 address? Yeah. XDP_COPY mode has the descriptor validation which is exported via recvmsg errno, but zerocopy path seems to be too deep in the stack to report something back. And there is no place, as you mention, in the completion ring to report the status. > Could error conditions be reported on tx completion in the metadata, > using xsk_tx_metadata_complete? That would be one way to do it, yes. But then the error reporting depends on the metadata opt-in. Having a separate ring to export the errors, or having a v2 tx-completions layout with extra 'status' field would also work. But this seems like something that should be handled separately? Because we'd have to teach all existing zc drivers to report those errors back instead of dropping these descriptors..