Re: [dpdk-dev] [RFC] ethdev: configure SR-IOV VF from host

[Jerin Jacob Kollanukkaran <jerinj@marvell.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon
> Sent: Thursday, August 15, 2019 8:36 PM
> To: Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>
> Cc: dev@dpdk.org; Bernard Iremonger <bernard.iremonger@intel.com>;
> Shahaf Shuler <shahafs@mellanox.com>; E. Scott Daniels
> <daniels@research.att.com>; Wenzhuo Lu <wenzhuo.lu@intel.com>; Alex
> Zelezniak <alexz@att.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>;
> Declan Doherty <declan.doherty@intel.com>
> 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)

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.

diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 2922d5b7c..19a60cab2 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -621,6 +621,7 @@ struct rte_eth_dev_data {
                all_multicast : 1, /**< RX all multicast mode ON(1) / OFF(0). */
                dev_started : 1,   /**< Device state: STARTED(1) / STOPPED(0). */
                lro         : 1;   /**< RX LRO is ON(1) / OFF(0) */
+               vf         : 1;   /**< SR-IOV VF device */
        uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT];
                        /**< Queues state: STARTED(1) / STOPPED(0). */
        uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT];


> 
> 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)  {
> +       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;
>  }
> 
>