DPDK patches and discussions
 help / color / mirror / Atom feed
From: "lihuisong (C)" <lihuisong@huawei.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	huangdengdui <huangdengdui@huawei.com>,
	Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
Cc: <ajit.khaparde@broadcom.com>, <roretzla@linux.microsoft.com>,
	Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>,
	<dev@dpdk.org>, <ferruh.yigit@amd.com>,
	<aman.deep.singh@intel.com>, <yuying.zhang@intel.com>,
	<andrew.rybchenko@oktetlabs.ru>, <stephen@networkplumber.org>,
	<jerinjacobk@gmail.com>, <liuyonglong@huawei.com>,
	<fengchengwen@huawei.com>, <haijie1@huawei.com>
Subject: Re: [PATCH v2 1/6] ethdev: support setting lanes
Date: Tue, 26 Mar 2024 09:42:05 +0800	[thread overview]
Message-ID: <68ee0a54-c0b4-293c-67ee-efed8964c33b@huawei.com> (raw)
In-Reply-To: <3325989.AxlXzFCzgd@thomas>


在 2024/3/25 17:30, Thomas Monjalon 写道:
> 25/03/2024 07:24, huangdengdui:
>> On 2024/3/22 21:58, Thomas Monjalon wrote:
>>> 22/03/2024 08:09, Dengdui Huang:
>>>> -#define RTE_ETH_LINK_SPEED_10G     RTE_BIT32(8)  /**< 10 Gbps */
>>>> -#define RTE_ETH_LINK_SPEED_20G     RTE_BIT32(9)  /**< 20 Gbps */
>>>> -#define RTE_ETH_LINK_SPEED_25G     RTE_BIT32(10) /**< 25 Gbps */
>>>> -#define RTE_ETH_LINK_SPEED_40G     RTE_BIT32(11) /**< 40 Gbps */
>>>> -#define RTE_ETH_LINK_SPEED_50G     RTE_BIT32(12) /**< 50 Gbps */
>>>> -#define RTE_ETH_LINK_SPEED_56G     RTE_BIT32(13) /**< 56 Gbps */
>>>> -#define RTE_ETH_LINK_SPEED_100G    RTE_BIT32(14) /**< 100 Gbps */
>>>> -#define RTE_ETH_LINK_SPEED_200G    RTE_BIT32(15) /**< 200 Gbps */
>>>> -#define RTE_ETH_LINK_SPEED_400G    RTE_BIT32(16) /**< 400 Gbps */
>>>> +#define RTE_ETH_LINK_SPEED_10G            RTE_BIT32(8)  /**< 10 Gbps */
>>>> +#define RTE_ETH_LINK_SPEED_20G            RTE_BIT32(9)  /**< 20 Gbps 2lanes */
>>>> +#define RTE_ETH_LINK_SPEED_25G            RTE_BIT32(10) /**< 25 Gbps */
>>>> +#define RTE_ETH_LINK_SPEED_40G            RTE_BIT32(11) /**< 40 Gbps 4lanes */
>>>> +#define RTE_ETH_LINK_SPEED_50G            RTE_BIT32(12) /**< 50 Gbps */
>>>> +#define RTE_ETH_LINK_SPEED_56G            RTE_BIT32(13) /**< 56 Gbps 4lanes */
>>>> +#define RTE_ETH_LINK_SPEED_100G           RTE_BIT32(14) /**< 100 Gbps */
>>>> +#define RTE_ETH_LINK_SPEED_200G           RTE_BIT32(15) /**< 200 Gbps 4lanes */
>>>> +#define RTE_ETH_LINK_SPEED_400G           RTE_BIT32(16) /**< 400 Gbps 4lanes */
>>>> +#define RTE_ETH_LINK_SPEED_10G_4LANES     RTE_BIT32(17)  /**< 10 Gbps 4lanes */
>>>> +#define RTE_ETH_LINK_SPEED_50G_2LANES     RTE_BIT32(18) /**< 50 Gbps 2 lanes */
>>>> +#define RTE_ETH_LINK_SPEED_100G_2LANES    RTE_BIT32(19) /**< 100 Gbps 2 lanes */
>>>> +#define RTE_ETH_LINK_SPEED_100G_4LANES    RTE_BIT32(20) /**< 100 Gbps 4lanes */
>>>> +#define RTE_ETH_LINK_SPEED_200G_2LANES    RTE_BIT32(21) /**< 200 Gbps 2lanes */
>>>> +#define RTE_ETH_LINK_SPEED_400G_8LANES    RTE_BIT32(22) /**< 400 Gbps 8lanes */
>>> I don't think it is a good idea to make this more complex.
>>> It brings nothing as far as I can see, compared to having speed and lanes separated.
>>> Can we have lanes information a separate value? no need for bitmask.
>>>
>> Hi,Thomas, Ajit, roretzla, damodharam
>>
>> I also considered the option at the beginning of the design.
>> But this option is not used due to the following reasons:
>> 1. For the user, ethtool couples speed and lanes.
>> The result of querying the NIC capability is as follows:
>> Supported link modes:
>>          100000baseSR4/Full
>>          100000baseSR2/Full
>> The NIC capability is configured as follows:
>> ethtool -s eth1 speed 100000 lanes 4 autoneg off
>> ethtool -s eth1 speed 100000 lanes 2 autoneg off
>>
>> Therefore, users are more accustomed to the coupling of speed and lanes.
>>
>> 2. For the PHY, When the physical layer capability is configured through the MDIO,
>> the speed and lanes are also coupled.
>> For example:
>> Table 45–7—PMA/PMD control 2 register bit definitions[1]
>> PMA/PMD type selection
>>                          1 0 0 1 0 1 0 = 100GBASE-SR2 PMA/PMD
>>                          0 1 0 1 1 1 1 = 100GBASE-SR4 PMA/PMD
>>
>> Therefore, coupling speeds and lanes is easier to understand.
>> And it is easier for the driver to report the support lanes.
>>
>> In addition, the code implementation is compatible with the old version.
>> When the driver does not support the lanes setting, the code does not need to be modified.
>>
>> So I think the speed and lanes coupling is better.
> I don't think so.
> You are mixing hardware implementation, user tool, and API.
> Having a separate and simple API is cleaner and not more difficult to handle
> in some get/set style functions.
Having a separate and simple API is cleaner. It's good.
But supported lane capabilities have a lot to do with the specified 
speed. This is determined by releated specification.
If we add a separate API for speed lanes, it probably is hard to check 
the validity of the configuration for speed and lanes.
And the setting lane API sepparated from speed is not good for 
uniforming all PMD's behavior in ethdev layer.

