[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-30 Thread Bruce Richardson
On Wed, Sep 28, 2016 at 08:02:21PM +0200, Thomas Monjalon wrote:
> 2016-09-28 16:52, Ananyev, Konstantin:
> > 
> > > 
> > > 2016-09-28 14:30, Ananyev, Konstantin:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2016-09-28 13:26, Ananyev, Konstantin:
> > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > > > > > If we  this way (force user to include driver specific headers
> > > > > > > > and call driver specific functions), how you guys plan to make 
> > > > > > > > this functionality available for multiple driver types.
> > > > > > >
> > > > > > > Multiple drivers won't have exactly the same specific features.
> > > > > > > But yes, there are some things common to several Intel NICs.
> > > > > > >
> > > > > > > > From discussion with Bernard  understand that customers would 
> > > > > > > > need similar functionality for i40e.
> > > > > > > > Does it mean that they'll have to re-implement this part of 
> > > > > > > > their code again?
> > > > > > > > Or would have to create (and maintain) their own shim layer 
> > > > > > > > that would provide some s of abstraction?
> > > > > > > > Basically their own version of rte_ethdev?
> > > > > > >
> > > > > > > No definitive answer.
> > > > > > > But we can argue the contrary: how to handle a generic API which
> > > > > > > is implemented only in 1 or 2 drivers? If the application tries 
> > > > > > > to use it, we can imagine that a specific range of hardware is
> > > expected.
> > > > > >
> > > > > > Yes, as I understand, it is a specific subset of supported HW (just 
> > > > > > Inel NICs for now, but different models/drivers).
> > > > > > Obviously users would like to have an ability to run their app on 
> > > > > > all HW from this subset without rebuilding/implementing the
> > > app.
> > > > > >
> > > > > > >
> > > > > > > I think it is an important question.
> > > > > > > Previously we had the issue of having some API which are too
> > > > > > > specific and need a rework to be used with other NICs. In order
> > > > > > > to avoid such rework and API break, we can try to make them
> > > > > > > available in a driver-specific or vendor-specific staging area,
> > > > > > > waiting for
> > > > > a later generalization.
> > > > > >
> > > > > > Could you remind me why you guys were that opposed to ioctl style 
> > > > > > approach?
> > > > > > It is not my favorite thing either, but it seems pretty generic way 
> > > > > > to handle such situations.
> > > > >
> > > > > We prefer having well-defined functions instead of opaque ioctl-style 
> > > > > encoding.
> > > > > And it was not clear what is the benefit of ioctl.
> > > > > Now I think I understand you would like to have a common ioctl 
> > > > > service for features available on 2 drivers. Right?
> > > >
> > > > Yes.
> > > >
> > > > > Example (trying to  read your mind):
> > > > >   rte_ethdev_ioctl(port_id,  > > > > id>); instead of
> > > > >   rte_pmd_ixgbe_vf_ping(port_id, vf_id);
> > > > >   rte_pmd_i40e_vf_ping(port_id, vf_id); Please confirm I 
> > > > > understand
> > > > > what you are thinking about.
> > > >
> > > > Yep, you read my mind correctly :)
> > > 
> > > Both could coexist (if ioctl was accepted by community).
> > 
> > True.
> > 
> > > What about starting to implement the PMD functions and postpone ioctl to 
> > > later with a dedicated thread?
> > 
> > You mean something like:
> > - 16.11: implement rte_pmd_ixgbe_vf_ping()
> > - 17.02:
> > a) implement rte_pmd_i40e_vf_ping()
> > b) introduce ioctl PMD API
> > c) make possible to vf_ping via ioctl API
> > ?
> > If so, then it sounds like reasonable approach to me.
> > Though would be inserting to hear what other guys think.
> 
> Yes.
> I would just add that we have to start a discussion thread to decide
> wether we'll add an ioctl call in 17.02 or not.

I still don't like IOCTLs as an option, I still think that they are awkward to
use and hinder code readability. Here is an addition suggestion to get people
thinking.

How about allowing a set of "vendor-specific" operations for each device. We
could do this using Bernard's suggestion of adding a set of additional 
function pointers to the device struct. The reason for doing this is that it
would allow us to group functionality into families of functionality, rather
than having to things either fully generically or specifically to each NIC. If
we look at the capabilities of most NICs, the other NICs which support similar
functions are generally other NICs from the same vendor. We could therefore
have ops common across ixgbe and i40e and other ops mlx4 and mlx5 drivers.

Could such a scheme work? Would it be worth doing?

/Bruce


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Thomas Monjalon
2016-09-28 16:52, Ananyev, Konstantin:
> 
> > 
> > 2016-09-28 14:30, Ananyev, Konstantin:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2016-09-28 13:26, Ananyev, Konstantin:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > > > > If we  this way (force user to include driver specific headers
> > > > > > > and call driver specific functions), how you guys plan to make 
> > > > > > > this functionality available for multiple driver types.
> > > > > >
> > > > > > Multiple drivers won't have exactly the same specific features.
> > > > > > But yes, there are some things common to several Intel NICs.
> > > > > >
> > > > > > > From discussion with Bernard  understand that customers would 
> > > > > > > need similar functionality for i40e.
> > > > > > > Does it mean that they'll have to re-implement this part of their 
> > > > > > > code again?
> > > > > > > Or would have to create (and maintain) their own shim layer that 
> > > > > > > would provide some s of abstraction?
> > > > > > > Basically their own version of rte_ethdev?
> > > > > >
> > > > > > No definitive answer.
> > > > > > But we can argue the contrary: how to handle a generic API which
> > > > > > is implemented only in 1 or 2 drivers? If the application tries to 
> > > > > > use it, we can imagine that a specific range of hardware is
> > expected.
> > > > >
> > > > > Yes, as I understand, it is a specific subset of supported HW (just 
> > > > > Inel NICs for now, but different models/drivers).
> > > > > Obviously users would like to have an ability to run their app on all 
> > > > > HW from this subset without rebuilding/implementing the
> > app.
> > > > >
> > > > > >
> > > > > > I think it is an important question.
> > > > > > Previously we had the issue of having some API which are too
> > > > > > specific and need a rework to be used with other NICs. In order
> > > > > > to avoid such rework and API break, we can try to make them
> > > > > > available in a driver-specific or vendor-specific staging area,
> > > > > > waiting for
> > > > a later generalization.
> > > > >
> > > > > Could you remind me why you guys were that opposed to ioctl style 
> > > > > approach?
> > > > > It is not my favorite thing either, but it seems pretty generic way 
> > > > > to handle such situations.
> > > >
> > > > We prefer having well-defined functions instead of opaque ioctl-style 
> > > > encoding.
> > > > And it was not clear what is the benefit of ioctl.
> > > > Now I think I understand you would like to have a common ioctl service 
> > > > for features available on 2 drivers. Right?
> > >
> > > Yes.
> > >
> > > > Example (trying to  read your mind):
> > > > rte_ethdev_ioctl(port_id,  > > > id>); instead of
> > > > rte_pmd_ixgbe_vf_ping(port_id, vf_id);
> > > > rte_pmd_i40e_vf_ping(port_id, vf_id); Please confirm I 
> > > > understand
> > > > what you are thinking about.
> > >
> > > Yep, you read my mind correctly :)
> > 
> > Both could coexist (if ioctl was accepted by community).
> 
> True.
> 
> > What about starting to implement the PMD functions and postpone ioctl to 
> > later with a dedicated thread?
> 
> You mean something like:
> - 16.11: implement rte_pmd_ixgbe_vf_ping()
> - 17.02:
>   a) implement rte_pmd_i40e_vf_ping()
>   b) introduce ioctl PMD API
>   c) make possible to vf_ping via ioctl API
> ?
> If so, then it sounds like reasonable approach to me.
> Though would be inserting to hear what other guys think.

Yes.
I would just add that we have to start a discussion thread to decide
wether we'll add an ioctl call in 17.02 or not.


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread ZELEZNIAK, ALEX






From: "az5157 at att.com<mailto:az5157 at att.com>" mailto:az5...@att.com>>
Date: Wednesday, September 28, 2016 at 12:23:06 PM
To: "Thomas Monjalon" mailto:thomas.monjalon at 
6wind.com>>
Cc: "Ananyev, Konstantin" mailto:konstantin.ananyev at intel.com>>, "Iremonger, Bernard" 
mailto:bernard.iremonger at intel.com>>, 
"Richardson, Bruce" mailto:bruce.richardson at 
intel.com>>, "dev at dpdk.org<mailto:dev at dpdk.org>" mailto:dev at dpdk.org>>, "Jerin Jacob" mailto:jerin.jacob at caviumnetworks.com>>, "Shah, Rahul R" 
mailto:rahul.r.shah at intel.com>>, "Lu, Wenzhuo" 
mailto:wenzhuo.lu at intel.com>>
Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF 
management

>
> 2016-09-28 16:52, Ananyev, Konstantin:
> >
> > >
> > > 2016-09-28 14:30, Ananyev, Konstantin:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 
> > > > 6wind.com<mailto:thomas.monjalon at 6wind.com>]
> > > > > 2016-09-28 13:26, Ananyev, Konstantin:
> > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 
> > > > > > 6wind.com<mailto:thomas.monjalon at 6wind.com>]
> > > > > > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > > > > > If we  this way (force user to include driver specific headers
> > > > > > > > and call driver specific functions), how you guys plan to make 
> > > > > > > > this functionality available for multiple driver types.
> > > > > > >
> > > > > > > Multiple drivers won't have exactly the same specific features.
> > > > > > > But yes, there are some things common to several Intel NICs.
> > > > > > >
> > > > > > > > From discussion with Bernard  understand that customers would 
> > > > > > > > need similar functionality for i40e.
> > > > > > > > Does it mean that they'll have to re-implement this part of 
> > > > > > > > their code again?
> > > > > > > > Or would have to create (and maintain) their own shim layer 
> > > > > > > > that would provide some s of abstraction?
> > > > > > > > Basically their own version of rte_ethdev?
> > > > > > >
> > > > > > > No definitive answer.
> > > > > > > But we can argue the contrary: how to handle a generic API which
> > > > > > > is implemented only in 1 or 2 drivers? If the application tries 
> > > > > > > to use it, we can imagine that a specific range of hardware is
> > > expected.
> > > > > >
> > > > > > Yes, as I understand, it is a specific subset of supported HW (just 
> > > > > > Inel NICs for now, but different models/drivers).
> > > > > > Obviously users would like to have an ability to run their app on 
> > > > > > all HW from this subset without rebuilding/implementing the
> > > app.
> > > > > >
> > > > > > >
> > > > > > > I think it is an important question.
> > > > > > > Previously we had the issue of having some API which are too
> > > > > > > specific and need a rework to be used with other NICs. In order
> > > > > > > to avoid such rework and API break, we can try to make them
> > > > > > > available in a driver-specific or vendor-specific staging area,
> > > > > > > waiting for
> > > > > a later generalization.
> > > > > >
> > > > > > Could you remind me why you guys were that opposed to ioctl style 
> > > > > > approach?
> > > > > > It is not my favorite thing either, but it seems pretty generic way 
> > > > > > to handle such situations.
> > > > >
> > > > > We prefer having well-defined functions instead of opaque ioctl-style 
> > > > > encoding.
> > > > > And it was not clear what is the benefit of ioctl.
> > > > > Now I think I understand you would like to have a common ioctl 
> > > > > service for features available on 2 drivers. Right?
> > > >
> > > > Yes.
> > > >
> > > > > Example (trying to  read your mind):
> > > > >  rte_eth

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Thomas Monjalon
2016-09-28 14:48, Iremonger, Bernard:
> 
> 
> > > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for
> > > VF management
> > >
> > > 2016-09-28 13:26, Ananyev, Konstantin:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > > > If we  this way (force user to include driver specific headers
> > > > > > and call driver specific functions), how you guys plan to make this
> > functionality available for multiple driver types.
> > > > >
> > > > > Multiple drivers won't have exactly the same specific features.
> > > > > But yes, there are some things common to several Intel NICs.
> > > > >
> > > > > > From discussion with Bernard  understand that customers would
> > need similar functionality for i40e.
> > > > > > Does it mean that they'll have to re-implement this part of their 
> > > > > > code
> > again?
> > > > > > Or would have to create (and maintain) their own shim layer that
> > would provide some s of abstraction?
> > > > > > Basically their own version of rte_ethdev?
> > > > >
> > > > > No definitive answer.
> > > > > But we can argue the contrary: how to handle a generic API which
> > > > > is implemented only in 1 or 2 drivers? If the application tries to 
> > > > > use it,
> > we can imagine that a specific range of hardware is expected.
> > > >
> > > > Yes, as I understand, it is a specific subset of supported HW (just 
> > > > Inel NICs
> > for now, but different models/drivers).
> > > > Obviously users would like to have an ability to run their app on all HW
> > from this subset without rebuilding/implementing the app.
> > > >
> > > > >
> > > > > I think it is an important question.
> > > > > Previously we had the issue of having some API which are too
> > > > > specific and need a rework to be used with other NICs. In order to
> > > > > avoid such rework and API break, we can try to make them available
> > > > > in a driver-specific or vendor-specific staging area, waiting for
> > > a later generalization.
> > > >
> > > > Could you remind me why you guys were that opposed to ioctl style
> > approach?
> > > > It is not my favorite thing either, but it seems pretty generic way to
> > handle such situations.
> > >
> > > We prefer having well-defined functions instead of opaque ioctl-style
> > encoding.
> > > And it was not clear what is the benefit of ioctl.
> > > Now I think I understand you would like to have a common ioctl service for
> > features available on 2 drivers. Right?
> > 
> > Yes.
> > 
> > > Example (trying to  read your mind):
> > >   rte_ethdev_ioctl(port_id,  > id>); instead of
> > >   rte_pmd_ixgbe_vf_ping(port_id, vf_id);
> > >   rte_pmd_i40e_vf_ping(port_id, vf_id); Please confirm I understand
> > > what you are thinking about.
> > 
> > Yep, you read my mind correctly :)
> > Konstantin
> > 
> Adding the pmd_ops field to struct eth_devops {} discussed previously in this 
> email thread will allow driver specific functions for multiple drivers and 
> will get rid of the driver specific header file rte_pmd_driver.h.
> Would this be an acceptable solution?

