Re: [dpdk-dev] [PATCH 2/2] net/hns3: support IEEE 1588 PTP

["Min Hu (Connor)" <humin29@huawei.com>]

在 2021/3/31 17:26, Ferruh Yigit 写道:
> On 3/31/2021 8:28 AM, Thomas Monjalon wrote:
>> 31/03/2021 04:35, Min Hu (Connor):
>>> 在 2021/3/30 21:59, Ferruh Yigit 写道:
>>>> On 3/26/2021 8:56 AM, Min Hu (Connor) wrote:
>>>>> Add hns3 support for new ethdev APIs to enable and read IEEE1588/
>>>>> 802.1AS PTP timestamps.
>>>>>
>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>> --- a/drivers/net/hns3/hns3_cmd.h
>>>>> +++ b/drivers/net/hns3/hns3_cmd.h
>>>>> @@ -123,6 +123,12 @@ enum hns3_opcode_type {
>>>>>        HNS3_OPC_CLEAR_MAC_TNL_INT      = 0x0312,
>>>>>        HNS3_OPC_CONFIG_FEC_MODE        = 0x031A,
>>>>> +#ifdef RTE_LIBRTE_IEEE1588
>>>>> +    /* PTP command */
>>>>> +    HNS3_OPC_PTP_INT_EN             = 0x0501,
>>>>> +    HNS3_OPC_CFG_PTP_MODE           = 0x0507,
>>>>> +#endif
>>>>> +
>>>>
>>>> Hi Connor,
>>>>
>>>> Does it needs to be a compile time configuration? What happens if it is
>>>> always enabled, or controlled by device argument?
>>>> .
>>> Hi Ferruh,
>>>     Firstly the "RTE_LIBRTE_IEEE1588" origins from the config file in 
>>> DPDK.
>>> Almost every nic driver use this macro in compile time.
>>>     For me, I think using this macro give one option for users to
>>> decide if his APPs contains this module. For example, in loT field,
>>> some microprocessor has small memory or small disk, So the APPs should
>>> be as small as possible. So, if user does not need "PTP", the APPs no
>>> need to contain it.
>>>     Well, another top, if is always enabled, for HNS3 PMD, it will
>>> work well for our nic. If user want to use "PTP", just call API. If user
>>> does not use it, it also doesn't matter. But we advise that if user
>>> don't need this function, just turn it off.
>>>     Thanks.
>>
>> Disabling at compile-time does not reduce the footprint significantly.
>> RTE_LIBRTE_IEEE1588 should disappear, so I advise not using it
>> in new code. Instead, you could enable/disable at runtime if needed.
>>
> 
> I am aware that 'RTE_LIBRTE_IEEE1588' already exists and used by some 
> drivers, but as you said they are from times we had a config option for 
> it, for the new support I believe it is better to have runtime 
> configuration.
> 
> And I agree with Thomas that it shouldn't increase the footprint much to 
> always compile the support in, if you have numbers please share.
> 
Well, thanks Thomas, Ferruh,
If there is no help to reduce the footprint significantly, I agree to
delete "RTE_LIBRTE_IEEE1588". As for runtime config, there is no need to
enable or disable PTP. Because We support the function, if user want to 
use it, just call related APIs, if not, don't use it.

So, I will delete the "RTE_LIBRTE_IEEE1588" in v3, what's your opinion?
.