> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf Of > Nadezhdin, Anton > Sent: Thursday, August 14, 2025 1:17 AM > To: Paul Menzel <[email protected]>; Olech, Milena > <[email protected]> > Cc: [email protected]; [email protected]; Nguyen, > Anthony L <[email protected]>; [email protected]; > Loktionov, Aleksandr <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH iwl-net] idpf: cleanup remaining SKBs in > PTP flows > > On 8/14/2025 7:19 AM, Paul Menzel wrote: > > Dear Anton, dear Milena, > > > > > > Thank you for the patch. > > > > Am 13.08.25 um 19:33 schrieb Anton Nadezhdin: > >> From: Milena Olech <[email protected]> > >> > >> When the driver requests Tx timestamp value, one of the first steps > >> is to clone SKB using skb_get. It increases the reference counter for > >> that SKB to prevent unexpected freeing by another component. > >> However, there may be a case where the index is requested, SKB is > >> assigned and never consumed by PTP flows - for example due to reset > >> during running PTP apps. > >> > >> Add a check in release timestamping function to verify if the SKB > >> assigned to Tx timestamp latch was freed, and release remaining SKBs. > >> > >> Fixes: 4901e83a94ef ("idpf: add Tx timestamp capabilities > >> negotiation") > >> Signed-off-by: Milena Olech <[email protected]> > >> Signed-off-by: Anton Nadezhdin <[email protected]> > >> Reviewed-by: Aleksandr Loktionov <[email protected]> > >> --- > >> drivers/net/ethernet/intel/idpf/idpf_ptp.c | 3 +++ > >> drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c | 1 + > >> 2 files changed, 4 insertions(+) > >> > >> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c > >> b/drivers/net/ ethernet/intel/idpf/idpf_ptp.c index > >> ee21f2ff0cad..63a41e688733 100644 > >> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c > >> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c > >> @@ -855,6 +855,9 @@ static void idpf_ptp_release_vport_tstamp(struct > >> idpf_vport *vport) > >> head = &vport->tx_tstamp_caps->latches_in_use; > >> list_for_each_entry_safe(ptp_tx_tstamp, tmp, head, list_member) > >> { > >> list_del(&ptp_tx_tstamp->list_member); > >> + if (ptp_tx_tstamp->skb) > >> + consume_skb(ptp_tx_tstamp->skb); > >> + > >> kfree(ptp_tx_tstamp); > >> } > >> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/ > >> drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c > >> index 4f1fb0cefe51..688a6f4e0acc 100644 > >> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c > >> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c > >> @@ -517,6 +517,7 @@ idpf_ptp_get_tstamp_value(struct idpf_vport > >> *vport, > >> shhwtstamps.hwtstamp = ns_to_ktime(tstamp); > >> skb_tstamp_tx(ptp_tx_tstamp->skb, &shhwtstamps); > >> consume_skb(ptp_tx_tstamp->skb); > >> + ptp_tx_tstamp->skb = NULL; > > > > This hunk is not clear to me from the commit message, and the whole diff. > > So in idpf_ptp_request_ts the code is making copy of skb, normally when this > copy is consumed in idpf_ptp_get_tstamp_value but in case of a reset the > normal flow is interrupted and skb is still in hold. > This patch release it during reset procedure. > > > > >> list_add(&ptp_tx_tstamp->list_member, > >> &tx_tstamp_caps->latches_free); > > > > Kind regards, > > > > Paul
Tested-by: Samuel Salin <[email protected]>
