On 8/25/2025 6:18 AM, Vadim Fedorenko wrote:
> On 23/08/2025 08:44, Kurt Kanzenbach wrote:
>> On Fri Aug 22 2025, Vadim Fedorenko wrote:
>>> On 22/08/2025 08:28, Kurt Kanzenbach wrote:
>>>> The current implementation uses schedule_work() which is executed by the
>>>> system work queue to retrieve Tx timestamps. This increases latency and can
>>>> lead to timeouts in case of heavy system load.
>>>>
>>>> Therefore, switch to the PTP aux worker which can be prioritized and pinned
>>>> according to use case. Tested on Intel i210.
>>>>
>>>> * igb_ptp_tx_work
>>>> - * @work: pointer to work struct
>>>> + * @ptp: pointer to ptp clock information
>>>> *
>>>> * This work function polls the TSYNCTXCTL valid bit to determine when a
>>>> * timestamp has been taken for the current stored skb.
>>>> **/
>>>> -static void igb_ptp_tx_work(struct work_struct *work)
>>>> +static long igb_ptp_tx_work(struct ptp_clock_info *ptp)
>>>> {
>>>> - struct igb_adapter *adapter = container_of(work, struct igb_adapter,
>>>> - ptp_tx_work);
>>>> + struct igb_adapter *adapter = container_of(ptp, struct igb_adapter,
>>>> + ptp_caps);
>>>> struct e1000_hw *hw = &adapter->hw;
>>>> u32 tsynctxctl;
>>>>
>>>> if (!adapter->ptp_tx_skb)
>>>> - return;
>>>> + return -1;
>>>>
>>>> if (time_is_before_jiffies(adapter->ptp_tx_start +
>>>> IGB_PTP_TX_TIMEOUT)) {
>>>> @@ -824,15 +824,17 @@ static void igb_ptp_tx_work(struct work_struct *work)
>>>> */
>>>> rd32(E1000_TXSTMPH);
>>>> dev_warn(&adapter->pdev->dev, "clearing Tx timestamp
>>>> hang\n");
>>>> - return;
>>>> + return -1;
>>>> }
>>>>
>>>> tsynctxctl = rd32(E1000_TSYNCTXCTL);
>>>> - if (tsynctxctl & E1000_TSYNCTXCTL_VALID)
>>>> + if (tsynctxctl & E1000_TSYNCTXCTL_VALID) {
>>>> igb_ptp_tx_hwtstamp(adapter);
>>>> - else
>>>> - /* reschedule to check later */
>>>> - schedule_work(&adapter->ptp_tx_work);
>>>> + return -1;
>>>> + }
>>>> +
>>>> + /* reschedule to check later */
>>>> + return 1;
>>>
>>> do_aux_work is expected to return delay in jiffies to re-schedule the
>>> work. it would be cleaner to use msec_to_jiffies macros to tell how much
>>> time the driver has to wait before the next try of retrieving the
>>> timestamp. AFAIU, the timestamp may come ASAP in this case, so it's
>>> actually reasonable to request immediate re-schedule of the worker by
>>> returning 0.
>>
>> I don't think so. The 'return 1' is only executed for 82576. For all
>> other NICs the TS is already available. For 82576 the packet is queued
>> to Tx ring and the worker is scheduled immediately. For example, in case
>> of other Tx traffic there's a chance that the TS is not available
>> yet. So we comeback one jiffy later, which is 10ms at worst. That looked
>> reasonable to me.
>
> Ok, LGTM
> Reviewed-by: Vadim Fedorenko <[email protected]>
> Agreed. Reviewed-by: Jacob Keller <[email protected]>
OpenPGP_signature.asc
Description: OpenPGP digital signature
