RE: [PATCH v2 2/7] net/idpf: save main time by alarm

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Qiao, Wenjing <wenjing.qiao@intel.com>
> Sent: Friday, April 21, 2023 3:16 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Qiao, Wenjing <wenjing.qiao@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v2 2/7] net/idpf: save main time by alarm
> 
> Using alarm to save main time from registers every 1 second.
> 
> Fixes: 8c6098afa075 ("common/idpf: add Rx/Tx data path")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wenjing Qiao <wenjing.qiao@intel.com>
> ---
>  drivers/net/idpf/idpf_ethdev.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/idpf/idpf_ethdev.c b/drivers/net/idpf/idpf_ethdev.c
> index e02ec2ec5a..3f33ffbc78 100644
> --- a/drivers/net/idpf/idpf_ethdev.c
> +++ b/drivers/net/idpf/idpf_ethdev.c
> @@ -761,6 +761,12 @@ idpf_dev_start(struct rte_eth_dev *dev)
>  		goto err_vec;
>  	}
> 
> +	if (dev->data->dev_conf.rxmode.offloads &
> RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
> +		rte_eal_alarm_set(1000 * 1000,

Please use a macro for easy read.

> +				  &idpf_dev_read_time_hw,
> +				  (void *)base);

It seems that the alarm logic in the driver/idpf is being continued in common/idpf, which can make the code messy. It would be better to wrap this as internal logic and expose API like "idpf_rx_timestamp_start/stop" in common/idpf for better organization and maintainability.

> +	}
> +
>  	ret = idpf_vc_vectors_alloc(vport, req_vecs_num);
>  	if (ret != 0) {
>  		PMD_DRV_LOG(ERR, "Failed to allocate interrupt vectors");
> @@ -810,6 +816,7 @@ static int  idpf_dev_stop(struct rte_eth_dev *dev)  {
>  	struct idpf_vport *vport = dev->data->dev_private;
> +	struct idpf_adapter *base = vport->adapter;
> 
>  	if (vport->stopped == 1)
>  		return 0;
> @@ -822,6 +829,11 @@ idpf_dev_stop(struct rte_eth_dev *dev)
> 
>  	idpf_vc_vectors_dealloc(vport);
> 
> +	if (dev->data->dev_conf.rxmode.offloads &
> RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
> +		rte_eal_alarm_cancel(idpf_dev_read_time_hw,
> +				     base);
> +	}
> +
>  	vport->stopped = 1;
> 
>  	return 0;
> --
> 2.25.1