On 11/24/2025 4:09 AM, Bruce Richardson wrote:
> On Fri, Nov 21, 2025 at 03:39:37PM -0800, Jacob Keller wrote:
>> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
>> index 3ef766de4704..9b07b11a6b51 100644
>> --- a/drivers/net/intel/iavf/iavf_ethdev.c
>> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
>> @@ -2887,6 +2887,14 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
>>  		}
>>  	}
>>  
>> +	/* Get PTP caps early to verify device capabilities */
>> +	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_PTP) {
>> +		if (iavf_get_ptp_cap(adapter)) {
>> +			PMD_INIT_LOG(ERR, "Failed to get ptp capability");
>> +			goto security_init_err;
>> +		}
>> +	}
>> +
> 
> With this code added here, do we still need to keep - or should we keep -
> the existing call in iavf_dev_start()? I would have expected the call to
> iavf_get_ptp_cap to be moved rather than duplicated. Is there a reason
> to keep the existing call?
> 
> /Bruce

So, I considered this, but I am not sure. In principle, we need to make
sure that we re-check capabilities after a reset. It is unlikely but it
is possible that a device reset could cause the PF to change its
decision on capabilities. I didn't see logic to call this during the
reset handler. If my understanding is right, thats because the start
function would get called after a reset, at which point we'd recheck
during the device start function.

I think its harmless to re-check the values as they should either be the
same or if they did somehow change we should get the updated value. The
intent of checking earlier is really just to make sure we try not to
report that we support timestamps when we definitely don't.

In particular, I wanted to get the current case with the upstream ice
driver and the DPDK code to fail closed instead of attempting to enable
timestamps but doing so incorrectly. (As outlined in my earlier cover
letter, we have a compatibility issue with the in-tree Linux kernel
drivers and the out-of-tree drivers regarding the capability struct layout).

That setup was trying to turn on timestamps but the PF did not actually
enable timestamps due to the capability layout issue, and thus the
timestamps were bogus. Instead, the new situation should be that DPDK
reports it can't enable timestamps, until we figure out the solution for
compatibility across these different variants of virtchnl. I'm working
on that but don't have an ETA or known time where we'll have a good
solution.

Thanks,
Jake