From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.17]) by mail.toke.dk (Postfix) with ESMTPS id B1418AD703E for ; Mon, 20 Jan 2025 07:26:26 +0100 (CET) Authentication-Results: mail.toke.dk; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=eQmxXCWe DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1737354387; x=1768890387; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=tnsWXzYfmvn62Lbs810i3rTWmbN0EXhNDxrDhCk8CD8=; b=eQmxXCWe72I8aBD1kuzF1eE77N5IMD49mnCoYsPUxvyRdvqaJD3MmBFV sIxU7x3K7TxpAl7VUh4mEglgsZKAUuoGlufL9u+0Mifar4SIYciib7HwN ZAU9G3UT2PlHTQ+BtkCWRrM6GzTyvqbK7V25megHuA8LtHWntYml541Gr KbyAFaGrqSZt0QbOZ2+biHYz8T5xp0u0d2EgbH6WIA3J4E2Wl2Op8QJyq 6NdjburCKEXm/o+Y2lx4mnc6Y46oh/9AkRw1mzlI0joRU+mhZ+GkuvvWK 6t7udp3T0+RSgxtnadTZOn7cLs8NJAAe1fS9cWENW7ts4tJ/YokFs1LrC A==; X-CSE-ConnectionGUID: +vFmooukQwWneao/pipnfw== X-CSE-MsgGUID: 2AYDaSD3SeqA8agUoIoaYA== X-IronPort-AV: E=McAfee;i="6700,10204,11320"; a="37622981" X-IronPort-AV: E=Sophos;i="6.13,218,1732608000"; d="scan'208";a="37622981" Received: from orviesa010.jf.intel.com ([10.64.159.150]) by fmvoesa111.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2025 22:26:15 -0800 X-CSE-ConnectionGUID: 2o19Sr1rR0+02sq9LFLQ2g== X-CSE-MsgGUID: X0kZiFOHTdmNQOkx3eVPsg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.12,224,1728975600"; d="scan'208";a="106227294" Received: from mohdfai2-mobl.gar.corp.intel.com (HELO [10.247.64.131]) ([10.247.64.131]) by orviesa010-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2025 22:26:01 -0800 Message-ID: <84770113-2546-4035-8bd4-bf9cedcfb00f@linux.intel.com> Date: Mon, 20 Jan 2025 14:25:45 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Song Yoong Siang , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Willem de Bruijn , Florian Bezdeka , Donald Hunter , Jonathan Corbet , Bjorn Topel , Magnus Karlsson , Maciej Fijalkowski , Jonathan Lemon , Andrew Lunn , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Joe Damato , Stanislav Fomichev , Xuan Zhuo , Mina Almasry , Daniel Jurgens , Andrii Nakryiko , Eduard Zingerman , Mykola Lysenko , Martin KaFai Lau , Song Liu , Yonghong Song , KP Singh , Hao Luo , Jiri Olsa , Shuah Khan , Alexandre Torgue , Jose Abreu , Maxime Coquelin , Tony Nguyen , Przemek Kitszel References: <20250116155350.555374-1-yoong.siang.song@intel.com> <20250116155350.555374-5-yoong.siang.song@intel.com> Content-Language: en-US From: "Abdul Rahim, Faizal" In-Reply-To: <20250116155350.555374-5-yoong.siang.song@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Message-ID-Hash: FWFJCQDBADU46PXSYH2YK2VLMCYUD63J X-Message-ID-Hash: FWFJCQDBADU46PXSYH2YK2VLMCYUD63J X-MailFrom: faizal.abdul.rahim@linux.intel.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, bpf@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, intel-wired-lan@lists.osuosl.org, xdp-hints@xdp-project.net X-Mailman-Version: 3.3.10 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next v6 4/4] igc: 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: Hi Siang. On 16/1/2025 11:53 pm, Song Yoong Siang wrote: > Enable Launch Time Control (LTC) support to XDP zero copy via XDP Tx > metadata framework. > > This patch is tested with tools/testing/selftests/bpf/xdp_hw_metadata on > Intel I225-LM Ethernet controller. Below are the test steps and result. > > Test Steps: > 1. At DUT, start xdp_hw_metadata selftest application: > $ sudo ./xdp_hw_metadata enp2s0 -l 1000000000 -L 1 > > 2. At Link Partner, send an UDP packet with VLAN priority 1 to port 9091 of > DUT. > > When launch time is set to 1s in the future, the delta between launch time > and transmit hardware timestamp is equal to 0.016us, as shown in result > below: > 0x562ff5dc8880: rx_desc[4]->addr=84110 addr=84110 comp_addr=84110 EoP > rx_hash: 0xE343384 with RSS type:0x1 > HW RX-time: 1734578015467548904 (sec:1734578015.4675) delta to User RX-time sec:0.0002 (183.103 usec) > XDP RX-time: 1734578015467651698 (sec:1734578015.4677) delta to User RX-time sec:0.0001 (80.309 usec) > No rx_vlan_tci or rx_vlan_proto, err=-95 > 0x562ff5dc8880: ping-pong with csum=561c (want c7dd) csum_start=34 csum_offset=6 > HW RX-time: 1734578015467548904 (sec:1734578015.4675) delta to HW Launch-time sec:1.0000 (1000000.000 usec) > 0x562ff5dc8880: complete tx idx=4 addr=4018 > HW Launch-time: 1734578016467548904 (sec:1734578016.4675) delta to HW TX-complete-time sec:0.0000 (0.016 usec) > HW TX-complete-time: 1734578016467548920 (sec:1734578016.4675) delta to User TX-complete-time sec:0.0000 (32.546 usec) > XDP RX-time: 1734578015467651698 (sec:1734578015.4677) delta to User TX-complete-time sec:0.9999 (999929.768 usec) > HW RX-time: 1734578015467548904 (sec:1734578015.4675) delta to HW TX-complete-time sec:1.0000 (1000000.016 usec) > 0x562ff5dc8880: complete rx idx=132 addr=84110 To be cautious, could we perform a stress test by sending a higher number of packets with launch time? For example, we could send 200 packets, each configured with a launch time, and verify that the driver continues to function correctly afterward. > Signed-off-by: Song Yoong Siang > --- > drivers/net/ethernet/intel/igc/igc_main.c | 78 ++++++++++++++++------- > 1 file changed, 56 insertions(+), 22 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c > index 27872bdea9bd..6857f5f5b4b2 100644 > --- a/drivers/net/ethernet/intel/igc/igc_main.c > +++ b/drivers/net/ethernet/intel/igc/igc_main.c > @@ -1566,6 +1566,26 @@ static bool igc_request_tx_tstamp(struct igc_adapter *adapter, struct sk_buff *s > return false; > } > > +static void igc_insert_empty_packet(struct igc_ring *tx_ring) > +{ > + struct igc_tx_buffer *empty_info; > + struct sk_buff *empty; > + void *data; > + > + empty_info = &tx_ring->tx_buffer_info[tx_ring->next_to_use]; > + empty = alloc_skb(IGC_EMPTY_FRAME_SIZE, GFP_ATOMIC); > + if (!empty) > + return; > + > + data = skb_put(empty, IGC_EMPTY_FRAME_SIZE); > + memset(data, 0, IGC_EMPTY_FRAME_SIZE); > + > + igc_tx_ctxtdesc(tx_ring, 0, false, 0, 0, 0); > + > + if (igc_init_tx_empty_descriptor(tx_ring, empty, empty_info) < 0) > + dev_kfree_skb_any(empty); > +} > + The function igc_insert_empty_packet() appears to wrap existing code to enhance reusability, with no new changes related to enabling launch-time XDP ZC functionality. If so, could we split this into a separate commit? This would make it clearer for the reader to distinguish between the refactoring changes and the new changes related to enabling launch-time XDP ZC support. > static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb, > struct igc_ring *tx_ring) > { > @@ -1603,26 +1623,8 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb, > skb->tstamp = ktime_set(0, 0); > launch_time = igc_tx_launchtime(tx_ring, txtime, &first_flag, &insert_empty); > > - if (insert_empty) { > - struct igc_tx_buffer *empty_info; > - struct sk_buff *empty; > - void *data; > - > - empty_info = &tx_ring->tx_buffer_info[tx_ring->next_to_use]; > - empty = alloc_skb(IGC_EMPTY_FRAME_SIZE, GFP_ATOMIC); > - if (!empty) > - goto done; > - > - data = skb_put(empty, IGC_EMPTY_FRAME_SIZE); > - memset(data, 0, IGC_EMPTY_FRAME_SIZE); > - > - igc_tx_ctxtdesc(tx_ring, 0, false, 0, 0, 0); > - > - if (igc_init_tx_empty_descriptor(tx_ring, > - empty, > - empty_info) < 0) > - dev_kfree_skb_any(empty); > - } > + if (insert_empty) > + igc_insert_empty_packet(tx_ring); > > done: > /* record the location of the first descriptor for this packet */ > @@ -2955,9 +2957,33 @@ static u64 igc_xsk_fill_timestamp(void *_priv) > return *(u64 *)_priv; > } > > +static void igc_xsk_request_launch_time(u64 launch_time, void *_priv) > +{ > + struct igc_metadata_request *meta_req = _priv; > + struct igc_ring *tx_ring = meta_req->tx_ring; > + __le32 launch_time_offset; > + bool insert_empty = false; > + bool first_flag = false; > + > + if (!tx_ring->launchtime_enable) > + return; > + > + launch_time_offset = igc_tx_launchtime(tx_ring, > + ns_to_ktime(launch_time), > + &first_flag, &insert_empty); > + if (insert_empty) { > + igc_insert_empty_packet(tx_ring); > + meta_req->tx_buffer = > + &tx_ring->tx_buffer_info[tx_ring->next_to_use]; > + } > + > + igc_tx_ctxtdesc(tx_ring, launch_time_offset, first_flag, 0, 0, 0); > +} > + > const struct xsk_tx_metadata_ops igc_xsk_tx_metadata_ops = { > .tmo_request_timestamp = igc_xsk_request_timestamp, > .tmo_fill_timestamp = igc_xsk_fill_timestamp, > + .tmo_request_launch_time = igc_xsk_request_launch_time, > }; > > static void igc_xdp_xmit_zc(struct igc_ring *ring) > @@ -2980,7 +3006,7 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring) > ntu = ring->next_to_use; > budget = igc_desc_unused(ring); > > - while (xsk_tx_peek_desc(pool, &xdp_desc) && budget--) { > + while (xsk_tx_peek_desc(pool, &xdp_desc) && budget >= 4) { Could we add some explanation on what & why the value "4" is used ?