How pmd_ops would be different of eth_devops?


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Thomas Monjalon
2016-09-28 14:30, Ananyev, Konstantin:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2016-09-28 13:26, Ananyev, Konstantin:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > > If we  this way (force user to include driver specific headers and
> > > > > call driver specific functions), how you guys plan to make this 
> > > > > functionality available for multiple driver types.
> > > >
> > > > Multiple drivers won't have exactly the same specific features.
> > > > But yes, there are some things common to several Intel NICs.
> > > >
> > > > > From discussion with Bernard  understand that customers would need 
> > > > > similar functionality for i40e.
> > > > > Does it mean that they'll have to re-implement this part of their 
> > > > > code again?
> > > > > Or would have to create (and maintain) their own shim layer that 
> > > > > would provide some s of abstraction?
> > > > > Basically their own version of rte_ethdev?
> > > >
> > > > No definitive answer.
> > > > But we can argue the contrary: how to handle a generic API which is
> > > > implemented only in 1 or 2 drivers? If the application tries to use it, 
> > > > we can imagine that a specific range of hardware is expected.
> > >
> > > Yes, as I understand, it is a specific subset of supported HW (just Inel 
> > > NICs for now, but different models/drivers).
> > > Obviously users would like to have an ability to run their app on all HW 
> > > from this subset without rebuilding/implementing the app.
> > >
> > > >
> > > > I think it is an important question.
> > > > Previously we had the issue of having some API which are too
> > > > specific and need a rework to be used with other NICs. In order to
> > > > avoid such rework and API break, we can try to make them available in a 
> > > > driver-specific or vendor-specific staging area, waiting for
> > a later generalization.
> > >
> > > Could you remind me why you guys were that opposed to ioctl style 
> > > approach?
> > > It is not my favorite thing either, but it seems pretty generic way to 
> > > handle such situations.
> > 
> > We prefer having well-defined functions instead of opaque ioctl-style 
> > encoding.
> > And it was not clear what is the benefit of ioctl.
> > Now I think I understand you would like to have a common ioctl service for 
> > features available on 2 drivers. Right?
> 
> Yes.
> 
> > Example (trying to  read your mind):
> > rte_ethdev_ioctl(port_id, ); 
> > instead of
> > rte_pmd_ixgbe_vf_ping(port_id, vf_id);
> > rte_pmd_i40e_vf_ping(port_id, vf_id);
> > Please confirm I understand what you are thinking about.
> 
> Yep, you read my mind correctly :)

Both could coexist (if ioctl was accepted by community).
What about starting to implement the PMD functions and postpone
ioctl to later with a dedicated thread?


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Ananyev, Konstantin

> 
> 2016-09-28 14:30, Ananyev, Konstantin:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-09-28 13:26, Ananyev, Konstantin:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > > > If we  this way (force user to include driver specific headers
> > > > > > and call driver specific functions), how you guys plan to make this 
> > > > > > functionality available for multiple driver types.
> > > > >
> > > > > Multiple drivers won't have exactly the same specific features.
> > > > > But yes, there are some things common to several Intel NICs.
> > > > >
> > > > > > From discussion with Bernard  understand that customers would need 
> > > > > > similar functionality for i40e.
> > > > > > Does it mean that they'll have to re-implement this part of their 
> > > > > > code again?
> > > > > > Or would have to create (and maintain) their own shim layer that 
> > > > > > would provide some s of abstraction?
> > > > > > Basically their own version of rte_ethdev?
> > > > >
> > > > > No definitive answer.
> > > > > But we can argue the contrary: how to handle a generic API which
> > > > > is implemented only in 1 or 2 drivers? If the application tries to 
> > > > > use it, we can imagine that a specific range of hardware is
> expected.
> > > >
> > > > Yes, as I understand, it is a specific subset of supported HW (just 
> > > > Inel NICs for now, but different models/drivers).
> > > > Obviously users would like to have an ability to run their app on all 
> > > > HW from this subset without rebuilding/implementing the
> app.
> > > >
> > > > >
> > > > > I think it is an important question.
> > > > > Previously we had the issue of having some API which are too
> > > > > specific and need a rework to be used with other NICs. In order
> > > > > to avoid such rework and API break, we can try to make them
> > > > > available in a driver-specific or vendor-specific staging area,
> > > > > waiting for
> > > a later generalization.
> > > >
> > > > Could you remind me why you guys were that opposed to ioctl style 
> > > > approach?
> > > > It is not my favorite thing either, but it seems pretty generic way to 
> > > > handle such situations.
> > >
> > > We prefer having well-defined functions instead of opaque ioctl-style 
> > > encoding.
> > > And it was not clear what is the benefit of ioctl.
> > > Now I think I understand you would like to have a common ioctl service 
> > > for features available on 2 drivers. Right?
> >
> > Yes.
> >
> > > Example (trying to  read your mind):
> > >   rte_ethdev_ioctl(port_id, ); 
> > > instead of
> > >   rte_pmd_ixgbe_vf_ping(port_id, vf_id);
> > >   rte_pmd_i40e_vf_ping(port_id, vf_id); Please confirm I understand
> > > what you are thinking about.
> >
> > Yep, you read my mind correctly :)
> 
> Both could coexist (if ioctl was accepted by community).

True.

> What about starting to implement the PMD functions and postpone ioctl to 
> later with a dedicated thread?

You mean something like:
- 16.11: implement rte_pmd_ixgbe_vf_ping()
- 17.02:
a) implement rte_pmd_i40e_vf_ping()
b) introduce ioctl PMD API
c) make possible to vf_ping via ioctl API
?
If so, then it sounds like reasonable approach to me.
Though would be inserting to hear what other guys think.
Konstantin




[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Thomas Monjalon
2016-09-28 13:26, Ananyev, Konstantin:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2016-09-28 11:23, Ananyev, Konstantin:
> > > If we  this way (force user to include driver specific headers and
> > > call driver specific functions), how you guys plan to make this 
> > > functionality available for multiple driver types.
> > 
> > Multiple drivers won't have exactly the same specific features.
> > But yes, there are some things common to several Intel NICs.
> > 
> > > From discussion with Bernard  understand that customers would need 
> > > similar functionality for i40e.
> > > Does it mean that they'll have to re-implement this part of their code 
> > > again?
> > > Or would have to create (and maintain) their own shim layer that would 
> > > provide some s of abstraction?
> > > Basically their own version of rte_ethdev?
> > 
> > No definitive answer.
> > But we can argue the contrary: how to handle a generic API which is 
> > implemented only in 1 or 2 drivers? If the application tries to use
> > it, we can imagine that a specific range of hardware is expected.
> 
> Yes, as I understand, it is a specific subset of supported HW (just Inel NICs 
> for now, but different models/drivers).
> Obviously users would like to have an ability to run their app on all HW from 
> this subset without rebuilding/implementing the app.
> 
> > 
> > I think it is an important question.
> > Previously we had the issue of having some API which are too specific and 
> > need a rework to be used with other NICs. In order to avoid
> > such rework and API break, we can try to make them available in a 
> > driver-specific or vendor-specific staging area, waiting for a later
> > generalization.
> 
> Could you remind me why you guys were that opposed to ioctl style approach?
> It is not my favorite thing either, but it seems pretty generic way to handle 
> such situations.

We prefer having well-defined functions instead of opaque ioctl-style encoding.
And it was not clear what is the benefit of ioctl.
Now I think I understand you would like to have a common ioctl service for
features available on 2 drivers. Right? Example (trying to read your mind):
rte_ethdev_ioctl(port_id, );
instead of
rte_pmd_ixgbe_vf_ping(port_id, vf_id);
rte_pmd_i40e_vf_ping(port_id, vf_id);
Please confirm I understand what you are thinking about.


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Iremonger, Bernard
Hi Thomas,



> -Original Message-
> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> 2016-09-28 14:48, Iremonger, Bernard:
> > 
> >
> > > > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's
> > > > for VF management
> > > >
> > > > 2016-09-28 13:26, Ananyev, Konstantin:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > > > > If we  this way (force user to include driver specific
> > > > > > > headers and call driver specific functions), how you guys
> > > > > > > plan to make this
> > > functionality available for multiple driver types.
> > > > > >
> > > > > > Multiple drivers won't have exactly the same specific features.
> > > > > > But yes, there are some things common to several Intel NICs.
> > > > > >
> > > > > > > From discussion with Bernard  understand that customers
> > > > > > > would
> > > need similar functionality for i40e.
> > > > > > > Does it mean that they'll have to re-implement this part of
> > > > > > > their code
> > > again?
> > > > > > > Or would have to create (and maintain) their own shim layer
> > > > > > > that
> > > would provide some s of abstraction?
> > > > > > > Basically their own version of rte_ethdev?
> > > > > >
> > > > > > No definitive answer.
> > > > > > But we can argue the contrary: how to handle a generic API
> > > > > > which is implemented only in 1 or 2 drivers? If the
> > > > > > application tries to use it,
> > > we can imagine that a specific range of hardware is expected.
> > > > >
> > > > > Yes, as I understand, it is a specific subset of supported HW
> > > > > (just Inel NICs
> > > for now, but different models/drivers).
> > > > > Obviously users would like to have an ability to run their app
> > > > > on all HW
> > > from this subset without rebuilding/implementing the app.
> > > > >
> > > > > >
> > > > > > I think it is an important question.
> > > > > > Previously we had the issue of having some API which are too
> > > > > > specific and need a rework to be used with other NICs. In
> > > > > > order to avoid such rework and API break, we can try to make
> > > > > > them available in a driver-specific or vendor-specific staging
> > > > > > area, waiting for
> > > > a later generalization.
> > > > >
> > > > > Could you remind me why you guys were that opposed to ioctl
> > > > > style
> > > approach?
> > > > > It is not my favorite thing either, but it seems pretty generic
> > > > > way to
> > > handle such situations.
> > > >
> > > > We prefer having well-defined functions instead of opaque
> > > > ioctl-style
> > > encoding.
> > > > And it was not clear what is the benefit of ioctl.
> > > > Now I think I understand you would like to have a common ioctl
> > > > service for
> > > features available on 2 drivers. Right?
> > >
> > > Yes.
> > >
> > > > Example (trying to  read your mind):
> > > > rte_ethdev_ioctl(port_id,  > > id>); instead of
> > > > rte_pmd_ixgbe_vf_ping(port_id, vf_id);
> > > > rte_pmd_i40e_vf_ping(port_id, vf_id); Please confirm I 
> > > > understand
> > > > what you are thinking about.
> > >
> > > Yep, you read my mind correctly :)
> > > Konstantin
> > >
> > Adding the pmd_ops field to struct eth_devops {} discussed previously in
> this email thread will allow driver specific functions for multiple drivers 
> and
> will get rid of the driver specific header file rte_pmd_driver.h.
> > Would this be an acceptable solution?
> 
> How pmd_ops would be different of eth_devops?

