[dpdk-dev] [PATCH v5 18/29] app/testpmd: use VFD APIs on i40e

Ferruh Yigit ferruh.yigit at intel.com
Mon Dec 19 12:03:58 CET 2016


On 12/16/2016 8:30 PM, Vincent JARDIN wrote:
> Le 16/12/2016 à 20:02, Ferruh Yigit a écrit :
>> +#ifdef RTE_LIBRTE_IXGBE_PMD
>>  			"set all queues drop (port_id) (on|off)\n"
>>  			"    Set drop enable bit for all queues.\n\n"
>>
>>  			"set vf split drop (port_id) (vf_id) (on|off)\n"
>>  			"    Set split drop enable bit for a VF from the PF.\n\n"
>> +#endif
> 
> it is not related to i40e. This serie should only be for i40e.

Please check the patch a little wider, it doesn't introduce the ifdef,
instead patch narrows down the scope of ifdef [1], because now some
functions are not just for ixgbe, but used both with i40e and ixgbe, so
the ifdef moved into the function itself [2].


[1]
-#ifdef RTE_LIBRTE_IXGBE_PMD
 	"set tx loopback (port_id) (on|off)\n"
 	"    Enable or disable tx loopback.\n\n"

+#ifdef RTE_LIBRTE_IXGBE_PMD
 	"set all queues drop (port_id) (on|off)\n"
 	"    Set drop enable bit for all queues.\n\n"

 	"set vf split drop (port_id) (vf_id) (on|off)\n"
 	"    Set split drop enable bit for a VF from the PF.\n\n"
+#endif

 	"set vf mac antispoof (port_id) (vf_id) (on|off).\n"
 	"    Set MAC antispoof for a VF from the PF.\n\n"
-#endif



[2]
@@ -11187,10 +11242,21 @@ cmd_set_tx_loopback_parsed(
...
+#ifdef RTE_LIBRTE_IXGBE_PMD
+	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
+		ret = rte_pmd_ixgbe_set_tx_loopback(res->port_id, is_on);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+	if (strstr(dev_info.driver_name, "i40e") != NULL)
+		ret = rte_pmd_i40e_set_tx_loopback(res->port_id, is_on);
+#endif

> 
> Moreover, it is a strange logic: how will it scale for all PMDs?

It won't.
That is why they are in ifdef.

These are PMD specific APIs, as naming convention shows "rte_pmd_i40e_..."

For the PMD features, which are not generic enough, but hardware is
capable and user may want to benefit from, PMD specific APIs exists.
Instead of these functions bloat the eth_dev layer, and give a fake
sense of abstraction, these APIs moved into PMDs.
And it is always possible to move these into ethdev layer, when multiple
PMDs supports same feature.
I agree this is something that needs to keep an eye on it, and be sure
if an API is generic, move it into eth_dev layer.

The application that use the PMD specific API, needs to be conditionally
compiled because that API may not always exits.

Thanks,
ferruh


More information about the dev mailing list