diff mbox series

[v3,1/2] examples/ptpclient: add the check for PTP capability

Message ID 20230817084226.55327-2-lihuisong@huawei.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: add the check for PTP capability |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

lihuisong (C) Aug. 17, 2023, 8:42 a.m. UTC 
  If a port doesn't support PTP, there is no need to keep running
app. So this patch adds the check for PTP capability.

Fixes: 7a04a4f67dca ("examples/ptpclient: enable Rx timestamp offload")
Cc: stable@dpdk.org

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(+)

Comments

Ferruh Yigit Sept. 15, 2023, 5:29 p.m. UTC | #1 
On 8/17/2023 9:42 AM, Huisong Li wrote:
> If a port doesn't support PTP, there is no need to keep running
> app. So this patch adds the check for PTP capability.
> 
> Fixes: 7a04a4f67dca ("examples/ptpclient: enable Rx timestamp offload")
> Cc: stable@dpdk.org
> 
> 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 cdf2da64df..181d8fb357 100644
> --- a/examples/ptpclient/ptpclient.c
> +++ b/examples/ptpclient/ptpclient.c
> @@ -196,6 +196,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;
> +	}
>  

I am not sure why TIMESTAMP offload is required for PTP, I think there
is a confusion.


Gagandeep, Hemant,
Can you please clarify why TIMESTAMP offload is enabled?
lihuisong (C) Sept. 21, 2023, 9:18 a.m. UTC | #2 
在 2023/9/16 1:29, Ferruh Yigit 写道:
> On 8/17/2023 9:42 AM, Huisong Li wrote:
>> If a port doesn't support PTP, there is no need to keep running
>> app. So this patch adds the check for PTP capability.
>>
>> Fixes: 7a04a4f67dca ("examples/ptpclient: enable Rx timestamp offload")
>> Cc: stable@dpdk.org
>>
>> 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 cdf2da64df..181d8fb357 100644
>> --- a/examples/ptpclient/ptpclient.c
>> +++ b/examples/ptpclient/ptpclient.c
>> @@ -196,6 +196,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;
>> +	}
>>   
> I am not sure why TIMESTAMP offload is required for PTP, I think there
> is a confusion.
If TIMESTAMP offload is not required for PTP, there isn't PTP offload in 
ethdev lib.
>
>
> Gagandeep, Hemant,
> Can you please clarify why TIMESTAMP offload is enabled?
looking forward to your reply.
>
> .
Ferruh Yigit Sept. 21, 2023, 11:02 a.m. UTC | #3 
On 9/21/2023 10:18 AM, lihuisong (C) wrote:
> 
> 在 2023/9/16 1:29, Ferruh Yigit 写道:
>> On 8/17/2023 9:42 AM, Huisong Li wrote:
>>> If a port doesn't support PTP, there is no need to keep running
>>> app. So this patch adds the check for PTP capability.
>>>
>>> Fixes: 7a04a4f67dca ("examples/ptpclient: enable Rx timestamp offload")
>>> Cc: stable@dpdk.org
>>>
>>> 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 cdf2da64df..181d8fb357 100644
>>> --- a/examples/ptpclient/ptpclient.c
>>> +++ b/examples/ptpclient/ptpclient.c
>>> @@ -196,6 +196,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;
>>> +    }
>>>   
>> I am not sure why TIMESTAMP offload is required for PTP, I think there
>> is a confusion.
> If TIMESTAMP offload is not required for PTP, there isn't PTP offload in
> ethdev lib.
>

What do you mean with "PTP offload"?

If you check the ptpclient sample app, it parses ptp packets in the
application.


>>
>>
>> Gagandeep, Hemant,
>> Can you please clarify why TIMESTAMP offload is enabled?
> looking forward to your reply.
>>
>> .
Hemant Agrawal Sept. 21, 2023, 11:22 a.m. UTC | #4 
> 
> On 9/21/2023 10:18 AM, lihuisong (C) wrote:
> >
> > 在 2023/9/16 1:29, Ferruh Yigit 写道:
> >> On 8/17/2023 9:42 AM, Huisong Li wrote:
> >>> If a port doesn't support PTP, there is no need to keep running app.
> >>> So this patch adds the check for PTP capability.
> >>>
> >>> Fixes: 7a04a4f67dca ("examples/ptpclient: enable Rx timestamp
> >>> offload")
> >>> Cc: stable@dpdk.org
> >>>
> >>> 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 cdf2da64df..181d8fb357 100644
> >>> --- a/examples/ptpclient/ptpclient.c
> >>> +++ b/examples/ptpclient/ptpclient.c
> >>> @@ -196,6 +196,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;
> >>> +    }
> >>>
> >> I am not sure why TIMESTAMP offload is required for PTP, I think
> >> there is a confusion.
> > If TIMESTAMP offload is not required for PTP, there isn't PTP offload
> > in ethdev lib.
> >
> 
> What do you mean with "PTP offload"?
> 
> If you check the ptpclient sample app, it parses ptp packets in the application.