There is not a lot of difference, however it would separate generic ethdev 
functions from driver specific functions.

Regards,

Bernard.




[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Thomas Monjalon
2016-09-28 11:23, Ananyev, Konstantin:
> If we  this way (force user to include driver specific headers and call 
> driver specific functions),
> how you guys plan to make this functionality available for multiple driver 
> types.

Multiple drivers won't have exactly the same specific features.
But yes, there are some things common to several Intel NICs.

> From discussion with Bernard  understand that customers would need similar 
> functionality for i40e.
> Does it mean that they'll have to re-implement this part of their code again?
> Or would have to create (and maintain) their own shim layer that would 
> provide some s of abstraction?
> Basically their own version of rte_ethdev?

No definitive answer.
But we can argue the contrary: how to handle a generic API which is implemented
only in 1 or 2 drivers? If the application tries to use it, we can imagine
that a specific range of hardware is expected.

I think it is an important question.
Previously we had the issue of having some API which are too specific
and need a rework to be used with other NICs. In order to avoid such
rework and API break, we can try to make them available in a driver-specific
or vendor-specific staging area, waiting for a later generalization.


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Iremonger, Bernard


> > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for
> > VF management
> >
> > 2016-09-28 13:26, Ananyev, Konstantin:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > > If we  this way (force user to include driver specific headers
> > > > > and call driver specific functions), how you guys plan to make this
> functionality available for multiple driver types.
> > > >
> > > > Multiple drivers won't have exactly the same specific features.
> > > > But yes, there are some things common to several Intel NICs.
> > > >
> > > > > From discussion with Bernard  understand that customers would
> need similar functionality for i40e.
> > > > > Does it mean that they'll have to re-implement this part of their code
> again?
> > > > > Or would have to create (and maintain) their own shim layer that
> would provide some s of abstraction?
> > > > > Basically their own version of rte_ethdev?
> > > >
> > > > No definitive answer.
> > > > But we can argue the contrary: how to handle a generic API which
> > > > is implemented only in 1 or 2 drivers? If the application tries to use 
> > > > it,
> we can imagine that a specific range of hardware is expected.
> > >
> > > Yes, as I understand, it is a specific subset of supported HW (just Inel 
> > > NICs
> for now, but different models/drivers).
> > > Obviously users would like to have an ability to run their app on all HW
> from this subset without rebuilding/implementing the app.
> > >
> > > >
> > > > I think it is an important question.
> > > > Previously we had the issue of having some API which are too
> > > > specific and need a rework to be used with other NICs. In order to
> > > > avoid such rework and API break, we can try to make them available
> > > > in a driver-specific or vendor-specific staging area, waiting for
> > a later generalization.
> > >
> > > Could you remind me why you guys were that opposed to ioctl style
> approach?
> > > It is not my favorite thing either, but it seems pretty generic way to
> handle such situations.
> >
> > We prefer having well-defined functions instead of opaque ioctl-style
> encoding.
> > And it was not clear what is the benefit of ioctl.
> > Now I think I understand you would like to have a common ioctl service for
> features available on 2 drivers. Right?
> 
> Yes.
> 
> > Example (trying to  read your mind):
> > rte_ethdev_ioctl(port_id,  id>); instead of
> > rte_pmd_ixgbe_vf_ping(port_id, vf_id);
> > rte_pmd_i40e_vf_ping(port_id, vf_id); Please confirm I understand
> > what you are thinking about.
> 
> Yep, you read my mind correctly :)
> Konstantin
> 
Adding the pmd_ops field to struct eth_devops {} discussed previously in this 
email thread will allow driver specific functions for multiple drivers and will 
get rid of the driver specific header file rte_pmd_driver.h.
Would this be an acceptable solution?

Regards,

Bernard.



[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Ananyev, Konstantin


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, September 28, 2016 3:24 PM
> To: Ananyev, Konstantin 
> Cc: Iremonger, Bernard ; Richardson, Bruce 
> ; dev at dpdk.org; Jerin
> Jacob ; Shah, Rahul R  intel.com>; Lu, Wenzhuo ;
> azelezniak 
> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF 
> management
> 
> 2016-09-28 13:26, Ananyev, Konstantin:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-09-28 11:23, Ananyev, Konstantin:
> > > > If we  this way (force user to include driver specific headers and
> > > > call driver specific functions), how you guys plan to make this 
> > > > functionality available for multiple driver types.
> > >
> > > Multiple drivers won't have exactly the same specific features.
> > > But yes, there are some things common to several Intel NICs.
> > >
> > > > From discussion with Bernard  understand that customers would need 
> > > > similar functionality for i40e.
> > > > Does it mean that they'll have to re-implement this part of their code 
> > > > again?
> > > > Or would have to create (and maintain) their own shim layer that would 
> > > > provide some s of abstraction?
> > > > Basically their own version of rte_ethdev?
> > >
> > > No definitive answer.
> > > But we can argue the contrary: how to handle a generic API which is
> > > implemented only in 1 or 2 drivers? If the application tries to use it, 
> > > we can imagine that a specific range of hardware is expected.
> >
> > Yes, as I understand, it is a specific subset of supported HW (just Inel 
> > NICs for now, but different models/drivers).
> > Obviously users would like to have an ability to run their app on all HW 
> > from this subset without rebuilding/implementing the app.
> >
> > >
> > > I think it is an important question.
> > > Previously we had the issue of having some API which are too
> > > specific and need a rework to be used with other NICs. In order to
> > > avoid such rework and API break, we can try to make them available in a 
> > > driver-specific or vendor-specific staging area, waiting for
> a later generalization.
> >
> > Could you remind me why you guys were that opposed to ioctl style approach?
> > It is not my favorite thing either, but it seems pretty generic way to 
> > handle such situations.
> 
> We prefer having well-defined functions instead of opaque ioctl-style 
> encoding.
> And it was not clear what is the benefit of ioctl.
> Now I think I understand you would like to have a common ioctl service for 
> features available on 2 drivers. Right?

Yes.

> Example (trying to  read your mind):
>   rte_ethdev_ioctl(port_id, ); 
> instead of
>   rte_pmd_ixgbe_vf_ping(port_id, vf_id);
>   rte_pmd_i40e_vf_ping(port_id, vf_id);
> Please confirm I understand what you are thinking about.

Yep, you read my mind correctly :)
Konstantin




[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Ananyev, Konstantin


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, September 28, 2016 2:03 PM
> To: Ananyev, Konstantin 
> Cc: Iremonger, Bernard ; Richardson, Bruce 
> ; dev at dpdk.org; Jerin
> Jacob ; Shah, Rahul R  intel.com>; Lu, Wenzhuo ;
> azelezniak 
> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF 
> management
> 
> 2016-09-28 11:23, Ananyev, Konstantin:
> > If we  this way (force user to include driver specific headers and
> > call driver specific functions), how you guys plan to make this 
> > functionality available for multiple driver types.
> 
> Multiple drivers won't have exactly the same specific features.
> But yes, there are some things common to several Intel NICs.
> 
> > From discussion with Bernard  understand that customers would need similar 
> > functionality for i40e.
> > Does it mean that they'll have to re-implement this part of their code 
> > again?
> > Or would have to create (and maintain) their own shim layer that would 
> > provide some s of abstraction?
> > Basically their own version of rte_ethdev?
> 
> No definitive answer.
> But we can argue the contrary: how to handle a generic API which is 
> implemented only in 1 or 2 drivers? If the application tries to use
> it, we can imagine that a specific range of hardware is expected.

Yes, as I understand, it is a specific subset of supported HW (just Inel NICs 
for now, but different models/drivers).
Obviously users would like to have an ability to run their app on all HW from 
this subset without rebuilding/implementing the app.

> 
> I think it is an important question.
> Previously we had the issue of having some API which are too specific and 
> need a rework to be used with other NICs. In order to avoid
> such rework and API break, we can try to make them available in a 
> driver-specific or vendor-specific staging area, waiting for a later
> generalization.

Could you remind me why you guys were that opposed to ioctl style approach?
It is not my favorite thing either, but it seems pretty generic way to handle 
such situations.

Konstantin




[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Richardson, Bruce


> -Original Message-
> From: Ananyev, Konstantin
> Sent: Wednesday, September 28, 2016 12:24 PM
> To: Iremonger, Bernard ; Richardson, Bruce
> 
> Cc: Thomas Monjalon ; dev at dpdk.org; Jerin 
> Jacob
> ; Shah, Rahul R  intel.com>;
> Lu, Wenzhuo ; azelezniak 
> Subject: RE: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> Hi lads,
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Iremonger,
> > Bernard
> > Sent: Tuesday, September 27, 2016 3:13 PM
> > To: Richardson, Bruce 
> > Cc: Thomas Monjalon ; dev at dpdk.org; Jerin
> > Jacob ; Shah, Rahul R
> > ; Lu, Wenzhuo ;
> > azelezniak 
> > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for
> > VF management
> >
> > Hi Bruce,
> >
> > 
> >
> > > > > > > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add
> > > > > > > API's for VF management
> > > > > > >
> > > > > > > 2016-09-23 17:02, Iremonger, Bernard:
> > > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > > 2016-09-23 09:53, Richardson, Bruce:
> > > > > > > > > > From: Thomas Monjalon
> > > > > > > > > > [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > > > > > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas
> > > > > > > > > > > > Monjalon
> > > > > wrote:
> > > > > > > > > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > > > > > > > > Do we really need to expose VF specific
> > > > > > > > > > > > > > > > > functions
> > > > > here?
> > > > > > > > > > > > > > > > > It can be generic(PF/VF) function
> > > > > > > > > > > > > > > > > indexed only through
> > > > > > > > > > > port_id.
> > > > > > > > > > > > > > > > > (example: as
> > > > > > > > > > > > > > > > > rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > > > > > > > > port_id, uint8_t on)) For instance, In
> > > > > > > > > > > > > > > > > Thunderx PMD, We are not exposing a
> > > > > > > > > > > > > > > > > separate port_id for PF. We only
> > > > > > > > > > > > > > > > > enumerate 0..N VFs as 0..N ethdev
> > > > > > > > > > > > > > > > > port_id
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Our intention with this patch is to
> > > > > > > > > > > > > > > > control the VF from the
> > > > > PF.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > The following librte_ether functions
> > > > > > > > > > > > > > > > already work in a similar
> > > > > > > > > > > way:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,
> > > > > > > > > > > > > > > > uint16_t vf, uint16_t rx_mode, uint8_t on)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id,
> > > > > > > > > > > > > > > > uint16_t vf, uint8_t
> > > > > > > > > > > > > > > > on)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id,
> > > > > > > > > > > > > > > > uint16_t 

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Iremonger, Bernard
Hi Bruce, Konstantin,



