All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] ethdev: add ioctl-like API to control device specific features
@ 2017-08-03 12:21 Chilikin, Andrey
  2017-08-03 13:21 ` Bruce Richardson
  0 siblings, 1 reply; 14+ messages in thread
From: Chilikin, Andrey @ 2017-08-03 12:21 UTC (permalink / raw)
  To: dev; +Cc: Richardson, Bruce, Ananyev, Konstantin, Wu, Jingjing

To control some device-specific features public device-specific functions
rte_pmd_*.h are used.

But this solution requires applications to distinguish devices at runtime
and, depending on the device type, call corresponding device-specific
functions even if functions' parameters are the same.

IOCTL-like API can be added to ethdev instead of public device-specific
functions to address the following:

* allow more usable support of features across a range of NIC from
  one vendor, but not others
* allow features to be implemented by multiple NIC drivers without
  relying on a critical mass to get the functionality in ethdev
* there are a large number of possible device specific functions, and
  creating individual APIs for each one is not a good solution
* IOCTLs are a proven method for solving this problem in other areas,
  i.e. OS kernels.

Control requests for this API will be globally defined at ethdev level, so
an application will use single API call to control different devices from
one/multiple vendors.

API call may look like as a classic ioctl with an extra parameter for
argument length for better sanity checks:

int
rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
        unsigned arg_length);

Regards,
Andrey

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-03 12:21 [RFC] ethdev: add ioctl-like API to control device specific features Chilikin, Andrey
@ 2017-08-03 13:21 ` Bruce Richardson
  2017-08-03 16:15   ` Stephen Hemminger
  0 siblings, 1 reply; 14+ messages in thread
From: Bruce Richardson @ 2017-08-03 13:21 UTC (permalink / raw)
  To: Chilikin, Andrey; +Cc: dev, Ananyev, Konstantin, Wu, Jingjing

On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
> To control some device-specific features public device-specific functions
> rte_pmd_*.h are used.
> 
> But this solution requires applications to distinguish devices at runtime
> and, depending on the device type, call corresponding device-specific
> functions even if functions' parameters are the same.
> 
> IOCTL-like API can be added to ethdev instead of public device-specific
> functions to address the following:
> 
> * allow more usable support of features across a range of NIC from
>   one vendor, but not others
> * allow features to be implemented by multiple NIC drivers without
>   relying on a critical mass to get the functionality in ethdev
> * there are a large number of possible device specific functions, and
>   creating individual APIs for each one is not a good solution
> * IOCTLs are a proven method for solving this problem in other areas,
>   i.e. OS kernels.
> 
> Control requests for this API will be globally defined at ethdev level, so
> an application will use single API call to control different devices from
> one/multiple vendors.
> 
> API call may look like as a classic ioctl with an extra parameter for
> argument length for better sanity checks:
> 
> int
> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
>         unsigned arg_length);
> 
> Regards,
> Andrey

I think we need to start putting in IOCTLs for ethdevs, much as I hate
to admit it, since I dislike IOCTLs and other functions with opaque
arguments! Having driver specific functions I don't think will scale
well as each vendor tries to expose as much of their driver specific
functionality as possible.

One other additional example: I discovered just this week another issue
with driver specific functions and testpmd, when I was working on the
meson build rework.

* With shared libraries, when we do "ninja install" we want our DPDK
  libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
  driver folder, so that they can be automatically loaded from that
  single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
* However, testpmd, as well as using the drivers as plugins, uses
  driver-specific functions, which means that it explicitly links
  against the pmd .so files.
* Those driver .so files are not in with the other libraries, so ld.so
  does not find the pmd, and the installed testpmd fails to run due to
  missing library dependencies.
* The workaround is to add the drivers path to the ld load path, but we
  should not require ld library path changes just to get DPDK apps to
  work.

Using ioctls instead of driver-specific functions would solve this.

My 2c.

Regards,
/Bruce

PS: Following discussion on-list, this probably should be discussed at a
tech board meeting, to close it out.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-03 13:21 ` Bruce Richardson
@ 2017-08-03 16:15   ` Stephen Hemminger
  2017-08-03 19:53     ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2017-08-03 16:15 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Chilikin, Andrey, dev, Ananyev, Konstantin, Wu, Jingjing

On Thu, 3 Aug 2017 14:21:38 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
> > To control some device-specific features public device-specific functions
> > rte_pmd_*.h are used.
> > 
> > But this solution requires applications to distinguish devices at runtime
> > and, depending on the device type, call corresponding device-specific
> > functions even if functions' parameters are the same.
> > 
> > IOCTL-like API can be added to ethdev instead of public device-specific
> > functions to address the following:
> > 
> > * allow more usable support of features across a range of NIC from
> >   one vendor, but not others
> > * allow features to be implemented by multiple NIC drivers without
> >   relying on a critical mass to get the functionality in ethdev
> > * there are a large number of possible device specific functions, and
> >   creating individual APIs for each one is not a good solution
> > * IOCTLs are a proven method for solving this problem in other areas,
> >   i.e. OS kernels.
> > 
> > Control requests for this API will be globally defined at ethdev level, so
> > an application will use single API call to control different devices from
> > one/multiple vendors.
> > 
> > API call may look like as a classic ioctl with an extra parameter for
> > argument length for better sanity checks:
> > 
> > int
> > rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
> >         unsigned arg_length);
> > 
> > Regards,
> > Andrey  
> 
> I think we need to start putting in IOCTLs for ethdevs, much as I hate
> to admit it, since I dislike IOCTLs and other functions with opaque
> arguments! Having driver specific functions I don't think will scale
> well as each vendor tries to expose as much of their driver specific
> functionality as possible.
> 
> One other additional example: I discovered just this week another issue
> with driver specific functions and testpmd, when I was working on the
> meson build rework.
> 
> * With shared libraries, when we do "ninja install" we want our DPDK
>   libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
>   driver folder, so that they can be automatically loaded from that
>   single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
> * However, testpmd, as well as using the drivers as plugins, uses
>   driver-specific functions, which means that it explicitly links
>   against the pmd .so files.
> * Those driver .so files are not in with the other libraries, so ld.so
>   does not find the pmd, and the installed testpmd fails to run due to
>   missing library dependencies.
> * The workaround is to add the drivers path to the ld load path, but we
>   should not require ld library path changes just to get DPDK apps to
>   work.
> 
> Using ioctls instead of driver-specific functions would solve this.
> 
> My 2c.

My 2c. No.

Short answer:
Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
despised by Linux kernel developers. They provide an unstructured, unsecured,
back door for device driver abuse. Try to get a new driver in Linux with
a unique ioctl, and it will be hard to get accepted.

Long answer:
So far every device specific feature has fit into ethdev model. Doing ioctl
is admitting "it is too hard to be general, we need need an out". For something
that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
For a real feature (think flow direction), we want a first class API for that.
For a wart, then devargs will do.

Give a good example of something that should be an ioctl. Don't build the
API first and then let it get cluttered.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-03 16:15   ` Stephen Hemminger
@ 2017-08-03 19:53     ` Thomas Monjalon
  2017-08-04  9:59       ` Chilikin, Andrey
  2017-08-04 11:58       ` Ferruh Yigit
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Monjalon @ 2017-08-03 19:53 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Bruce Richardson, Chilikin, Andrey, Ananyev,
	Konstantin, Wu, Jingjing

03/08/2017 18:15, Stephen Hemminger:
> On Thu, 3 Aug 2017 14:21:38 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
> > > To control some device-specific features public device-specific functions
> > > rte_pmd_*.h are used.
> > > 
> > > But this solution requires applications to distinguish devices at runtime
> > > and, depending on the device type, call corresponding device-specific
> > > functions even if functions' parameters are the same.
> > > 
> > > IOCTL-like API can be added to ethdev instead of public device-specific
> > > functions to address the following:
> > > 
> > > * allow more usable support of features across a range of NIC from
> > >   one vendor, but not others
> > > * allow features to be implemented by multiple NIC drivers without
> > >   relying on a critical mass to get the functionality in ethdev
> > > * there are a large number of possible device specific functions, and
> > >   creating individual APIs for each one is not a good solution
> > > * IOCTLs are a proven method for solving this problem in other areas,
> > >   i.e. OS kernels.
> > > 
> > > Control requests for this API will be globally defined at ethdev level, so
> > > an application will use single API call to control different devices from
> > > one/multiple vendors.
> > > 
> > > API call may look like as a classic ioctl with an extra parameter for
> > > argument length for better sanity checks:
> > > 
> > > int
> > > rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
> > >         unsigned arg_length);
> > > 
> > > Regards,
> > > Andrey  
> > 
> > I think we need to start putting in IOCTLs for ethdevs, much as I hate
> > to admit it, since I dislike IOCTLs and other functions with opaque
> > arguments! Having driver specific functions I don't think will scale
> > well as each vendor tries to expose as much of their driver specific
> > functionality as possible.
> > 
> > One other additional example: I discovered just this week another issue
> > with driver specific functions and testpmd, when I was working on the
> > meson build rework.
> > 
> > * With shared libraries, when we do "ninja install" we want our DPDK
> >   libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
> >   driver folder, so that they can be automatically loaded from that
> >   single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
> > * However, testpmd, as well as using the drivers as plugins, uses
> >   driver-specific functions, which means that it explicitly links
> >   against the pmd .so files.
> > * Those driver .so files are not in with the other libraries, so ld.so
> >   does not find the pmd, and the installed testpmd fails to run due to
> >   missing library dependencies.
> > * The workaround is to add the drivers path to the ld load path, but we
> >   should not require ld library path changes just to get DPDK apps to
> >   work.
> > 
> > Using ioctls instead of driver-specific functions would solve this.
> > 
> > My 2c.
> 
> My 2c. No.
> 
> Short answer:
> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
> despised by Linux kernel developers. They provide an unstructured, unsecured,
> back door for device driver abuse. Try to get a new driver in Linux with
> a unique ioctl, and it will be hard to get accepted.
> 
> Long answer:
> So far every device specific feature has fit into ethdev model. Doing ioctl
> is admitting "it is too hard to be general, we need need an out". For something
> that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
> For a real feature (think flow direction), we want a first class API for that.
> For a wart, then devargs will do.
> 
> Give a good example of something that should be an ioctl. Don't build the
> API first and then let it get cluttered.

I agree with Stephen.

And please do not forget that ioctl still requires an API:
the argument that you put in ioctl is the API of the feature.
So it is the same thing as defining a new function.

The real debate is to decide if we want to continue adding more
control path features in DPDK or focus on Rx/Tx.
But this discussion would be better lead with some examples/requests.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-03 19:53     ` Thomas Monjalon
@ 2017-08-04  9:59       ` Chilikin, Andrey
  2017-08-04 10:08         ` Thomas Monjalon
  2017-08-04 11:58       ` Ferruh Yigit
  1 sibling, 1 reply; 14+ messages in thread
From: Chilikin, Andrey @ 2017-08-04  9:59 UTC (permalink / raw)
  To: Thomas Monjalon, dev
  Cc: Stephen Hemminger, Richardson, Bruce, Ananyev, Konstantin, Wu, Jingjing

> 03/08/2017 18:15, Stephen Hemminger:
> > On Thu, 3 Aug 2017 14:21:38 +0100
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> >
> > > On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
> > > > To control some device-specific features public device-specific
> functions
> > > > rte_pmd_*.h are used.
> > > >
> > > > But this solution requires applications to distinguish devices at runtime
> > > > and, depending on the device type, call corresponding device-specific
> > > > functions even if functions' parameters are the same.
> > > >
> > > > IOCTL-like API can be added to ethdev instead of public device-specific
> > > > functions to address the following:
> > > >
> > > > * allow more usable support of features across a range of NIC from
> > > >   one vendor, but not others
> > > > * allow features to be implemented by multiple NIC drivers without
> > > >   relying on a critical mass to get the functionality in ethdev
> > > > * there are a large number of possible device specific functions, and
> > > >   creating individual APIs for each one is not a good solution
> > > > * IOCTLs are a proven method for solving this problem in other areas,
> > > >   i.e. OS kernels.
> > > >
> > > > Control requests for this API will be globally defined at ethdev level, so
> > > > an application will use single API call to control different devices from
> > > > one/multiple vendors.
> > > >
> > > > API call may look like as a classic ioctl with an extra parameter for
> > > > argument length for better sanity checks:
> > > >
> > > > int
> > > > rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
> > > >         unsigned arg_length);
> > > >
> > > > Regards,
> > > > Andrey
> > >
> > > I think we need to start putting in IOCTLs for ethdevs, much as I hate
> > > to admit it, since I dislike IOCTLs and other functions with opaque
> > > arguments! Having driver specific functions I don't think will scale
> > > well as each vendor tries to expose as much of their driver specific
> > > functionality as possible.
> > >
> > > One other additional example: I discovered just this week another issue
> > > with driver specific functions and testpmd, when I was working on the
> > > meson build rework.
> > >
> > > * With shared libraries, when we do "ninja install" we want our DPDK
> > >   libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
> > >   driver folder, so that they can be automatically loaded from that
> > >   single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
> > > * However, testpmd, as well as using the drivers as plugins, uses
> > >   driver-specific functions, which means that it explicitly links
> > >   against the pmd .so files.
> > > * Those driver .so files are not in with the other libraries, so ld.so
> > >   does not find the pmd, and the installed testpmd fails to run due to
> > >   missing library dependencies.
> > > * The workaround is to add the drivers path to the ld load path, but we
> > >   should not require ld library path changes just to get DPDK apps to
> > >   work.
> > >
> > > Using ioctls instead of driver-specific functions would solve this.
> > >
> > > My 2c.
> >
> > My 2c. No.
> >
> > Short answer:
> > Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
> > despised by Linux kernel developers. They provide an unstructured,
> unsecured,
> > back door for device driver abuse. Try to get a new driver in Linux with
> > a unique ioctl, and it will be hard to get accepted.
> >
> > Long answer:
> > So far every device specific feature has fit into ethdev model. Doing ioctl
> > is admitting "it is too hard to be general, we need need an out". For
> something
> > that is a flag, it should fit into existing config model; ignoring silly ABI
> constraints.
> > For a real feature (think flow direction), we want a first class API for that.
> > For a wart, then devargs will do.
> >
> > Give a good example of something that should be an ioctl. Don't build the
> > API first and then let it get cluttered.
> 
> I agree with Stephen.
> 
> And please do not forget that ioctl still requires an API:
> the argument that you put in ioctl is the API of the feature.
> So it is the same thing as defining a new function.
> 
> The real debate is to decide if we want to continue adding more
> control path features in DPDK or focus on Rx/Tx.
> But this discussion would be better lead with some examples/requests.

In addition to what Bruce mentioned above, anything that requires dynamic re-configuration at run time would be a good example:
* Internal resources partitioning, for example, RX buffers allocation for different traffic classes/flow types, depending on the load
* Mapping user priorities from different sources (VLAN's PCP bits, IP DSCP, MPLS Exp) to traffic classes
* Dynamic queue regions allocation for traffic classes
* Dynamic statistics allocation
* Dynamic flow types configuration depending on loaded parser profile

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-04  9:59       ` Chilikin, Andrey
@ 2017-08-04 10:08         ` Thomas Monjalon
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2017-08-04 10:08 UTC (permalink / raw)
  To: Chilikin, Andrey
  Cc: dev, Stephen Hemminger, Richardson, Bruce, Ananyev, Konstantin,
	Wu, Jingjing

04/08/2017 11:59, Chilikin, Andrey:
> > 03/08/2017 18:15, Stephen Hemminger:
> > > On Thu, 3 Aug 2017 14:21:38 +0100
> > > Bruce Richardson <bruce.richardson@intel.com> wrote:
> > >
> > > > On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
> > > > > To control some device-specific features public device-specific
> > functions
> > > > > rte_pmd_*.h are used.
> > > > >
> > > > > But this solution requires applications to distinguish devices at runtime
> > > > > and, depending on the device type, call corresponding device-specific
> > > > > functions even if functions' parameters are the same.
> > > > >
> > > > > IOCTL-like API can be added to ethdev instead of public device-specific
> > > > > functions to address the following:
> > > > >
> > > > > * allow more usable support of features across a range of NIC from
> > > > >   one vendor, but not others
> > > > > * allow features to be implemented by multiple NIC drivers without
> > > > >   relying on a critical mass to get the functionality in ethdev
> > > > > * there are a large number of possible device specific functions, and
> > > > >   creating individual APIs for each one is not a good solution
> > > > > * IOCTLs are a proven method for solving this problem in other areas,
> > > > >   i.e. OS kernels.
> > > > >
> > > > > Control requests for this API will be globally defined at ethdev level, so
> > > > > an application will use single API call to control different devices from
> > > > > one/multiple vendors.
> > > > >
> > > > > API call may look like as a classic ioctl with an extra parameter for
> > > > > argument length for better sanity checks:
> > > > >
> > > > > int
> > > > > rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
> > > > >         unsigned arg_length);
> > > > >
> > > > > Regards,
> > > > > Andrey
> > > >
> > > > I think we need to start putting in IOCTLs for ethdevs, much as I hate
> > > > to admit it, since I dislike IOCTLs and other functions with opaque
> > > > arguments! Having driver specific functions I don't think will scale
> > > > well as each vendor tries to expose as much of their driver specific
> > > > functionality as possible.
> > > >
> > > > One other additional example: I discovered just this week another issue
> > > > with driver specific functions and testpmd, when I was working on the
> > > > meson build rework.
> > > >
> > > > * With shared libraries, when we do "ninja install" we want our DPDK
> > > >   libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
> > > >   driver folder, so that they can be automatically loaded from that
> > > >   single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
> > > > * However, testpmd, as well as using the drivers as plugins, uses
> > > >   driver-specific functions, which means that it explicitly links
> > > >   against the pmd .so files.
> > > > * Those driver .so files are not in with the other libraries, so ld.so
> > > >   does not find the pmd, and the installed testpmd fails to run due to
> > > >   missing library dependencies.
> > > > * The workaround is to add the drivers path to the ld load path, but we
> > > >   should not require ld library path changes just to get DPDK apps to
> > > >   work.
> > > >
> > > > Using ioctls instead of driver-specific functions would solve this.
> > > >
> > > > My 2c.
> > >
> > > My 2c. No.
> > >
> > > Short answer:
> > > Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
> > > despised by Linux kernel developers. They provide an unstructured,
> > unsecured,
> > > back door for device driver abuse. Try to get a new driver in Linux with
> > > a unique ioctl, and it will be hard to get accepted.
> > >
> > > Long answer:
> > > So far every device specific feature has fit into ethdev model. Doing ioctl
> > > is admitting "it is too hard to be general, we need need an out". For
> > something
> > > that is a flag, it should fit into existing config model; ignoring silly ABI
> > constraints.
> > > For a real feature (think flow direction), we want a first class API for that.
> > > For a wart, then devargs will do.
> > >
> > > Give a good example of something that should be an ioctl. Don't build the
> > > API first and then let it get cluttered.
> > 
> > I agree with Stephen.
> > 
> > And please do not forget that ioctl still requires an API:
> > the argument that you put in ioctl is the API of the feature.
> > So it is the same thing as defining a new function.
> > 
> > The real debate is to decide if we want to continue adding more
> > control path features in DPDK or focus on Rx/Tx.
> > But this discussion would be better lead with some examples/requests.
> 
> In addition to what Bruce mentioned above, anything that requires dynamic re-configuration at run time would be a good example:
> * Internal resources partitioning, for example, RX buffers allocation for different traffic classes/flow types, depending on the load
> * Mapping user priorities from different sources (VLAN's PCP bits, IP DSCP, MPLS Exp) to traffic classes
> * Dynamic queue regions allocation for traffic classes
> * Dynamic statistics allocation
> * Dynamic flow types configuration depending on loaded parser profile

Why should it be device-specific?
If capabilities are well advertised, it could be generic.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-03 19:53     ` Thomas Monjalon
  2017-08-04  9:59       ` Chilikin, Andrey
@ 2017-08-04 11:58       ` Ferruh Yigit
  2017-08-04 12:56         ` Bruce Richardson
                           ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Ferruh Yigit @ 2017-08-04 11:58 UTC (permalink / raw)
  To: Thomas Monjalon, dev
  Cc: Stephen Hemminger, Bruce Richardson, Chilikin, Andrey, Ananyev,
	Konstantin, Wu, Jingjing

On 8/3/2017 8:53 PM, Thomas Monjalon wrote:
> 03/08/2017 18:15, Stephen Hemminger:
>> On Thu, 3 Aug 2017 14:21:38 +0100
>> Bruce Richardson <bruce.richardson@intel.com> wrote:
>>
>>> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
>>>> To control some device-specific features public device-specific functions
>>>> rte_pmd_*.h are used.
>>>>
>>>> But this solution requires applications to distinguish devices at runtime
>>>> and, depending on the device type, call corresponding device-specific
>>>> functions even if functions' parameters are the same.
>>>>
>>>> IOCTL-like API can be added to ethdev instead of public device-specific
>>>> functions to address the following:
>>>>
>>>> * allow more usable support of features across a range of NIC from
>>>>   one vendor, but not others
>>>> * allow features to be implemented by multiple NIC drivers without
>>>>   relying on a critical mass to get the functionality in ethdev
>>>> * there are a large number of possible device specific functions, and
>>>>   creating individual APIs for each one is not a good solution
>>>> * IOCTLs are a proven method for solving this problem in other areas,
>>>>   i.e. OS kernels.
>>>>
>>>> Control requests for this API will be globally defined at ethdev level, so
>>>> an application will use single API call to control different devices from
>>>> one/multiple vendors.
>>>>
>>>> API call may look like as a classic ioctl with an extra parameter for
>>>> argument length for better sanity checks:
>>>>
>>>> int
>>>> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
>>>>         unsigned arg_length);
>>>>
>>>> Regards,
>>>> Andrey  
>>>
>>> I think we need to start putting in IOCTLs for ethdevs, much as I hate
>>> to admit it, since I dislike IOCTLs and other functions with opaque
>>> arguments! Having driver specific functions I don't think will scale
>>> well as each vendor tries to expose as much of their driver specific
>>> functionality as possible.
>>>
>>> One other additional example: I discovered just this week another issue
>>> with driver specific functions and testpmd, when I was working on the
>>> meson build rework.
>>>
>>> * With shared libraries, when we do "ninja install" we want our DPDK
>>>   libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
>>>   driver folder, so that they can be automatically loaded from that
>>>   single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
>>> * However, testpmd, as well as using the drivers as plugins, uses
>>>   driver-specific functions, which means that it explicitly links
>>>   against the pmd .so files.
>>> * Those driver .so files are not in with the other libraries, so ld.so
>>>   does not find the pmd, and the installed testpmd fails to run due to
>>>   missing library dependencies.
>>> * The workaround is to add the drivers path to the ld load path, but we
>>>   should not require ld library path changes just to get DPDK apps to
>>>   work.
>>>
>>> Using ioctls instead of driver-specific functions would solve this.
>>>
>>> My 2c.
>>
>> My 2c. No.
>>
>> Short answer:
>> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
>> despised by Linux kernel developers. They provide an unstructured, unsecured,
>> back door for device driver abuse. Try to get a new driver in Linux with
>> a unique ioctl, and it will be hard to get accepted.
>>
>> Long answer:
>> So far every device specific feature has fit into ethdev model. Doing ioctl
>> is admitting "it is too hard to be general, we need need an out". For something
>> that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
>> For a real feature (think flow direction), we want a first class API for that.
>> For a wart, then devargs will do.
>>
>> Give a good example of something that should be an ioctl. Don't build the
>> API first and then let it get cluttered.
> 
> I agree with Stephen.
> 
> And please do not forget that ioctl still requires an API:
> the argument that you put in ioctl is the API of the feature.
> So it is the same thing as defining a new function.

I am also not fan of the ioctl usage. I believe it hides APIs behind ids
and prevent argument check by compiler.

BUT, the number of the increasing PMD specific APIs are also worrying,
it is becoming harder to maintain, and I believe this is something NOT
sustainable in long run.


What about having *eth_dev_extended_ops* ?


As a part of the rte_eth_dev. This can be in the librte_ether library
but in a separated file.

And the APIs for these ops can be less strict on compatibility, and
easier to add.

Benefits of having this new dev_ops:

* Having an abstraction layer for common checks.

* Even feature is not generic for all NICs, still a few NICs can share
the ops.

* All APIs are in the same file makes it easy to see PMD specific APIs
comparing to scattered into various PMDs.

* This is very like ioctl approach, but APIs are more clear and
arguments can be verified.

Thanks,
ferruh


> 
> The real debate is to decide if we want to continue adding more
> control path features in DPDK or focus on Rx/Tx.
> But this discussion would be better lead with some examples/requests.
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-04 11:58       ` Ferruh Yigit
@ 2017-08-04 12:56         ` Bruce Richardson
  2017-08-08  8:32           ` Ferruh Yigit
  2017-08-08 17:23         ` Wiles, Keith
  2017-08-08 17:28         ` Wiles, Keith
  2 siblings, 1 reply; 14+ messages in thread
