DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: "Xueming(Steven) Li" <xuemingl@nvidia.com>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Somnath Kotur <somnath.kotur@broadcom.com>,
	John Daley <johndale@cisco.com>,
	Hyong Youb Kim <hyonkim@cisco.com>,
	Beilei Xing <beilei.xing@intel.com>,
	Qiming Yang <qiming.yang@intel.com>,
	Qi Zhang <qi.z.zhang@intel.com>,
	Haiyue Wang <haiyue.wang@intel.com>,
	Matan Azrad <matan@nvidia.com>,
	Shahaf Shuler <shahafs@nvidia.com>,
	Slava Ovsiienko <viacheslavo@nvidia.com>,
	NBU-Contact-Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] ethdev: fix representor port ID search by name
Date: Tue, 20 Jul 2021 11:59:31 +0300	[thread overview]
Message-ID: <d62e6234-fb71-baa7-b5d5-d0ccdef19425@oktetlabs.ru> (raw)
In-Reply-To: <DM4PR12MB5373AC091379D0D71B9DAED0A1E19@DM4PR12MB5373.namprd12.prod.outlook.com>

On 7/19/21 3:50 PM, Xueming(Steven) Li wrote:
> 
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, July 19, 2021 8:36 PM
>> To: Xueming(Steven) Li <xuemingl@nvidia.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>; Somnath Kotur
>> <somnath.kotur@broadcom.com>; John Daley <johndale@cisco.com>; Hyong Youb Kim <hyonkim@cisco.com>; Beilei Xing
>> <beilei.xing@intel.com>; Qiming Yang <qiming.yang@intel.com>; Qi Zhang <qi.z.zhang@intel.com>; Haiyue Wang
>> <haiyue.wang@intel.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Slava Ovsiienko
>> <viacheslavo@nvidia.com>; NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>
>> Cc: dev@dpdk.org; Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>; stable@dpdk.org
>> Subject: Re: [PATCH] ethdev: fix representor port ID search by name
>>
>> On 7/19/21 2:54 PM, Xueming(Steven) Li wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Monday, July 19, 2021 4:46 PM
>>>> To: Xueming(Steven) Li <xuemingl@nvidia.com>; Ajit Khaparde
>>>> <ajit.khaparde@broadcom.com>; Somnath Kotur
>>>> <somnath.kotur@broadcom.com>; John Daley <johndale@cisco.com>; Hyong
>>>> Youb Kim <hyonkim@cisco.com>; Beilei Xing <beilei.xing@intel.com>;
>>>> Qiming Yang <qiming.yang@intel.com>; Qi Zhang <qi.z.zhang@intel.com>;
>>>> Haiyue Wang <haiyue.wang@intel.com>; Matan Azrad <matan@nvidia.com>;
>>>> Shahaf Shuler <shahafs@nvidia.com>; Slava Ovsiienko
>>>> <viacheslavo@nvidia.com>; NBU-Contact-Thomas Monjalon
>>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Cc: dev@dpdk.org; Viacheslav Galaktionov
>>>> <viacheslav.galaktionov@oktetlabs.ru>; stable@dpdk.org
>>>> Subject: Re: [PATCH] ethdev: fix representor port ID search by name
>>>>
>>>> On 7/19/21 9:58 AM, Xueming(Steven) Li wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>>> Sent: Tuesday, July 13, 2021 12:18 AM
>>>>>> To: Ajit Khaparde <ajit.khaparde@broadcom.com>; Somnath Kotur
>>>>>> <somnath.kotur@broadcom.com>; John Daley <johndale@cisco.com>;
>>>>>> Hyong Youb Kim <hyonkim@cisco.com>; Beilei Xing
>>>>>> <beilei.xing@intel.com>; Qiming Yang <qiming.yang@intel.com>; Qi
>>>>>> Zhang <qi.z.zhang@intel.com>; Haiyue Wang <haiyue.wang@intel.com>;
>>>>>> Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
>>>>>> Slava Ovsiienko <viacheslavo@nvidia.com>; NBU-Contact-Thomas
>>>>>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
>>>>>> <ferruh.yigit@intel.com>;
>>>>>> Xueming(Steven) Li <xuemingl@nvidia.com>
>>>>>> Cc: dev@dpdk.org; Viacheslav Galaktionov
>>>>>> <viacheslav.galaktionov@oktetlabs.ru>; stable@dpdk.org
>>>>>> Subject: [PATCH] ethdev: fix representor port ID search by name
>>>>>>
>>>>>> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>>>>>
>>>>>> Fix representor port ID search by name if the representor itself
>>>>>> does not provide representors info. Getting a list of representors
>>>>>> from a representor does not make sense. Instead, a parent device
>>>> should be used.
>>>>>>
>>>>>> To this end, extend the rte_eth_dev_data structure to include the port ID of the parent device for representors.
>>>>>>
>>>>>> Fixes: df7547a6a2cc ("ethdev: add helper function to get
>>>>>> representor
>>>>>> ID")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Viacheslav Galaktionov
>>>>>> <viacheslav.galaktionov@oktetlabs.ru>
>>>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>>> ---
>>>>>> The new field is added into the hole in rte_eth_dev_data structure.
>>>>>> The patch does not change ABI, but extra care is required since ABI
>>>>>> check is disabled for the structure because of the libabigail bug
>>>> [1].
>>>>>>
>>>>>> Potentially it is bad for out-of-tree drivers which implement
>>>>>> representors but do not fill in a new parert_port_id field in rte_eth_dev_data structure. Do we care?
>>>>>>
>>>>>> May be the patch should add lines to release notes, but I'd like to get initial feedback first.
>>>>>>
>>>>>> mlx5 changes should be reviwed by maintainers very carefully, since we are not sure if we patch it correctly.
>>>>>>
>>>>>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=28060
>>>>
>>>> [snip]
>>>>
>>>>>> --- a/lib/ethdev/ethdev_driver.h
>>>>>> +++ b/lib/ethdev/ethdev_driver.h
>>>>>> @@ -1248,8 +1248,8 @@ struct rte_eth_devargs {
>>>>>>      * For backward compatibility, if no representor info, direct
>>>>>>      * map legacy VF (no controller and pf).
>>>>>>      *
>>>>>> - * @param ethdev
>>>>>> - *  Handle of ethdev port.
>>>>>> + * @param parent_port_id
>>>>>> + *  Port ID of the backing device.
>>>>>>      * @param type
>>>>>>      *  Representor type.
>>>>>>      * @param controller
>>>>>> @@ -1266,7 +1266,7 @@ struct rte_eth_devargs {
>>>>>>      */
>>>>>>     __rte_internal
>>>>>>     int
>>>>>> -rte_eth_representor_id_get(const struct rte_eth_dev *ethdev,
>>>>>> +rte_eth_representor_id_get(uint16_t parent_port_id,
>>>>>
>>>>> It make more sense to get representor info from parent port.
>>>>> Representor is a member of switch domain, PMD owns the information
>>>>> of the representor owner port and info of representors. This change
>>>>> looks better, but not sure whether it valuable to introduce a new
>>>> member to the EAL data structure.
>>>>
>>>> IMHO, it is simply incorrect to return representors info on a
>>>> representor itself. Representor info is an information which representors may be populated using the device.
>>>>
>>>> If above statement is correct, we need a way to get parent device by
>>>> representor to do name to representor ID mapping. I see two options to do it:
>>>>     A. Dedicated field in rte_eth_dev_data as the patch does.
>>>>     B. Dedicated ethdev op (since representor knows parent port ID anyway).
>>>> We have chosen (A) because of simplicity.
>>>
>>> Just recalled that representor port could be probed w/o owner PF, is a force for parent port?
>>
>> I thought that it is impossible and parent port is absolutely required for a representor. Could you provide an example and explain how
>> will it work?
> 
> In case of bonding, PF0 and PF1 become one PF port `bond0`, PCI address is PF0.
> 	-a <PF0>,representor=pf[0-1]vf[0-99] // this is the syntax we proposed.

Is it net/bonding or vendor-specific bonding in HW?
If I remember correctly in the case of net/bonding we have ethdev ports
for bonded devices.

> 
> To be backward compatible, also support the following 2 devargs:
> 	-a <pf0>,representor=[0-99] // probe bond0 and representor on pf0
> 	-a <pf1>,representor=[0-99] // probe representors on pf1.
> If devargs start with PF1 devargs, no owner PF1 created as it disabled in bonding. Can't create bond0(PF0) automatically here as
> device is located by PCI address(PF1) from devargs.

So, I guess the problem is vendor-specific bonding in HW. Anyway
legacy backward compatible representor spec should not require
representors info since it worked before without it. So, it does
not sound like a reason to have representors info on a representor
itself.

  reply	other threads:[~2021-07-20  8:59 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-12 16:17 Andrew Rybchenko
2021-07-19  6:58 ` Xueming(Steven) Li
2021-07-19  8:46   ` Andrew Rybchenko
2021-07-19 11:54     ` Xueming(Steven) Li
2021-07-19 12:36       ` Andrew Rybchenko
2021-07-19 12:50         ` Xueming(Steven) Li
2021-07-20  8:59           ` Andrew Rybchenko [this message]
2021-07-29  4:13             ` Xueming(Steven) Li
2021-08-01  8:40               ` Andrew Rybchenko
2021-08-01 14:25                 ` Xueming(Steven) Li
2021-07-29  4:20 ` Xueming(Steven) Li
2021-08-01  8:50   ` Andrew Rybchenko
2021-08-01 14:15     ` Xueming(Steven) Li
2021-08-18 14:00 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
2021-08-27  9:18   ` Xueming(Steven) Li
2021-08-27  9:48     ` Viacheslav Galaktionov
2021-08-28 13:22       ` Xueming(Steven) Li
2021-08-29  8:23         ` Andrew Rybchenko
2021-08-29 12:17           ` Xueming(Steven) Li
2021-08-31 15:42             ` Andrew Rybchenko
2021-08-20 12:18 ` [dpdk-dev] [PATCH v3] " Andrew Rybchenko
2021-08-31 15:41   ` Andrew Rybchenko
2021-08-31 16:06 ` [dpdk-dev] [PATCH v4] " Andrew Rybchenko
2021-08-31 16:32   ` Wang, Haiyue
2021-08-31 16:37     ` Andrew Rybchenko
2021-09-01  5:15   ` Xing, Beilei
2021-09-01 14:55   ` Ferruh Yigit
2021-09-06 16:16     ` Viacheslav Galaktionov
2021-09-13 11:26 ` [dpdk-dev] [PATCH v5] " Andrew Rybchenko
2021-09-29 11:13   ` Singh, Aman Deep
2021-09-30 12:03     ` Andrew Rybchenko
2021-09-30 12:51       ` Singh, Aman Deep
2021-09-30 13:40         ` Andrew Rybchenko
2021-10-01 11:39   ` Andrew Rybchenko
2021-10-08  8:39     ` Xueming(Steven) Li
2021-10-05 21:56   ` Thomas Monjalon
2021-10-07 10:20     ` Andrew Rybchenko
2021-10-07 12:39       ` Thomas Monjalon
2021-10-08  9:27 ` [dpdk-dev] [PATCH v6] " Andrew Rybchenko
2021-10-11  7:56   ` Slava Ovsiienko
2021-10-11 12:30 ` [dpdk-dev] [PATCH v7] " Andrew Rybchenko
2021-10-11 12:53 ` [dpdk-dev] [PATCH v8] " Andrew Rybchenko
2021-10-12 15:09   ` Ferruh Yigit

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=d62e6234-fb71-baa7-b5d5-d0ccdef19425@oktetlabs.ru \
    --to=andrew.rybchenko@oktetlabs.ru \
    --cc=ajit.khaparde@broadcom.com \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=haiyue.wang@intel.com \
    --cc=hyonkim@cisco.com \
    --cc=johndale@cisco.com \
    --cc=matan@nvidia.com \
    --cc=qi.z.zhang@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=shahafs@nvidia.com \
    --cc=somnath.kotur@broadcom.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=viacheslav.galaktionov@oktetlabs.ru \
    --cc=viacheslavo@nvidia.com \
    --cc=xuemingl@nvidia.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).