> Subject: RE: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> Hi lads,
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Iremonger,
> > Bernard
> > Sent: Tuesday, September 27, 2016 3:13 PM
> > To: Richardson, Bruce 
> > Cc: Thomas Monjalon ; dev at dpdk.org;
> Jerin
> > Jacob ; Shah, Rahul R
> > ; Lu, Wenzhuo ;
> > azelezniak 
> > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for
> > VF management
> >
> > Hi Bruce,
> >
> > 
> >
> > > > > > > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add
> > > > > > > API's for VF management
> > > > > > >
> > > > > > > 2016-09-23 17:02, Iremonger, Bernard:
> > > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > > 2016-09-23 09:53, Richardson, Bruce:
> > > > > > > > > > From: Thomas Monjalon
> > > > > > > > > > [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > > > > > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas
> > > > > > > > > > > > Monjalon
> > > > > wrote:
> > > > > > > > > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > > > > > > > > Do we really need to expose VF specific
> > > > > > > > > > > > > > > > > functions
> > > > > here?
> > > > > > > > > > > > > > > > > It can be generic(PF/VF) function
> > > > > > > > > > > > > > > > > indexed only through
> > > > > > > > > > > port_id.
> > > > > > > > > > > > > > > > > (example: as
> > > > > > > > > > > > > > > > > rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > > > > > > > > port_id, uint8_t on)) For instance, In
> > > > > > > > > > > > > > > > > Thunderx PMD, We are not exposing a
> > > > > > > > > > > > > > > > > separate port_id for PF. We only
> > > > > > > > > > > > > > > > > enumerate 0..N VFs as 0..N ethdev
> > > > > > > > > > > > > > > > > port_id
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Our intention with this patch is to
> > > > > > > > > > > > > > > > control the VF from the
> > > > > PF.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > The following librte_ether functions
> > > > > > > > > > > > > > > > already work in a similar
> > > > > > > > > > > way:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,
> > > > > > > > > > > > > > > > uint16_t vf, uint16_t rx_mode, uint8_t on)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id,
> > > > > > > > > > > > > > > > uint16_t vf, uint8_t
> > > > > > > > > > > > > > > > on)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id,
> > > > > > > > > > > > > > > > uint16_t vf, uint8_t
> > > > > > > > > > > > > > > > on)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > int rte_eth_set_vf

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-28 Thread Ananyev, Konstantin
Hi lads,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Iremonger, Bernard
> Sent: Tuesday, September 27, 2016 3:13 PM
> To: Richardson, Bruce 
> Cc: Thomas Monjalon ; dev at dpdk.org; Jerin 
> Jacob ; Shah, Rahul
> R ; Lu, Wenzhuo ; 
> azelezniak 
> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF 
> management
> 
> Hi Bruce,
> 
> 
> 
> > > > > > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add
> > > > > > API's for VF management
> > > > > >
> > > > > > 2016-09-23 17:02, Iremonger, Bernard:
> > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > 2016-09-23 09:53, Richardson, Bruce:
> > > > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > > > > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas
> > > > > > > > > > > Monjalon
> > > > wrote:
> > > > > > > > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > > > > > > > Do we really need to expose VF specific
> > > > > > > > > > > > > > > > functions
> > > > here?
> > > > > > > > > > > > > > > > It can be generic(PF/VF) function indexed
> > > > > > > > > > > > > > > > only through
> > > > > > > > > > port_id.
> > > > > > > > > > > > > > > > (example: as
> > > > > > > > > > > > > > > > rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > > > > > > > port_id, uint8_t on)) For instance, In
> > > > > > > > > > > > > > > > Thunderx PMD, We are not exposing a
> > > > > > > > > > > > > > > > separate port_id for PF. We only enumerate
> > > > > > > > > > > > > > > > 0..N VFs as 0..N ethdev port_id
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Our intention with this patch is to control
> > > > > > > > > > > > > > > the VF from the
> > > > PF.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > The following librte_ether functions already
> > > > > > > > > > > > > > > work in a similar
> > > > > > > > > > way:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,
> > > > > > > > > > > > > > > uint16_t vf, uint16_t rx_mode, uint8_t on)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id,
> > > > > > > > > > > > > > > uint16_t vf, uint8_t
> > > > > > > > > > > > > > > on)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id,
> > > > > > > > > > > > > > > uint16_t vf, uint8_t
> > > > > > > > > > > > > > > on)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > int rte_eth_set_vf_rate_limit(uint8_t
> > > > > > > > > > > > > > > port_id, uint16_t vf, uint16_t tx_rate,
> > > > > > > > > > > > > > > uint64_t q_msk)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I have a bad feeling with these functions
> > > > > > > > > > > > > > dedicated to VF from
> > > > > > PF.
> > > > > > > > > > > 

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-27 Thread Iremonger, Bernard
Hi Bruce,



> > > > > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add
> > > > > API's for VF management
> > > > >
> > > > > 2016-09-23 17:02, Iremonger, Bernard:
> > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > 2016-09-23 09:53, Richardson, Bruce:
> > > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > > > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas
> > > > > > > > > > Monjalon
> > > wrote:
> > > > > > > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > > > > > > Do we really need to expose VF specific
> > > > > > > > > > > > > > > functions
> > > here?
> > > > > > > > > > > > > > > It can be generic(PF/VF) function indexed
> > > > > > > > > > > > > > > only through
> > > > > > > > > port_id.
> > > > > > > > > > > > > > > (example: as
> > > > > > > > > > > > > > > rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > > > > > > port_id, uint8_t on)) For instance, In
> > > > > > > > > > > > > > > Thunderx PMD, We are not exposing a separate
> > > > > > > > > > > > > > > port_id for PF. We only enumerate 0..N VFs
> > > > > > > > > > > > > > > as 0..N ethdev port_id
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Our intention with this patch is to control
> > > > > > > > > > > > > > the VF from the
> > > PF.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > The following librte_ether functions already
> > > > > > > > > > > > > > work in a similar
> > > > > > > > > way:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,
> > > > > > > > > > > > > > uint16_t vf, uint16_t rx_mode, uint8_t on)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id,
> > > > > > > > > > > > > > uint16_t vf, uint8_t
> > > > > > > > > > > > > > on)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id,
> > > > > > > > > > > > > > uint16_t vf, uint8_t
> > > > > > > > > > > > > > on)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id,
> > > > > > > > > > > > > > uint16_t vf, uint16_t tx_rate, uint64_t q_msk)
> > > > > > > > > > > > >
> > > > > > > > > > > > > I have a bad feeling with these functions
> > > > > > > > > > > > > dedicated to VF from
> > > > > PF.
> > > > > > > > > > > > > Are we sure there is no other way?
> > > > > > > > > > > > > I mean we just need to know the VF with a port ID.
> > > > > > > > > > > >
> > > > > > > > > > > > When the VF is used in a VM the port ID of the VF
> > > > > > > > > > > > is not visible to
> > > > > > > > > the PF.
> > > > > > > > > > > > I don't think there is another way to do this.
> > > > > > > > > > >
> > > > > > > > > > > I don't understand why we could not assign a port id
> > > > 

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-27 Thread Bruce Richardson
On Tue, Sep 27, 2016 at 11:31:06AM +0100, Iremonger, Bernard wrote:
> Hi Thomas, Bruce,
> 
> 
> 
> > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> > management
> > 
> > 2016-09-26 15:37, Iremonger, Bernard:
> > > Hi Thomas, Bruce,
> > >
> > > 
> > >
> > > > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's
> > > > for VF management
> > > >
> > > > 2016-09-23 17:02, Iremonger, Bernard:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > 2016-09-23 09:53, Richardson, Bruce:
> > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon
> > wrote:
> > > > > > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > > > > > Do we really need to expose VF specific functions
> > here?
> > > > > > > > > > > > > > It can be generic(PF/VF) function indexed only
> > > > > > > > > > > > > > through
> > > > > > > > port_id.
> > > > > > > > > > > > > > (example: as
> > > > > > > > > > > > > > rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > > > > > port_id, uint8_t on)) For instance, In Thunderx
> > > > > > > > > > > > > > PMD, We are not exposing a separate port_id for
> > > > > > > > > > > > > > PF. We only enumerate 0..N VFs as 0..N ethdev
> > > > > > > > > > > > > > port_id
> > > > > > > > > > > > >
> > > > > > > > > > > > > Our intention with this patch is to control the VF 
> > > > > > > > > > > > > from the
> > PF.
> > > > > > > > > > > > >
> > > > > > > > > > > > > The following librte_ether functions already work
> > > > > > > > > > > > > in a similar
> > > > > > > > way:
> > > > > > > > > > > > >
> > > > > > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,
> > > > > > > > > > > > > uint16_t vf, uint16_t rx_mode, uint8_t on)
> > > > > > > > > > > > >
> > > > > > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t
> > > > > > > > > > > > > vf, uint8_t
> > > > > > > > > > > > > on)
> > > > > > > > > > > > >
> > > > > > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t
> > > > > > > > > > > > > vf, uint8_t
> > > > > > > > > > > > > on)
> > > > > > > > > > > > >
> > > > > > > > > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id,
> > > > > > > > > > > > > uint16_t vf, uint16_t tx_rate, uint64_t q_msk)
> > > > > > > > > > > >
> > > > > > > > > > > > I have a bad feeling with these functions dedicated
> > > > > > > > > > > > to VF from
> > > > PF.
> > > > > > > > > > > > Are we sure there is no other way?
> > > > > > > > > > > > I mean we just need to know the VF with a port ID.
> > > > > > > > > > >
> > > > > > > > > > > When the VF is used in a VM the port ID of the VF is
> > > > > > > > > > > not visible to
> > > > > > > > the PF.
> > > > > > > > > > > I don't think there is another way to do this.
> > > > > > > > > >
> > > > > > > > > > I don't understand why we could not assign a port id to
> > > > > > > > > > the VF from the host instead of h

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-27 Thread Iremonger, Bernard
Hi Thomas, Bruce,



> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> 2016-09-26 15:37, Iremonger, Bernard:
> > Hi Thomas, Bruce,
> >
> > 
> >
> > > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's
> > > for VF management
> > >
> > > 2016-09-23 17:02, Iremonger, Bernard:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2016-09-23 09:53, Richardson, Bruce:
> > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon
> wrote:
> > > > > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > > > > Do we really need to expose VF specific functions
> here?
> > > > > > > > > > > > > It can be generic(PF/VF) function indexed only
> > > > > > > > > > > > > through
> > > > > > > port_id.
> > > > > > > > > > > > > (example: as
> > > > > > > > > > > > > rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > > > > port_id, uint8_t on)) For instance, In Thunderx
> > > > > > > > > > > > > PMD, We are not exposing a separate port_id for
> > > > > > > > > > > > > PF. We only enumerate 0..N VFs as 0..N ethdev
> > > > > > > > > > > > > port_id
> > > > > > > > > > > >
> > > > > > > > > > > > Our intention with this patch is to control the VF from 
> > > > > > > > > > > > the
> PF.
> > > > > > > > > > > >
> > > > > > > > > > > > The following librte_ether functions already work
> > > > > > > > > > > > in a similar
> > > > > > > way:
> > > > > > > > > > > >
> > > > > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,
> > > > > > > > > > > > uint16_t vf, uint16_t rx_mode, uint8_t on)
> > > > > > > > > > > >
> > > > > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t
> > > > > > > > > > > > vf, uint8_t
> > > > > > > > > > > > on)
> > > > > > > > > > > >
> > > > > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t
> > > > > > > > > > > > vf, uint8_t
> > > > > > > > > > > > on)
> > > > > > > > > > > >
> > > > > > > > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id,
> > > > > > > > > > > > uint16_t vf, uint16_t tx_rate, uint64_t q_msk)
> > > > > > > > > > >
> > > > > > > > > > > I have a bad feeling with these functions dedicated
> > > > > > > > > > > to VF from
> > > PF.
> > > > > > > > > > > Are we sure there is no other way?
> > > > > > > > > > > I mean we just need to know the VF with a port ID.
> > > > > > > > > >
> > > > > > > > > > When the VF is used in a VM the port ID of the VF is
> > > > > > > > > > not visible to
> > > > > > > the PF.
> > > > > > > > > > I don't think there is another way to do this.
> > > > > > > > >
> > > > > > > > > I don't understand why we could not assign a port id to
> > > > > > > > > the VF from the host instead of having the couple PF port id /
> VF id.
> > > > > > > > > Can we enumerate all the VFs associated to a PF?
> > > > > > > > > Then can we allocate them a port id in the array
> rte_eth_devices?
> > > > > > > >
> > > > > > > > Hi Thomas,
> > > > > > > >
> > > > > > > &

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-26 Thread Thomas Monjalon
2016-09-26 15:37, Iremonger, Bernard:
> Hi Thomas, Bruce,
> 
> 
> 
> > Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> > management
> > 
> > 2016-09-23 17:02, Iremonger, Bernard:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2016-09-23 09:53, Richardson, Bruce:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> > > > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > > > Do we really need to expose VF specific functions here?
> > > > > > > > > > > > It can be generic(PF/VF) function indexed only
> > > > > > > > > > > > through
> > > > > > port_id.
> > > > > > > > > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > > > port_id, uint8_t on)) For instance, In Thunderx PMD,
> > > > > > > > > > > > We are not exposing a separate port_id for PF. We
> > > > > > > > > > > > only enumerate 0..N VFs as 0..N ethdev port_id
> > > > > > > > > > >
> > > > > > > > > > > Our intention with this patch is to control the VF from 
> > > > > > > > > > > the PF.
> > > > > > > > > > >
> > > > > > > > > > > The following librte_ether functions already work in a
> > > > > > > > > > > similar
> > > > > > way:
> > > > > > > > > > >
> > > > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t
> > > > > > > > > > > vf, uint16_t rx_mode, uint8_t on)
> > > > > > > > > > >
> > > > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf,
> > > > > > > > > > > uint8_t
> > > > > > > > > > > on)
> > > > > > > > > > >
> > > > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf,
> > > > > > > > > > > uint8_t
> > > > > > > > > > > on)
> > > > > > > > > > >
> > > > > > > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id,
> > > > > > > > > > > uint16_t vf, uint16_t tx_rate, uint64_t q_msk)
> > > > > > > > > >
> > > > > > > > > > I have a bad feeling with these functions dedicated to VF 
> > > > > > > > > > from
> > PF.
> > > > > > > > > > Are we sure there is no other way?
> > > > > > > > > > I mean we just need to know the VF with a port ID.
> > > > > > > > >
> > > > > > > > > When the VF is used in a VM the port ID of the VF is not
> > > > > > > > > visible to
> > > > > > the PF.
> > > > > > > > > I don't think there is another way to do this.
> > > > > > > >
> > > > > > > > I don't understand why we could not assign a port id to the
> > > > > > > > VF from the host instead of having the couple PF port id / VF 
> > > > > > > > id.
> > > > > > > > Can we enumerate all the VFs associated to a PF?
> > > > > > > > Then can we allocate them a port id in the array 
> > > > > > > > rte_eth_devices?
> > > > > > >
> > > > > > > Hi Thomas,
> > > > > > >
> > > > > > > The VF is not a port visible to DPDK, though, so it shouldn't
> > > > > > > have a port id IMHO. DPDK can't actually do anything with it.
> > > > > >
> > > > > > You say the contrary below.
> > > > >
> > > > > Well, yes and no. The driver can manipulate things for the VF, but
> > > > > DPDK
> > > > doesn't actually have a device that corresponds to the VF. There are
> > &

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-26 Thread Iremonger, Bernard
Hi Thomas, Bruce,



> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> 2016-09-23 17:02, Iremonger, Bernard:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-09-23 09:53, Richardson, Bruce:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> > > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > > Do we really need to expose VF specific functions here?
> > > > > > > > > > > It can be generic(PF/VF) function indexed only
> > > > > > > > > > > through
> > > > > port_id.
> > > > > > > > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > > port_id, uint8_t on)) For instance, In Thunderx PMD,
> > > > > > > > > > > We are not exposing a separate port_id for PF. We
> > > > > > > > > > > only enumerate 0..N VFs as 0..N ethdev port_id
> > > > > > > > > >
> > > > > > > > > > Our intention with this patch is to control the VF from the 
> > > > > > > > > > PF.
> > > > > > > > > >
> > > > > > > > > > The following librte_ether functions already work in a
> > > > > > > > > > similar
> > > > > way:
> > > > > > > > > >
> > > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t
> > > > > > > > > > vf, uint16_t rx_mode, uint8_t on)
> > > > > > > > > >
> > > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf,
> > > > > > > > > > uint8_t
> > > > > > > > > > on)
> > > > > > > > > >
> > > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf,
> > > > > > > > > > uint8_t
> > > > > > > > > > on)
> > > > > > > > > >
> > > > > > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id,
> > > > > > > > > > uint16_t vf, uint16_t tx_rate, uint64_t q_msk)
> > > > > > > > >
> > > > > > > > > I have a bad feeling with these functions dedicated to VF from
> PF.
> > > > > > > > > Are we sure there is no other way?
> > > > > > > > > I mean we just need to know the VF with a port ID.
> > > > > > > >
> > > > > > > > When the VF is used in a VM the port ID of the VF is not
> > > > > > > > visible to
> > > > > the PF.
> > > > > > > > I don't think there is another way to do this.
> > > > > > >
> > > > > > > I don't understand why we could not assign a port id to the
> > > > > > > VF from the host instead of having the couple PF port id / VF id.
> > > > > > > Can we enumerate all the VFs associated to a PF?
> > > > > > > Then can we allocate them a port id in the array rte_eth_devices?
> > > > > >
> > > > > > Hi Thomas,
> > > > > >
> > > > > > The VF is not a port visible to DPDK, though, so it shouldn't
> > > > > > have a port id IMHO. DPDK can't actually do anything with it.
> > > > >
> > > > > You say the contrary below.
> > > >
> > > > Well, yes and no. The driver can manipulate things for the VF, but
> > > > DPDK
> > > doesn't actually have a device that corresponds to the VF. There are
> > > no PCI bar mappings for it, DPDK can't do RX and TX with it etc.?
> > >
> > > Very good point.
> > > There are only few ethdev functions which are supported by every
> > > drivers, like Rx/Tx and would not be available for VF from PF interface.
> > >
> > > > > > The PCI device for the VF is likely passed through to a
> > > > > > different VM and being used there. Unfortunately, the VF still
> > > > > > needs 

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-23 Thread Thomas Monjalon
2016-09-23 17:02, Iremonger, Bernard:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2016-09-23 09:53, Richardson, Bruce:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2016-09-23 10:20, Bruce Richardson:
> > > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> > > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > > Do we really need to expose VF specific functions here?
> > > > > > > > > > It can be generic(PF/VF) function indexed only through
> > > > port_id.
> > > > > > > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > > port_id, uint8_t on)) For instance, In Thunderx PMD, We
> > > > > > > > > > are not exposing a separate port_id for PF. We only
> > > > > > > > > > enumerate 0..N VFs as 0..N ethdev port_id
> > > > > > > > >
> > > > > > > > > Our intention with this patch is to control the VF from the 
> > > > > > > > > PF.
> > > > > > > > >
> > > > > > > > > The following librte_ether functions already work in a
> > > > > > > > > similar
> > > > way:
> > > > > > > > >
> > > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
> > > > > > > > > uint16_t rx_mode, uint8_t on)
> > > > > > > > >
> > > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf,
> > > > > > > > > uint8_t
> > > > > > > > > on)
> > > > > > > > >
> > > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf,
> > > > > > > > > uint8_t
> > > > > > > > > on)
> > > > > > > > >
> > > > > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t
> > > > > > > > > vf, uint16_t tx_rate, uint64_t q_msk)
> > > > > > > >
> > > > > > > > I have a bad feeling with these functions dedicated to VF from 
> > > > > > > > PF.
> > > > > > > > Are we sure there is no other way?
> > > > > > > > I mean we just need to know the VF with a port ID.
> > > > > > >
> > > > > > > When the VF is used in a VM the port ID of the VF is not
> > > > > > > visible to
> > > > the PF.
> > > > > > > I don't think there is another way to do this.
> > > > > >
> > > > > > I don't understand why we could not assign a port id to the VF
> > > > > > from the host instead of having the couple PF port id / VF id.
> > > > > > Can we enumerate all the VFs associated to a PF?
> > > > > > Then can we allocate them a port id in the array rte_eth_devices?
> > > > >
> > > > > Hi Thomas,
> > > > >
> > > > > The VF is not a port visible to DPDK, though, so it shouldn't have
> > > > > a port id IMHO. DPDK can't actually do anything with it.
> > > >
> > > > You say the contrary below.
> > >
> > > Well, yes and no. The driver can manipulate things for the VF, but DPDK
> > doesn't actually have a device that corresponds to the VF. There are no PCI
> > bar mappings for it, DPDK can't do RX and TX with it etc.?
> > 
> > Very good point.
> > There are only few ethdev functions which are supported by every drivers,
> > like Rx/Tx and would not be available for VF from PF interface.
> > 
> > > > > The PCI device for the VF is likely passed through to a different
> > > > > VM and being used there. Unfortunately, the VF still needs certain
> > > > > things done for it by the PF, so if the PF is under DPDK control,
> > > > > it needs to provide the functionality to assist the VF.
> > > >
> > > > Why not have a VF_from_PF driver which does the mailbox things?
> > > > So you can manage the VF from the PF with a simple port id.
> > > > It really seems to be the cleanest design to me.
> > >
> > > While I see your point, and it could work, I just want to be sure that we 
> > > are
> > ok with the results of that. Suppose we do create ethdevs for the VFs
> > controlled by the PF. Does the new VF get counted in the
> > rte_eth_dev_count() value (I assume yes)? How are apps meant to use the
> > port? Do they have to put in a special case when iterating through all the 
> > port
> > ids to check that it's not a pseudo port that can't do anything. None of the
> > standard ethdev calls from an app will work on it, you can't configure nb 
> > rx/tx
> > queues on it, you can't start or stop it, you can't do rx or tx on it, etc, 
> > etc.
> > 
> > Yes these devices would be special because their supported API would be
> > quite different. I was thinking that in the future you could add most of the
> > configuration functions through the VF mailbox.
> > But the Intel mailbox currently support only some special configurations
> > which are not supported by other devices even its own VF device (except
> > setting MAC address).
> > And when I read "set drop enable bit in the VF split rx control register", 
> > it
> > becomes clear it is really specific and has nothing to do in the generic 
> > ethdev
> > API.
> > That's why it is a NACK.
> > 
> > When we want to use these very specific features we are aware of the
> > underlying device and driver. So we can directly include a header from the
> > driver. I suggest to retrieve a handler for the device which is not a port 
> > id

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-23 Thread Iremonger, Bernard
Hi Thoms

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, September 23, 2016 2:15 PM
> To: Richardson, Bruce ; Iremonger, Bernard
> 
> Cc: dev at dpdk.org; Jerin Jacob ; Shah,
> Rahul R ; Lu, Wenzhuo ;
> azelezniak 
> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> 2016-09-23 09:53, Richardson, Bruce:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-09-23 10:20, Bruce Richardson:
> > > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> > > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > > Do we really need to expose VF specific functions here?
> > > > > > > > > It can be generic(PF/VF) function indexed only through
> > > port_id.
> > > > > > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > > port_id, uint8_t on)) For instance, In Thunderx PMD, We
> > > > > > > > > are not exposing a separate port_id for PF. We only
> > > > > > > > > enumerate 0..N VFs as 0..N ethdev port_id
> > > > > > > >
> > > > > > > > Our intention with this patch is to control the VF from the PF.
> > > > > > > >
> > > > > > > > The following librte_ether functions already work in a
> > > > > > > > similar
> > > way:
> > > > > > > >
> > > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
> > > > > > > > uint16_t rx_mode, uint8_t on)
> > > > > > > >
> > > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf,
> > > > > > > > uint8_t
> > > > > > > > on)
> > > > > > > >
> > > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf,
> > > > > > > > uint8_t
> > > > > > > > on)
> > > > > > > >
> > > > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t
> > > > > > > > vf, uint16_t tx_rate, uint64_t q_msk)
> > > > > > >
> > > > > > > I have a bad feeling with these functions dedicated to VF from PF.
> > > > > > > Are we sure there is no other way?
> > > > > > > I mean we just need to know the VF with a port ID.
> > > > > >
> > > > > > When the VF is used in a VM the port ID of the VF is not
> > > > > > visible to
> > > the PF.
> > > > > > I don't think there is another way to do this.
> > > > >
> > > > > I don't understand why we could not assign a port id to the VF
> > > > > from the host instead of having the couple PF port id / VF id.
> > > > > Can we enumerate all the VFs associated to a PF?
> > > > > Then can we allocate them a port id in the array rte_eth_devices?
> > > >
> > > > Hi Thomas,
> > > >
> > > > The VF is not a port visible to DPDK, though, so it shouldn't have
> > > > a port id IMHO. DPDK can't actually do anything with it.
> > >
> > > You say the contrary below.
> >
> > Well, yes and no. The driver can manipulate things for the VF, but DPDK
> doesn't actually have a device that corresponds to the VF. There are no PCI
> bar mappings for it, DPDK can't do RX and TX with it etc.?
> 
> Very good point.
> There are only few ethdev functions which are supported by every drivers,
> like Rx/Tx and would not be available for VF from PF interface.
> 
> > > > The PCI device for the VF is likely passed through to a different
> > > > VM and being used there. Unfortunately, the VF still needs certain
> > > > things done for it by the PF, so if the PF is under DPDK control,
> > > > it needs to provide the functionality to assist the VF.
> > >
> > > Why not have a VF_from_PF driver which does the mailbox things?
> > > So you can manage the VF from the PF with a simple port id.
> > > It really seems to be the cleanest design to me.
> >
> > While I see your point, and it could work, I just want to be sure that we 
> > are
> ok with the results of that. Suppose we do create ethdevs for the VFs
> controlled by the PF. Does the new V