From: Bruce Richardson @ 2017-08-04 12:56 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Thomas Monjalon, dev, Stephen Hemminger, Chilikin, Andrey,
	Ananyev, Konstantin, Wu, Jingjing

On Fri, Aug 04, 2017 at 12:58:01PM +0100, Ferruh Yigit wrote:
> On 8/3/2017 8:53 PM, Thomas Monjalon wrote:
> > 03/08/2017 18:15, Stephen Hemminger:
> >> On Thu, 3 Aug 2017 14:21:38 +0100
> >> Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>
> >>> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
> >>>> To control some device-specific features public device-specific functions
> >>>> rte_pmd_*.h are used.
> >>>>
> >>>> But this solution requires applications to distinguish devices at runtime
> >>>> and, depending on the device type, call corresponding device-specific
> >>>> functions even if functions' parameters are the same.
> >>>>
> >>>> IOCTL-like API can be added to ethdev instead of public device-specific
> >>>> functions to address the following:
> >>>>
> >>>> * allow more usable support of features across a range of NIC from
> >>>>   one vendor, but not others
> >>>> * allow features to be implemented by multiple NIC drivers without
> >>>>   relying on a critical mass to get the functionality in ethdev
> >>>> * there are a large number of possible device specific functions, and
> >>>>   creating individual APIs for each one is not a good solution
> >>>> * IOCTLs are a proven method for solving this problem in other areas,
> >>>>   i.e. OS kernels.
> >>>>
> >>>> Control requests for this API will be globally defined at ethdev level, so
> >>>> an application will use single API call to control different devices from
> >>>> one/multiple vendors.
> >>>>
> >>>> API call may look like as a classic ioctl with an extra parameter for
> >>>> argument length for better sanity checks:
> >>>>
> >>>> int
> >>>> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
> >>>>         unsigned arg_length);
> >>>>
> >>>> Regards,
> >>>> Andrey  
> >>>
> >>> I think we need to start putting in IOCTLs for ethdevs, much as I hate
> >>> to admit it, since I dislike IOCTLs and other functions with opaque
> >>> arguments! Having driver specific functions I don't think will scale
> >>> well as each vendor tries to expose as much of their driver specific
> >>> functionality as possible.
> >>>
> >>> One other additional example: I discovered just this week another issue
> >>> with driver specific functions and testpmd, when I was working on the
> >>> meson build rework.
> >>>
> >>> * With shared libraries, when we do "ninja install" we want our DPDK
> >>>   libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
> >>>   driver folder, so that they can be automatically loaded from that
> >>>   single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
> >>> * However, testpmd, as well as using the drivers as plugins, uses
> >>>   driver-specific functions, which means that it explicitly links
> >>>   against the pmd .so files.
> >>> * Those driver .so files are not in with the other libraries, so ld.so
> >>>   does not find the pmd, and the installed testpmd fails to run due to
> >>>   missing library dependencies.
> >>> * The workaround is to add the drivers path to the ld load path, but we
> >>>   should not require ld library path changes just to get DPDK apps to
> >>>   work.
> >>>
> >>> Using ioctls instead of driver-specific functions would solve this.
> >>>
> >>> My 2c.
> >>
> >> My 2c. No.
> >>
> >> Short answer:
> >> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
> >> despised by Linux kernel developers. They provide an unstructured, unsecured,
> >> back door for device driver abuse. Try to get a new driver in Linux with
> >> a unique ioctl, and it will be hard to get accepted.
> >>
> >> Long answer:
> >> So far every device specific feature has fit into ethdev model. Doing ioctl
> >> is admitting "it is too hard to be general, we need need an out". For something
> >> that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
> >> For a real feature (think flow direction), we want a first class API for that.
> >> For a wart, then devargs will do.
> >>
> >> Give a good example of something that should be an ioctl. Don't build the
> >> API first and then let it get cluttered.
> > 
> > I agree with Stephen.
> > 
> > And please do not forget that ioctl still requires an API:
> > the argument that you put in ioctl is the API of the feature.
> > So it is the same thing as defining a new function.
> 
> I am also not fan of the ioctl usage. I believe it hides APIs behind ids
> and prevent argument check by compiler.
> 
> BUT, the number of the increasing PMD specific APIs are also worrying,
> it is becoming harder to maintain, and I believe this is something NOT
> sustainable in long run.
> 
> 
> What about having *eth_dev_extended_ops* ?
> 
> 
> As a part of the rte_eth_dev. This can be in the librte_ether library
> but in a separated file.
> 
> And the APIs for these ops can be less strict on compatibility, and
> easier to add.
> 
> Benefits of having this new dev_ops:
> 
> * Having an abstraction layer for common checks.
> 
> * Even feature is not generic for all NICs, still a few NICs can share
> the ops.
> 
> * All APIs are in the same file makes it easy to see PMD specific APIs
> comparing to scattered into various PMDs.
> 
> * This is very like ioctl approach, but APIs are more clear and
> arguments can be verified.
> 

