ethdev: configure SR-IOV VF from host

-----Original Message-----
From: dev <[email protected]> On Behalf Of Thomas Monjalon
Sent: Thursday, August 15, 2019 8:36 PM
To: Ferruh Yigit <[email protected]>; Andrew Rybchenko <[email protected]>
Cc: [email protected]; Bernard Iremonger <[email protected]>; Shahaf Shuler <[email protected]>; E. Scott Daniels <[email protected]>; Wenzhuo Lu <[email protected]>; Alex Zelezniak <[email protected]>; Ajit Khaparde <[email protected]>; Declan Doherty <[email protected]>
Subject: [dpdk-dev] [RFC] ethdev: configure SR-IOV VF from host

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is drived by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required.

This RFC proposes to use generic DPDK API for VF configuration. The impacted functions are (can be extended):

- rte_eth_dev_is_valid_port
- rte_eth_promiscuous_enable
- rte_eth_promiscuous_disable
- rte_eth_promiscuous_get
- rte_eth_allmulticast_enable
- rte_eth_allmulticast_disable
- rte_eth_allmulticast_get
- rte_eth_dev_set_mc_addr_list
- rte_eth_dev_default_mac_addr_set
- rte_eth_macaddr_get
- rte_eth_dev_mac_addr_add
- rte_eth_dev_mac_addr_remove
- rte_eth_dev_vlan_filter
- rte_eth_dev_get_mtu
- rte_eth_dev_set_mtu

In order to target these functions to a VF (which has no port id in the host), the higher bit of port id is reserved:

#define RTE_ETH_VF_PORT_FLAG (1 << 15)

This bit can be combined only with the port id of a representor. The meaning is to target the VF connected with the representor port, instead of the representor port itself.

If a function is not expected to support VF configuration, it will return - EINVAL, i.e. there is no code change.
If an API function (listed above) can support VF configuration, but the PMD does not support it, then -ENOTSUP must be returned.

As an example, this is the change required in rte_eth_dev_is_valid_port:

int
rte_eth_dev_is_valid_port(uint16_t port_id) {

(quoted diff) +13 -2

+       uint32_t dev_flags;
+       uint16_t vf_flag;
+
+       vf_flag = port_id & RTE_ETH_VF_PORT_FLAG;
+       port_id &= RTE_ETH_VF_PORT_FLAG - 1; /* remove VF flag */
+
        if (port_id >= RTE_MAX_ETHPORTS ||
            (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED))
                return 0;
-       else
-               return 1;
+
+       dev_flags = rte_eth_dev_shared_data->data[port_id].dev_flags;
+       if (vf_flag != 0 && (dev_flags & RTE_ETH_DEV_REPRESENTOR) == 0)
+               return 0; /* VF flag has no meaning if not a representor
+ */
+
+       return 1;
 }

From: Thomas Monjalon

Quoted 29 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is drived by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required.

This RFC proposes to use generic DPDK API for VF configuration. The impacted functions are (can be extended):

- rte_eth_dev_is_valid_port
- rte_eth_promiscuous_enable
- rte_eth_promiscuous_disable
- rte_eth_promiscuous_get
- rte_eth_allmulticast_enable
- rte_eth_allmulticast_disable
- rte_eth_allmulticast_get
- rte_eth_dev_set_mc_addr_list
- rte_eth_dev_default_mac_addr_set
- rte_eth_macaddr_get
- rte_eth_dev_mac_addr_add
- rte_eth_dev_mac_addr_remove
- rte_eth_dev_vlan_filter
- rte_eth_dev_get_mtu
- rte_eth_dev_set_mtu

Quoted 4 lines

In order to target these functions to a VF (which has no port id in the host), the higher bit of port id is reserved:

#define RTE_ETH_VF_PORT_FLAG (1 << 15)

Instead of changing the port number behavior, How about adding a bit field/ I think, There is no ABI breakage as the parent type of bit field is uint8_t and there is still more room.

-----Original Message-----
From: Thomas Monjalon [mailto:[email protected]]
Sent: Thursday, August 15, 2019 4:06 PM
To: Yigit, Ferruh <[email protected]>; Andrew Rybchenko <[email protected]>
Cc: [email protected]; Iremonger, Bernard <[email protected]>; Shahaf Shuler <[email protected]>; E. Scott Daniels <[email protected]>; Lu, Wenzhuo <[email protected]>; Alex Zelezniak <[email protected]>; Ajit Khaparde <[email protected]>; Doherty, Declan <[email protected]>
Subject: [RFC] ethdev: configure SR-IOV VF from host

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is drived by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required.

This RFC proposes to use generic DPDK API for VF configuration. The impacted functions are (can be extended):

- rte_eth_dev_is_valid_port
- rte_eth_promiscuous_enable
- rte_eth_promiscuous_disable
- rte_eth_promiscuous_get
- rte_eth_allmulticast_enable
- rte_eth_allmulticast_disable
- rte_eth_allmulticast_get
- rte_eth_dev_set_mc_addr_list
- rte_eth_dev_default_mac_addr_set
- rte_eth_macaddr_get
- rte_eth_dev_mac_addr_add
- rte_eth_dev_mac_addr_remove
- rte_eth_dev_vlan_filter
- rte_eth_dev_get_mtu
- rte_eth_dev_set_mtu

In order to target these functions to a VF (which has no port id in the host), the higher bit of port id is reserved:

#define RTE_ETH_VF_PORT_FLAG (1 << 15)

This bit can be combined only with the port id of a representor. The meaning is to target the VF connected with the representor port, instead of the representor port itself.

If a function is not expected to support VF configuration, it will return - EINVAL, i.e. there is no code change.
If an API function (listed above) can support VF configuration, but the PMD does not support it, then -ENOTSUP must be returned.

As an example, this is the change required in rte_eth_dev_is_valid_port:

