[dpdk-dev] [PATCH v8 03/14] eal/pci, ethdev: Remove assumption that port will not be detached

[Iremonger, Bernard]

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tetsuya Mukawa
> Sent: Wednesday, February 18, 2015 10:58 AM
> To: Richardson, Bruce; Thomas Monjalon
> Cc: dev at dpdk.org; Neil Horman
> Subject: Re: [dpdk-dev] [PATCH v8 03/14] eal/pci, ethdev: Remove assumption that port will not be
> detached
> 
> On 2015/02/18 19:03, Bruce Richardson wrote:
> > On Wed, Feb 18, 2015 at 10:57:25AM +0100, Thomas Monjalon wrote:
> >> 2015-02-18 15:10, Tetsuya Mukawa:
> >>> On 2015/02/18 10:54, Tetsuya Mukawa wrote:
> >>>> On 2015/02/18 9:31, Thomas Monjalon wrote:
> >>>>> 2015-02-17 15:14, Tetsuya Mukawa:
> >>>>>> On 2015/02/17 9:36, Thomas Monjalon wrote:
> >>>>>>> 2015-02-16 13:14, Tetsuya Mukawa:
> >>>>>>> Is uint8_t sill a good size for hotpluggable virtual device ids?
> >>>>>> I am not sure it's enough, but uint8_t is widely used in "rte_ethdev.c"
> >>>>>> as port id.
> >>>>>> If someone reports it doesn't enough, I guess it will be the time
> >>>>>> to write a patch to change all uint_8 in one patch.
> >>>>> It's a big ABI breakage. So if we feel it's going to be required,
> >>>>> it's better to do it now in 2.0 release I think.
> >>>>>
> >>>>> Any opinion?
> >>>>>
> >>>> Hi Thomas,
> >>>>
> >>>> I agree with it.
> >>>> I will add an one more patch to change uint8_t to uint16_t.
> >>>>
> >>>> Thanks,
> >>>> Tetsuya
> >>>>
> >>> Hi Thomas,
> >>>
> >>> Could I make sure.
> >>> After changing uint8_t to uint16_t in "rte_ethdev.[ch]", must I also
> >>> need to change other applications and libraries that call ethdev APIs?
> >>> If so, I would not finish it by 23rd.
> >>>
> >>> I've counted how many lines call ethdev APIs that are related to port_id.
> >>> Could you please check an attached file?
> >>> It's over 1200 lines. Probably to fix  one of caller, I will need to
> >>> check how port_id is used, and fix more related lines. So probably
> >>> thousands lines may need to be fixed.
> >>>
> >>> When is deadline for fixing this changing?
> >>> Also, if you have a good idea to fix it easier, could you please let
> >>> me know?
> >> It was an open question.
> >> If everybody is fine with 255 ports maximum, let's keep it as is.
> >>
> > I think we are probably ok for now (and forseeable future) with 255 max.
> >
> > However, if we did change it, I agree that in 2.0 is a very good time to do so.
> > Since we are expanding the field, rather than shrinking it, I don't
> > see why we can't just make the change at the ethdev level (and in libs
> > API) in 2.0 and then in later releases (e.g. 2.1) update the apps and
> > examples to match. That way the ABI stays the same from 2.0 onwards,
> > and we don't have a huge amount of churn changing it everywhere late in the 2.0 release cycle.
> 
> Hi Bruce,
> 
> Could you please check my RFC patch I will send soon?
> I wrote the patch like below.
> 
> 1. Copy header file like below.
> $ cp lib/librte_ether/rte_ethdev.h lib/librte_ether/rte_ethdev_internal.h
> 2. Change "rte_ethdev.c" to include "rte_ethdev_internal.h"
> 3. Change type of port id in "rte_ethdev.c" and "rte_ethdev_internal.h".
> 
> If the patch is OK, I wll send it with hotplug patches.
> 
> Thanks,
> Tetsuya
> 
> 
> > /Bruce
> 
Hi Tetsuya,

After this change there will be two header files with a lot of the same information.
lib/librte_ether/rte_ethdev.h
lib/librte_ether/rte_ethdev_internal.h
I don't think this is a good idea for maintenance in the future.
If 255 is ok for the foreseeable future, why change it now.

Regards,

Bernard.