Sounds like an ethdev-staging library, where features can be put until
such time as they get critical mass for acceptance and promoted to
ethdev? It's sounds better than IOCTL, while giving the same benefits.

I'd be happy enough with any solution that allows NIC features to be
exposed that does not have functions limited to each individual driver,
so that common functionality can be exposed to apps via an API even if
only 2 drivers support it.

> Thanks,
> ferruh
> 
> 
> > 
> > The real debate is to decide if we want to continue adding more
> > control path features in DPDK or focus on Rx/Tx.
I don't see dropping control path as an option. It would severely limit
the usefulness of DPDK.

/Bruce

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-04 12:56         ` Bruce Richardson
@ 2017-08-08  8:32           ` Ferruh Yigit
  2017-08-08 15:27             ` Stephen Hemminger
  0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2017-08-08  8:32 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Thomas Monjalon, dev, Stephen Hemminger, Chilikin, Andrey,
	Ananyev, Konstantin, Wu, Jingjing

On 8/4/2017 1:56 PM, Bruce Richardson wrote:
> On Fri, Aug 04, 2017 at 12:58:01PM +0100, Ferruh Yigit wrote:
>> On 8/3/2017 8:53 PM, Thomas Monjalon wrote:
>>> 03/08/2017 18:15, Stephen Hemminger:
>>>> On Thu, 3 Aug 2017 14:21:38 +0100
>>>> Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>
>>>>> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
>>>>>> To control some device-specific features public device-specific functions
>>>>>> rte_pmd_*.h are used.
>>>>>>
>>>>>> But this solution requires applications to distinguish devices at runtime
>>>>>> and, depending on the device type, call corresponding device-specific
>>>>>> functions even if functions' parameters are the same.
>>>>>>
>>>>>> IOCTL-like API can be added to ethdev instead of public device-specific
>>>>>> functions to address the following:
>>>>>>
>>>>>> * allow more usable support of features across a range of NIC from
>>>>>>   one vendor, but not others
>>>>>> * allow features to be implemented by multiple NIC drivers without
>>>>>>   relying on a critical mass to get the functionality in ethdev
>>>>>> * there are a large number of possible device specific functions, and
>>>>>>   creating individual APIs for each one is not a good solution
>>>>>> * IOCTLs are a proven method for solving this problem in other areas,
>>>>>>   i.e. OS kernels.
>>>>>>
>>>>>> Control requests for this API will be globally defined at ethdev level, so
>>>>>> an application will use single API call to control different devices from
>>>>>> one/multiple vendors.
>>>>>>
>>>>>> API call may look like as a classic ioctl with an extra parameter for
>>>>>> argument length for better sanity checks:
>>>>>>
>>>>>> int
>>>>>> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
>>>>>>         unsigned arg_length);
>>>>>>
>>>>>> Regards,
>>>>>> Andrey  
>>>>>
>>>>> I think we need to start putting in IOCTLs for ethdevs, much as I hate
>>>>> to admit it, since I dislike IOCTLs and other functions with opaque
>>>>> arguments! Having driver specific functions I don't think will scale
>>>>> well as each vendor tries to expose as much of their driver specific
>>>>> functionality as possible.
>>>>>
>>>>> One other additional example: I discovered just this week another issue
>>>>> with driver specific functions and testpmd, when I was working on the
>>>>> meson build rework.
>>>>>
>>>>> * With shared libraries, when we do "ninja install" we want our DPDK
>>>>>   libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
>>>>>   driver folder, so that they can be automatically loaded from that
>>>>>   single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
>>>>> * However, testpmd, as well as using the drivers as plugins, uses
>>>>>   driver-specific functions, which means that it explicitly links
>>>>>   against the pmd .so files.
>>>>> * Those driver .so files are not in with the other libraries, so ld.so
>>>>>   does not find the pmd, and the installed testpmd fails to run due to
>>>>>   missing library dependencies.
>>>>> * The workaround is to add the drivers path to the ld load path, but we
>>>>>   should not require ld library path changes just to get DPDK apps to
>>>>>   work.
>>>>>
>>>>> Using ioctls instead of driver-specific functions would solve this.
>>>>>
>>>>> My 2c.
>>>>
>>>> My 2c. No.
>>>>
>>>> Short answer:
>>>> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
>>>> despised by Linux kernel developers. They provide an unstructured, unsecured,
>>>> back door for device driver abuse. Try to get a new driver in Linux with
>>>> a unique ioctl, and it will be hard to get accepted.
>>>>
>>>> Long answer:
>>>> So far every device specific feature has fit into ethdev model. Doing ioctl
>>>> is admitting "it is too hard to be general, we need need an out". For something
>>>> that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
>>>> For a real feature (think flow direction), we want a first class API for that.
>>>> For a wart, then devargs will do.
>>>>
>>>> Give a good example of something that should be an ioctl. Don't build the
>>>> API first and then let it get cluttered.
>>>
>>> I agree with Stephen.
>>>
>>> And please do not forget that ioctl still requires an API:
>>> the argument that you put in ioctl is the API of the feature.
>>> So it is the same thing as defining a new function.
>>
>> I am also not fan of the ioctl usage. I believe it hides APIs behind ids
>> and prevent argument check by compiler.
>>
>> BUT, the number of the increasing PMD specific APIs are also worrying,
>> it is becoming harder to maintain, and I believe this is something NOT
>> sustainable in long run.
>>
>>
>> What about having *eth_dev_extended_ops* ?
>>
>>
>> As a part of the rte_eth_dev. This can be in the librte_ether library
>> but in a separated file.
>>
>> And the APIs for these ops can be less strict on compatibility, and
>> easier to add.
>>
>> Benefits of having this new dev_ops:
>>
>> * Having an abstraction layer for common checks.
>>
>> * Even feature is not generic for all NICs, still a few NICs can share
>> the ops.
>>
>> * All APIs are in the same file makes it easy to see PMD specific APIs
>> comparing to scattered into various PMDs.
>>
>> * This is very like ioctl approach, but APIs are more clear and
>> arguments can be verified.
>>
> 
> Sounds like an ethdev-staging library, where features can be put until
> such time as they get critical mass for acceptance and promoted to
> ethdev? It's sounds better than IOCTL, while giving the same benefits.
> 
> I'd be happy enough with any solution that allows NIC features to be
> exposed that does not have functions limited to each individual driver,
> so that common functionality can be exposed to apps via an API even if
> only 2 drivers support it.

This is not decided yet, but to enable working on this for next release,
is a deprecation notice required to add a new field to "struct
rte_eth_dev" ?

"struct rte_eth_dev" is marked as "@internal", so I believe deprecation
notice is NOT required, but I would like to confirm.

> 
>> Thanks,
>> ferruh
>>
>>
>>>
>>> The real debate is to decide if we want to continue adding more
>>> control path features in DPDK or focus on Rx/Tx.
> I don't see dropping control path as an option. It would severely limit
> the usefulness of DPDK.
> 
> /Bruce
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-08  8:32           ` Ferruh Yigit
@ 2017-08-08 15:27             ` Stephen Hemminger
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2017-08-08 15:27 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Bruce Richardson, Thomas Monjalon, dev, Chilikin, Andrey,
	Ananyev, Konstantin, Wu, Jingjing

On Tue, 8 Aug 2017 09:32:07 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 8/4/2017 1:56 PM, Bruce Richardson wrote:
> > On Fri, Aug 04, 2017 at 12:58:01PM +0100, Ferruh Yigit wrote:  
> >> On 8/3/2017 8:53 PM, Thomas Monjalon wrote:  
> >>> 03/08/2017 18:15, Stephen Hemminger:  
> >>>> On Thu, 3 Aug 2017 14:21:38 +0100
> >>>> Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>>>  
> >>>>> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:  
> >>>>>> To control some device-specific features public device-specific functions
> >>>>>> rte_pmd_*.h are used.
> >>>>>>
> >>>>>> But this solution requires applications to distinguish devices at runtime
> >>>>>> and, depending on the device type, call corresponding device-specific
> >>>>>> functions even if functions' parameters are the same.
> >>>>>>
> >>>>>> IOCTL-like API can be added to ethdev instead of public device-specific
> >>>>>> functions to address the following:
> >>>>>>
> >>>>>> * allow more usable support of features across a range of NIC from
> >>>>>>   one vendor, but not others
> >>>>>> * allow features to be implemented by multiple NIC drivers without
> >>>>>>   relying on a critical mass to get the functionality in ethdev
> >>>>>> * there are a large number of possible device specific functions, and
> >>>>>>   creating individual APIs for each one is not a good solution
> >>>>>> * IOCTLs are a proven method for solving this problem in other areas,
> >>>>>>   i.e. OS kernels.
> >>>>>>
> >>>>>> Control requests for this API will be globally defined at ethdev level, so
> >>>>>> an application will use single API call to control different devices from
> >>>>>> one/multiple vendors.
> >>>>>>
> >>>>>> API call may look like as a classic ioctl with an extra parameter for
> >>>>>> argument length for better sanity checks:
> >>>>>>
> >>>>>> int
> >>>>>> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
> >>>>>>         unsigned arg_length);
> >>>>>>
> >>>>>> Regards,
> >>>>>> Andrey    
> >>>>>
> >>>>> I think we need to start putting in IOCTLs for ethdevs, much as I hate
> >>>>> to admit it, since I dislike IOCTLs and other functions with opaque
> >>>>> arguments! Having driver specific functions I don't think will scale
> >>>>> well as each vendor tries to expose as much of their driver specific
> >>>>> functionality as possible.
> >>>>>
> >>>>> One other additional example: I discovered just this week another issue
> >>>>> with driver specific functions and testpmd, when I was working on the
> >>>>> meson build rework.
> >>>>>
> >>>>> * With shared libraries, when we do "ninja install" we want our DPDK
> >>>>>   libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
> >>>>>   driver folder, so that they can be automatically loaded from that
> >>>>>   single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
> >>>>> * However, testpmd, as well as using the drivers as plugins, uses
> >>>>>   driver-specific functions, which means that it explicitly links
> >>>>>   against the pmd .so files.
> >>>>> * Those driver .so files are not in with the other libraries, so ld.so
> >>>>>   does not find the pmd, and the installed testpmd fails to run due to
> >>>>>   missing library dependencies.
> >>>>> * The workaround is to add the drivers path to the ld load path, but we
> >>>>>   should not require ld library path changes just to get DPDK apps to
> >>>>>   work.
> >>>>>
> >>>>> Using ioctls instead of driver-specific functions would solve this.
> >>>>>
> >>>>> My 2c.  
> >>>>
> >>>> My 2c. No.
> >>>>
> >>>> Short answer:
> >>>> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
> >>>> despised by Linux kernel developers. They provide an unstructured, unsecured,
> >>>> back door for device driver abuse. Try to get a new driver in Linux with
> >>>> a unique ioctl, and it will be hard to get accepted.
> >>>>
> >>>> Long answer:
> >>>> So far every device specific feature has fit into ethdev model. Doing ioctl
> >>>> is admitting "it is too hard to be general, we need need an out". For something
> >>>> that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
> >>>> For a real feature (think flow direction), we want a first class API for that.
> >>>> For a wart, then devargs will do.
> >>>>
> >>>> Give a good example of something that should be an ioctl. Don't build the
> >>>> API first and then let it get cluttered.  
> >>>
> >>> I agree with Stephen.
> >>>
> >>> And please do not forget that ioctl still requires an API:
> >>> the argument that you put in ioctl is the API of the feature.
> >>> So it is the same thing as defining a new function.  
> >>
> >> I am also not fan of the ioctl usage. I believe it hides APIs behind ids
> >> and prevent argument check by compiler.
> >>
> >> BUT, the number of the increasing PMD specific APIs are also worrying,
> >> it is becoming harder to maintain, and I believe this is something NOT
> >> sustainable in long run.
> >>
> >>
> >> What about having *eth_dev_extended_ops* ?
> >>
> >>
> >> As a part of the rte_eth_dev. This can be in the librte_ether library
> >> but in a separated file.
> >>
> >> And the APIs for these ops can be less strict on compatibility, and
> >> easier to add.
> >>
> >> Benefits of having this new dev_ops:
> >>
> >> * Having an abstraction layer for common checks.
> >>
> >> * Even feature is not generic for all NICs, still a few NICs can share
> >> the ops.
> >>
> >> * All APIs are in the same file makes it easy to see PMD specific APIs
> >> comparing to scattered into various PMDs.
> >>
> >> * This is very like ioctl approach, but APIs are more clear and
> >> arguments can be verified.
> >>  
> > 
> > Sounds like an ethdev-staging library, where features can be put until
> > such time as they get critical mass for acceptance and promoted to
> > ethdev? It's sounds better than IOCTL, while giving the same benefits.
> > 
> > I'd be happy enough with any solution that allows NIC features to be
> > exposed that does not have functions limited to each individual driver,
> > so that common functionality can be exposed to apps via an API even if
> > only 2 drivers support it.  
> 
> This is not decided yet, but to enable working on this for next release,
> is a deprecation notice required to add a new field to "struct
> rte_eth_dev" ?
> 
> "struct rte_eth_dev" is marked as "@internal", so I believe deprecation
> notice is NOT required, but I would like to confirm.

Increasing the size of a structure used by API calls
will break ABI since the new version
of DPDK will read garbage off the end of the caller's input.
The problem could have been avoided if original DPDK API's had
used configuration structure and size of that struct.

Only structures allocated and only used internally could change.
It looks like rte_eth_dev is safe.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-04 11:58       ` Ferruh Yigit
  2017-08-04 12:56         ` Bruce Richardson
@ 2017-08-08 17:23         ` Wiles, Keith
  2017-08-08 17:28         ` Wiles, Keith
  2 siblings, 0 replies; 14+ messages in thread
From: Wiles, Keith @ 2017-08-08 17:23 UTC (permalink / raw)
  To: Yigit, Ferruh
  Cc: Thomas Monjalon, dev, Stephen Hemminger, Richardson, Bruce,
	Chilikin, Andrey, Ananyev, Konstantin, Wu, Jingjing


On Aug 4, 2017, at 6:58 AM, Ferruh Yigit <ferruh.yigit@intel.com<mailto:ferruh.yigit@intel.com>> wrote:

On 8/3/2017 8:53 PM, Thomas Monjalon wrote:
03/08/2017 18:15, Stephen Hemminger:
On Thu, 3 Aug 2017 14:21:38 +0100
Bruce Richardson <bruce.richardson@intel.com<mailto:bruce.richardson@intel.com>> wrote:

On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
To control some device-specific features public device-specific functions
rte_pmd_*.h are used.

But this solution requires applications to distinguish devices at runtime
and, depending on the device type, call corresponding device-specific
functions even if functions' parameters are the same.

IOCTL-like API can be added to ethdev instead of public device-specific
functions to address the following:

* allow more usable support of features across a range of NIC from
 one vendor, but not others
* allow features to be implemented by multiple NIC drivers without
 relying on a critical mass to get the functionality in ethdev
* there are a large number of possible device specific functions, and
 creating individual APIs for each one is not a good solution
* IOCTLs are a proven method for solving this problem in other areas,
 i.e. OS kernels.

Control requests for this API will be globally defined at ethdev level, so
an application will use single API call to control different devices from
one/multiple vendors.

API call may look like as a classic ioctl with an extra parameter for
argument length for better sanity checks:

int
rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
       unsigned arg_length);

Regards,
Andrey

I think we need to start putting in IOCTLs for ethdevs, much as I hate
to admit it, since I dislike IOCTLs and other functions with opaque
arguments! Having driver specific functions I don't think will scale
well as each vendor tries to expose as much of their driver specific
functionality as possible.

One other additional example: I discovered just this week another issue
with driver specific functions and testpmd, when I was working on the
meson build rework.

* With shared libraries, when we do "ninja install" we want our DPDK
 libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
 driver folder, so that they can be automatically loaded from that
 single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
* However, testpmd, as well as using the drivers as plugins, uses
 driver-specific functions, which means that it explicitly links
 against the pmd .so files.
* Those driver .so files are not in with the other libraries, so ld.so
 does not find the pmd, and the installed testpmd fails to run due to
 missing library dependencies.
* The workaround is to add the drivers path to the ld load path, but we
 should not require ld library path changes just to get DPDK apps to
 work.

Using ioctls instead of driver-specific functions would solve this.

My 2c.

My 2c. No.

Short answer:
Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
despised by Linux kernel developers. They provide an unstructured, unsecured,
back door for device driver abuse. Try to get a new driver in Linux with
a unique ioctl, and it will be hard to get accepted.

Long answer:
So far every device specific feature has fit into ethdev model. Doing ioctl
is admitting "it is too hard to be general, we need need an out". For something
that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
For a real feature (think flow direction), we want a first class API for that.
For a wart, then devargs will do.

Give a good example of something that should be an ioctl. Don't build the
API first and then let it get cluttered.

I agree with Stephen.

And please do not forget that ioctl still requires an API:
the argument that you put in ioctl is the API of the feature.
So it is the same thing as defining a new function.

I am also not fan of the ioctl usage. I believe it hides APIs behind ids
and prevent argument check by compiler.

BUT, the number of the increasing PMD specific APIs are also worrying,
it is becoming harder to maintain, and I believe this is something NOT
sustainable in long run.


What about having *eth_dev_extended_ops* ?

We had talk about adding something like device specific APIs to DPDK in the past, which to me are just IOCTL like APIs. The big problem with IOCTLs is trying to cram a bunch of specific requests into a very generic API and I do not like ioctl as defined in Linux/Unix today. The old IOCTLs calls are too opaque and difficult for compilers to test args and many other issues.

We talked about having a single API in rte_eth_dev that would allow a user to ask for and possible get a list of function pointers in a given structure for the requested type. If a user calls this API to get some feature from a given NIC he would get NULL or a pointer to a set of functions. The generic API in rte_eth would allow the user to request what structures or types of APIs it supports.

Using a specific API to get the list of APIs or supported features in a NIC, will allow the developer to request the set of APIs (in an array or some method). Then we have real APIs for specific control or requests and not a generic API like ioctl.

Cristian had suggested an API like this to make it easy to add any IOCTL like needs to a driver. We can define a set of structures that seem generic for some IOCTL like needs or just allow the NIC to define his own structures and APIs. Allowing the developer to define his own structures and APIs is not very generic or usable by the users, so I would lean toward defining structures set need today and expand those structures in the future or add more structures.

int rte_eth_dev_something(uint16_t port_id, const char *feature, void **obj);

Using strings we can define or the NIC vendor can define to ask for a pointer to a structure he knows via a driver header. Strings are good, because we can read them via the debug or print them out quickly instead of trying to use some lookup table. Plus we can have any length or characters for defining the structure request.

Just off the top of my head, but it can be changed if needed.




As a part of the rte_eth_dev. This can be in the librte_ether library
but in a separated file.

And the APIs for these ops can be less strict on compatibility, and
easier to add.

Benefits of having this new dev_ops:

* Having an abstraction layer for common checks.

* Even feature is not generic for all NICs, still a few NICs can share
the ops.

* All APIs are in the same file makes it easy to see PMD specific APIs
comparing to scattered into various PMDs.

* This is very like ioctl approach, but APIs are more clear and
arguments can be verified.

Thanks,
ferruh



The real debate is to decide if we want to continue adding more
control path features in DPDK or focus on Rx/Tx.
But this discussion would be better lead with some examples/requests.

Regards,
Keith

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-04 11:58       ` Ferruh Yigit
  2017-08-04 12:56         ` Bruce Richardson
  2017-08-08 17:23         ` Wiles, Keith