> 
> 
> >>
> >>
> >> Gagandeep, Hemant,
> >> Can you please clarify why TIMESTAMP offload is enabled?
> > looking forward to your reply.

[Hemant] as explained in other mail, it is a requirement for dpaa2. So, we are just passing the offload argument.

Well, currently there is no such offload to know HW PTP support in DPDK. It can be introduced. 

And I agree the above else should not be there atleast w.r.t TIMESTAMP OFFLOAD.

> >> .
lihuisong (C) Sept. 21, 2023, 11:36 a.m. UTC | #5 
在 2023/9/21 19:02, Ferruh Yigit 写道:
> On 9/21/2023 10:18 AM, lihuisong (C) wrote:
>> 在 2023/9/16 1:29, Ferruh Yigit 写道:
>>> On 8/17/2023 9:42 AM, Huisong Li wrote:
>>>> If a port doesn't support PTP, there is no need to keep running
>>>> app. So this patch adds the check for PTP capability.
>>>>
>>>> Fixes: 7a04a4f67dca ("examples/ptpclient: enable Rx timestamp offload")
>>>> Cc: stable@dpdk.org
>>>>
>>>> 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 cdf2da64df..181d8fb357 100644
>>>> --- a/examples/ptpclient/ptpclient.c
>>>> +++ b/examples/ptpclient/ptpclient.c
>>>> @@ -196,6 +196,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;
>>>> +    }
>>>>    
>>> I am not sure why TIMESTAMP offload is required for PTP, I think there
>>> is a confusion.
>> If TIMESTAMP offload is not required for PTP, there isn't PTP offload in
>> ethdev lib.
>>
> What do you mean with "PTP offload"?
Yes, I mean we need like a PTP offload.
>
> If you check the ptpclient sample app, it parses ptp packets in the
> application.
App did parse PTP packet already.
What is the relationship between this and the verification in the app?
>
>
>>>
>>> Gagandeep, Hemant,
>>> Can you please clarify why TIMESTAMP offload is enabled?
>> looking forward to your reply.
>>> .
> .
lihuisong (C) Oct. 20, 2023, 4:05 a.m. UTC | #6 
Hi Hemant and Ferruh,

在 2023/9/21 19:22, Hemant Agrawal 写道:
>> On 9/21/2023 10:18 AM, lihuisong (C) wrote:
>>> 在 2023/9/16 1:29, Ferruh Yigit 写道:
>>>> On 8/17/2023 9:42 AM, Huisong Li wrote:
>>>>> If a port doesn't support PTP, there is no need to keep running app.
>>>>> So this patch adds the check for PTP capability.
>>>>>
>>>>> Fixes: 7a04a4f67dca ("examples/ptpclient: enable Rx timestamp
>>>>> offload")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> 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 cdf2da64df..181d8fb357 100644
>>>>> --- a/examples/ptpclient/ptpclient.c
>>>>> +++ b/examples/ptpclient/ptpclient.c
>>>>> @@ -196,6 +196,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;
>>>>> +    }
>>>>>
>>>> I am not sure why TIMESTAMP offload is required for PTP, I think
>>>> there is a confusion.
>>> If TIMESTAMP offload is not required for PTP, there isn't PTP offload
>>> in ethdev lib.
>>>
>> What do you mean with "PTP offload"?
>>
>> If you check the ptpclient sample app, it parses ptp packets in the application.
>>
>>>>
>>>> Gagandeep, Hemant,
>>>> Can you please clarify why TIMESTAMP offload is enabled?
>>> looking forward to your reply.
> [Hemant] as explained in other mail, it is a requirement for dpaa2. So, we are just passing the offload argument.
>
> Well, currently there is no such offload to know HW PTP support in DPDK. It can be introduced.
I agree with you, Heman.
>
> And I agree the above else should not be there atleast w.r.t TIMESTAMP OFFLOAD.

Ack.

@Ferruh, I wonder what you think. Looking forward to your reply.

>
>>>> .
diff mbox series

Patch

diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index cdf2da64df..181d8fb357 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -196,6 +196,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 |=