netdev - RE: [xdp-hints] Re: [Intel-wired-lan] [PATCH iwl-next, v3 2/2] igc: Add Tx hardware timestamp request for AF

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] [day] [month] [year] [list]

Message-ID: <PH0PR11MB58304D1FA254E9CE8FA1AEF4D8272@PH0PR11MB5830.namprd11.prod.outlook.com>
Date: Fri, 8 Mar 2024 03:39:15 +0000
From: "Song, Yoong Siang" <yoong.siang.song@...el.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, "Fijalkowski, Maciej"
	<maciej.fijalkowski@...el.com>
CC: "Brandeburg, Jesse" <jesse.brandeburg@...el.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "David S . Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>, "Alexei
 Starovoitov" <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Jesper
 Dangaard Brouer <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>,
	Stanislav Fomichev <sdf@...gle.com>, "Gomes, Vinicius"
	<vinicius.gomes@...el.com>, "Bezdeka, Florian" <florian.bezdeka@...mens.com>,
	Andrii Nakryiko <andrii@...nel.org>, "Eduard Zingerman" <eddyz87@...il.com>,
	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>,
	"xdp-hints@...-project.net" <xdp-hints@...-project.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
	"bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: RE: [xdp-hints] Re: [Intel-wired-lan] [PATCH iwl-next, v3 2/2] igc:
 Add Tx hardware timestamp request for AF_XDP zero-copy packet

On Thursday, March 7, 2024 9:39 PM, Kurt Kanzenbach <kurt@...utronix.de> wrote:
>Hi Maciej,
>
>On Wed Mar 06 2024, Maciej Fijalkowski wrote:
>> On Sun, Mar 03, 2024 at 04:32:25PM +0800, Song Yoong Siang wrote:
>>> -	tstamp->skb = NULL;
>>> +	/* Copy the tx hardware timestamp into xdp metadata or skb */
>>> +	if (tstamp->buffer_type == IGC_TX_BUFFER_TYPE_XSK)
>>
>> I believe this should also be protected with xp_tx_metadata_enabled()
>> check. We recently had following bugfix, PTAL:
>>
>> https://lore.kernel.org/bpf/20240222-stmmac_xdp-v2-1-
>4beee3a037e4@...utronix.de/
>>
>> I'll take a deeper look at patch tomorrow, might be the case that you've
>> addressed that or you were aware of this issue but anyways wanted to bring
>> it up. Just check that you don't break standard XDP/AF_XDP traffic :)
>
>This one doesn't crash standard AF_XDP traffic. Don't know why, but it
>seems to work.
>
>Thanks,
>Kurt

Thanks Maciej and Kurt for the comments.
In stmmac, xsk_tx_metadata_complete() is called by generic tx complete irq,
thus causing the NULL pointer issue.
In igc, xsk_tx_metadata_complete() is called by ptp irq,
and we use  tstamp->buffer_type == IGC_TX_BUFFER_TYPE_XSK
as a flag to check whether XDP Tx metadata is used. Thus other data path won't
call into xsk_tx_metadata_complete().
Even it work, but I think I should still add xp_tx_metadata_enabled() checking
for safety measure. Any thoughts?

In case Maciej have more comments, I will wait few days before I add the checking in v4.

Btw, thank for fixing the issue on stmmac. 

Thanks & Regards
Siang