On 8/31/2021 5:06 PM, Andrew Rybchenko wrote:

From: Viacheslav Galaktionov <redacted>

Getting a list of representors from a representor does not make sense.
Instead, a parent device should be used.

Which code is getting list of the representors?

As far as I can see impacted APIs are:
'rte_eth_representor_id_get()'
'rte_eth_representor_info_get()'

Which are now getting 'parent_port_id' as argument, instead of representro port id.

'rte_eth_representor_info_get()' is using 'representor_info_get()' dev_ops,
which is only implemented by 'mlx5', so is this problem only valid for 'mlx5'
and can it be solved within PMD implementation?

To this end, extend the rte_eth_dev_data structure to include the port ID
of the backing device for representors.

Signed-off-by: Viacheslav Galaktionov <redacted>
Signed-off-by: Andrew Rybchenko <redacted>
---
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?

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

v4:
    - apply mlx5 review notes: remove fallback from generic ethdev
      code and add fallback to mlx5 code to handle legacy usecase

v3:
    - fix mlx5 build breakage

v2:
    - fix mlx5 review notes
    - try device port ID first before parent in order to address
      backward compatibility issue

<...>

Why 'parent'? Isn't this for the port that port representator represents, does
it called 'parent'? Above that device mentioned as 'backing device' a few times,
so would something like 'peer_port_id' better?

+			/**< Port ID of the backing device.
+			 *   This device will be used to query representor
+			 *   info and calculate representor IDs.
+			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
+			 */
 

Overall I am not feeling good about adding representor port related information
withing the device data struct.
I wonder if this information can be kept in the device private data.

Or, it is hard to explain but can we use something like inheritance, a
representor specific dev_data derived from original dev_data. We can store
dev_data pointers in 'struct rte_eth_dev' but can cast it to representor
specific dev_data when type is representor.

struct rte_eth_dev_data_rep
	struct rte_eth_dev_data
	<representor specific fields>

This brings lots of complexity though, specially in allocating/freeing this
struct, not sure if it worth to the effort.