> -----Original Message----- > From: Andrew Rybchenko <[email protected]> > Sent: Tuesday, July 20, 2021 5:00 PM > To: Xueming(Steven) Li <[email protected]>; Ajit Khaparde > <[email protected]>; Somnath Kotur > <[email protected]>; John Daley <[email protected]>; Hyong Youb Kim > <[email protected]>; Beilei Xing > <[email protected]>; Qiming Yang <[email protected]>; Qi Zhang > <[email protected]>; Haiyue Wang > <[email protected]>; Matan Azrad <[email protected]>; Shahaf Shuler > <[email protected]>; Slava Ovsiienko > <[email protected]>; NBU-Contact-Thomas Monjalon <[email protected]>; > Ferruh Yigit <[email protected]> > Cc: [email protected]; Viacheslav Galaktionov > <[email protected]>; [email protected] > Subject: Re: [PATCH] ethdev: fix representor port ID search by name > > On 7/19/21 3:50 PM, Xueming(Steven) Li wrote: > > > > > >> -----Original Message----- > >> From: Andrew Rybchenko <[email protected]> > >> Sent: Monday, July 19, 2021 8:36 PM > >> To: Xueming(Steven) Li <[email protected]>; Ajit Khaparde > >> <[email protected]>; Somnath Kotur > >> <[email protected]>; John Daley <[email protected]>; Hyong > >> Youb Kim <[email protected]>; Beilei Xing <[email protected]>; > >> Qiming Yang <[email protected]>; Qi Zhang <[email protected]>; > >> Haiyue Wang <[email protected]>; Matan Azrad <[email protected]>; > >> Shahaf Shuler <[email protected]>; Slava Ovsiienko > >> <[email protected]>; NBU-Contact-Thomas Monjalon > >> <[email protected]>; Ferruh Yigit <[email protected]> > >> Cc: [email protected]; Viacheslav Galaktionov > >> <[email protected]>; [email protected] > >> 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 <[email protected]> > >>>> Sent: Monday, July 19, 2021 4:46 PM > >>>> To: Xueming(Steven) Li <[email protected]>; Ajit Khaparde > >>>> <[email protected]>; Somnath Kotur > >>>> <[email protected]>; John Daley <[email protected]>; > >>>> Hyong Youb Kim <[email protected]>; Beilei Xing > >>>> <[email protected]>; Qiming Yang <[email protected]>; Qi > >>>> Zhang <[email protected]>; Haiyue Wang <[email protected]>; > >>>> Matan Azrad <[email protected]>; Shahaf Shuler <[email protected]>; > >>>> Slava Ovsiienko <[email protected]>; NBU-Contact-Thomas > >>>> Monjalon <[email protected]>; Ferruh Yigit > >>>> <[email protected]> > >>>> Cc: [email protected]; Viacheslav Galaktionov > >>>> <[email protected]>; [email protected] > >>>> 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 <[email protected]> > >>>>>> Sent: Tuesday, July 13, 2021 12:18 AM > >>>>>> To: Ajit Khaparde <[email protected]>; Somnath Kotur > >>>>>> <[email protected]>; John Daley <[email protected]>; > >>>>>> Hyong Youb Kim <[email protected]>; Beilei Xing > >>>>>> <[email protected]>; Qiming Yang <[email protected]>; Qi > >>>>>> Zhang <[email protected]>; Haiyue Wang > >>>>>> <[email protected]>; Matan Azrad <[email protected]>; Shahaf > >>>>>> Shuler <[email protected]>; Slava Ovsiienko > >>>>>> <[email protected]>; NBU-Contact-Thomas Monjalon > >>>>>> <[email protected]>; Ferruh Yigit <[email protected]>; > >>>>>> Xueming(Steven) Li <[email protected]> > >>>>>> Cc: [email protected]; Viacheslav Galaktionov > >>>>>> <[email protected]>; [email protected] > >>>>>> Subject: [PATCH] ethdev: fix representor port ID search by name > >>>>>> > >>>>>> From: Viacheslav Galaktionov > >>>>>> <[email protected]> > >>>>>> > >>>>>> 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: [email protected] > >>>>>> > >>>>>> Signed-off-by: Viacheslav Galaktionov > >>>>>> <[email protected]> > >>>>>> Signed-off-by: Andrew Rybchenko <[email protected]> > >>>>>> --- > >>>>>> 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.
Not net/bonding pmd, it's Linux bonding, supported by hw driver. > > > > > 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. Legacy backward logic could be something like this: if PF owner port found, use it, fallback to current representor. This won't break anything I guess, how do you think?
