Received-SPF: pass (google.com: domain of linux-kernel+bounces-96484-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: "Song, Yoong Siang" <yoong.siang.song@intel.com>
To: Kurt Kanzenbach <kurt@linutronix.de>, "Fijalkowski, Maciej"
	<maciej.fijalkowski@intel.com>
CC: "Brandeburg, Jesse" <jesse.brandeburg@intel.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@intel.com>, "David S . Miller" <davem@davemloft.net>, "Eric
 Dumazet" <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni
	<pabeni@redhat.com>, Richard Cochran <richardcochran@gmail.com>, "Alexei
 Starovoitov" <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Jesper
 Dangaard Brouer <hawk@kernel.org>, John Fastabend <john.fastabend@gmail.com>,
	Stanislav Fomichev <sdf@google.com>, "Gomes, Vinicius"
	<vinicius.gomes@intel.com>, "Bezdeka, Florian" <florian.bezdeka@siemens.com>,
	Andrii Nakryiko <andrii@kernel.org>, "Eduard Zingerman" <eddyz87@gmail.com>,
	Mykola Lysenko <mykolal@fb.com>, "Martin KaFai Lau" <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yonghong.song@linux.dev>, KP Singh
	<kpsingh@kernel.org>, Hao Luo <haoluo@google.com>, Jiri Olsa
	<jolsa@kernel.org>, Shuah Khan <shuah@kernel.org>,
	"xdp-hints@xdp-project.net" <xdp-hints@xdp-project.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"intel-wired-lan@lists.osuosl.org" <intel-wired-lan@lists.osuosl.org>,
	"linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
	"bpf@vger.kernel.org" <bpf@vger.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
Thread-Topic: [xdp-hints] Re: [Intel-wired-lan] [PATCH iwl-next, v3 2/2] igc:
 Add Tx hardware timestamp request for AF_XDP zero-copy packet
Thread-Index: AQHab+vcS1TDt8kjEUSsQjqKNGdoS7EsSaUAgADcFYA=
Date: Fri, 8 Mar 2024 03:39:15 +0000
Message-ID: <PH0PR11MB58304D1FA254E9CE8FA1AEF4D8272@PH0PR11MB5830.namprd11.prod.outlook.com>
References: <20240303083225.1184165-1-yoong.siang.song@intel.com>
 <20240303083225.1184165-3-yoong.siang.song@intel.com>
 <Zein8XvWkqj8VrHs@boxer> <87a5nago13.fsf@kurt.kurt.home>
In-Reply-To: <87a5nago13.fsf@kurt.kurt.home>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?a/iDHadEsomSJ0EjGOMaR/nCNqyCnAOc0gzEB98b/+1C7dwjOsKvUWFaR340?=
 =?us-ascii?Q?uvlor6PlA51gIMTKhwjASIrvJDoJ/ZScHtEjEggy+tUj7RMaxCf5SapEkK74?=
 =?us-ascii?Q?1MaBDPFZiBm+XOABwt4hctoxmZpE+XsU2/6+RFGSnhPuD31Pt8etSRJ0E9hs?=
 =?us-ascii?Q?U6c4FyYYg6bo9DvoEdYhSh8uXzLLiyljJKy6Zrf9BhcMQ9vH0CXNXITT+jsX?=
 =?us-ascii?Q?LJtuDHHOsF3mZ2b8BZB4WTdDynKfTDMtB6xbVsVEflFC56SNKf/frCbUK7S3?=
 =?us-ascii?Q?4hRL0bPCJIqF30VTAek9j/NS6lWRensu3JxT7mEUX5t12ttoZTAtF7WOeYbT?=
 =?us-ascii?Q?ukMWzoS8sv/xc8lf3aVolpmWxJxXRo8OiGRtn20bXCZuVqHUTCfpto2706LB?=
 =?us-ascii?Q?M2qg/56pMoc0t+ACHJk0O+S3DFaa5/Ju5+K7o7ACsI7vstep0Y7/yWSFHjBR?=
 =?us-ascii?Q?OHwzNIXSjkl+7qH1PzSaZBtMPhG5RNe7+1kMy5FxR1aysuenQ9OCiB0HTFLj?=
 =?us-ascii?Q?2ejOyCblMmtfRlcfkhslM4DzYBjOpDsFSBJD7F5zOvNoOI9h0ZVc9tAXtJkz?=
 =?us-ascii?Q?hTZY1Vw2eCtyXb6k/LmzJ3ZUfXyvWgnK7tmBEWglV0H0tvLOCAiR7+TBSoG7?=
 =?us-ascii?Q?MsldG+vly72c0KgQgsRicce5UHHZpejhGHhClVu2lJ/0lmPxXaFIReP70yJ6?=
 =?us-ascii?Q?ThAmb45L7DqNH9hruJDugk6/1gaapQldWA7QnChj54uNSXEPu4/T19SIpOgL?=
 =?us-ascii?Q?ZyBrWy73FYyhp9wyLU86hNaovSDR1a09mR4YoSg4UeUUMitoHKbqSFUkDa7P?=
 =?us-ascii?Q?gduZcvCDwfKXzU95E7PIhVpDiDKWs64b9ebPgvYCgVD6S1TZuBzjiXHIV3E3?=
 =?us-ascii?Q?VGHqKH/e2/apBkz00vk4I6DSvEpvI8ThNqjwJk6SFgm+crUuhUO9phyEKW6R?=
 =?us-ascii?Q?8c1kB3Iad7bSU/hL/k25EacGy3aBNxUJ9YU1fWOYKU1i6YGX7ViDL1L/mbFm?=
 =?us-ascii?Q?ayiyZV6fAZMjYTOlPvQ7bIRDVabN5x8X5A1mXUpLCZlvK173RzynNrUmPQQL?=
 =?us-ascii?Q?wlYgSCSMy2u6wQQBj08GnUlHW710Qy3unNstAC7Pt2XgshzSmQiyWA8IQ3aO?=
 =?us-ascii?Q?N9XRzgsOkYVCZkbUa/ajjZuKO6sM8YdgyFj5MZjlO0d8e3s2sKxaod/Ei7/0?=
 =?us-ascii?Q?1eAKD5ieFkGsLdMQpMwhSCydKkMUW0NShQVG0LK2kolzK7t9n20h+81m1vjC?=
 =?us-ascii?Q?3zvenRlz0wChW9/OSJCMoha2cS8oeytHeTUDkTupUAH74CkJnTMnHD2GO74E?=
 =?us-ascii?Q?emIzUpvsNgwODJNHiftuJdzSYgOkzMt1i9iKSxxAMQPWomgi6tCTEvNZ56Ji?=
 =?us-ascii?Q?eVrq6CTKz+m5du5Nlpm9CqtEcUmoZDq16ooqWRNthvUSHnQTaIL1jrZ89yQv?=
 =?us-ascii?Q?ncanhI9thZbe9O+RZzZLQtJecS6324WIBsycf0VUzQcga1KINfiVmOQc3are?=
 =?us-ascii?Q?9R3nPZ3QAS7qA4gfmZjfFPz5/4b77NIBHSSk1zWgGaOWSW58QYhcNACJk2hR?=
 =?us-ascii?Q?udoFqWHilSfUH13k9Tf65i69POPfoEN8fwzDR+NUSVyPg81NKXh7Puhrzu6e?=
 =?us-ascii?Q?dA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5830.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c7654ef5-fb8d-438a-4de6-08dc3f2153c0
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Mar 2024 03:39:15.1009
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: jUxhL3+ckX4DwUSbgu9tPOqAD00uIMCIr8TZYOMqBvoV/5crsxuPs+vobusPDo+7oslSs3E1PhL9dnS8qJrOfaMGk2oGhguESx4ufe/XJzg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7294
X-OriginatorOrg: intel.com

On Thursday, March 7, 2024 9:39 PM, Kurt Kanzenbach <kurt@linutronix.de> wr=
ote:
>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 =3D NULL;
>>> +	/* Copy the tx hardware timestamp into xdp metadata or skb */
>>> +	if (tstamp->buffer_type =3D=3D 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@linutronix.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 bri=
ng
>> 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 =3D=3D IGC_TX_BUFFER_TYPE_XSK
as a flag to check whether XDP Tx metadata is used. Thus other data path wo=
n't
call into xsk_tx_metadata_complete().
Even it work, but I think I should still add xp_tx_metadata_enabled() check=
ing
for safety measure. Any thoughts?

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

Btw, thank for fixing the issue on stmmac.=20

Thanks & Regards
Siang

=20