* [PATCH 0/3] some bugfixes for PTP
@ 2022-06-28 13:39 Dongdong Liu
2022-06-28 13:39 ` [PATCH 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Dongdong Liu @ 2022-06-28 13:39 UTC (permalink / raw)
To: dev, ferruh.yigit, andrew.rybchenko, thomas; +Cc: stable
The patchset include some bugfixes for PTP.
Huisong Li (3):
examples/ptpclient: add the check for PTP capability
net/hns3: fix fail to receive PTP packet
ethdev: add the check for the valitity of timestamp offload
drivers/net/hns3/hns3_ptp.c | 1 -
drivers/net/hns3/hns3_rxtx_vec.c | 22 +++++------
examples/ptpclient/ptpclient.c | 5 +++
lib/ethdev/rte_ethdev.c | 65 +++++++++++++++++++++++++++++++-
4 files changed, 79 insertions(+), 14 deletions(-)
--
2.22.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] examples/ptpclient: add the check for PTP capability 2022-06-28 13:39 [PATCH 0/3] some bugfixes for PTP Dongdong Liu @ 2022-06-28 13:39 ` Dongdong Liu 2022-06-28 13:39 ` [PATCH 2/3] net/hns3: fix fail to receive PTP packet Dongdong Liu ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Dongdong Liu @ 2022-06-28 13:39 UTC (permalink / raw) To: dev, ferruh.yigit, andrew.rybchenko, thomas; +Cc: stable From: Huisong Li <lihuisong@huawei.com> If a port doesn't support PTP, there is no need to keep running app. So this patch adds the check for PTP capability. Signe-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> --- examples/ptpclient/ptpclient.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c index 1f1c9c9c52..9689f42f86 100644 --- a/examples/ptpclient/ptpclient.c +++ b/examples/ptpclient/ptpclient.c @@ -195,6 +195,11 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool) if (dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_TIMESTAMP) port_conf.rxmode.offloads |= RTE_ETH_RX_OFFLOAD_TIMESTAMP; + else { + printf("port(%u) doesn't support PTP: %s\n", port, + strerror(-retval)); + return -ENOTSUP; + } if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) port_conf.txmode.offloads |= -- 2.22.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] net/hns3: fix fail to receive PTP packet 2022-06-28 13:39 [PATCH 0/3] some bugfixes for PTP Dongdong Liu 2022-06-28 13:39 ` [PATCH 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu @ 2022-06-28 13:39 ` Dongdong Liu 2022-06-28 13:39 ` [PATCH 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 0/3] some bugfixes for PTP Dongdong Liu 3 siblings, 0 replies; 10+ messages in thread From: Dongdong Liu @ 2022-06-28 13:39 UTC (permalink / raw) To: dev, ferruh.yigit, andrew.rybchenko, thomas; +Cc: stable From: Huisong Li <lihuisong@huawei.com> The Rx and Tx vector algorithm of hns3 PMD don't support PTP function. Currently, hns3 driver uses 'pf->ptp_enable' to check whether PTP is enabled so as to not select Rx and Tx vector algorithm. And the variable is set when call rte_eth_timesync_enable(). Namely, it may not be set before selecting Rx/Tx function, let's say the case: set PTP offload in dev_configure(), do dev_start() and then call rte_eth_timesync_enable(). In this case, all PTP packets can not be received to application. So this patch fixes the check based on the RTE_ETH_RX_OFFLOAD_TIMESTAMP flag. Fixes: 71f4d1aae11f ("net/hns3: fix vector burst unsupported when PTP capability set") Cc: stable@dpdk.org Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> --- drivers/net/hns3/hns3_ptp.c | 1 - drivers/net/hns3/hns3_rxtx_vec.c | 22 ++++++++++------------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/net/hns3/hns3_ptp.c b/drivers/net/hns3/hns3_ptp.c index 0b0061bba5..6bbd85ba23 100644 --- a/drivers/net/hns3/hns3_ptp.c +++ b/drivers/net/hns3/hns3_ptp.c @@ -125,7 +125,6 @@ hns3_timesync_enable(struct rte_eth_dev *dev) if (pf->ptp_enable) return 0; - hns3_warn(hw, "note: please ensure Rx/Tx burst mode is simple or common when enabling PTP!"); rte_spinlock_lock(&hw->lock); ret = hns3_timesync_configure(hns, true); diff --git a/drivers/net/hns3/hns3_rxtx_vec.c b/drivers/net/hns3/hns3_rxtx_vec.c index 73f0ab6bc8..22c087b9a7 100644 --- a/drivers/net/hns3/hns3_rxtx_vec.c +++ b/drivers/net/hns3/hns3_rxtx_vec.c @@ -17,15 +17,18 @@ int hns3_tx_check_vec_support(struct rte_eth_dev *dev) { struct rte_eth_txmode *txmode = &dev->data->dev_conf.txmode; - struct hns3_adapter *hns = dev->data->dev_private; - struct hns3_pf *pf = &hns->pf; + struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; /* Only support RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE */ if (txmode->offloads != RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) return -ENOTSUP; - /* Vec is not supported when PTP enabled */ - if (pf->ptp_enable) + /* + * PTP function requires the cooperation of Rx and Tx. + * Tx vector isn't supported if RTE_ETH_RX_OFFLOAD_TIMESTAMP is set + * in Rx offloads. + */ + if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) return -ENOTSUP; return 0; @@ -232,10 +235,9 @@ hns3_rx_check_vec_support(struct rte_eth_dev *dev) { struct rte_eth_fdir_conf *fconf = &dev->data->dev_conf.fdir_conf; struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; - uint64_t offloads_mask = RTE_ETH_RX_OFFLOAD_TCP_LRO | - RTE_ETH_RX_OFFLOAD_VLAN; - struct hns3_adapter *hns = dev->data->dev_private; - struct hns3_pf *pf = &hns->pf; + uint64_t offloads_mask = RTE_ETH_RX_OFFLOAD_TCP_LRO | \ + RTE_ETH_RX_OFFLOAD_VLAN | \ + RTE_ETH_RX_OFFLOAD_TIMESTAMP; if (dev->data->scattered_rx) return -ENOTSUP; @@ -249,9 +251,5 @@ hns3_rx_check_vec_support(struct rte_eth_dev *dev) if (hns3_rxq_iterate(dev, hns3_rxq_vec_check, NULL) != 0) return -ENOTSUP; - /* Vec is not supported when PTP enabled */ - if (pf->ptp_enable) - return -ENOTSUP; - return 0; } -- 2.22.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] ethdev: add the check for the valitity of timestamp offload 2022-06-28 13:39 [PATCH 0/3] some bugfixes for PTP Dongdong Liu 2022-06-28 13:39 ` [PATCH 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu 2022-06-28 13:39 ` [PATCH 2/3] net/hns3: fix fail to receive PTP packet Dongdong Liu @ 2022-06-28 13:39 ` Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 0/3] some bugfixes for PTP Dongdong Liu 3 siblings, 0 replies; 10+ messages in thread From: Dongdong Liu @ 2022-06-28 13:39 UTC (permalink / raw) To: dev, ferruh.yigit, andrew.rybchenko, thomas; +Cc: stable From: Huisong Li <lihuisong@huawei.com> This patch adds the check for the valitity of timestamp offload. Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> --- lib/ethdev/rte_ethdev.c | 65 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index 1979dc0850..9b8ba3a348 100644 --- a/lib/ethdev/rte_ethdev.c +++ b/lib/ethdev/rte_ethdev.c @@ -5167,15 +5167,48 @@ rte_eth_dev_set_mc_addr_list(uint16_t port_id, mc_addr_set, nb_mc_addr)); } +static int +rte_eth_timestamp_offload_valid(struct rte_eth_dev *dev) +{ + struct rte_eth_dev_info dev_info; + struct rte_eth_rxmode *rxmode; + int ret; + + ret = rte_eth_dev_info_get(dev->data->port_id, &dev_info); + if (ret != 0) { + RTE_ETHDEV_LOG(ERR, "Cannot get port (%u) device information.\n", + dev->data->port_id); + return ret; + } + + if ((dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_TIMESTAMP) == 0) { + RTE_ETHDEV_LOG(ERR, "Driver does not support PTP.\n"); + return -ENOTSUP; + } + + rxmode = &dev->data->dev_conf.rxmode; + if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) == 0) { + RTE_ETHDEV_LOG(ERR, "Please enable 'RTE_ETH_RX_OFFLOAD_TIMESTAMP' offload.\n"); + return -EINVAL; + } + + return 0; +} + int rte_eth_timesync_enable(uint16_t port_id) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_enable, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_enable)(dev)); } @@ -5183,11 +5216,15 @@ int rte_eth_timesync_disable(uint16_t port_id) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_disable, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; return eth_err(port_id, (*dev->dev_ops->timesync_disable)(dev)); } @@ -5196,6 +5233,7 @@ rte_eth_timesync_read_rx_timestamp(uint16_t port_id, struct timespec *timestamp, uint32_t flags) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -5207,7 +5245,12 @@ rte_eth_timesync_read_rx_timestamp(uint16_t port_id, struct timespec *timestamp, return -EINVAL; } - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_rx_timestamp, -ENOTSUP); + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_rx_timestamp, + -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_read_rx_timestamp) (dev, timestamp, flags)); } @@ -5217,6 +5260,7 @@ rte_eth_timesync_read_tx_timestamp(uint16_t port_id, struct timespec *timestamp) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -5229,6 +5273,10 @@ rte_eth_timesync_read_tx_timestamp(uint16_t port_id, } RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_tx_timestamp, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_read_tx_timestamp) (dev, timestamp)); } @@ -5237,11 +5285,16 @@ int rte_eth_timesync_adjust_time(uint16_t port_id, int64_t delta) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_adjust_time, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_adjust_time)(dev, delta)); } @@ -5249,6 +5302,7 @@ int rte_eth_timesync_read_time(uint16_t port_id, struct timespec *timestamp) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -5261,6 +5315,10 @@ rte_eth_timesync_read_time(uint16_t port_id, struct timespec *timestamp) } RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_time, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_read_time)(dev, timestamp)); } @@ -5269,6 +5327,7 @@ int rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *timestamp) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -5281,6 +5340,10 @@ rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *timestamp) } RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_write_time, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_write_time)(dev, timestamp)); } -- 2.22.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 0/3] some bugfixes for PTP 2022-06-28 13:39 [PATCH 0/3] some bugfixes for PTP Dongdong Liu ` (2 preceding siblings ...) 2022-06-28 13:39 ` [PATCH 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu @ 2022-07-02 8:17 ` Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu ` (2 more replies) 3 siblings, 3 replies; 10+ messages in thread From: Dongdong Liu @ 2022-07-02 8:17 UTC (permalink / raw) To: dev, ferruh.yigit, andrew.rybchenko, thomas; +Cc: stable, Dongdong Liu The patchset include some bugfixes for PTP. v1->v2: - fix the checkpatch warning. Huisong Li (3): examples/ptpclient: add the check for PTP capability net/hns3: fix fail to receive PTP packet ethdev: add the check for the valitity of timestamp offload drivers/net/hns3/hns3_ptp.c | 1 - drivers/net/hns3/hns3_rxtx_vec.c | 20 +++++----- examples/ptpclient/ptpclient.c | 5 +++ lib/ethdev/rte_ethdev.c | 65 +++++++++++++++++++++++++++++++- 4 files changed, 78 insertions(+), 13 deletions(-) -- 2.22.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] examples/ptpclient: add the check for PTP capability 2022-07-02 8:17 ` [PATCH v2 0/3] some bugfixes for PTP Dongdong Liu @ 2022-07-02 8:17 ` Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 2/3] net/hns3: fix fail to receive PTP packet Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu 2 siblings, 0 replies; 10+ messages in thread From: Dongdong Liu @ 2022-07-02 8:17 UTC (permalink / raw) To: dev, ferruh.yigit, andrew.rybchenko, thomas Cc: stable, Huisong Li, Dongdong Liu, Kirill Rybalchenko From: Huisong Li <lihuisong@huawei.com> If a port doesn't support PTP, there is no need to keep running app. So this patch adds the check for PTP capability. Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> --- examples/ptpclient/ptpclient.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c index 1f1c9c9c52..9689f42f86 100644 --- a/examples/ptpclient/ptpclient.c +++ b/examples/ptpclient/ptpclient.c @@ -195,6 +195,11 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool) if (dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_TIMESTAMP) port_conf.rxmode.offloads |= RTE_ETH_RX_OFFLOAD_TIMESTAMP; + else { + printf("port(%u) doesn't support PTP: %s\n", port, + strerror(-retval)); + return -ENOTSUP; + } if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) port_conf.txmode.offloads |= -- 2.22.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] net/hns3: fix fail to receive PTP packet 2022-07-02 8:17 ` [PATCH v2 0/3] some bugfixes for PTP Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu @ 2022-07-02 8:17 ` Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu 2 siblings, 0 replies; 10+ messages in thread From: Dongdong Liu @ 2022-07-02 8:17 UTC (permalink / raw) To: dev, ferruh.yigit, andrew.rybchenko, thomas Cc: stable, Huisong Li, Dongdong Liu, Yisen Zhuang, Min Hu (Connor) From: Huisong Li <lihuisong@huawei.com> The Rx and Tx vector algorithm of hns3 PMD don't support PTP function. Currently, hns3 driver uses 'pf->ptp_enable' to check whether PTP is enabled so as to not select Rx and Tx vector algorithm. And the variable is set when call rte_eth_timesync_enable(). Namely, it may not be set before selecting Rx/Tx function, let's say the case: set PTP offload in dev_configure(), do dev_start() and then call rte_eth_timesync_enable(). In this case, all PTP packets can not be received to application. So this patch fixes the check based on the RTE_ETH_RX_OFFLOAD_TIMESTAMP flag. Fixes: 71f4d1aae11f ("net/hns3: fix vector burst unsupported when PTP capability set") Cc: stable@dpdk.org Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> --- drivers/net/hns3/hns3_ptp.c | 1 - drivers/net/hns3/hns3_rxtx_vec.c | 20 +++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/drivers/net/hns3/hns3_ptp.c b/drivers/net/hns3/hns3_ptp.c index 0b0061bba5..6bbd85ba23 100644 --- a/drivers/net/hns3/hns3_ptp.c +++ b/drivers/net/hns3/hns3_ptp.c @@ -125,7 +125,6 @@ hns3_timesync_enable(struct rte_eth_dev *dev) if (pf->ptp_enable) return 0; - hns3_warn(hw, "note: please ensure Rx/Tx burst mode is simple or common when enabling PTP!"); rte_spinlock_lock(&hw->lock); ret = hns3_timesync_configure(hns, true); diff --git a/drivers/net/hns3/hns3_rxtx_vec.c b/drivers/net/hns3/hns3_rxtx_vec.c index 73f0ab6bc8..153866cf03 100644 --- a/drivers/net/hns3/hns3_rxtx_vec.c +++ b/drivers/net/hns3/hns3_rxtx_vec.c @@ -17,15 +17,18 @@ int hns3_tx_check_vec_support(struct rte_eth_dev *dev) { struct rte_eth_txmode *txmode = &dev->data->dev_conf.txmode; - struct hns3_adapter *hns = dev->data->dev_private; - struct hns3_pf *pf = &hns->pf; + struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; /* Only support RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE */ if (txmode->offloads != RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) return -ENOTSUP; - /* Vec is not supported when PTP enabled */ - if (pf->ptp_enable) + /* + * PTP function requires the cooperation of Rx and Tx. + * Tx vector isn't supported if RTE_ETH_RX_OFFLOAD_TIMESTAMP is set + * in Rx offloads. + */ + if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) return -ENOTSUP; return 0; @@ -233,9 +236,8 @@ hns3_rx_check_vec_support(struct rte_eth_dev *dev) struct rte_eth_fdir_conf *fconf = &dev->data->dev_conf.fdir_conf; struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; uint64_t offloads_mask = RTE_ETH_RX_OFFLOAD_TCP_LRO | - RTE_ETH_RX_OFFLOAD_VLAN; - struct hns3_adapter *hns = dev->data->dev_private; - struct hns3_pf *pf = &hns->pf; + RTE_ETH_RX_OFFLOAD_VLAN | + RTE_ETH_RX_OFFLOAD_TIMESTAMP; if (dev->data->scattered_rx) return -ENOTSUP; @@ -249,9 +251,5 @@ hns3_rx_check_vec_support(struct rte_eth_dev *dev) if (hns3_rxq_iterate(dev, hns3_rxq_vec_check, NULL) != 0) return -ENOTSUP; - /* Vec is not supported when PTP enabled */ - if (pf->ptp_enable) - return -ENOTSUP; - return 0; } -- 2.22.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload 2022-07-02 8:17 ` [PATCH v2 0/3] some bugfixes for PTP Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 2/3] net/hns3: fix fail to receive PTP packet Dongdong Liu @ 2022-07-02 8:17 ` Dongdong Liu 2022-07-06 14:57 ` Andrew Rybchenko 2 siblings, 1 reply; 10+ messages in thread From: Dongdong Liu @ 2022-07-02 8:17 UTC (permalink / raw) To: dev, ferruh.yigit, andrew.rybchenko, thomas Cc: stable, Huisong Li, Dongdong Liu From: Huisong Li <lihuisong@huawei.com> This patch adds the check for the valitity of timestamp offload. Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> --- lib/ethdev/rte_ethdev.c | 65 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index 1979dc0850..9b8ba3a348 100644 --- a/lib/ethdev/rte_ethdev.c +++ b/lib/ethdev/rte_ethdev.c @@ -5167,15 +5167,48 @@ rte_eth_dev_set_mc_addr_list(uint16_t port_id, mc_addr_set, nb_mc_addr)); } +static int +rte_eth_timestamp_offload_valid(struct rte_eth_dev *dev) +{ + struct rte_eth_dev_info dev_info; + struct rte_eth_rxmode *rxmode; + int ret; + + ret = rte_eth_dev_info_get(dev->data->port_id, &dev_info); + if (ret != 0) { + RTE_ETHDEV_LOG(ERR, "Cannot get port (%u) device information.\n", + dev->data->port_id); + return ret; + } + + if ((dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_TIMESTAMP) == 0) { + RTE_ETHDEV_LOG(ERR, "Driver does not support PTP.\n"); + return -ENOTSUP; + } + + rxmode = &dev->data->dev_conf.rxmode; + if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) == 0) { + RTE_ETHDEV_LOG(ERR, "Please enable 'RTE_ETH_RX_OFFLOAD_TIMESTAMP' offload.\n"); + return -EINVAL; + } + + return 0; +} + int rte_eth_timesync_enable(uint16_t port_id) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_enable, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_enable)(dev)); } @@ -5183,11 +5216,15 @@ int rte_eth_timesync_disable(uint16_t port_id) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_disable, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; return eth_err(port_id, (*dev->dev_ops->timesync_disable)(dev)); } @@ -5196,6 +5233,7 @@ rte_eth_timesync_read_rx_timestamp(uint16_t port_id, struct timespec *timestamp, uint32_t flags) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -5207,7 +5245,12 @@ rte_eth_timesync_read_rx_timestamp(uint16_t port_id, struct timespec *timestamp, return -EINVAL; } - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_rx_timestamp, -ENOTSUP); + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_rx_timestamp, + -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_read_rx_timestamp) (dev, timestamp, flags)); } @@ -5217,6 +5260,7 @@ rte_eth_timesync_read_tx_timestamp(uint16_t port_id, struct timespec *timestamp) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -5229,6 +5273,10 @@ rte_eth_timesync_read_tx_timestamp(uint16_t port_id, } RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_tx_timestamp, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_read_tx_timestamp) (dev, timestamp)); } @@ -5237,11 +5285,16 @@ int rte_eth_timesync_adjust_time(uint16_t port_id, int64_t delta) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_adjust_time, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_adjust_time)(dev, delta)); } @@ -5249,6 +5302,7 @@ int rte_eth_timesync_read_time(uint16_t port_id, struct timespec *timestamp) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -5261,6 +5315,10 @@ rte_eth_timesync_read_time(uint16_t port_id, struct timespec *timestamp) } RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_time, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_read_time)(dev, timestamp)); } @@ -5269,6 +5327,7 @@ int rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *timestamp) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -5281,6 +5340,10 @@ rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *timestamp) } RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_write_time, -ENOTSUP); + ret = rte_eth_timestamp_offload_valid(dev); + if (ret != 0) + return ret; + return eth_err(port_id, (*dev->dev_ops->timesync_write_time)(dev, timestamp)); } -- 2.22.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload 2022-07-02 8:17 ` [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu @ 2022-07-06 14:57 ` Andrew Rybchenko 2022-07-07 2:05 ` lihuisong (C) 0 siblings, 1 reply; 10+ messages in thread From: Andrew Rybchenko @ 2022-07-06 14:57 UTC (permalink / raw) To: Dongdong Liu, dev, ferruh.yigit, thomas; +Cc: stable, Huisong Li On 7/2/22 11:17, Dongdong Liu wrote: > From: Huisong Li <lihuisong@huawei.com> > > This patch adds the check for the valitity of timestamp offload. > > Signed-off-by: Huisong Li <lihuisong@huawei.com> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> > --- > lib/ethdev/rte_ethdev.c | 65 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 64 insertions(+), 1 deletion(-) > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > index 1979dc0850..9b8ba3a348 100644 > --- a/lib/ethdev/rte_ethdev.c > +++ b/lib/ethdev/rte_ethdev.c > @@ -5167,15 +5167,48 @@ rte_eth_dev_set_mc_addr_list(uint16_t port_id, > mc_addr_set, nb_mc_addr)); > } > > +static int > +rte_eth_timestamp_offload_valid(struct rte_eth_dev *dev) > +{ > + struct rte_eth_dev_info dev_info; > + struct rte_eth_rxmode *rxmode; > + int ret; > + > + ret = rte_eth_dev_info_get(dev->data->port_id, &dev_info); > + if (ret != 0) { > + RTE_ETHDEV_LOG(ERR, "Cannot get port (%u) device information.\n", > + dev->data->port_id); > + return ret; > + } > + > + if ((dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_TIMESTAMP) == 0) { As I understand strictly speaking the Rx offload is not the same as PTP support. May be it is a corner case, but I can imagine case when HW cannot provide timestamp for each packet (lack of space in extra information associated with a packet), but can return timestamps out-of-band using timestamp get API. I have no strong opinion. May be we are not interested in the corner case, but I think it requires acks from maintainers of all drivers which support PTP. > + RTE_ETHDEV_LOG(ERR, "Driver does not support PTP.\n"); > + return -ENOTSUP; > + } > + > + rxmode = &dev->data->dev_conf.rxmode; > + if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) == 0) { > + RTE_ETHDEV_LOG(ERR, "Please enable 'RTE_ETH_RX_OFFLOAD_TIMESTAMP' offload.\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + > int > rte_eth_timesync_enable(uint16_t port_id) > { > struct rte_eth_dev *dev; > + int ret; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_enable, -ENOTSUP); > + ret = rte_eth_timestamp_offload_valid(dev); > + if (ret != 0) > + return ret; > + Typically ops pointer check is done just before usage. So, it is better to avoid adding code between check and usage. Same in all cases below. if there is a good reason to do so, please, explain it. > return eth_err(port_id, (*dev->dev_ops->timesync_enable)(dev)); > } > [snip] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload 2022-07-06 14:57 ` Andrew Rybchenko @ 2022-07-07 2:05 ` lihuisong (C) 0 siblings, 0 replies; 10+ messages in thread From: lihuisong (C) @ 2022-07-07 2:05 UTC (permalink / raw) To: Andrew Rybchenko, Dongdong Liu, dev, ferruh.yigit, thomas; +Cc: stable 在 2022/7/6 22:57, Andrew Rybchenko 写道: > On 7/2/22 11:17, Dongdong Liu wrote: >> From: Huisong Li <lihuisong@huawei.com> >> >> This patch adds the check for the valitity of timestamp offload. >> >> Signed-off-by: Huisong Li <lihuisong@huawei.com> >> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> >> --- >> lib/ethdev/rte_ethdev.c | 65 ++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 64 insertions(+), 1 deletion(-) >> >> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >> index 1979dc0850..9b8ba3a348 100644 >> --- a/lib/ethdev/rte_ethdev.c >> +++ b/lib/ethdev/rte_ethdev.c >> @@ -5167,15 +5167,48 @@ rte_eth_dev_set_mc_addr_list(uint16_t port_id, >> mc_addr_set, nb_mc_addr)); >> } >> +static int >> +rte_eth_timestamp_offload_valid(struct rte_eth_dev *dev) >> +{ >> + struct rte_eth_dev_info dev_info; >> + struct rte_eth_rxmode *rxmode; >> + int ret; >> + >> + ret = rte_eth_dev_info_get(dev->data->port_id, &dev_info); >> + if (ret != 0) { >> + RTE_ETHDEV_LOG(ERR, "Cannot get port (%u) device >> information.\n", >> + dev->data->port_id); >> + return ret; >> + } >> + >> + if ((dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_TIMESTAMP) == >> 0) { > > As I understand strictly speaking the Rx offload is not the same as PTP > support. May be it is a corner case, but I can imagine case when HW > cannot provide timestamp for each packet (lack of space in extra > information associated with a packet), but can return timestamps > out-of-band using timestamp get API. > Generally speaking, Rx offload is a data plane thing and done in hardware. If hardware cannot provide timestamp for each packet, and can implement PTP only by using timestamp APIs. Can this corner case still be classified as Rx offload? If so, It is more of a control plane thing, like MTU. On the other hand, this offload is a capability obtained from driver. If driver doesn't support PTP, the ethdev layer should block them in timestamp APIs. > I have no strong opinion. May be we are not interested in the corner > case, but I think it requires acks from maintainers of all drivers > which support PTP. > >> + RTE_ETHDEV_LOG(ERR, "Driver does not support PTP.\n"); >> + return -ENOTSUP; >> + } >> + >> + rxmode = &dev->data->dev_conf.rxmode; >> + if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) == 0) { >> + RTE_ETHDEV_LOG(ERR, "Please enable >> 'RTE_ETH_RX_OFFLOAD_TIMESTAMP' offload.\n"); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> int >> rte_eth_timesync_enable(uint16_t port_id) >> { >> struct rte_eth_dev *dev; >> + int ret; >> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> dev = &rte_eth_devices[port_id]; >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_enable, -ENOTSUP); >> + ret = rte_eth_timestamp_offload_valid(dev); >> + if (ret != 0) >> + return ret; >> + > > Typically ops pointer check is done just before usage. So, it is > better to avoid adding code between check and usage. > Same in all cases below. > if there is a good reason to do so, please, explain it. A coin has two sides. Both styles exist in the ethdev layer API. There is no need to check input configuration if driver doesn't support the API. I am ok for them. > >> return eth_err(port_id, (*dev->dev_ops->timesync_enable)(dev)); >> } > > [snip] > . ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-07-07 2:05 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-28 13:39 [PATCH 0/3] some bugfixes for PTP Dongdong Liu 2022-06-28 13:39 ` [PATCH 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu 2022-06-28 13:39 ` [PATCH 2/3] net/hns3: fix fail to receive PTP packet Dongdong Liu 2022-06-28 13:39 ` [PATCH 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 0/3] some bugfixes for PTP Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 2/3] net/hns3: fix fail to receive PTP packet Dongdong Liu 2022-07-02 8:17 ` [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu 2022-07-06 14:57 ` Andrew Rybchenko 2022-07-07 2:05 ` lihuisong (C)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).