[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-23 Thread Thomas Monjalon
2016-09-23 09:53, Richardson, Bruce:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2016-09-23 10:20, Bruce Richardson:
> > > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> > > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > > Do we really need to expose VF specific functions here?
> > > > > > > > It can be generic(PF/VF) function indexed only through
> > port_id.
> > > > > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > > port_id, uint8_t on)) For instance, In Thunderx PMD, We are
> > > > > > > > not exposing a separate port_id for PF. We only enumerate
> > > > > > > > 0..N VFs as 0..N ethdev port_id
> > > > > > >
> > > > > > > Our intention with this patch is to control the VF from the PF.
> > > > > > >
> > > > > > > The following librte_ether functions already work in a similar
> > way:
> > > > > > >
> > > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
> > > > > > > uint16_t rx_mode, uint8_t on)
> > > > > > >
> > > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t
> > > > > > > on)
> > > > > > >
> > > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t
> > > > > > > on)
> > > > > > >
> > > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf,
> > > > > > > uint16_t tx_rate, uint64_t q_msk)
> > > > > >
> > > > > > I have a bad feeling with these functions dedicated to VF from PF.
> > > > > > Are we sure there is no other way?
> > > > > > I mean we just need to know the VF with a port ID.
> > > > >
> > > > > When the VF is used in a VM the port ID of the VF is not visible to
> > the PF.
> > > > > I don't think there is another way to do this.
> > > >
> > > > I don't understand why we could not assign a port id to the VF from
> > > > the host instead of having the couple PF port id / VF id.
> > > > Can we enumerate all the VFs associated to a PF?
> > > > Then can we allocate them a port id in the array rte_eth_devices?
> > >
> > > Hi Thomas,
> > >
> > > The VF is not a port visible to DPDK, though, so it shouldn't have a
> > > port id IMHO. DPDK can't actually do anything with it.
> > 
> > You say the contrary below.
> 
> Well, yes and no. The driver can manipulate things for the VF, but DPDK 
> doesn't actually have a device that corresponds to the VF. There are no PCI 
> bar mappings for it, DPDK can't do RX and TX with it etc.?

Very good point.
There are only few ethdev functions which are supported by every drivers,
like Rx/Tx and would not be available for VF from PF interface.

> > > The PCI device for the VF is likely passed through to a different VM
> > > and being used there. Unfortunately, the VF still needs certain things
> > > done for it by the PF, so if the PF is under DPDK control, it needs to
> > > provide the functionality to assist the VF.
> > 
> > Why not have a VF_from_PF driver which does the mailbox things?
> > So you can manage the VF from the PF with a simple port id.
> > It really seems to be the cleanest design to me.
> 
> While I see your point, and it could work, I just want to be sure that we are 
> ok with the results of that. Suppose we do create ethdevs for the VFs 
> controlled by the PF. Does the new VF get counted in the rte_eth_dev_count() 
> value (I assume yes)? How are apps meant to use the port? Do they have to put 
> in a special case when iterating through all the port ids to check that it's 
> not a pseudo port that can't do anything. None of the standard ethdev calls 
> from an app will work on it, you can't configure nb rx/tx queues on it, you 
> can't start or stop it, you can't do rx or tx on it, etc, etc.

Yes these devices would be special because their supported API would be
quite different. I was thinking that in the future you could add most
of the configuration functions through the VF mailbox.
But the Intel mailbox currently support only some special configurations
which are not supported by other devices even its own VF device (except
setting MAC address).
And when I read "set drop enable bit in the VF split rx control register",
it becomes clear it is really specific and has nothing to do in the
generic ethdev API.
That's why it is a NACK.

When we want to use these very specific features we are aware of the
underlying device and driver. So we can directly include a header from
the driver. I suggest to retrieve a handler for the device which is not
a port id and will allow to call ixgbe functions directly.
It could be achieved by adding an ethdev function like discussed here:
http://dpdk.org/ml/archives/dev/2016-September/047392.html




[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-23 Thread Thomas Monjalon
2016-09-23 10:20, Bruce Richardson:
> On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > Do we really need to expose VF specific functions here?
> > > > > > It can be generic(PF/VF) function indexed only through port_id.
> > > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t port_id,
> > > > > > uint8_t on)) For instance, In Thunderx PMD, We are not exposing a
> > > > > > separate port_id for PF. We only enumerate 0..N VFs as 0..N ethdev
> > > > > > port_id
> > > > >
> > > > > Our intention with this patch is to control the VF from the PF.
> > > > >
> > > > > The following librte_ether functions already work in a similar way:
> > > > >
> > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf, uint16_t
> > > > > rx_mode, uint8_t on)
> > > > >
> > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t on)
> > > > >
> > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t on)
> > > > >
> > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t
> > > > > tx_rate, uint64_t q_msk)
> > > > 
> > > > I have a bad feeling with these functions dedicated to VF from PF.
> > > > Are we sure there is no other way?
> > > > I mean we just need to know the VF with a port ID.
> > > 
> > > When the VF is used in a VM the port ID of the VF is not visible to the 
> > > PF.
> > > I don't think there is another way to do this.
> > 
> > I don't understand why we could not assign a port id to the VF from the
> > host instead of having the couple PF port id / VF id.
> > Can we enumerate all the VFs associated to a PF?
> > Then can we allocate them a port id in the array rte_eth_devices?
> 
> Hi Thomas,
> 
> The VF is not a port visible to DPDK, though, so it shouldn't have a port id
> IMHO. DPDK can't actually do anything with it.

You say the contrary below.

> The PCI device for the VF is likely passed through to a different VM and being
> used there. Unfortunately, the VF still needs certain things done for it by 
> the
> PF, so if the PF is under DPDK control, it needs to provide the functionality
> to assist the VF.

Why not have a VF_from_PF driver which does the mailbox things?
So you can manage the VF from the PF with a simple port id.
It really seems to be the cleanest design to me.


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-23 Thread Bruce Richardson
On Fri, Sep 23, 2016 at 11:36:21AM +0200, Thomas Monjalon wrote:
> 2016-09-23 10:20, Bruce Richardson:
> > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > Do we really need to expose VF specific functions here?
> > > > > > > It can be generic(PF/VF) function indexed only through port_id.
> > > > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t port_id,
> > > > > > > uint8_t on)) For instance, In Thunderx PMD, We are not exposing a
> > > > > > > separate port_id for PF. We only enumerate 0..N VFs as 0..N ethdev
> > > > > > > port_id
> > > > > >
> > > > > > Our intention with this patch is to control the VF from the PF.
> > > > > >
> > > > > > The following librte_ether functions already work in a similar way:
> > > > > >
> > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf, uint16_t
> > > > > > rx_mode, uint8_t on)
> > > > > >
> > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t on)
> > > > > >
> > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t on)
> > > > > >
> > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t
> > > > > > tx_rate, uint64_t q_msk)
> > > > > 
> > > > > I have a bad feeling with these functions dedicated to VF from PF.
> > > > > Are we sure there is no other way?
> > > > > I mean we just need to know the VF with a port ID.
> > > > 
> > > > When the VF is used in a VM the port ID of the VF is not visible to the 
> > > > PF.
> > > > I don't think there is another way to do this.
> > > 
> > > I don't understand why we could not assign a port id to the VF from the
> > > host instead of having the couple PF port id / VF id.
> > > Can we enumerate all the VFs associated to a PF?
> > > Then can we allocate them a port id in the array rte_eth_devices?
> > 
> > Hi Thomas,
> > 
> > The VF is not a port visible to DPDK, though, so it shouldn't have a port id
> > IMHO. DPDK can't actually do anything with it.
> 
> You say the contrary below.
> 
> > The PCI device for the VF is likely passed through to a different VM and 
> > being
> > used there. Unfortunately, the VF still needs certain things done for it by 
> > the
> > PF, so if the PF is under DPDK control, it needs to provide the 
> > functionality
> > to assist the VF.
> 
> Why not have a VF_from_PF driver which does the mailbox things?
> So you can manage the VF from the PF with a simple port id.
> It really seems to be the cleanest design to me.

