Re: [dpdk-dev] [RFC PATCH] lib/librte_ethdev: Expand port identifier

[Marc Sune <marc.sune@bisdn.de>]

On 18/02/15 13:31, Bruce Richardson wrote:
> On Wed, Feb 18, 2015 at 12:30:07PM +0000, Bruce Richardson wrote:
>> On Wed, Feb 18, 2015 at 08:02:49PM +0900, Tetsuya Mukawa wrote:
>>> Currently uint8_t is used for port identifier. This patch changes it,
>>> and use uint16_t as port identifier.
>>> This patch only changes ethdev library. ABI of the library will be
>>> kept even after applying it.
>>>
>>> Also, this patch involves following fixes.
>>> - Use "port_id" as variable name instead of "port".
>>>
>>>
>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>>> ---
>>>   lib/librte_ether/rte_ethdev.c          |  212 +-
>>>   lib/librte_ether/rte_ethdev_internal.h | 3672 ++++++++++++++++++++++++++++++++
>>>   2 files changed, 3778 insertions(+), 106 deletions(-)
>>>   create mode 100644 lib/librte_ether/rte_ethdev_internal.h
>>>
>> I'm not sure I follow why we need a new header file for this.
>> Also, thinking about this change, a more fundamental problem is going to be
>> the mbuf structure, which stores a port id inside it in an 8-bit value.
>> Upgrading that to a 16-bit value requires some thought, and verification to
>> ensure any adjustment of fields does not lead to serious performance issues.
>>
>> Therefore, I suggest we leave the port id values as 8-bits until such time
>> as we need greater than 255 port values in a real-world use case.
>> Out of interest - anyone have a DPDK app where they use >16 port id values? If
>> so, how high does the port id value get?

Just a though on port_id in general; I wouldn't see why other type of 
ports could fall into the same abstraction of using port_ids as we do 
for PHY ports, if eventually we could create a unified API TX/RX 
routines he same regardless of the port (I know KNI deprecated this 
approach in the past). Of course initialization routines should be 
different for each type of port.

I see quite a bit of code duplicity, basically in TX/RX routines for PHY 
ports, KNI ports, SHMEM (ring) ports like ivshmem etc.., which are very 
similar, and we put into the shoulders of all users of DPDK to have to 
do the "switch() - case" based on the type of port (which is state that 
they have to store themselves too). This seems to me it could be 
improved from a DPDK user's point of view.

By no means I am saying lower level APIs should not be exposed (current 
APIs)... There is the need to, since users using one type of ports only 
should be able to by-pass that (small?) extra overhead of this higher 
level APIs.

If the implementation would eventually would go into this direction, 
there would be more pressure in the port_id identifier; e.g. KNI 
interfaces and other SW like interfaces can be created and destroyed 
quite frequently (e.g. VMs), so more than 8 bits for addressing would 
probably be needed.

I know it not helping in the short-term, but let's see if someone thinks 
this makes any sense at all.

Marc

>>
>> Regards,
>> /Bruce
>>
> Resending with correct email addr for Neil.
>
> /Bruce