@ 2017-08-08 17:28         ` Wiles, Keith
  2017-08-08 18:02           ` Stephen Hemminger
  2 siblings, 1 reply; 14+ messages in thread
From: Wiles, Keith @ 2017-08-08 17:28 UTC (permalink / raw)
  To: Yigit, Ferruh
  Cc: Thomas Monjalon, DPDK, Stephen Hemminger, Richardson, Bruce,
	Chilikin, Andrey, Ananyev, Konstantin, Wu, Jingjing

Fix format.

> On Aug 4, 2017, at 6:58 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> On 8/3/2017 8:53 PM, Thomas Monjalon wrote:
>> 03/08/2017 18:15, Stephen Hemminger:
>>> On Thu, 3 Aug 2017 14:21:38 +0100
>>> Bruce Richardson <bruce.richardson@intel.com> wrote:
>>> 
>>>> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:
>>>>> To control some device-specific features public device-specific functions
>>>>> rte_pmd_*.h are used.
>>>>> 
>>>>> But this solution requires applications to distinguish devices at runtime
>>>>> and, depending on the device type, call corresponding device-specific
>>>>> functions even if functions' parameters are the same.
>>>>> 
>>>>> IOCTL-like API can be added to ethdev instead of public device-specific
>>>>> functions to address the following:
>>>>> 
>>>>> * allow more usable support of features across a range of NIC from
>>>>>  one vendor, but not others
>>>>> * allow features to be implemented by multiple NIC drivers without
>>>>>  relying on a critical mass to get the functionality in ethdev
>>>>> * there are a large number of possible device specific functions, and
>>>>>  creating individual APIs for each one is not a good solution
>>>>> * IOCTLs are a proven method for solving this problem in other areas,
>>>>>  i.e. OS kernels.
>>>>> 
>>>>> Control requests for this API will be globally defined at ethdev level, so
>>>>> an application will use single API call to control different devices from
>>>>> one/multiple vendors.
>>>>> 
>>>>> API call may look like as a classic ioctl with an extra parameter for
>>>>> argument length for better sanity checks:
>>>>> 
>>>>> int
>>>>> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
>>>>>        unsigned arg_length);
>>>>> 
>>>>> Regards,
>>>>> Andrey  
>>>> 
>>>> I think we need to start putting in IOCTLs for ethdevs, much as I hate
>>>> to admit it, since I dislike IOCTLs and other functions with opaque
>>>> arguments! Having driver specific functions I don't think will scale
>>>> well as each vendor tries to expose as much of their driver specific
>>>> functionality as possible.
>>>> 
>>>> One other additional example: I discovered just this week another issue
>>>> with driver specific functions and testpmd, when I was working on the
>>>> meson build rework.
>>>> 
>>>> * With shared libraries, when we do "ninja install" we want our DPDK
>>>>  libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
>>>>  driver folder, so that they can be automatically loaded from that
>>>>  single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
>>>> * However, testpmd, as well as using the drivers as plugins, uses
>>>>  driver-specific functions, which means that it explicitly links
>>>>  against the pmd .so files.
>>>> * Those driver .so files are not in with the other libraries, so ld.so
>>>>  does not find the pmd, and the installed testpmd fails to run due to
>>>>  missing library dependencies.
>>>> * The workaround is to add the drivers path to the ld load path, but we
>>>>  should not require ld library path changes just to get DPDK apps to
>>>>  work.
>>>> 
>>>> Using ioctls instead of driver-specific functions would solve this.
>>>> 
>>>> My 2c.
>>> 
>>> My 2c. No.
>>> 
>>> Short answer:
>>> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
>>> despised by Linux kernel developers. They provide an unstructured, unsecured,
>>> back door for device driver abuse. Try to get a new driver in Linux with
>>> a unique ioctl, and it will be hard to get accepted.
>>> 
>>> Long answer:
>>> So far every device specific feature has fit into ethdev model. Doing ioctl
>>> is admitting "it is too hard to be general, we need need an out". For something
>>> that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
>>> For a real feature (think flow direction), we want a first class API for that.
>>> For a wart, then devargs will do.
>>> 
>>> Give a good example of something that should be an ioctl. Don't build the
>>> API first and then let it get cluttered.
>> 
>> I agree with Stephen.
>> 
>> And please do not forget that ioctl still requires an API:
>> the argument that you put in ioctl is the API of the feature.
>> So it is the same thing as defining a new function.
> 
> I am also not fan of the ioctl usage. I believe it hides APIs behind ids
> and prevent argument check by compiler.
> 
> BUT, the number of the increasing PMD specific APIs are also worrying,
> it is becoming harder to maintain, and I believe this is something NOT
> sustainable in long run.
> 
> 
> What about having *eth_dev_extended_ops* ?