Just to confirm I am understanding your suggestion correctly:

* for a normal app, eg. testpmd or l2fwd, things stay exactly as they are
* for an app that wants to control VFs from PFs, then we provide a new 
  ethdev driver, and a call in the PF to create an new instance of it 
  e.g. rte_eth_vf_from_pf(pf_port_id, vf_number)
* then to control the VF, you make regular rte_ethdev calls using the new
  ethdev port id you got from the vf_from_pf call.
* it's up to that app to know what ports are regular ports that can do IO,
  and what ports are VF control ports, though we may add in an ethdev flag value
  somewhere to assist in this.

Is that basically it?

/Bruce


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-23 Thread Bruce Richardson
On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> 2016-09-15 16:46, Iremonger, Bernard:
> > > > > Do we really need to expose VF specific functions here?
> > > > > It can be generic(PF/VF) function indexed only through port_id.
> > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t port_id,
> > > > > uint8_t on)) For instance, In Thunderx PMD, We are not exposing a
> > > > > separate port_id for PF. We only enumerate 0..N VFs as 0..N ethdev
> > > > > port_id
> > > >
> > > > Our intention with this patch is to control the VF from the PF.
> > > >
> > > > The following librte_ether functions already work in a similar way:
> > > >
> > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf, uint16_t
> > > > rx_mode, uint8_t on)
> > > >
> > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t on)
> > > >
> > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t on)
> > > >
> > > > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t
> > > > tx_rate, uint64_t q_msk)
> > > 
> > > I have a bad feeling with these functions dedicated to VF from PF.
> > > Are we sure there is no other way?
> > > I mean we just need to know the VF with a port ID.
> > 
> > When the VF is used in a VM the port ID of the VF is not visible to the PF.
> > I don't think there is another way to do this.
> 
> I don't understand why we could not assign a port id to the VF from the
> host instead of having the couple PF port id / VF id.
> Can we enumerate all the VFs associated to a PF?
> Then can we allocate them a port id in the array rte_eth_devices?

Hi Thomas,

The VF is not a port visible to DPDK, though, so it shouldn't have a port id
IMHO. DPDK can't actually do anything with it.

The PCI device for the VF is likely passed through to a different VM and being
used there. Unfortunately, the VF still needs certain things done for it by the
PF, so if the PF is under DPDK control, it needs to provide the functionality
to assist the VF.

/Bruce


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-23 Thread Richardson, Bruce
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, September 23, 2016 10:36 AM
> To: Richardson, Bruce 
> Cc: Iremonger, Bernard ; dev at dpdk.org; 
> Jerin
> Jacob ; Shah, Rahul R
> ; Lu, Wenzhuo ; azelezniak
> 
> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> 2016-09-23 10:20, Bruce Richardson:
> > On Thu, Sep 22, 2016 at 07:04:37PM +0200, Thomas Monjalon wrote:
> > > 2016-09-15 16:46, Iremonger, Bernard:
> > > > > > > Do we really need to expose VF specific functions here?
> > > > > > > It can be generic(PF/VF) function indexed only through
> port_id.
> > > > > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t
> > > > > > > port_id, uint8_t on)) For instance, In Thunderx PMD, We are
> > > > > > > not exposing a separate port_id for PF. We only enumerate
> > > > > > > 0..N VFs as 0..N ethdev port_id
> > > > > >
> > > > > > Our intention with this patch is to control the VF from the PF.
> > > > > >
> > > > > > The following librte_ether functions already work in a similar
> way:
> > > > > >
> > > > > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
> > > > > > uint16_t rx_mode, uint8_t on)
> > > > > >
> > > > > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t
> > > > > > on)
> > > > > >
> > > > > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t
> > > > > > on)
> > > > > >
> > > > > > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf,
> > > > > > uint16_t tx_rate, uint64_t q_msk)
> > > > >
> > > > > I have a bad feeling with these functions dedicated to VF from PF.
> > > > > Are we sure there is no other way?
> > > > > I mean we just need to know the VF with a port ID.
> > > >
> > > > When the VF is used in a VM the port ID of the VF is not visible to
> the PF.
> > > > I don't think there is another way to do this.
> > >
> > > I don't understand why we could not assign a port id to the VF from
> > > the host instead of having the couple PF port id / VF id.
> > > Can we enumerate all the VFs associated to a PF?
> > > Then can we allocate them a port id in the array rte_eth_devices?
> >
> > Hi Thomas,
> >
> > The VF is not a port visible to DPDK, though, so it shouldn't have a
> > port id IMHO. DPDK can't actually do anything with it.
> 
> You say the contrary below.

Well, yes and no. The driver can manipulate things for the VF, but DPDK doesn't 
actually have a device that corresponds to the VF. There are no PCI bar 
mappings for it, DPDK can't do RX and TX with it etc.?
> 
> > The PCI device for the VF is likely passed through to a different VM
> > and being used there. Unfortunately, the VF still needs certain things
> > done for it by the PF, so if the PF is under DPDK control, it needs to
> > provide the functionality to assist the VF.
> 
> Why not have a VF_from_PF driver which does the mailbox things?
> So you can manage the VF from the PF with a simple port id.
> It really seems to be the cleanest design to me.

While I see your point, and it could work, I just want to be sure that we are 
ok with the results of that. Suppose we do create ethdevs for the VFs 
controlled by the PF. Does the new VF get counted in the rte_eth_dev_count() 
value (I assume yes)? How are apps meant to use the port? Do they have to put 
in a special case when iterating through all the port ids to check that it's 
not a pseudo port that can't do anything. None of the standard ethdev calls 
from an app will work on it, you can't configure nb rx/tx queues on it, you 
can't start or stop it, you can't do rx or tx on it, etc, etc.

/Bruce


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-22 Thread Thomas Monjalon
2016-09-15 16:46, Iremonger, Bernard:
> > > > Do we really need to expose VF specific functions here?
> > > > It can be generic(PF/VF) function indexed only through port_id.
> > > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t port_id,
> > > > uint8_t on)) For instance, In Thunderx PMD, We are not exposing a
> > > > separate port_id for PF. We only enumerate 0..N VFs as 0..N ethdev
> > > > port_id
> > >
> > > Our intention with this patch is to control the VF from the PF.
> > >
> > > The following librte_ether functions already work in a similar way:
> > >
> > > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf, uint16_t
> > > rx_mode, uint8_t on)
> > >
> > > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t on)
> > >
> > > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t on)
> > >
> > > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t
> > > tx_rate, uint64_t q_msk)
> > 
> > I have a bad feeling with these functions dedicated to VF from PF.
> > Are we sure there is no other way?
> > I mean we just need to know the VF with a port ID.
> 
> When the VF is used in a VM the port ID of the VF is not visible to the PF.
> I don't think there is another way to do this.

I don't understand why we could not assign a port id to the VF from the
host instead of having the couple PF port id / VF id.
Can we enumerate all the VFs associated to a PF?
Then can we allocate them a port id in the array rte_eth_devices?


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-15 Thread Iremonger, Bernard
Hi Thomas,



> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> 2016-09-12 16:28, Iremonger, Bernard:
> > > On Fri, Aug 26, 2016 at 10:10:18AM +0100, Bernard Iremonger wrote:
> > > > Add new API functions to configure and manage VF's on a NIC.
> > > >
> > > > add rte_eth_dev_vf_ping function.
> > > > add rte_eth_dev_set_vf_vlan_anti_spoof function.
> > > > add rte_eth_dev_set_vf_mac_anti_spoof function.
> > > >
> > > > Signed-off-by: azelezniak 
> > > >
> > > > add rte_eth_dev_set_vf_vlan_strip function.
> > > > add rte_eth_dev_set_vf_vlan_insert function.
> > > > add rte_eth_dev_set_loopback function.
> > > > add rte_eth_dev_set_all_queues_drop function.
> > > > add rte_eth_dev_set_vf_split_drop_en function add
> > > > rte_eth_dev_set_vf_mac_addr function.
> > >
> > > Do we really need to expose VF specific functions here?
> > > It can be generic(PF/VF) function indexed only through port_id.
> > > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t port_id,
> > > uint8_t on)) For instance, In Thunderx PMD, We are not exposing a
> > > separate port_id for PF. We only enumerate 0..N VFs as 0..N ethdev
> > > port_id
> >
> > Our intention with this patch is to control the VF from the PF.
> >
> > The following librte_ether functions already work in a similar way:
> >
> > rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf, uint16_t
> > rx_mode, uint8_t on)
> >
> > rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t on)
> >
> > rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t on)
> >
> > int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t
> > tx_rate, uint64_t q_msk)
> 
> I have a bad feeling with these functions dedicated to VF from PF.
> Are we sure there is no other way?
> I mean we just need to know the VF with a port ID.

When the VF is used in a VM the port ID of the VF is not visible to the PF.
I don't think there is another way to do this.

Regards,

Bernard.



[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-13 Thread Thomas Monjalon
2016-09-12 16:28, Iremonger, Bernard:
> > On Fri, Aug 26, 2016 at 10:10:18AM +0100, Bernard Iremonger wrote:
> > > Add new API functions to configure and manage VF's on a NIC.
> > >
> > > add rte_eth_dev_vf_ping function.
> > > add rte_eth_dev_set_vf_vlan_anti_spoof function.
> > > add rte_eth_dev_set_vf_mac_anti_spoof function.
> > >
> > > Signed-off-by: azelezniak 
> > >
> > > add rte_eth_dev_set_vf_vlan_strip function.
> > > add rte_eth_dev_set_vf_vlan_insert function.
> > > add rte_eth_dev_set_loopback function.
> > > add rte_eth_dev_set_all_queues_drop function.
> > > add rte_eth_dev_set_vf_split_drop_en function add
> > > rte_eth_dev_set_vf_mac_addr function.
> > 
> > Do we really need to expose VF specific functions here?
> > It can be generic(PF/VF) function indexed only through port_id.
> > (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t port_id, uint8_t on))
> > For instance, In Thunderx PMD, We are not exposing a separate port_id for
> > PF. We only enumerate 0..N VFs as 0..N ethdev port_id
> 
> Our intention with this patch is to control the VF from the PF.
> 
> The following librte_ether functions already work in a similar way:
> 
> rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf, uint16_t rx_mode, 
> uint8_t on)
> 
> rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t on)
> 
> rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t on)
> 
> int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t tx_rate, 
> uint64_t q_msk)

I have a bad feeling with these functions dedicated to VF from PF.
Are we sure there is no other way?
I mean we just need to know the VF with a port ID.


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-12 Thread Iremonger, Bernard
Hi Jerin,



> Subject: Re: [dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF
> management
> 
> On Fri, Aug 26, 2016 at 10:10:18AM +0100, Bernard Iremonger wrote:
> > Add new API functions to configure and manage VF's on a NIC.
> >
> > add rte_eth_dev_vf_ping function.
> > add rte_eth_dev_set_vf_vlan_anti_spoof function.
> > add rte_eth_dev_set_vf_mac_anti_spoof function.
> >
> > Signed-off-by: azelezniak 
> >
> > add rte_eth_dev_set_vf_vlan_strip function.
> > add rte_eth_dev_set_vf_vlan_insert function.
> > add rte_eth_dev_set_loopback function.
> > add rte_eth_dev_set_all_queues_drop function.
> > add rte_eth_dev_set_vf_split_drop_en function add
> > rte_eth_dev_set_vf_mac_addr function.
> 
> Do we really need to expose VF specific functions here?
> It can be generic(PF/VF) function indexed only through port_id.
> (example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t port_id, uint8_t on))
> For instance, In Thunderx PMD, We are not exposing a separate port_id for
> PF. We only enumerate 0..N VFs as 0..N ethdev port_id