The patch[1] is an example for this separate API.
I think it is not very good. It cannot tell user and PMD the follow points:
1) user don't know what lanes should or can be set for a specified speed 
on one NIC.
2) how should PMD do for a supported lanes in their HW?

Anyway, if we add setting speed lanes feature, we must report and set 
speed and lanes capabilities for user well.
otherwise, user will be more confused.

[1] https://patchwork.dpdk.org/project/dpdk/list/?series=31606

BR,
/Huisong
>
>
>
> .

  parent reply	other threads:[~2024-03-26  1:42 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-12  7:52 [PATCH 0/3] " Dengdui Huang
2024-03-12  7:52 ` [PATCH 1/3] ethdev: " Dengdui Huang
2024-03-19  3:02   ` Stephen Hemminger
2024-03-20  1:16     ` huangdengdui
2024-03-12  7:52 ` [PATCH 2/3] net/hns3: " Dengdui Huang
2024-03-12  7:52 ` [PATCH 3/3] app/testpmd: " Dengdui Huang
2024-03-15 21:47   ` Damodharam Ammepalli
2024-03-19  2:38     ` huangdengdui
2024-03-22  7:09   ` [PATCH v2 0/6] " Dengdui Huang
2024-03-22  7:09     ` [PATCH v2 1/6] ethdev: " Dengdui Huang
2024-03-22 13:58       ` Thomas Monjalon
2024-03-22 15:15         ` Ajit Khaparde
2024-03-22 17:32           ` Tyler Retzlaff
2024-03-22 22:30             ` Damodharam Ammepalli
2024-03-25  6:24         ` huangdengdui
2024-03-25  9:30           ` Thomas Monjalon
2024-03-25 21:14             ` Damodharam Ammepalli
2024-03-26  1:42             ` lihuisong (C) [this message]
2024-03-26  3:45               ` Ajit Khaparde
2024-03-26 10:30               ` Thomas Monjalon
2024-03-26 11:15                 ` lihuisong (C)
2024-03-26 13:47                   ` Ajit Khaparde
2024-03-26 18:11                     ` Ajit Khaparde
2024-03-26 18:21                       ` Damodharam Ammepalli
2024-03-30 11:38                         ` huangdengdui
2024-04-01 20:07                           ` Thomas Monjalon
2024-04-01 22:29                             ` Damodharam Ammepalli
2024-04-02  8:37                             ` huangdengdui
2024-04-02 15:28                               ` Stephen Hemminger
2024-04-04 13:45                               ` Ferruh Yigit
2024-03-29  3:25                     ` lihuisong (C)
2024-03-22  7:09     ` [PATCH v2 2/6] test: updated UT for " Dengdui Huang
2024-03-22  7:09     ` [PATCH v2 3/6] ethdev: add function to parse link mode info Dengdui Huang
2024-03-22  7:09     ` [PATCH v2 4/6] net/hns3: use parse link mode info function Dengdui Huang
2024-03-22  7:09     ` [PATCH v2 5/6] net/hns3: support setting lanes Dengdui Huang
2024-03-22  7:09     ` [PATCH v2 6/6] app/testpmd: " Dengdui Huang
2024-04-04 13:58     ` [PATCH v2 0/6] " Ferruh Yigit
2024-05-16 12:48       ` huangdengdui
2024-03-18 14:56 ` [PATCH 0/3] " Thomas Monjalon
2024-03-18 21:26   ` Damodharam Ammepalli
2024-03-18 21:42     ` Stephen Hemminger
2024-03-18 22:55       ` Ajit Khaparde
2024-03-20 11:41     ` Thomas Monjalon
2024-03-20 12:31     ` Ferruh Yigit
2024-03-21  2:02       ` huangdengdui
2024-03-21  8:28         ` Thomas Monjalon
2024-03-22  2:28           ` huangdengdui
2024-03-22  4:38             ` Jerin Jacob
2024-03-22  5:25               ` Ajit Khaparde
2024-03-22  5:51                 ` Jerin Jacob
2024-03-22 13:51                   ` Thomas Monjalon
2024-03-25 14:04                     ` Jerin Jacob

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=68ee0a54-c0b4-293c-67ee-efed8964c33b@huawei.com \
    --to=lihuisong@huawei.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=aman.deep.singh@intel.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=damodharam.ammepalli@broadcom.com \
    --cc=dev@dpdk.org \
    --cc=fengchengwen@huawei.com \
    --cc=ferruh.yigit@amd.com \
    --cc=haijie1@huawei.com \
    --cc=huangdengdui@huawei.com \
    --cc=jerinjacobk@gmail.com \
    --cc=liuyonglong@huawei.com \
    --cc=roretzla@linux.microsoft.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    --cc=yuying.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).