We had talk about adding something like device specific APIs to DPDK in the past, which to me are just IOCTL like APIs. The big problem with IOCTLs is trying to cram a bunch of specific requests into a very generic API and I do not like ioctl as defined in Linux/Unix today. The old IOCTLs calls are too opaque and difficult for compilers to test args and many other issues.

We talked about having a single API in rte_eth_dev that would allow a user to ask for and possible get a list of function pointers in a given structure for the requested type. If a user calls this API to get some feature from a given NIC he would get NULL or a pointer to a set of functions. The generic API in rte_eth would allow the user to request what structures or types of APIs it supports.

Using a specific API to get the list of APIs or supported features in a NIC, will allow the developer to request the set of APIs (in an array or some method). Then we have real APIs for specific control or requests and not a generic API like ioctl.

Cristian had suggested an API like this to make it easy to add any IOCTL like needs to a driver. We can define a set of structures that seem generic for some IOCTL like needs or just allow the NIC to define his own structures and APIs. Allowing the developer to define his own structures and APIs is not very generic or usable by the users, so I would lean toward defining structures set need today and expand those structures in the future or add more structures.

int rte_eth_dev_something(uint16_t port_id, const char *feature, void **obj);

Using strings we can define or the NIC vendor can define to ask for a pointer to a structure he knows via a driver header. Strings are good, because we can read them via the debug or print them out quickly instead of trying to use some lookup table. Plus we can have any length or characters for defining the structure request.

