From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 45AA1A0543 for ; Wed, 6 Jul 2022 16:57:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3C66F40DF7; Wed, 6 Jul 2022 16:57:38 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 590BA40156; Wed, 6 Jul 2022 16:57:35 +0200 (CEST) Received: from [192.168.1.38] (unknown [188.170.75.69]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 6A75AF1; Wed, 6 Jul 2022 17:57:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 6A75AF1 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1657119454; bh=zZNu/JGQSk/UigBWkJNZtY+kRxrg+AyL1xjGrQrHJXU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=T5hfReuOAhSkCVTIf/debKskocKLSRwl4EGb8+cMbbICnDX7of+MiILUj+iUFe6vc 7pYmDlFr1cVcViHnEREvqEIavy/nuR6f3NBYmHpR3DYni5Jd/caVWLkDYx+362xwGP 47GBWwvoPm2DYFStk+OYwEgemBvXgecKkIOfKp3o= Message-ID: Date: Wed, 6 Jul 2022 17:57:33 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload Content-Language: en-US To: Dongdong Liu , dev@dpdk.org, ferruh.yigit@xilinx.com, thomas@monjalon.net Cc: stable@dpdk.org, Huisong Li References: <20220628133959.21381-1-liudongdong3@huawei.com> <20220702081730.1168-1-liudongdong3@huawei.com> <20220702081730.1168-4-liudongdong3@huawei.com> From: Andrew Rybchenko In-Reply-To: <20220702081730.1168-4-liudongdong3@huawei.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On 7/2/22 11:17, Dongdong Liu wrote: > From: Huisong Li > > This patch adds the check for the valitity of timestamp offload. > > Signed-off-by: Huisong Li > Signed-off-by: Dongdong Liu > --- > 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]