Re: [dpdk-dev] [PATCH v5] net/ice: support IEEE 1588 PTP

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

> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Monday, September 27, 2021 4:28 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Su, Simei
> <simei.su@intel.com>
> Subject: [PATCH v5] net/ice: support IEEE 1588 PTP
> 
> Add ice support for new ethdev APIs to enable/disable and read/write/adjust
> IEEE1588 PTP timestamps. Currently, only scalar path supports 1588 PTP,
> vector path doesn't.
> 
> The example command for running ptpclient is as below:
> ./build/examples/dpdk-ptpclient -c 1 -n 3 -- -T 0 -p 0x1
> 
> Signed-off-by: Simei Su <simei.su@intel.com>
> ---
> v5:
> * Refine patch title and commit log.
> * Simplify judge logic in ice_timesync_enable and ice_program_hw_rx_queue.
> * Add flag reset in ice_timesync_disable.
> 
> v4:
> * Rework code to consider ice_dev_start and ice_timesync_enable order.
> 
> v3:
> * Rework code to support scalar path only.
> * Update the doc/guides/nics/features/ice.ini to add "Timesync" feature.
> * Add release notes.
> 
> v2:
> * Change patchset to one patch based on share code update.
> * Change per device offload to per queue offload.
> 
>  doc/guides/nics/features/ice.ini       |   1 +
>  doc/guides/rel_notes/release_21_11.rst |   1 +
>  drivers/net/ice/ice_ethdev.c           | 201
> ++++++++++++++++++++++++++++++++-
>  drivers/net/ice/ice_ethdev.h           |   6 +
>  drivers/net/ice/ice_rxtx.c             |  41 ++++++-
>  drivers/net/ice/ice_rxtx.h             |   1 +
>  6 files changed, 248 insertions(+), 3 deletions(-)
> 
......
> 
>  static int
> +ice_timesync_enable(struct rte_eth_dev *dev) {
> +	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct ice_adapter *ad =
> +			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> +	int ret;
> +
> +	if (hw->func_caps.ts_func_info.src_tmr_owned) {
> +		ret = ice_ptp_init_phc(hw);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR, "Failed to initialize PHC");
> +			return -1;
> +		}
> +
> +		ret = ice_ptp_write_incval(hw, ICE_PTP_NOMINAL_INCVAL_E810);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR,
> +				"Failed to write PHC increment time value");
> +			return -1;
> +		}
> +	}
> +
> +	/* Initialize cycle counters for system time/RX/TX timestamp */
> +	memset(&ad->systime_tc, 0, sizeof(struct rte_timecounter));
> +	memset(&ad->rx_tstamp_tc, 0, sizeof(struct rte_timecounter));
> +	memset(&ad->tx_tstamp_tc, 0, sizeof(struct rte_timecounter));
> +
> +	ad->systime_tc.cc_mask = ICE_CYCLECOUNTER_MASK;
> +	ad->systime_tc.cc_shift = 0;
> +	ad->systime_tc.nsec_mask = 0;
> +
> +	ad->rx_tstamp_tc.cc_mask = ICE_CYCLECOUNTER_MASK;
> +	ad->rx_tstamp_tc.cc_shift = 0;
> +	ad->rx_tstamp_tc.nsec_mask = 0;
> +
> +	ad->tx_tstamp_tc.cc_mask = ICE_CYCLECOUNTER_MASK;
> +	ad->tx_tstamp_tc.cc_shift = 0;
> +	ad->tx_tstamp_tc.nsec_mask = 0;
> +
> +	if (dev->data->dev_started && !(dev->data->dev_conf.rxmode.offloads &
> +	    DEV_RX_OFFLOAD_TIMESTAMP)) {
> +		PMD_DRV_LOG(ERR, "Rx timestamp offload not configured");
> +		return -1;
> +	} else {
> +		ad->ptp_ena = 1;
> +	}

No need "else" branch if already return in "if" branch.

> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c index
> bb75183..7089202 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -270,6 +270,7 @@ ice_program_hw_rx_queue(struct ice_rx_queue *rxq)
>  	struct rte_eth_rxmode *rxmode = &dev_data->dev_conf.rxmode;
>  	uint32_t rxdid = ICE_RXDID_COMMS_OVS;
>  	uint32_t regval;
> +	struct ice_adapter *ad = rxq->vsi->adapter;
> 
>  	/* Set buffer size as the head split is disabled. */
>  	buf_size = (uint16_t)(rte_pktmbuf_data_room_size(rxq->mp) - @@ -366,7
> +367,8 @@ ice_program_hw_rx_queue(struct ice_rx_queue *rxq)
>  	regval |= (0x03 << QRXFLXP_CNTXT_RXDID_PRIO_S) &
>  		QRXFLXP_CNTXT_RXDID_PRIO_M;
> 
> -	if (rxq->offloads & DEV_RX_OFFLOAD_TIMESTAMP)
> +	if ((!ad->ptp_ena && (rxq->offloads & DEV_RX_OFFLOAD_TIMESTAMP)) ||
> +	    ad->ptp_ena)

it can be simplified to ptp_ena || offloads & TIMESTAMP.