Just off the top of my head, but it can be changed if needed.


> 
> 
> As a part of the rte_eth_dev. This can be in the librte_ether library
> but in a separated file.
> 
> And the APIs for these ops can be less strict on compatibility, and
> easier to add.
> 
> Benefits of having this new dev_ops:
> 
> * Having an abstraction layer for common checks.
> 
> * Even feature is not generic for all NICs, still a few NICs can share
> the ops.
> 
> * All APIs are in the same file makes it easy to see PMD specific APIs
> comparing to scattered into various PMDs.
> 
> * This is very like ioctl approach, but APIs are more clear and
> arguments can be verified.
> 
> Thanks,
> ferruh
> 
> 
>> 
>> The real debate is to decide if we want to continue adding more
>> control path features in DPDK or focus on Rx/Tx.
>> But this discussion would be better lead with some examples/requests.

Regards,
Keith

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-08 17:28         ` Wiles, Keith
@ 2017-08-08 18:02           ` Stephen Hemminger
  2017-08-08 18:21             ` Wiles, Keith
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2017-08-08 18:02 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Yigit, Ferruh, Thomas Monjalon, DPDK, Richardson, Bruce,
	Chilikin, Andrey, Ananyev, Konstantin, Wu, Jingjing

On Tue, 8 Aug 2017 17:28:19 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> Fix format.
> 
> > On Aug 4, 2017, at 6:58 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > 
> > On 8/3/2017 8:53 PM, Thomas Monjalon wrote:  
> >> 03/08/2017 18:15, Stephen Hemminger:  
> >>> On Thu, 3 Aug 2017 14:21:38 +0100
> >>> Bruce Richardson <bruce.richardson@intel.com> wrote:
> >>>   
> >>>> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:  
> >>>>> To control some device-specific features public device-specific functions
> >>>>> rte_pmd_*.h are used.
> >>>>> 
> >>>>> But this solution requires applications to distinguish devices at runtime
> >>>>> and, depending on the device type, call corresponding device-specific
> >>>>> functions even if functions' parameters are the same.
> >>>>> 
> >>>>> IOCTL-like API can be added to ethdev instead of public device-specific
> >>>>> functions to address the following:
> >>>>> 
> >>>>> * allow more usable support of features across a range of NIC from
> >>>>>  one vendor, but not others
> >>>>> * allow features to be implemented by multiple NIC drivers without
> >>>>>  relying on a critical mass to get the functionality in ethdev
> >>>>> * there are a large number of possible device specific functions, and
> >>>>>  creating individual APIs for each one is not a good solution
> >>>>> * IOCTLs are a proven method for solving this problem in other areas,
> >>>>>  i.e. OS kernels.
> >>>>> 
> >>>>> Control requests for this API will be globally defined at ethdev level, so
> >>>>> an application will use single API call to control different devices from
> >>>>> one/multiple vendors.
> >>>>> 
> >>>>> API call may look like as a classic ioctl with an extra parameter for
> >>>>> argument length for better sanity checks:
> >>>>> 
> >>>>> int
> >>>>> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
> >>>>>        unsigned arg_length);
> >>>>> 
> >>>>> Regards,
> >>>>> Andrey    
> >>>> 
> >>>> I think we need to start putting in IOCTLs for ethdevs, much as I hate
> >>>> to admit it, since I dislike IOCTLs and other functions with opaque
> >>>> arguments! Having driver specific functions I don't think will scale
> >>>> well as each vendor tries to expose as much of their driver specific
> >>>> functionality as possible.
> >>>> 
> >>>> One other additional example: I discovered just this week another issue
> >>>> with driver specific functions and testpmd, when I was working on the
> >>>> meson build rework.
> >>>> 
> >>>> * With shared libraries, when we do "ninja install" we want our DPDK
> >>>>  libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
> >>>>  driver folder, so that they can be automatically loaded from that
> >>>>  single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
> >>>> * However, testpmd, as well as using the drivers as plugins, uses
> >>>>  driver-specific functions, which means that it explicitly links
> >>>>  against the pmd .so files.
> >>>> * Those driver .so files are not in with the other libraries, so ld.so
> >>>>  does not find the pmd, and the installed testpmd fails to run due to
> >>>>  missing library dependencies.
> >>>> * The workaround is to add the drivers path to the ld load path, but we
> >>>>  should not require ld library path changes just to get DPDK apps to
> >>>>  work.
> >>>> 
> >>>> Using ioctls instead of driver-specific functions would solve this.
> >>>> 
> >>>> My 2c.  
> >>> 
> >>> My 2c. No.
> >>> 
> >>> Short answer:
> >>> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
> >>> despised by Linux kernel developers. They provide an unstructured, unsecured,
> >>> back door for device driver abuse. Try to get a new driver in Linux with
> >>> a unique ioctl, and it will be hard to get accepted.
> >>> 
> >>> Long answer:
> >>> So far every device specific feature has fit into ethdev model. Doing ioctl
> >>> is admitting "it is too hard to be general, we need need an out". For something
> >>> that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
> >>> For a real feature (think flow direction), we want a first class API for that.
> >>> For a wart, then devargs will do.
> >>> 
> >>> Give a good example of something that should be an ioctl. Don't build the
> >>> API first and then let it get cluttered.  
> >> 
> >> I agree with Stephen.
> >> 
> >> And please do not forget that ioctl still requires an API:
> >> the argument that you put in ioctl is the API of the feature.
> >> So it is the same thing as defining a new function.  
> > 
> > I am also not fan of the ioctl usage. I believe it hides APIs behind ids
> > and prevent argument check by compiler.
> > 
> > BUT, the number of the increasing PMD specific APIs are also worrying,
> > it is becoming harder to maintain, and I believe this is something NOT
> > sustainable in long run.
> > 
> > 
> > What about having *eth_dev_extended_ops* ?  
> 
> 
> We had talk about adding something like device specific APIs to DPDK in the past, which to me are just IOCTL like APIs. The big problem with IOCTLs is trying to cram a bunch of specific requests into a very generic API and I do not like ioctl as defined in Linux/Unix today. The old IOCTLs calls are too opaque and difficult for compilers to test args and many other issues.
> 
> We talked about having a single API in rte_eth_dev that would allow a user to ask for and possible get a list of function pointers in a given structure for the requested type. If a user calls this API to get some feature from a given NIC he would get NULL or a pointer to a set of functions. The generic API in rte_eth would allow the user to request what structures or types of APIs it supports.
> 
> Using a specific API to get the list of APIs or supported features in a NIC, will allow the developer to request the set of APIs (in an array or some method). Then we have real APIs for specific control or requests and not a generic API like ioctl.
> 
> Cristian had suggested an API like this to make it easy to add any IOCTL like needs to a driver. We can define a set of structures that seem generic for some IOCTL like needs or just allow the NIC to define his own structures and APIs. Allowing the developer to define his own structures and APIs is not very generic or usable by the users, so I would lean toward defining structures set need today and expand those structures in the future or add more structures.
> 
> int rte_eth_dev_something(uint16_t port_id, const char *feature, void **obj);
> 
> Using strings we can define or the NIC vendor can define to ask for a pointer to a structure he knows via a driver header. Strings are good, because we can read them via the debug or print them out quickly instead of trying to use some lookup table. Plus we can have any length or characters for defining the structure request.
> 
> Just off the top of my head, but it can be changed if needed.
> 
> 
> > 
> > 
> > As a part of the rte_eth_dev. This can be in the librte_ether library
> > but in a separated file.
> > 
> > And the APIs for these ops can be less strict on compatibility, and
> > easier to add.
> > 
> > Benefits of having this new dev_ops:
> > 
> > * Having an abstraction layer for common checks.
> > 
> > * Even feature is not generic for all NICs, still a few NICs can share
> > the ops.
> > 
> > * All APIs are in the same file makes it easy to see PMD specific APIs
> > comparing to scattered into various PMDs.
> > 
> > * This is very like ioctl approach, but APIs are more clear and
> > arguments can be verified.
> > 
> > Thanks,
> > ferruh
> > 
> >   
> >> 
> >> The real debate is to decide if we want to continue adding more
> >> control path features in DPDK or focus on Rx/Tx.
> >> But this discussion would be better lead with some examples/requests.  
> 

The real question is how important is that DPDK is the playground for HW features?
My impression is that the current process is HW supplier driven "we support offload of XYZZY".

The existing control of device model is already a multi-path mess of config API's, device args,
and magic scripts. That needs to be addressed first.

There is very little community input from users, that is the problem.

IMHO if a new hardware feature can't be made to fit into a standard OS model like Linux
because it is too hard then adding it to DPDK is a mistake.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] ethdev: add ioctl-like API to control device specific features
  2017-08-08 18:02           ` Stephen Hemminger
