Re: [dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF

["Ouyang, Changchun" <changchun.ouyang@intel.com>]

Hi Thomas,

As we can see below, There are already 4 existing functions for vf in the header file:
rte_ethdev.h:int rte_eth_dev_set_vf_rxmode(uint8_t port, uint16_t vf, uint16_t rx_mode,
rte_ethdev.h:rte_eth_dev_set_vf_tx(uint8_t port,uint16_t vf, uint8_t on);
rte_ethdev.h:rte_eth_dev_set_vf_rx(uint8_t port,uint16_t vf, uint8_t on);
rte_ethdev.h:rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,

So do we have plan to move them or remove them as they are all for VF specifically?
 
If no, why we can accept those functions, but not accept the rte_eth_set_vf_rate_limit? :-)

I have 2 new api in this patch, the rte_eth_set_queue_rate_limit is more generic, whose
Argument only have port and queue. 
but PRC customer has the requirement of API function to limit the vf tx rate,
so personally I think rte_eth_set_vf_rate_limit is necessary for them.

Thanks and regards,
Changchun

-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
Sent: Wednesday, May 28, 2014 6:48 AM
To: Ouyang, Changchun
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF

Hi Changchun,

2014-05-26 15:45, Ouyang Changchun:
>  /**
> + * Set the rate limitation for a queue on an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param queue_idx
> + *   The queue id.
> + * @param tx_rate
> + *   The tx rate allocated from the total link speed for this queue.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +int rte_eth_set_queue_rate_limit(uint8_t port_id, uint16_t queue_idx,
> +			uint16_t tx_rate);
> +
> +/**
> + * Set the rate limitation for a vf on an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param vf
> + *   VF id.
> + * @param tx_rate
> + *   The tx rate allocated from the total link speed for this VF id.
> + * @param q_msk
> + *   The queue mask which need to set the rate.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf,
> +			uint16_t tx_rate, uint64_t q_msk);

You are defining an API function specifically for VF. It's not generic and shouldn't appear in the API. We now have to be careful about the API and try to build a robust generic API which could become stable.

Is it possible to imagine another API where only port and queue parameters are required? 

Thanks
--
Thomas