Our intention with this patch is to control the VF from the PF.

The following librte_ether functions already work in a similar way:

rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf, uint16_t rx_mode, 
uint8_t on)

rte_eth_dev_set_vf_rx(uint8_t port_id, uint16_t vf, uint8_t on)

rte_eth_dev_set_vf_tx(uint8_t port_id, uint16_t vf, uint8_t on)

int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t tx_rate, 
uint64_t q_msk)

> 
> > increment LIBABIVER to 5.
> >
> > Signed-off-by: Bernard Iremonger 
> > ---
> >  lib/librte_ether/rte_ethdev.c  | 159 +++
> >  lib/librte_ether/rte_ethdev.h  | 223
> +
> >  lib/librte_ether/rte_ether_version.map |   9 ++
> >  3 files changed, 391 insertions(+)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.c
> > b/lib/librte_ether/rte_ethdev.c index 1388ea3..2a3d2ae 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -2306,6 +2306,22 @@ rte_eth_dev_default_mac_addr_set(uint8_t
> > port_id, struct ether_addr *addr)  }
> >
> >  int
> > +rte_eth_dev_set_vf_mac_addr(uint8_t port_id, uint16_t vf, struct
> > +ether_addr *addr) {
> > +   struct rte_eth_dev *dev;
> > +
> > +   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +
> > +   if (!is_valid_assigned_ether_addr(addr))
> > +   return -EINVAL;
> > +
> > +   dev = &rte_eth_devices[port_id];
> > +   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_mac_addr, -
> ENOTSUP);
> > +
> > +   return (*dev->dev_ops->set_vf_mac_addr)(dev, vf, addr); }
> > +
> > +int
> >  rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
> > uint16_t rx_mode, uint8_t on)
> >  {
> > @@ -2490,6 +2506,149 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t
> port_id, uint16_t vlan_id,
> >vf_mask, vlan_on);
> >  }
> >
> > +int
> > +rte_eth_dev_set_vf_vlan_anti_spoof(uint8_t port_id,
> > +  uint16_t vf, uint8_t on)
> > +{
> > +   struct rte_eth_dev *dev;
> > +
> > +   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +
> > +   dev = &rte_eth_devices[port_id];
> > +   if (vf > 63) {
> 
> PMD may have more than 64 VFs.

Yes, it would be better to check on max_vfs,  the same way as the already 
implemented functions mentioned above.

> 
> 
> > +   RTE_PMD_DEBUG_TRACE("VF VLAN anti spoof:VF %d >
> 63\n", vf);
> > +   return -EINVAL;
> > +   }
> > +
> > +   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >set_vf_vlan_anti_spoof, -ENOTSUP);
> > +   (*dev->dev_ops->set_vf_vlan_anti_spoof)(dev, vf, on);
> > +   return 0;
> > +}
> > +

Thanks for reviewing.
Regards,

Bernard.



[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-09-09 Thread Jerin Jacob
On Fri, Aug 26, 2016 at 10:10:18AM +0100, Bernard Iremonger wrote:
> Add new API functions to configure and manage VF's on a NIC.
> 
> add rte_eth_dev_vf_ping function.
> add rte_eth_dev_set_vf_vlan_anti_spoof function.
> add rte_eth_dev_set_vf_mac_anti_spoof function.
> 
> Signed-off-by: azelezniak 
> 
> add rte_eth_dev_set_vf_vlan_strip function.
> add rte_eth_dev_set_vf_vlan_insert function.
> add rte_eth_dev_set_loopback function.
> add rte_eth_dev_set_all_queues_drop function.
> add rte_eth_dev_set_vf_split_drop_en function
> add rte_eth_dev_set_vf_mac_addr function.

Do we really need to expose VF specific functions here?
It can be generic(PF/VF) function indexed only through port_id.
(example: as rte_eth_dev_set_vlan_anti_spoof(uint8_t port_id, uint8_t on))
For instance, In Thunderx PMD, We are not exposing a separate port_id
for PF. We only enumerate 0..N VFs as 0..N ethdev port_id

> increment LIBABIVER to 5.
> 
> Signed-off-by: Bernard Iremonger 
> ---
>  lib/librte_ether/rte_ethdev.c  | 159 +++
>  lib/librte_ether/rte_ethdev.h  | 223 
> +
>  lib/librte_ether/rte_ether_version.map |   9 ++
>  3 files changed, 391 insertions(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 1388ea3..2a3d2ae 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -2306,6 +2306,22 @@ rte_eth_dev_default_mac_addr_set(uint8_t port_id, 
> struct ether_addr *addr)
>  }
>  
>  int
> +rte_eth_dev_set_vf_mac_addr(uint8_t port_id, uint16_t vf, struct ether_addr 
> *addr)
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> + if (!is_valid_assigned_ether_addr(addr))
> + return -EINVAL;
> +
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_mac_addr, -ENOTSUP);
> +
> + return (*dev->dev_ops->set_vf_mac_addr)(dev, vf, addr);
> +}
> +
> +int
>  rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
>   uint16_t rx_mode, uint8_t on)
>  {
> @@ -2490,6 +2506,149 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port_id, 
> uint16_t vlan_id,
>  vf_mask, vlan_on);
>  }
>  
> +int
> +rte_eth_dev_set_vf_vlan_anti_spoof(uint8_t port_id,
> +uint16_t vf, uint8_t on)
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> + dev = &rte_eth_devices[port_id];
> + if (vf > 63) {

PMD may have more than 64 VFs.


> + RTE_PMD_DEBUG_TRACE("VF VLAN anti spoof:VF %d > 63\n", vf);
> + return -EINVAL;
> + }
> +
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_vlan_anti_spoof, 
> -ENOTSUP);
> + (*dev->dev_ops->set_vf_vlan_anti_spoof)(dev, vf, on);
> + return 0;
> +}
> +


[dpdk-dev] [RFC PATCH v2 3/5] librte_ether: add API's for VF management

2016-08-26 Thread Bernard Iremonger
Add new API functions to configure and manage VF's on a NIC.

add rte_eth_dev_vf_ping function.
add rte_eth_dev_set_vf_vlan_anti_spoof function.
add rte_eth_dev_set_vf_mac_anti_spoof function.

Signed-off-by: azelezniak 

add rte_eth_dev_set_vf_vlan_strip function.
add rte_eth_dev_set_vf_vlan_insert function.
add rte_eth_dev_set_loopback function.
add rte_eth_dev_set_all_queues_drop function.
add rte_eth_dev_set_vf_split_drop_en function
add rte_eth_dev_set_vf_mac_addr function.
increment LIBABIVER to 5.

Signed-off-by: Bernard Iremonger 
---
 lib/librte_ether/rte_ethdev.c  | 159 +++
 lib/librte_ether/rte_ethdev.h  | 223 +
 lib/librte_ether/rte_ether_version.map |   9 ++
 3 files changed, 391 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 1388ea3..2a3d2ae 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2306,6 +2306,22 @@ rte_eth_dev_default_mac_addr_set(uint8_t port_id, struct 
ether_addr *addr)
 }

 int
+rte_eth_dev_set_vf_mac_addr(uint8_t port_id, uint16_t vf, struct ether_addr 
*addr)
+{
+   struct rte_eth_dev *dev;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+   if (!is_valid_assigned_ether_addr(addr))
+   return -EINVAL;
+
+   dev = &rte_eth_devices[port_id];
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_mac_addr, -ENOTSUP);
+
+   return (*dev->dev_ops->set_vf_mac_addr)(dev, vf, addr);
+}
+
+int
 rte_eth_dev_set_vf_rxmode(uint8_t port_id,  uint16_t vf,
uint16_t rx_mode, uint8_t on)
 {
@@ -2490,6 +2506,149 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port_id, 
uint16_t vlan_id,
   vf_mask, vlan_on);
 }

+int
+rte_eth_dev_set_vf_vlan_anti_spoof(uint8_t port_id,
+  uint16_t vf, uint8_t on)
+{
+   struct rte_eth_dev *dev;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+   dev = &rte_eth_devices[port_id];
+   if (vf > 63) {
+   RTE_PMD_DEBUG_TRACE("VF VLAN anti spoof:VF %d > 63\n", vf);
+   return -EINVAL;
+   }
+
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_vlan_anti_spoof, 
-ENOTSUP);
+   (*dev->dev_ops->set_vf_vlan_anti_spoof)(dev, vf, on);
+   return 0;
+}
+
+int
+rte_eth_dev_set_vf_mac_anti_spoof(uint8_t port_id,
+  uint16_t vf, uint8_t on)
+{
+   struct rte_eth_dev *dev;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+   dev = &rte_eth_devices[port_id];
+   if (vf > 63) {
+   RTE_PMD_DEBUG_TRACE("VF MAC anti spoof:VF %d > 63\n", vf);
+   return -EINVAL;
+   }
+
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_mac_anti_spoof, -ENOTSUP);
+   (*dev->dev_ops->set_vf_mac_anti_spoof)(dev, vf, on);
+   return 0;
+}
+
+int
+rte_eth_dev_vf_ping(uint8_t port_id, int32_t vf)
+{
+   struct rte_eth_dev *dev;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+   dev = &rte_eth_devices[port_id];
+   if (vf > 63) {
+   RTE_PMD_DEBUG_TRACE("VF ping: VF %d > 64\n", vf);
+   return -EINVAL;
+   }
+
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vf_ping, -ENOTSUP);
+   return (*dev->dev_ops->vf_ping)(dev, vf);
+}
+
+int
+rte_eth_dev_set_vf_vlan_strip(uint8_t port_id, uint16_t vf, int on)
+{
+   struct rte_eth_dev *dev;
+   struct rte_eth_dev_info dev_info;
+   uint16_t queues_per_pool;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+   dev = &rte_eth_devices[port_id];
+   if (vf > 63) {
+   RTE_PMD_DEBUG_TRACE("VF vlan strip set VF %d > 63\n", vf);
+   return -EINVAL;
+   }
+
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_vlan_strip, -ENOTSUP);
+
+   rte_eth_dev_info_get(port_id, &dev_info);
+   queues_per_pool = dev_info.vmdq_queue_num/dev_info.max_vmdq_pools;
+
+   (*dev->dev_ops->set_vf_vlan_strip)(dev, on, queues_per_pool);
+   return 0;
+}
+
+int
+rte_eth_dev_set_vf_vlan_insert(uint8_t port_id, uint16_t vf, int on)
+{
+   struct rte_eth_dev *dev;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+   dev = &rte_eth_devices[port_id];
+   if (vf > 63) {
+   RTE_PMD_DEBUG_TRACE("VF vlan insert set VF %d > 63\n", vf);
+   return -EINVAL;
+   }
+
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_vlan_insert, -ENOTSUP);
+
+   (*dev->dev_ops->set_vf_vlan_insert)(dev, vf, on);
+   return 0;
+}
+
+int
+rte_eth_dev_set_tx_loopback(uint8_t port_id, int on)
+{
+   struct rte_eth_dev *dev;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+   dev = &rte_eth_devices[port_id];
+
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_tx_loopback, -ENOTSUP);
+
+   (*dev->dev_ops->set_tx_loopback)(dev, on);
+   return 0;
+}
+
+int