int
rte_eth_dev_is_valid_port(uint16_t port_id) {

(quoted diff) +6 -0

+       uint32_t dev_flags;
+       uint16_t vf_flag;
+
+       vf_flag = port_id & RTE_ETH_VF_PORT_FLAG;
+       port_id &= RTE_ETH_VF_PORT_FLAG - 1; /* remove VF flag */
+
        if (port_id >= RTE_MAX_ETHPORTS ||
            (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED))
                return 0;

Hi Thomas,

From: Thomas Monjalon [mailto:[email protected]]

Quoted 61 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is drived by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required.

This RFC proposes to use generic DPDK API for VF configuration. The impacted functions are (can be extended):

- rte_eth_dev_is_valid_port
- rte_eth_promiscuous_enable
- rte_eth_promiscuous_disable
- rte_eth_promiscuous_get
- rte_eth_allmulticast_enable
- rte_eth_allmulticast_disable
- rte_eth_allmulticast_get
- rte_eth_dev_set_mc_addr_list
- rte_eth_dev_default_mac_addr_set
- rte_eth_macaddr_get
- rte_eth_dev_mac_addr_add
- rte_eth_dev_mac_addr_remove
- rte_eth_dev_vlan_filter
- rte_eth_dev_get_mtu
- rte_eth_dev_set_mtu

In order to target these functions to a VF (which has no port id in the host), the higher bit of port id is reserved:

#define RTE_ETH_VF_PORT_FLAG (1 << 15)

This bit can be combined only with the port id of a representor. The meaning is to target the VF connected with the representor port, instead of the representor port itself.

If a function is not expected to support VF configuration, it will return - EINVAL, i.e. there is no code change.
If an API function (listed above) can support VF configuration, but the PMD does not support it, then -ENOTSUP must be returned.

As an example, this is the change required in rte_eth_dev_is_valid_port:

int
rte_eth_dev_is_valid_port(uint16_t port_id) {

(quoted diff) +9 -2

+       uint32_t dev_flags;
+       uint16_t vf_flag;
+
+       vf_flag = port_id & RTE_ETH_VF_PORT_FLAG;
+       port_id &= RTE_ETH_VF_PORT_FLAG - 1; /* remove VF flag */
+
        if (port_id >= RTE_MAX_ETHPORTS ||
            (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED))
                return 0;
-       else
-               return 1;
+
+       dev_flags = rte_eth_dev_shared_data->data[port_id].dev_flags;
+       if (vf_flag != 0 && (dev_flags & RTE_ETH_DEV_REPRESENTOR) == 0)

Review

Quoted 5 lines

(quoted diff) +4 -0

+               return 0; /* VF flag has no meaning if not a representor
+ */
+
+       return 1;
 }

Some of the functions in the list above for example, rte_eth_dev_promiscuous_enable() use the dev_ops structure, is it intended to add more rte_eth_dev_* functions to the dev_ops structure?

At present the ixgbe and i40e PMD's have sets of private functions for configuring SRIOV VF's from the DPDK PF, rte_pmd_ixgbe_* and rte_pmd_i40e_* functions (see rte_pmd_ixgbe.h and rte_pmd_i40e.h).

At the time these functions were not allowed to be added to the dev_ops structure as there were so many of them. There was a proposal to add a dev_ctrl function to the dev_ops structure which would access the private functions. Maybe adding the dev_ctrl function should be considered again.

Having two ways (through dev_ops and private PMD functions) to configure DPDK VF's from the DPDK PF will be confusing for developers.

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

and PMD-specific APIs were used.
This new generic API will avoid vendors fragmentation.

Some PMD-specific API could migrate to this generic model. As an example, the default MAC address configuration is demonstrated for a VF mapped to mlx5 representor port.

As it breaks the ABI, I propose to merge this API in DPDK 19.11-rc2.

I am sorry I had not send a patch since proposing a RFC in August. (I gave priority to the summit and the -rc1 release)

Thomas Monjalon (3):
ethdev: identify SR-IOV VF from host
ethdev: set VF MAC address from host
net/mlx5: set VF MAC address from host

 drivers/net/mlx5/mlx5.c                  |  6 +++
 drivers/net/mlx5/mlx5.h                  |  1 +
 drivers/net/mlx5/mlx5_mac.c              | 19 ++++++++
 lib/librte_ethdev/rte_ethdev.c           | 55 +++++++++++++++++++++---
 lib/librte_ethdev/rte_ethdev.h           | 38 ++++++++++++++++
 lib/librte_ethdev/rte_ethdev_core.h      |  1 +
 lib/librte_ethdev/rte_ethdev_version.map |  1 +
 7 files changed, 114 insertions(+), 7 deletions(-)

--
2.23.0

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 3 lines · Thomas Monjalon

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 6 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Quoted 4 lines

and PMD-specific APIs were used.
This new generic API will avoid vendors fragmentation.

The API is good. But I have concerns about the vendor implementation of this API.
It can support only vendors with bifurcated driver(Mellanox case). or using igb_uio(non iommu case) but not the devices with VFIO(Which is the first-class citizen).

All the control plane control stuff to replace Linux with "port representor" logic will be of the mercy of an "out of tree" driver either with igb_uio or
https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
es.dpdk.org%2Fpatch%2F58810%2F&amp;data=02%7C01%7Cshahafs%40mel lanox.com%7C8da6ffacecde48af24f608d75ceee28c%7Ca652971c7d2e4d9ba6 a4d149256f461b%7C0%7C0%7C637080053397844419&amp;sdata=sAIRqTnAN G8lIb2eYhvcylU%2F6%2F81eXPDeGbnrUdMnis%3D&amp;reserved=0

I am _not against_ on DPDK supports port representor or controlling netdev VF traffic, but if we have taken that path then DPDK should have the infrastructure to support for all driver models like VFIO(Addressed in [1])

I would have this question when DPDK starts supporting port representor(but I was not aware that kernel security issue on netdev ports controlled by DPDK in non-bifurcated driver case and concise effort block such scheme by kernel [2])

[1]
https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
es.dpdk.org%2Fpatch%2F58810%2F&amp;data=02%7C01%7Cshahafs%40mel lanox.com%7C8da6ffacecde48af24f608d75ceee28c%7Ca652971c7d2e4d9ba6 a4d149256f461b%7C0%7C0%7C637080053397844419&amp;sdata=sAIRqTnAN G8lIb2eYhvcylU%2F6%2F81eXPDeGbnrUdMnis%3D&amp;reserved=0
[2]
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
work.kernel.org%2Fpatch%2F10522381%2F&amp;data=02%7C01%7Cshahafs %40mellanox.com%7C8da6ffacecde48af24f608d75ceee28c%7Ca652971c7d2e 4d9ba6a4d149256f461b%7C0%7C0%7C637080053397844419&amp;sdata=fyEo fHJQM51L8ssvLNyaLwrsCK8bBJiuPT%2FgMje3QxE%3D&amp;reserved=0

Quoted 28 lines

Some PMD-specific API could migrate to this generic model. As an example, the default MAC address configuration is demonstrated for a VF mapped to mlx5 representor port.

As it breaks the ABI, I propose to merge this API in DPDK 19.11-rc2.

I am sorry I had not send a patch since proposing a RFC in August. (I gave priority to the summit and the -rc1 release)

Thomas Monjalon (3):
ethdev: identify SR-IOV VF from host
ethdev: set VF MAC address from host
net/mlx5: set VF MAC address from host

 drivers/net/mlx5/mlx5.c                  |  6 +++
 drivers/net/mlx5/mlx5.h                  |  1 +
 drivers/net/mlx5/mlx5_mac.c              | 19 ++++++++
 lib/librte_ethdev/rte_ethdev.c           | 55 +++++++++++++++++++++---
 lib/librte_ethdev/rte_ethdev.h           | 38 ++++++++++++++++
 lib/librte_ethdev/rte_ethdev_core.h      |  1 +
 lib/librte_ethdev/rte_ethdev_version.map |  1 +
 7 files changed, 114 insertions(+), 7 deletions(-)

--
2.23.0

Wednesday, October 30, 2019 6:09 AM, Jerin Jacob:

Quoted 19 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 3 lines · Thomas Monjalon

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 6 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

Quoted 3 lines

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Per my understanding Thomas proposal is not to create the VFs from the PF device. it is to configure their network attributes from the PF after they have been created.

Quoted 24 lines

Wednesday, October 30, 2019 6:09 AM, Jerin Jacob:

Quoted 22 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 3 lines · Thomas Monjalon

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 6 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Quoted 3 lines

Per my understanding Thomas proposal is not to create the VFs from the PF device. it is to configure their network attributes from the PF after they have been created.

Yes. My question is without creating the VF, How do you control them?

30/10/2019 10:24, Jerin Jacob:

Quoted 25 lines · Shahaf Shuler

Quoted 24 lines

Wednesday, October 30, 2019 6:09 AM, Jerin Jacob:

Quoted 22 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 3 lines · Thomas Monjalon

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 6 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Quoted 5 lines

Quoted 3 lines

Per my understanding Thomas proposal is not to create the VFs from the PF device. it is to configure their network attributes from the PF after they have been created.

Yes. My question is without creating the VF, How do you control them?

We can create the VF via the kernel PF driver, before binding the PF to VFIO.

Quoted 32 lines · Thomas Monjalon

30/10/2019 10:24, Jerin Jacob:

Quoted 30 lines · Shahaf Shuler

Quoted 20 lines

Wednesday, October 30, 2019 6:09 AM, Jerin Jacob:

Quoted 19 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 3 lines · Thomas Monjalon

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 6 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
     - driver is bifurcated (Mellanox case),
       so the VF can be configured via the kernel.
     - driver is on top of UIO or VFIO, so DPDK API is required,

Quoted 7 lines

Quoted 3 lines

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Per my understanding Thomas proposal is not to create the VFs from the PF device. it is to configure their network attributes from the PF after they have been created.

Yes. My question is without creating the VF, How do you control them?

Quoted 1 line

We can create the VF via the kernel PF driver, before binding the PF to VFIO.

AFAIK, this is not possible. VFs are gone as soon as you're unbinding kernel PF driver.  And after binding of vfio-pci you can no longer create VFs.

I tried to check some representor functionality about 2 months ago and didn't find a way to enable VFs on Intel NICs if PF is under control of vfio-pci.

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 5 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Quoted 2 lines

and PMD-specific APIs were used.
This new generic API will avoid vendors fragmentation.

The API is good. But I have concerns about the vendor implementation of this API.
It can support only vendors with bifurcated driver(Mellanox case). or using igb_uio(non iommu case) but not the devices with VFIO(Which is the first-class citizen).

All the control plane control stuff to replace Linux with "port representor" logic
will be of the mercy of an "out of tree" driver either with igb_uio or http://patches.dpdk.org/patch/58810/

I am _not against_ on DPDK supports port representor or controlling netdev VF traffic,
but if we have taken that path then DPDK should have the infrastructure to support for
all driver models like VFIO(Addressed in [1])

I would have this question when DPDK starts supporting port representor(but I was not
aware that kernel security issue on netdev ports controlled by DPDK in non-bifurcated driver case
and concise effort block such scheme by kernel [2])

[1]
http://patches.dpdk.org/patch/58810/
[2]
https://patchwork.kernel.org/patch/10522381/

30/10/2019 05:08, Jerin Jacob:

Quoted 5 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Quoted 2 lines

Just to understand, Could you explain more details/examples for security reasons?

Examples are setting the MAC address or the promiscuous mode. These settings should be decided by the hypervisor, and not freely set by the VM.

Quoted 7 lines

Quoted 6 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

Quoted 3 lines

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Not sure I said the contrary.
igb_uio and vfio_pf are 2 implementations of UIO and VFIO.

Quoted 4 lines

Quoted 3 lines

and PMD-specific APIs were used.
This new generic API will avoid vendors fragmentation.

Quoted 5 lines

The API is good. But I have concerns about the vendor implementation of this API.
It can support only vendors with bifurcated driver(Mellanox case). or using igb_uio(non iommu case) but not the devices with VFIO(Which is the first-class citizen).

Why not? (see above)
The API is agnostic to the kernel driver in use.

Quoted 19 lines

All the control plane control stuff to replace Linux with "port representor" logic
will be of the mercy of an "out of tree" driver either with igb_uio or http://patches.dpdk.org/patch/58810/

I am _not against_ on DPDK supports port representor or controlling netdev VF traffic,
but if we have taken that path then DPDK should have the infrastructure to support for
all driver models like VFIO(Addressed in [1])

I would have this question when DPDK starts supporting port representor(but I was not
aware that kernel security issue on netdev ports controlled by DPDK in non-bifurcated driver case
and concise effort block such scheme by kernel [2])

Quoted 4 lines

[1]
http://patches.dpdk.org/patch/58810/
[2]
https://patchwork.kernel.org/patch/10522381/

I feel you are using this thread to promote your vfio_pf implementation.

But this API and the kernel module are orthogonals. Please can we focus on the DPDK API in this thread, and talk about VFIO implementation in the other thread?

Quoted 10 lines

30/10/2019 05:08, Jerin Jacob:

Quoted 7 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 3 lines

Examples are setting the MAC address or the promiscuous mode. These settings should be decided by the hypervisor, and not freely set by the VM.

What is hypervisor here, rte_flow based DPDK application over using port representor?

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

Quoted 6 lines

Quoted 3 lines

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Not sure I said the contrary.
igb_uio and vfio_pf are 2 implementations of UIO and VFIO.

Yes. I am saying without out of tree module it is not possible.

Quoted 9 lines

Quoted 8 lines

Quoted 2 lines

and PMD-specific APIs were used.
This new generic API will avoid vendors fragmentation.

The API is good. But I have concerns about the vendor implementation of this API.
It can support only vendors with bifurcated driver(Mellanox case). or using igb_uio(non iommu case) but not the devices with VFIO(Which is the first-class citizen).

Quoted 2 lines

Why not? (see above)
The API is agnostic to the kernel driver in use.

My question is how do you enable in DPDK with VFIO if DPDK is giving this feature?

Quoted 18 lines

Quoted 18 lines

All the control plane control stuff to replace Linux with "port representor" logic
will be of the mercy of an "out of tree" driver either with igb_uio or http://patches.dpdk.org/patch/58810/

I am _not against_ on DPDK supports port representor or controlling netdev VF traffic,
but if we have taken that path then DPDK should have the infrastructure to support for
all driver models like VFIO(Addressed in [1])

I would have this question when DPDK starts supporting port representor(but I was not
aware that kernel security issue on netdev ports controlled by DPDK in non-bifurcated driver case
and concise effort block such scheme by kernel [2])

Quoted 6 lines

Quoted 4 lines

[1]
http://patches.dpdk.org/patch/58810/
[2]
https://patchwork.kernel.org/patch/10522381/

I feel you are using this thread to promote your vfio_pf implementation.

Yes. I am. Because, I need to think, How I can enable this API with VFIO. Otherwise, I can not implement this API.

Quoted 3 lines

But this API and the kernel module are orthogonals. Please can we focus on the DPDK API in this thread, and talk about VFIO implementation in the other thread?

Yes. My concerns are we keep adding APIs without thinking about how it can be implemented
in the overall scheme of things. Just API is not enough.

30/10/2019 10:15, Jerin Jacob:

Quoted 11 lines · Thomas Monjalon

Quoted 10 lines

30/10/2019 05:08, Jerin Jacob:

Quoted 7 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 6 lines

Quoted 3 lines

Examples are setting the MAC address or the promiscuous mode. These settings should be decided by the hypervisor, and not freely set by the VM.

What is hypervisor here, rte_flow based DPDK application over using port representor?

Yes, something like that. An example is OpenStack/OVS.

Quoted 27 lines

Quoted 6 lines

Quoted 6 lines

When the PF (host port) is driven by DPDK (OVS-DPDK case),

Quoted 4 lines

we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required,

Quoted 6 lines

Quoted 3 lines

Not true. Both UIO and VFIO are NOT allowed to create SRIOV VF from the PF device.
It is only allowed through igb-uio out of tree driver without iommu support.

Not sure I said the contrary.
igb_uio and vfio_pf are 2 implementations of UIO and VFIO.

Yes. I am saying without out of tree module it is not possible.

Quoted 11 lines

Quoted 8 lines

Quoted 2 lines

and PMD-specific APIs were used.
This new generic API will avoid vendors fragmentation.

The API is good. But I have concerns about the vendor implementation of this API.
It can support only vendors with bifurcated driver(Mellanox case). or using igb_uio(non iommu case) but not the devices with VFIO(Which is the first-class citizen).

Why not? (see above)
The API is agnostic to the kernel driver in use.

Quoted 2 lines

My question is how do you enable in DPDK with VFIO if DPDK is giving this feature?

I didn't get your question.
If it is the same question as before, I think you can create the VF before binding the PF to VFIO.

Quoted 33 lines

Quoted 19 lines

Quoted 19 lines

All the control plane control stuff to replace Linux with "port representor" logic
will be of the mercy of an "out of tree" driver either with igb_uio or http://patches.dpdk.org/patch/58810/

I am _not against_ on DPDK supports port representor or controlling netdev VF traffic,
but if we have taken that path then DPDK should have the infrastructure to support for
all driver models like VFIO(Addressed in [1])

I would have this question when DPDK starts supporting port representor(but I was not
aware that kernel security issue on netdev ports controlled by DPDK in non-bifurcated driver case
and concise effort block such scheme by kernel [2])

Quoted 6 lines

Quoted 4 lines

[1]
http://patches.dpdk.org/patch/58810/
[2]
https://patchwork.kernel.org/patch/10522381/

I feel you are using this thread to promote your vfio_pf implementation.

Yes. I am. Because, I need to think, How I can enable this API with VFIO. Otherwise, I can not implement this API.

Quoted 3 lines

But this API and the kernel module are orthogonals. Please can we focus on the DPDK API in this thread, and talk about VFIO implementation in the other thread?

Quoted 3 lines

Yes. My concerns are we keep adding APIs without thinking about how it can be implemented
in the overall scheme of things. Just API is not enough.

We keep thinking about all scenarios (maybe in other threads). And adding an API is a step in the right direction in my opinion.

30/10/2019 10:15, Jerin Jacob:

Quoted 11 lines · Thomas Monjalon

Quoted 10 lines

30/10/2019 05:08, Jerin Jacob:

Quoted 7 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Quoted 6 lines

Quoted 3 lines

Examples are setting the MAC address or the promiscuous mode. These settings should be decided by the hypervisor, and not freely set by the VM.

What is hypervisor here, rte_flow based DPDK application over using port representor?

Yes, something like that. An example is OpenStack/OVS.

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 3 lines · Thomas Monjalon

30/10/2019 10:15, Jerin Jacob:

Quoted 1 line

On Wed, Oct 30, 2019 at 2:26 PM Thomas Monjalon

Quoted 3 lines · <[email protected]> wrote:

Quoted 3 lines

Quoted 3 lines

30/10/2019 05:08, Jerin Jacob:

Quoted 1 line

On Wed, Oct 30, 2019 at 12:21 AM Thomas Monjalon

Quoted 11 lines · <[email protected]> wrote:

Quoted 11 lines

Quoted 10 lines

Quoted 6 lines

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Just to understand, Could you explain more details/examples for security reasons?

Examples are setting the MAC address or the promiscuous mode. These settings should be decided by the hypervisor, and not freely set by the VM.

Quoted 4 lines

Quoted 2 lines

What is hypervisor here, rte_flow based DPDK application over using port representor?

Yes, something like that. An example is OpenStack/OVS.

So it is more of an orchestration primitive. Not the security primitive as mentioned above,

the guest. Right?

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required, and PMD-specific APIs were used.

This new generic API will avoid vendors fragmentation.

Some PMD-specific API could migrate to this generic model. As an example, the default MAC address configuration is demonstrated for a VF mapped to mlx5 representor port.

As it breaks the ABI, I propose to merge this API in DPDK 19.11-rc2.

I am sorry I had not send a patch since proposing a RFC in August. (I gave priority to the summit and the -rc1 release)

Thomas Monjalon (3):
ethdev: identify SR-IOV VF from host
ethdev: set VF MAC address from host
net/mlx5: set VF MAC address from host

  drivers/net/mlx5/mlx5.c                  |  6 +++
  drivers/net/mlx5/mlx5.h                  |  1 +
  drivers/net/mlx5/mlx5_mac.c              | 19 ++++++++
  lib/librte_ethdev/rte_ethdev.c           | 55 +++++++++++++++++++++---
  lib/librte_ethdev/rte_ethdev.h           | 38 ++++++++++++++++
  lib/librte_ethdev/rte_ethdev_core.h      |  1 +
  lib/librte_ethdev/rte_ethdev_version.map |  1 +
  7 files changed, 114 insertions(+), 7 deletions(-)

Quoted 9 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required, and PMD-specific APIs were used.

So, what is wrong with setting VF mac via the representor port as it done now? From our previous discussion, I thought that we concluded that is enough to have current API, i.e. just call set_default_mac() for a representor port and this will lead to setting mac for VF. This is how it's implemented in Linux kernel and this is how it's implemented in current DPDK drivers that supports setting mac for the representor.

The only use case for this new API is to be able to control mac of the representor itself, which doesn't make much sense. At least there are only hypothetical use cases. And once again, Linux kernel doesn't support this behavior.

Quoted 1 line

This new generic API will avoid vendors fragmentation.

I don't see the fragmentation. Right now you can set VF mac from DPDK by calling set_default_mac() for representor. This API exists for all vendors. Not implemented for Intel, but new API is not implemented too.

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

30/10/2019 16:07, Ilya Maximets:

Quoted 11 lines · Thomas Monjalon

Quoted 9 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required, and PMD-specific APIs were used.

Quoted 7 lines

So, what is wrong with setting VF mac via the representor port as it done now? From our previous discussion, I thought that we concluded that is enough to have current API, i.e. just call set_default_mac() for a representor port and this will lead to setting mac for VF. This is how it's implemented in Linux kernel and this is how it's implemented in current DPDK drivers that supports setting mac for the representor.

I don't know what is done in the Linux kernel.
In DPDK, setting the MAC of the representor is really setting the MAC of the representor. Is it crazy?

Quoted 5 lines

The only use case for this new API is to be able to control mac of the representor itself, which doesn't make much sense. At least there are only hypothetical use cases. And once again, Linux kernel doesn't support this behavior.

I think it is sane to be able to set different MAC addresses for the representor and the VF.

Quoted 5 lines

Quoted 1 line

This new generic API will avoid vendors fragmentation.

I don't see the fragmentation. Right now you can set VF mac from DPDK by calling set_default_mac() for representor. This API exists for all vendors. Not implemented for Intel, but new API is not implemented too.

No, the current situation is the following:
- for mlx5, VF MAC can be configured with iproute2 or netlink
- for ixgbe, rte_pmd_ixgbe_set_vf_mac_addr()
- for i40e, rte_pmd_i40e_set_vf_mac_addr()
- for bnxt, rte_pmd_bnxt_set_vf_mac_addr()

Quoted 6 lines

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

I am not responsible of the choices in Linux.
But I agree it would be interesting to check the reason of such decision. Rony, please could you explain?

Quoted 20 lines

30/10/2019 16:07, Ilya Maximets:

Quoted 11 lines · Thomas Monjalon

Quoted 9 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required, and PMD-specific APIs were used.

Quoted 7 lines

So, what is wrong with setting VF mac via the representor port as it done now? From our previous discussion, I thought that we concluded that is enough to have current API, i.e. just call set_default_mac() for a representor port and this will lead to setting mac for VF. This is how it's implemented in Linux kernel and this is how it's implemented in current DPDK drivers that supports setting mac for the representor.

Quoted 3 lines

I don't know what is done in the Linux kernel.
In DPDK, setting the MAC of the representor is really setting the MAC of the representor. Is it crazy?

Kind of. And no, it doesn't work this way in DPDK. Just look at the i40e driver:

325 static int
326 i40e_vf_representor_mac_addr_set(struct rte_eth_dev *ethdev,
327 struct ether_addr *mac_addr)
328 {
329 struct i40e_vf_representor *representor = ethdev->data->dev_private;
330
331 return rte_pmd_i40e_set_vf_mac_addr(
332 representor->adapter->eth_dev->data->port_id,
333 representor->vf_id, mac_addr);
334 }
....

423 static const struct eth_dev_ops i40e_representor_dev_ops = {
     <...>

445 .mac_addr_set = i40e_vf_representor_mac_addr_set,

It really calls the function to set VF mac address.

And for MLX drivers it's the same.
MLX driver call netlink to set representor MAC, but netlink is in kernel and this call inside the kernel translates to the setting of mac address of the VF.

Am I missing something?

Quoted 7 lines

Quoted 4 lines

The only use case for this new API is to be able to control mac of the representor itself, which doesn't make much sense. At least there are only hypothetical use cases. And once again, Linux kernel doesn't support this behavior.

I think it is sane to be able to set different MAC addresses for the representor and the VF.

Quoted 11 lines

Quoted 5 lines

Quoted 1 line

This new generic API will avoid vendors fragmentation.

I don't see the fragmentation. Right now you can set VF mac from DPDK by calling set_default_mac() for representor. This API exists for all vendors. Not implemented for Intel, but new API is not implemented too.

No, the current situation is the following:
- for mlx5, VF MAC can be configured with iproute2 or netlink
- for ixgbe, rte_pmd_ixgbe_set_vf_mac_addr()
- for i40e, rte_pmd_i40e_set_vf_mac_addr()
- for bnxt, rte_pmd_bnxt_set_vf_mac_addr()

Quoted 10 lines

Quoted 6 lines

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

I am not responsible of the choices in Linux.
But I agree it would be interesting to check the reason of such decision. Rony, please could you explain?

30/10/2019 17:09, Ilya Maximets:

Quoted 39 lines · Thomas Monjalon

Quoted 20 lines

30/10/2019 16:07, Ilya Maximets:

Quoted 11 lines · Thomas Monjalon

Quoted 9 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

When the PF (host port) is driven by DPDK (OVS-DPDK case), we face two different cases:
- driver is bifurcated (Mellanox case),
so the VF can be configured via the kernel.
- driver is on top of UIO or VFIO, so DPDK API is required, and PMD-specific APIs were used.

Quoted 7 lines

So, what is wrong with setting VF mac via the representor port as it done now? From our previous discussion, I thought that we concluded that is enough to have current API, i.e. just call set_default_mac() for a representor port and this will lead to setting mac for VF. This is how it's implemented in Linux kernel and this is how it's implemented in current DPDK drivers that supports setting mac for the representor.

Quoted 3 lines

I don't know what is done in the Linux kernel.
In DPDK, setting the MAC of the representor is really setting the MAC of the representor. Is it crazy?

Kind of. And no, it doesn't work this way in DPDK. Just look at the i40e driver:

325 static int
326 i40e_vf_representor_mac_addr_set(struct rte_eth_dev *ethdev,
327 struct ether_addr *mac_addr)
328 {
329 struct i40e_vf_representor *representor = ethdev->data->dev_private;
330
331 return rte_pmd_i40e_set_vf_mac_addr(
332 representor->adapter->eth_dev->data->port_id,
333 representor->vf_id, mac_addr);
334 }
....

Quoted 6 lines

423 static const struct eth_dev_ops i40e_representor_dev_ops = {
      <...>

445 .mac_addr_set = i40e_vf_representor_mac_addr_set,

It really calls the function to set VF mac address.

Indeed, I missed that i40e_vf_representor_mac_addr_set() is calling rte_pmd_i40e_set_vf_mac_addr().

Quoted 6 lines

And for MLX drivers it's the same.
MLX driver call netlink to set representor MAC, but netlink is in kernel and this call inside the kernel translates to the setting of mac address of the VF.

Am I missing something?

I am not sure about this kernel translation. At least not in mlx5. Setting MAC address of a VF representor seems not supported on mlx5. But there is a specific netlink request to set a VF MAC address:
ip link set <PF> vf <VF> mac <MAC>

Quoted 7 lines

Quoted 7 lines

Quoted 4 lines

The only use case for this new API is to be able to control mac of the representor itself, which doesn't make much sense. At least there are only hypothetical use cases. And once again, Linux kernel doesn't support this behavior.

I think it is sane to be able to set different MAC addresses for the representor and the VF.

Let me explain better my thoughts.
In the switchdev design, VF and uplink ports are all connected together via a switch.
The representors are mirrors of those switch ports. So a VF representor port is supposed to mirror the switch port where a VF is connected to. It is different of the VF itself.

This is a drawing of my understanding of switchdev design:

VF1 ------ VF1 rep /--------\
                    | switch | uplink rep ------ uplink ------ wire

VF2 ------ VF2 rep \--------/ (PF)

Of course, there can be more VF and uplink ports.

With some switch-aware protocols, it might be interesting to have different MAC addresses on a VF and its representor. And more generally, I imagine that the config of a VF representor could be different of the config of a VF.

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Quoted 1 line

This new generic API will avoid vendors fragmentation.

I don't see the fragmentation. Right now you can set VF mac from DPDK by calling set_default_mac() for representor. This API exists for all vendors. Not implemented for Intel, but new API is not implemented too.

No, the current situation is the following:
- for mlx5, VF MAC can be configured with iproute2 or netlink
- for ixgbe, rte_pmd_ixgbe_set_vf_mac_addr()
- for i40e, rte_pmd_i40e_set_vf_mac_addr()
- for bnxt, rte_pmd_bnxt_set_vf_mac_addr()

Thanks to Ilya's comment, this is an update of the DPDK situation:
- for mlx5, VF MAC can be configured with iproute2 or netlink and rte_eth_dev_default_mac_addr_set(rep) is not supported
- for ixgbe, rte_pmd_ixgbe_set_vf_mac_addr()
and rte_eth_dev_default_mac_addr_set(rep) does the same
- for i40e, rte_pmd_i40e_set_vf_mac_addr()
and rte_eth_dev_default_mac_addr_set(rep) does the same
- for bnxt, rte_pmd_bnxt_set_vf_mac_addr()
and no representor

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

Quoted 3 lines

Quoted 3 lines

I am not responsible of the choices in Linux.
But I agree it would be interesting to check the reason of such decision. Rony, please could you explain?

I looked at few Linux drivers:

bnxt_vf_rep_netdev_ops has no op to set MAC bnxt_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

lio_vf_rep_ndev_ops has no op to set MAC lionetdevops.ndo_set_vf_mac = set VF MAC from PF

mlx5e_netdev_ops_rep has no op to set MAC mlx5e_netdev_ops.ndo_set_vf_mac = set VF MAC from PF mlx5e_netdev_ops_uplink_rep.ndo_set_vf_mac = set VF MAC from PF

nfp_repr_netdev_ops.ndo_set_mac_address = set representor MAC nfp_repr_netdev_ops.ndo_set_vf_mac = set VF MAC from representor nfp_net_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

There is a big chance that the behaviour is not standardized in Linux (as usual). So it is already confusing for users of Linux.

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 7 lines · Thomas Monjalon

30/10/2019 17:09, Ilya Maximets:

Quoted 6 lines · Thomas Monjalon

Quoted 5 lines

30/10/2019 16:07, Ilya Maximets:

Quoted 4 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

Quoted 7 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Since we already have a confusion about what is configured when operations are performed on a representor port we have 2 options:

1. Have this proposed API to configure representor itself while setting config to representor and configuring VF if special bit enabled.
2. Reverse the logic of current proposal, i.e. always apply configuration to VF while working with representor and apply configuration to representor itself if special bit is set.

I'd probably prefer option #2, because:
- From the OVS and OpenStack point of view, I think, we don't really need to configure representor itself in most cases. And OVS really should not know if it works with representor or some real port.

- It seems that most of the existing code in DPDK already works like this, i.e. applying configs to VF itself. Intel drivers works like this and Mellanox drivers, as Thomas said, doesn't have this functionality at all.

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

Quoted 3 lines

Quoted 3 lines

I am not responsible of the choices in Linux.
But I agree it would be interesting to check the reason of such decision. Rony, please could you explain?

I looked at few Linux drivers:

bnxt_vf_rep_netdev_ops has no op to set MAC bnxt_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

Quoted 6 lines

lio_vf_rep_ndev_ops has no op to set MAC lionetdevops.ndo_set_vf_mac = set VF MAC from PF

mlx5e_netdev_ops_rep has no op to set MAC mlx5e_netdev_ops.ndo_set_vf_mac = set VF MAC from PF mlx5e_netdev_ops_uplink_rep.ndo_set_vf_mac = set VF MAC from

PF

Quoted 2 lines

nfp_repr_netdev_ops.ndo_set_mac_address = set representor

MAC

Quoted 1 line

nfp_repr_netdev_ops.ndo_set_vf_mac = set VF MAC from

representor

Quoted 6 lines

nfp_net_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

There is a big chance that the behaviour is not standardized in Linux (as usual). So it is already confusing for users of Linux.

Quoted 10 lines

Quoted 9 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Quoted 2 lines

Since we already have a confusion about what is configured when operations are performed on a representor port we have 2 options:

I don't agree we have. I don't think there is any design note or API doc that says the ethdev configuration on representor should be applied on VF (please share if I missed it).
The fact that there are some drivers that implemented it doesn't mean it is correct.

Quoted 8 lines

1. Have this proposed API to configure representor itself while setting config to representor and configuring VF if special bit enabled.
2. Reverse the logic of current proposal, i.e. always apply configuration to VF while working with representor and apply configuration to representor itself if special bit is set.

Quoted 5 lines

I'd probably prefer option #2, because:
- From the OVS and OpenStack point of view, I think, we don't really need to configure representor itself in most cases. And OVS really should not know if it works with representor or some real port.

I don't thinks OVS can be really agnostic to the fact it runs on top of representors:
1. probing of representor has different command line -w <bdf>,representor=XXX
2. the whole acceleration framework based on insertion of flow rules for direct forward from the VF to target entity. Rules are applied on the representor and would not work if port is not such.
3. some multi-port devices cannot do direct fwd between its different port. This is why rep has switch_id and application should query it and act upon.
4. representor carry the VF port id. This is how application know to which VF (or vport) they associated with on their other side.

Quoted 4 lines

- It seems that most of the existing code in DPDK already works like this, i.e. applying configs to VF itself. Intel drivers works like this and Mellanox drivers, as Thomas said, doesn't have this functionality at all.

As I said above, I don't think we need to refer to specific driver behavior, rather the API guidelines.
To me, it is a bit strange and not natural that ethdev configuration is applied to different port w/o any explicit request from the application. This is why I would prefer #1 above.

Quoted 37 lines

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

Quoted 3 lines

Quoted 3 lines

I am not responsible of the choices in Linux.
But I agree it would be interesting to check the reason of such decision. Rony, please could you explain?

I looked at few Linux drivers:

bnxt_vf_rep_netdev_ops has no op to set MAC bnxt_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

Quoted 6 lines

lio_vf_rep_ndev_ops has no op to set MAC lionetdevops.ndo_set_vf_mac = set VF MAC from PF

mlx5e_netdev_ops_rep has no op to set MAC mlx5e_netdev_ops.ndo_set_vf_mac = set VF MAC from PF mlx5e_netdev_ops_uplink_rep.ndo_set_vf_mac = set VF MAC from

PF

Quoted 2 lines

nfp_repr_netdev_ops.ndo_set_mac_address = set representor

MAC

Quoted 1 line

nfp_repr_netdev_ops.ndo_set_vf_mac = set VF MAC from

representor

Quoted 6 lines

nfp_net_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

There is a big chance that the behaviour is not standardized in Linux (as usual). So it is already confusing for users of Linux.

Quoted 12 lines

Quoted 11 lines

Quoted 8 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Since we already have a confusion about what is configured when operations are performed on a representor port we have 2 options:

Quoted 3 lines

I don't agree we have. I don't think there is any design note or API doc that says the ethdev configuration on representor should be applied on VF (please share if I missed it).
The fact that there are some drivers that implemented it doesn't mean it is correct.

Well, it means that at least authors and reviewers of these patches,
plus probably next-net maintainers believe that it is correct.
At least they did - when patch was applied.
If that is not clearly stated in the doc - it might be just the gap in the documentation,

that needs to be fixed, not a mandate to break existing behavior.

03/11/2019 16:27, Ananyev, Konstantin:

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Since we already have a confusion about what is configured when operations are performed on a representor port we have 2 options:

Quoted 7 lines

Quoted 3 lines

I don't agree we have. I don't think there is any design note or API doc that says the ethdev configuration on representor should be applied on VF (please share if I missed it).
The fact that there are some drivers that implemented it doesn't mean it is correct.

Well, it means that at least authors and reviewers of these patches, plus probably next-net maintainers believe that it is correct. At least they did - when patch was applied.

Quoted 1 line

If that is not clearly stated in the doc - it might be just the gap in the documentation,

Gap in the documentation? We should state that the config should be applied to the port specified with port_id and no other one? Funny

Quoted 1 line

that needs to be fixed, not a mandate to break existing behavior.

So because you managed to have a wrong patch applied in your PMD, you want to make it the generic API in ethdev?
What a process!

Hey guys, if you want to change an API behaviour in a way others don't, you just have to implement what you want in your PMD silently, then you will be able to change the API to comply with your behaviour. Wonderful.
If we allow such practice, DPDK is dead.

Friday, November 1, 2019 11:33 AM, Ilya Maximets:

Quoted 11 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 7 lines · Thomas Monjalon

30/10/2019 17:09, Ilya Maximets:

Quoted 6 lines · Thomas Monjalon

Quoted 5 lines

30/10/2019 16:07, Ilya Maximets:

Quoted 4 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

[...]

Quoted 10 lines

Quoted 7 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Since we already have a confusion about what is configured when operations are performed on a representor port we have 2 options:

I don't agree we have. I don't think there is any design note or API doc that says the ethdev configuration on representor should be applied on VF (please share if I missed it). The fact that there are some drivers that implemented it doesn't mean it is correct.

Quoted 7 lines

1. Have this proposed API to configure representor itself while setting config to representor and configuring VF if special bit enabled.
2. Reverse the logic of current proposal, i.e. always apply configuration to VF while working with representor and apply configuration to representor itself if special bit is set.

Quoted 5 lines

I'd probably prefer option #2, because:
- From the OVS and OpenStack point of view, I think, we don't really need to configure representor itself in most cases. And OVS really should not know if it works with representor or some real port.

I don't thinks OVS can be really agnostic to the fact it runs on top of representors:
1. probing of representor has different command line -w <bdf>,representor=XXX

2. the whole acceleration framework based on insertion of flow rules for direct forward from the VF to target entity. Rules are applied on the representor and would not work if port is not such.

3. some multi-port devices cannot do direct fwd between its different port. This is why rep has switch_id and application should query it and act upon.

4. representor carry the VF port id. This is how application know to which VF (or vport) they associated with on their other side.

Quoted 5 lines

- It seems that most of the existing code in DPDK already works like this, i.e. applying configs to VF itself. Intel drivers works like this and Mellanox drivers, as Thomas said, doesn't have this functionality at all.

As I said above, I don't think we need to refer to specific driver behavior, rather the API guidelines.
To me, it is a bit strange and not natural that ethdev configuration is applied to different port w/o any explicit request from the application. This is why I would prefer #1 above.

Quoted 38 lines

Quoted 20 lines

Quoted 11 lines

Quoted 11 lines

Quoted 10 lines

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

Quoted 3 lines

Quoted 3 lines

I am not responsible of the choices in Linux.
But I agree it would be interesting to check the reason of such decision. Rony, please could you explain?

I looked at few Linux drivers:

bnxt_vf_rep_netdev_ops has no op to set MAC bnxt_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

Quoted 6 lines

lio_vf_rep_ndev_ops has no op to set MAC lionetdevops.ndo_set_vf_mac = set VF MAC from PF

mlx5e_netdev_ops_rep has no op to set MAC mlx5e_netdev_ops.ndo_set_vf_mac = set VF MAC from PF mlx5e_netdev_ops_uplink_rep.ndo_set_vf_mac = set VF MAC from

PF

Quoted 2 lines

nfp_repr_netdev_ops.ndo_set_mac_address = set representor

MAC

Quoted 1 line

nfp_repr_netdev_ops.ndo_set_vf_mac = set VF MAC from

representor

Quoted 6 lines

nfp_net_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

There is a big chance that the behaviour is not standardized in Linux (as usual). So it is already confusing for users of Linux.

-----Original Message-----
From: dev <[email protected]> On Behalf Of Ilya Maximets
Sent: Monday, November 4, 2019 12:28 PM
To: Shahaf Shuler <[email protected]>; Ilya Maximets <[email protected]>; Thomas Monjalon <[email protected]>
Cc: [email protected]; Jerin Jacob <[email protected]>; Andrew Rybchenko <[email protected]>; Ferruh Yigit <[email protected]>; Stephen Hemminger <[email protected]>; Roni Bar Yanai <[email protected]>; Rony Efraim <[email protected]>; [email protected]; [email protected]; [email protected]; Ananyev, Konstantin <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 5 lines · Shahaf Shuler

Friday, November 1, 2019 11:33 AM, Ilya Maximets:

Quoted 4 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

On 30.10.2019 22:42, Thomas Monjalon wrote:

Quoted 18 lines

Quoted 7 lines

Quoted 7 lines

30/10/2019 17:09, Ilya Maximets:

Quoted 6 lines · Thomas Monjalon

Quoted 5 lines

30/10/2019 16:07, Ilya Maximets:

Quoted 4 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

[...]

Quoted 9 lines

Quoted 7 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Since we already have a confusion about what is configured when

operations

Quoted 3 lines

Quoted 1 line

are performed on a representor port we have 2 options:

I don't agree we have. I don't think there is any design note or API doc that

says the ethdev configuration on representor should be applied on VF (please share if I missed it).

Quoted 1 line

The fact that there are some drivers that implemented it doesn't mean it is

correct.

Quoted 8 lines

Quoted 8 lines

1. Have this proposed API to configure representor itself while setting config to representor and configuring VF if special bit enabled.
2. Reverse the logic of current proposal, i.e. always apply configuration to VF while working with representor and apply configuration to representor itself if special bit is set.

Quoted 7 lines

Quoted 5 lines

I'd probably prefer option #2, because:
- From the OVS and OpenStack point of view, I think, we don't really need to configure representor itself in most cases. And OVS really should not know if it works with representor or some real port.

I don't thinks OVS can be really agnostic to the fact it runs on top of

representors:

Quoted 1 line

1. probing of representor has different command line -w

<bdf>,representor=XXX

OVS doesn't care about content of devargs. It just passes them to hotplug engine without any parsing (except a single case that must be eliminated with a proper device iterators, not an OVS issue).

Quoted 1 line

2. the whole acceleration framework based on insertion of flow rules for

direct forward from the VF to target entity. Rules are applied on the representor and would not work if port is not such.

OVS tries to offload rules to the netdev from which packet was received. That's it. If it succeeds - OK. If not, OVS doesn't care.

Quoted 1 line

3. some multi-port devices cannot do direct fwd between its different port.

This is why rep has switch_id and application should query it and act upon.

This is part of offloading engine that doesn't affect the generic code. If needed, OVS could request switch_id for netdev it tries to offload rules on. OVS should not know if it representor port or not. If this operation will not succeed for non-representors, OVS should not care because we can't offload anything for non-representors anyway.

Quoted 1 line

4. representor carry the VF port id. This is how application know to which

VF (or vport) they associated with on their other side.

This is just part of devargs, i.e. part of device unique identifier. Once again, OVS doesn't parse devargs and should not do that.

Quoted 7 lines

Quoted 5 lines

- It seems that most of the existing code in DPDK already works like this, i.e. applying configs to VF itself. Intel drivers works like this and Mellanox drivers, as Thomas said, doesn't have this functionality at all.

As I said above, I don't think we need to refer to specific driver behavior,

rather the API guidelines.

Quoted 1 line

To me, it is a bit strange and not natural that ethdev configuration is applied

to different port w/o any explicit request from the application.

Quoted 1 line

This is why I would prefer #1 above.

IMHO, the whole concept of representors is that representor is a way of attaching same port both to VM and vSwitch/hypervisor.

If you're looking at representors as a separate ports on a switch, well.. In this case, for me VF configuration looks like something that vSwitch should not do at all, because it should not configure ports that doesn't attached to it. It's like configuring the other side of veth pair, which is nonsense.

BTW, I don't know a way to find out if port is a representor of something or not in Linux kernel.

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I am not responsible of the choices in Linux.
But I agree it would be interesting to check the reason of such

decision.

Quoted 25 lines

Quoted 25 lines

Quoted 7 lines

Quoted 1 line

Quoted 1 line

Rony, please could you explain?

I looked at few Linux drivers:

bnxt_vf_rep_netdev_ops has no op to set MAC bnxt_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

Quoted 6 lines

lio_vf_rep_ndev_ops has no op to set MAC lionetdevops.ndo_set_vf_mac = set VF MAC from PF

mlx5e_netdev_ops_rep has no op to set MAC mlx5e_netdev_ops.ndo_set_vf_mac = set VF MAC from PF mlx5e_netdev_ops_uplink_rep.ndo_set_vf_mac = set VF MAC from

PF

Quoted 2 lines

nfp_repr_netdev_ops.ndo_set_mac_address = set representor

MAC

Quoted 1 line

nfp_repr_netdev_ops.ndo_set_vf_mac = set VF MAC from

representor

Quoted 6 lines

nfp_net_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

There is a big chance that the behaviour is not standardized in Linux (as usual). So it is already confusing for users of Linux.

Regards,
Asaf Penso

Quoted 114 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Ilya Maximets
Sent: Monday, November 4, 2019 12:28 PM
To: Shahaf Shuler <[email protected]>; Ilya Maximets <[email protected]>; Thomas Monjalon <[email protected]>
Cc: [email protected]; Jerin Jacob <[email protected]>; Andrew Rybchenko <[email protected]>; Ferruh Yigit <[email protected]>; Stephen Hemminger <[email protected]>; Roni Bar Yanai <[email protected]>; Rony Efraim <[email protected]>; [email protected]; [email protected]; [email protected]; Ananyev, Konstantin <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 5 lines · Shahaf Shuler

Friday, November 1, 2019 11:33 AM, Ilya Maximets:

Quoted 4 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

On 30.10.2019 22:42, Thomas Monjalon wrote:

Quoted 18 lines

Quoted 7 lines

Quoted 7 lines

30/10/2019 17:09, Ilya Maximets:

Quoted 6 lines · Thomas Monjalon

Quoted 5 lines

30/10/2019 16:07, Ilya Maximets:

Quoted 4 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

[...]

Quoted 9 lines

Quoted 7 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Since we already have a confusion about what is configured when

operations

Quoted 3 lines

Quoted 1 line

are performed on a representor port we have 2 options:

I don't agree we have. I don't think there is any design note or API doc that

says the ethdev configuration on representor should be applied on VF (please share if I missed it).

Quoted 1 line

The fact that there are some drivers that implemented it doesn't mean it is

correct.

Quoted 8 lines

Quoted 8 lines

1. Have this proposed API to configure representor itself while setting config to representor and configuring VF if special bit enabled.
2. Reverse the logic of current proposal, i.e. always apply configuration to VF while working with representor and apply configuration to representor itself if special bit is set.

Quoted 7 lines

Quoted 5 lines

I'd probably prefer option #2, because:
- From the OVS and OpenStack point of view, I think, we don't really need to configure representor itself in most cases. And OVS really should not know if it works with representor or some real port.

I don't thinks OVS can be really agnostic to the fact it runs on top of

representors:

Quoted 1 line

1. probing of representor has different command line -w

<bdf>,representor=XXX

OVS doesn't care about content of devargs. It just passes them to hotplug engine without any parsing (except a single case that must be eliminated with a proper device iterators, not an OVS issue).

Quoted 1 line

2. the whole acceleration framework based on insertion of flow rules for

direct forward from the VF to target entity. Rules are applied on the representor and would not work if port is not such.

OVS tries to offload rules to the netdev from which packet was received. That's it. If it succeeds - OK. If not, OVS doesn't care.

Quoted 1 line

3. some multi-port devices cannot do direct fwd between its different port.

This is why rep has switch_id and application should query it and act upon.

This is part of offloading engine that doesn't affect the generic code. If needed, OVS could request switch_id for netdev it tries to offload rules on. OVS should not know if it representor port or not. If this operation will not succeed for non-representors, OVS should not care because we can't offload anything for non-representors anyway.

Quoted 1 line

4. representor carry the VF port id. This is how application know to which

VF (or vport) they associated with on their other side.

This is just part of devargs, i.e. part of device unique identifier. Once again, OVS doesn't parse devargs and should not do that.

Quoted 7 lines

Quoted 5 lines

- It seems that most of the existing code in DPDK already works like this, i.e. applying configs to VF itself. Intel drivers works like this and Mellanox drivers, as Thomas said, doesn't have this functionality at all.

As I said above, I don't think we need to refer to specific driver behavior,

rather the API guidelines.

Quoted 1 line

To me, it is a bit strange and not natural that ethdev configuration is applied

to different port w/o any explicit request from the application.

Quoted 1 line

This is why I would prefer #1 above.

IMHO, the whole concept of representors is that representor is a way of attaching same port both to VM and vSwitch/hypervisor.

If you're looking at representors as a separate ports on a switch, well.. In this case, for me VF configuration looks like something that vSwitch should not do at all, because it should not configure ports that doesn't attached to it. It's like configuring the other side of veth pair, which is nonsense.

Quoted 2 lines

BTW, I don't know a way to find out if port is a representor of something or not in Linux kernel.

Specifically in OVS, in function dpdk_eth_dev_init, you can do this to detect:
*info.dev_flags & RTE_ETH_DEV_REPRESENTOR

Quoted 41 lines

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 12 lines

The this is that this new API will produce conceptual fragmentation between DPDK and the Linux kernel, because to do the same thing you'll have to use different ways. I mean, to change mac of VF in kernel you need to set mac to the representor, but in DPDK changing setting mac to representor will lead to changing the mac of the representor itself, not the VF. This will be really confusing for users.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I am not responsible of the choices in Linux.
But I agree it would be interesting to check the reason of such

decision.

Quoted 25 lines

Quoted 25 lines

Quoted 7 lines

Quoted 1 line

Quoted 1 line

Rony, please could you explain?

I looked at few Linux drivers:

bnxt_vf_rep_netdev_ops has no op to set MAC bnxt_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

Quoted 6 lines

lio_vf_rep_ndev_ops has no op to set MAC lionetdevops.ndo_set_vf_mac = set VF MAC from PF

mlx5e_netdev_ops_rep has no op to set MAC mlx5e_netdev_ops.ndo_set_vf_mac = set VF MAC from PF mlx5e_netdev_ops_uplink_rep.ndo_set_vf_mac = set VF MAC from

PF

Quoted 2 lines

nfp_repr_netdev_ops.ndo_set_mac_address = set representor

MAC

Quoted 1 line

nfp_repr_netdev_ops.ndo_set_vf_mac = set VF MAC from

representor

Quoted 6 lines

nfp_net_netdev_ops.ndo_set_vf_mac = set VF MAC from PF

There is a big chance that the behaviour is not standardized in Linux (as usual). So it is already confusing for users of Linux.

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 19 lines · Shahaf Shuler

Friday, November 1, 2019 11:33 AM, Ilya Maximets:

Quoted 11 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 7 lines · Thomas Monjalon

30/10/2019 17:09, Ilya Maximets:

Quoted 6 lines · Thomas Monjalon

Quoted 5 lines

30/10/2019 16:07, Ilya Maximets:

Quoted 4 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

[...]

Quoted 5 lines

Quoted 5 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Since we already have a confusion about what is configured when operations are performed on a representor port we have 2 options:

I don't agree we have. I don't think there is any design note or API doc that

says the ethdev configuration on representor should be applied on VF (please share if I missed it).

Quoted 1 line

The fact that there are some drivers that implemented it doesn't mean it is

correct.

Quoted 8 lines

Quoted 8 lines

1. Have this proposed API to configure representor itself while setting config to representor and configuring VF if special bit enabled.
2. Reverse the logic of current proposal, i.e. always apply configuration to VF while working with representor and apply configuration to representor itself if special bit is set.

Quoted 7 lines

Quoted 5 lines

I'd probably prefer option #2, because:
- From the OVS and OpenStack point of view, I think, we don't really need to configure representor itself in most cases. And OVS really should not know if it works with representor or some real port.

I don't thinks OVS can be really agnostic to the fact it runs on top of

representors:

Quoted 2 lines

1. probing of representor has different command line -w <bdf>,representor=XXX

OVS doesn't care about content of devargs. It just passes them to hotplug engine without any parsing (except a single case that must be eliminated with a proper device iterators, not an OVS issue).

Quoted 1 line

2. the whole acceleration framework based on insertion of flow rules for

direct forward from the VF to target entity. Rules are applied on the representor and would not work if port is not such.

OVS tries to offload rules to the netdev from which packet was received. That's it. If it succeeds - OK. If not, OVS doesn't care.

Quoted 1 line

3. some multi-port devices cannot do direct fwd between its different port.

This is why rep has switch_id and application should query it and act upon.

This is part of offloading engine that doesn't affect the generic code. If needed, OVS could request switch_id for netdev it tries to offload rules on. OVS should not know if it representor port or not. If this operation will not succeed for non-representors, OVS should not care because we can't offload anything for non-representors anyway.

Quoted 2 lines

4. representor carry the VF port id. This is how application know to which

VF (or vport) they associated with on their other side.

This is just part of devargs,

Once again, OVS doesn't parse devargs and should not do that.

Quoted 8 lines

Quoted 6 lines

- It seems that most of the existing code in DPDK already works like this, i.e. applying configs to VF itself. Intel drivers works like this and Mellanox drivers, as Thomas said, doesn't have this functionality at all.

As I said above, I don't think we need to refer to specific driver behavior,

rather the API guidelines.

Quoted 1 line

To me, it is a bit strange and not natural that ethdev configuration is applied

to different port w/o any explicit request from the application.

Quoted 1 line

This is why I would prefer #1 above.

IMHO, the whole concept of representors is that representor is a way of attaching same port both to VM and vSwitch/hypervisor.

If you're looking at representors as a separate ports on a switch, well..

In this case, for me VF configuration looks like something that vSwitch should not do at all, because it should not configure ports that doesn't attached to it. It's like configuring the other side of veth pair, which is nonsense.

BTW, I don't know a way to find out if port is a representor of something or not in Linux kernel.

Monday, November 4, 2019 12:28 PM, Ilya Maximets:

Quoted 64 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 19 lines · Shahaf Shuler

Friday, November 1, 2019 11:33 AM, Ilya Maximets:

Quoted 11 lines

Subject: Re: [dpdk-dev] [PATCH v2 0/3] ethdev: configure SR-IOV VF from host

Quoted 7 lines · Thomas Monjalon

30/10/2019 17:09, Ilya Maximets:

Quoted 6 lines · Thomas Monjalon

Quoted 5 lines

30/10/2019 16:07, Ilya Maximets:

Quoted 4 lines · Thomas Monjalon

Quoted 3 lines

In a virtual environment, the network controller may have to configure some SR-IOV VF parameters for security reasons.

[...]

Quoted 5 lines

Quoted 5 lines

If we consider what Intel did, i.e. configure VF in place of representor for some operations, there are two drawbacks:
- confusing that some ops apply to representor, others apply to VF
- some ops are not possible on representor (because targetted to VF)

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

I still feel that the addition of one single bit in the port ID is an elegant solution to target either the VF or its representor.

Since we already have a confusion about what is configured when operations are performed on a representor port we have 2 options:

I don't agree we have. I don't think there is any design note or API doc that

says the ethdev configuration on representor should be applied on VF (please share if I missed it).

Quoted 1 line

The fact that there are some drivers that implemented it doesn't mean it is

correct.

Quoted 8 lines

Quoted 8 lines

1. Have this proposed API to configure representor itself while setting config to representor and configuring VF if special bit enabled.
2. Reverse the logic of current proposal, i.e. always apply configuration to VF while working with representor and apply configuration to representor itself if special bit is set.

Quoted 7 lines

Quoted 5 lines

I'd probably prefer option #2, because:
- From the OVS and OpenStack point of view, I think, we don't really need to configure representor itself in most cases. And OVS really should not know if it works with representor or some real port.

I don't thinks OVS can be really agnostic to the fact it runs on top of

representors:

Quoted 2 lines

1. probing of representor has different command line -w <bdf>,representor=XXX

OVS doesn't care about content of devargs. It just passes them to hotplug engine without any parsing (except a single case that must be eliminated with a proper device iterators, not an OVS issue).

Quoted 1 line

2. the whole acceleration framework based on insertion of flow rules for

direct forward from the VF to target entity. Rules are applied on the representor and would not work if port is not such.

OVS tries to offload rules to the netdev from which packet was received. That's it. If it succeeds - OK. If not, OVS doesn't care.

Quoted 8 lines

Quoted 1 line

3. some multi-port devices cannot do direct fwd between its different port.

This is why rep has switch_id and application should query it and act upon.

This is part of offloading engine that doesn't affect the generic code. If needed, OVS could request switch_id for netdev it tries to offload rules on. OVS should not know if it representor port or not. If this operation will not succeed for non-representors, OVS should not care because we can't offload anything for non-representors anyway.

I am not speaking on specific module in OVS if it is aware or not, rather OVS as a whole.
There is a big code part in OVS that its sole purpose is to create flow rule on representors.
There is no meaning for this code w/o representors and it should be used only when OVS runs on top of representors (otherwise I would expect bad perf due to the constant preparation of rte_flow rules).

Quoted 6 lines

Quoted 3 lines

4. representor carry the VF port id. This is how application know to which

VF (or vport) they associated with on their other side.

This is just part of devargs,

It is not part of devargs, it is part of the ethdev info.
OVS should expose such info to the user in order to help w/ the open flow rules creation.

i.e. part of device unique identifier.

Quoted 1 line

Once again, OVS doesn't parse devargs and should not do that.

So how can OVS user know the mapping between representor and its VF?

Quoted 10 lines

Quoted 9 lines

Quoted 7 lines

- It seems that most of the existing code in DPDK already works like this, i.e. applying configs to VF itself. Intel drivers works like this and Mellanox drivers, as Thomas said, doesn't have this functionality at all.

As I said above, I don't think we need to refer to specific driver behavior,

rather the API guidelines.

Quoted 8 lines

Quoted 1 line

To me, it is a bit strange and not natural that ethdev configuration is applied

to different port w/o any explicit request from the application.

Quoted 1 line

This is why I would prefer #1 above.

IMHO, the whole concept of representors is that representor is a way of attaching same port both to VM and vSwitch/hypervisor.

If you're looking at representors as a separate ports on a switch, well..

I think we cannot ignore the fact that both in Linux and DPDK those are netdev/ethdev and as such holds some network configuration (even though on some case it is very minimal or some random values).

Quoted 3 lines

In this case, for me VF configuration looks like something that vSwitch should not do at all, because it should not configure ports that doesn't attached to it. It's like configuring the other side of veth pair, which is nonsense.

It seems to me we configure the remote VF on every possible solution. The suggest one discussed here just make it more explicit.

Quoted 4 lines

BTW, I don't know a way to find out if port is a representor of something or not in Linux kernel.

One example is by querying the netdev phys_port_name. representor ports will have specific strings to describe the remote end (e.g. pf0vf0) while regular port (e.g. uplink) will have pf0.