@ 2017-08-08 18:21             ` Wiles, Keith
  0 siblings, 0 replies; 14+ messages in thread
From: Wiles, Keith @ 2017-08-08 18:21 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Yigit, Ferruh, Thomas Monjalon, DPDK, Richardson, Bruce,
	Chilikin, Andrey, Ananyev, Konstantin, Wu, Jingjing


> On Aug 8, 2017, at 1:02 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Tue, 8 Aug 2017 17:28:19 +0000
> "Wiles, Keith" <keith.wiles@intel.com> wrote:
> 
>> Fix format.
>> 
>>> On Aug 4, 2017, at 6:58 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>> 
>>> On 8/3/2017 8:53 PM, Thomas Monjalon wrote:  
>>>> 03/08/2017 18:15, Stephen Hemminger:  
>>>>> On Thu, 3 Aug 2017 14:21:38 +0100
>>>>> Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>>> 
>>>>>> On Thu, Aug 03, 2017 at 01:21:35PM +0100, Chilikin, Andrey wrote:  
>>>>>>> To control some device-specific features public device-specific functions
>>>>>>> rte_pmd_*.h are used.
>>>>>>> 
>>>>>>> But this solution requires applications to distinguish devices at runtime
>>>>>>> and, depending on the device type, call corresponding device-specific
>>>>>>> functions even if functions' parameters are the same.
>>>>>>> 
>>>>>>> IOCTL-like API can be added to ethdev instead of public device-specific
>>>>>>> functions to address the following:
>>>>>>> 
>>>>>>> * allow more usable support of features across a range of NIC from
>>>>>>> one vendor, but not others
>>>>>>> * allow features to be implemented by multiple NIC drivers without
>>>>>>> relying on a critical mass to get the functionality in ethdev
>>>>>>> * there are a large number of possible device specific functions, and
>>>>>>> creating individual APIs for each one is not a good solution
>>>>>>> * IOCTLs are a proven method for solving this problem in other areas,
>>>>>>> i.e. OS kernels.
>>>>>>> 
>>>>>>> Control requests for this API will be globally defined at ethdev level, so
>>>>>>> an application will use single API call to control different devices from
>>>>>>> one/multiple vendors.
>>>>>>> 
>>>>>>> API call may look like as a classic ioctl with an extra parameter for
>>>>>>> argument length for better sanity checks:
>>>>>>> 
>>>>>>> int
>>>>>>> rte_eth_dev_ioctl(uint16_t port, uint64_t ctl, void *argp,
>>>>>>>       unsigned arg_length);
>>>>>>> 
>>>>>>> Regards,
>>>>>>> Andrey    
>>>>>> 
>>>>>> I think we need to start putting in IOCTLs for ethdevs, much as I hate
>>>>>> to admit it, since I dislike IOCTLs and other functions with opaque
>>>>>> arguments! Having driver specific functions I don't think will scale
>>>>>> well as each vendor tries to expose as much of their driver specific
>>>>>> functionality as possible.
>>>>>> 
>>>>>> One other additional example: I discovered just this week another issue
>>>>>> with driver specific functions and testpmd, when I was working on the
>>>>>> meson build rework.
>>>>>> 
>>>>>> * With shared libraries, when we do "ninja install" we want our DPDK
>>>>>> libs moved to e.g. /usr/local/lib, but the drivers moved to a separate
>>>>>> driver folder, so that they can be automatically loaded from that
>>>>>> single location by DPDK apps [== CONFIG_RTE_EAL_PMD_PATH].
>>>>>> * However, testpmd, as well as using the drivers as plugins, uses
>>>>>> driver-specific functions, which means that it explicitly links
>>>>>> against the pmd .so files.
>>>>>> * Those driver .so files are not in with the other libraries, so ld.so
>>>>>> does not find the pmd, and the installed testpmd fails to run due to
>>>>>> missing library dependencies.
>>>>>> * The workaround is to add the drivers path to the ld load path, but we
>>>>>> should not require ld library path changes just to get DPDK apps to
>>>>>> work.
>>>>>> 
>>>>>> Using ioctls instead of driver-specific functions would solve this.
>>>>>> 
>>>>>> My 2c.  
>>>>> 
>>>>> My 2c. No.
>>>>> 
>>>>> Short answer:
>>>>> Ioctl's were a bad idea in Unix (per Dennis Ritchie et al) and are now
>>>>> despised by Linux kernel developers. They provide an unstructured, unsecured,
>>>>> back door for device driver abuse. Try to get a new driver in Linux with
>>>>> a unique ioctl, and it will be hard to get accepted.
>>>>> 
>>>>> Long answer:
>>>>> So far every device specific feature has fit into ethdev model. Doing ioctl
>>>>> is admitting "it is too hard to be general, we need need an out". For something
>>>>> that is a flag, it should fit into existing config model; ignoring silly ABI constraints.
>>>>> For a real feature (think flow direction), we want a first class API for that.
>>>>> For a wart, then devargs will do.
>>>>> 
>>>>> Give a good example of something that should be an ioctl. Don't build the
>>>>> API first and then let it get cluttered.  
>>>> 
>>>> I agree with Stephen.
>>>> 
>>>> And please do not forget that ioctl still requires an API:
>>>> the argument that you put in ioctl is the API of the feature.
>>>> So it is the same thing as defining a new function.  
>>> 
>>> I am also not fan of the ioctl usage. I believe it hides APIs behind ids
>>> and prevent argument check by compiler.
>>> 
>>> BUT, the number of the increasing PMD specific APIs are also worrying,
>>> it is becoming harder to maintain, and I believe this is something NOT
>>> sustainable in long run.
>>> 
>>> 
>>> What about having *eth_dev_extended_ops* ?  
>> 
>> 
>> We had talk about adding something like device specific APIs to DPDK in the past, which to me are just IOCTL like APIs. The big problem with IOCTLs is trying to cram a bunch of specific requests into a very generic API and I do not like ioctl as defined in Linux/Unix today. The old IOCTLs calls are too opaque and difficult for compilers to test args and many other issues.
>> 
>> We talked about having a single API in rte_eth_dev that would allow a user to ask for and possible get a list of function pointers in a given structure for the requested type. If a user calls this API to get some feature from a given NIC he would get NULL or a pointer to a set of functions. The generic API in rte_eth would allow the user to request what structures or types of APIs it supports.
>> 
>> Using a specific API to get the list of APIs or supported features in a NIC, will allow the developer to request the set of APIs (in an array or some method). Then we have real APIs for specific control or requests and not a generic API like ioctl.
>> 
>> Cristian had suggested an API like this to make it easy to add any IOCTL like needs to a driver. We can define a set of structures that seem generic for some IOCTL like needs or just allow the NIC to define his own structures and APIs. Allowing the developer to define his own structures and APIs is not very generic or usable by the users, so I would lean toward defining structures set need today and expand those structures in the future or add more structures.
>> 
>> int rte_eth_dev_something(uint16_t port_id, const char *feature, void **obj);
>> 
>> Using strings we can define or the NIC vendor can define to ask for a pointer to a structure he knows via a driver header. Strings are good, because we can read them via the debug or print them out quickly instead of trying to use some lookup table. Plus we can have any length or characters for defining the structure request.
>> 
>> Just off the top of my head, but it can be changed if needed.
>> 
>> 
>>> 
>>> 
>>> As a part of the rte_eth_dev. This can be in the librte_ether library
>>> but in a separated file.
>>> 
>>> And the APIs for these ops can be less strict on compatibility, and
>>> easier to add.
>>> 
>>> Benefits of having this new dev_ops:
>>> 
>>> * Having an abstraction layer for common checks.
>>> 
>>> * Even feature is not generic for all NICs, still a few NICs can share
>>> the ops.
>>> 
>>> * All APIs are in the same file makes it easy to see PMD specific APIs
>>> comparing to scattered into various PMDs.
>>> 
>>> * This is very like ioctl approach, but APIs are more clear and
>>> arguments can be verified.
>>> 
>>> Thanks,
>>> ferruh
>>> 
>>> 
>>>> 
>>>> The real debate is to decide if we want to continue adding more
>>>> control path features in DPDK or focus on Rx/Tx.
>>>> But this discussion would be better lead with some examples/requests.  
>> 
> 
> The real question is how important is that DPDK is the playground for HW features?
> My impression is that the current process is HW supplier driven "we support offload of XYZZY”.

It is not really a playground as I see it, but defining clean usable solution for vendors and developers to gain access to HW features. Every vendor loves to expose its features and allowing them to create APIs called directly from an application will make it impossible for a generic application and we need to provide some structure to allow the application to determine what features are supported and how they gain access to those features.

> 
> The existing control of device model is already a multi-path mess of config API's, device args,
> and magic scripts. That needs to be addressed first.

Not sure where the ‘magic scripts’ support comes from :-), but I agree we need to clean up the current configuration of devices. I only really see two methods in the work I have done, which are device args strings and configuration from ethdev. The DPDK configuration is another problem in that it sometimes requires a huge command line to startup an application. I think I saw a patch to add better config file support, but it does not address everything IMO. Which BTW was one of the reasons I wrote the dpdk-run.py script to help solve this problem without having to change the current config file/command line interface.

> 
> There is very little community input from users, that is the problem.

I believe I am a user with Pktgen and my usage of DPDK today, but I maybe a bit more of an expert then most users (I hope). I see these usability problems and I try to address them, but we all need to be aware of the usability aspect of DPDK for the casual user.

> 
> IMHO if a new hardware feature can't be made to fit into a standard OS model like Linux
> because it is too hard then adding it to DPDK is a mistake.

I guess I agree, but what is the Linux model anyway and how does that effect FreeBSD or Windows or name your next OS. We need to provide good hardware support for DPDK and I feel it maybe impossible to always define everything in a super clean generic way, this is why IOCTL were created and I think we can define a better solution then IOCTL as in Linux/Unix.


Regards,
Keith


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-08-08 18:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-03 12:21 [RFC] ethdev: add ioctl-like API to control device specific features Chilikin, Andrey
2017-08-03 13:21 ` Bruce Richardson
2017-08-03 16:15   ` Stephen Hemminger
2017-08-03 19:53     ` Thomas Monjalon
2017-08-04  9:59       ` Chilikin, Andrey
2017-08-04 10:08         ` Thomas Monjalon
2017-08-04 11:58       ` Ferruh Yigit
2017-08-04 12:56         ` Bruce Richardson
2017-08-08  8:32           ` Ferruh Yigit
2017-08-08 15:27             ` Stephen Hemminger
2017-08-08 17:23         ` Wiles, Keith
2017-08-08 17:28         ` Wiles, Keith
2017-08-08 18:02           ` Stephen Hemminger
2017-08-08 18:21             ` Wiles, Keith

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.