[dpdk-dev] filter_ctl PMD API idea

[dpdk-dev] filter_ctl PMD API idea

[Wu, Jingjing]

Hi, all

When we develop filters feature in i40e driver for Intel® Ethernet Controller XL710/X710 [Fortville] (For both 10G/40G), we found that there are lots of new filters, there are also some changes on the existing filters, comparing to ixgbe. 
If we keep the way to add new ops in rte_eth_dev for each new filter, it can work. 
But we suggest to use a more generic API for all filters to avoid a superset dev_ops. It needs to be cleaner and easy-to-use. There is a need for technical discussion.

Here is the early design idea we are looking for comments.  

1.   Create two new APIs
-----------------------------------------------------
rte_eth_filter_supported(uint8_t port, uint16_t filter_type);
/* check whether this filter type is supported for the queried port */
rte_eth_filter_ctl(uint8_t port, uint16_t filter_type, uint16_t filter_op, void *arg);
/* configure filters, will call new ops eth_filter_ctl in eth_dev_ops */
-----------------------------------------------------

2.   Define filter types, operations, and structures in new header file lib/librte_eth/rte_eth_filter.h.
-----------------------------------------------------
#define RTE_ETH_FILTER_RSS		1
#define RTE_ETH_FILTER_SYN	    2
#define RTE_ETH_FILTER_5TUPLE	    3
#define RTE_ETH_FILTER_FDIR		4
.... <all other filter types we support>

#define RTE_ETH_FILTER_OP_GET      	1
#define RTE_ETH_FILTER_OP_ADD    	2
#define RTE_ETH_FILTER_OP_DELETE		3
#define RTE_ETH_FILTER_OP_SET			4
....< other operations if want to define>...

/* structures defined for corresponding filter type and operation */
/* take RTE_ETH_FILTER_FDIR and OP_SET for example*/

struct rte_eth_filter_fdir_cfg { 
#define RTE_ETH_FILTER_FDIR_SET_MASK   0
#define RTE_ETH_FILTER_FDIR_SET_OFFSET  1
…… <other sub operations in this structure>
	uint16_t cfg_type;
　　/* sub operation to defined what specific configuration it will take, 
　　　and which following fields are meaningful*/
　　……
　　/* fields, can be a union or combine of required specific items*/
　　……
　　
};

-----------------------------------------------------
By this way, It is easy to add more filter types or operation in future.
And the difference among the same filter and operation can be distinguish by sub command in defined structure, e.g. ”cfg_type” in above rte_eth_filter_fdir_cfg structure. 

3.   Define ops in driver (take i40e for example)
-----------------------------------------------------
static struct eth_dev_ops i40e_eth_dev_ops = {
         . filter_ctl = i40e_filter_ctl,
};
-----------------------------------------------------
Then the functions in drivers can be implemented separately.

4.   Use case In test-pmd/cmdline.c 
-----------------------------------------------------
#include <rte_eth_filter.h>
/* add or change commands e.g. fdir_set (arg1) (arg2) …… */

static void
cmd_fdir_parsed()
{
	……
　　/* take setting fdir mask for example*/
　　struct rte_eth_filter_fdir_cfg cfg;

　　if (rte_eth_filter_supported(port, RTE_ETH_FILTER_FDIR)) {
　　	cfg.cfg_type = RTE_ETH_FILTER_FDIR_SET_MASK; 
　　	/* fill the corresponding fields in cfg*/
　　	……
　　	rte_eth_filter_ctl(port, RTE_ETH_FILTER_FDIR, RTE_ETH_FILTER_OP_SET, &cfg);
　　}
	……
}
-----------------------------------------------------


Any comments are welcome! 

At the time being, only Intel PMD is only available on dpdk.org. We are lack of understanding on the other non-Intel PMD, the current design did not take them into account. But we are looking for the inputs from those PMD developers, we strongly look forward to those PMD are released as open source.

Thanks!
Jingjing

Re: [dpdk-dev] filter_ctl PMD API idea

[Wu, Jingjing]

Any comments or advises? 

Thanks!

Fortville Filter features' development will be started based on this design this week.


> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, September 4, 2014 8:05 PM
> To: dev@dpdk.org
> Cc: stephen@networkplumber.org; vincent.jardin@6wind.com
> Subject: filter_ctl PMD API idea
> 
> Hi, all
> 
> When we develop filters feature in i40e driver for Intel® Ethernet Controller XL710/X710
> [Fortville] (For both 10G/40G), we found that there are lots of new filters, there are also
> some changes on the existing filters, comparing to ixgbe.
> If we keep the way to add new ops in rte_eth_dev for each new filter, it can work.
> But we suggest to use a more generic API for all filters to avoid a superset dev_ops. It needs
> to be cleaner and easy-to-use. There is a need for technical discussion.
> 
> Here is the early design idea we are looking for comments.
> 
> 1.   Create two new APIs
> -----------------------------------------------------
> rte_eth_filter_supported(uint8_t port, uint16_t filter_type);
> /* check whether this filter type is supported for the queried port */
> rte_eth_filter_ctl(uint8_t port, uint16_t filter_type, uint16_t filter_op, void *arg);
> /* configure filters, will call new ops eth_filter_ctl in eth_dev_ops */
> -----------------------------------------------------
> 
> 2.   Define filter types, operations, and structures in new header file
> lib/librte_eth/rte_eth_filter.h.
> -----------------------------------------------------
> #define RTE_ETH_FILTER_RSS		1
> #define RTE_ETH_FILTER_SYN	    2
> #define RTE_ETH_FILTER_5TUPLE	    3
> #define RTE_ETH_FILTER_FDIR		4
> .... <all other filter types we support>
> 
> #define RTE_ETH_FILTER_OP_GET      	1
> #define RTE_ETH_FILTER_OP_ADD    	2
> #define RTE_ETH_FILTER_OP_DELETE		3
> #define RTE_ETH_FILTER_OP_SET			4
> ....< other operations if want to define>...
> 
> /* structures defined for corresponding filter type and operation */
> /* take RTE_ETH_FILTER_FDIR and OP_SET for example*/
> 
> struct rte_eth_filter_fdir_cfg {
> #define RTE_ETH_FILTER_FDIR_SET_MASK   0
> #define RTE_ETH_FILTER_FDIR_SET_OFFSET  1
> …… <other sub operations in this structure>
> 	uint16_t cfg_type;
> 　　/* sub operation to defined what specific configuration it will take,
> 　　　and which following fields are meaningful*/
> 　　……
> 　　/* fields, can be a union or combine of required specific items*/
> 　　……
> 
> };
> 
> -----------------------------------------------------
> By this way, It is easy to add more filter types or operation in future.
> And the difference among the same filter and operation can be distinguish by sub command
> in defined structure, e.g. ”cfg_type” in above rte_eth_filter_fdir_cfg structure.
> 
> 3.   Define ops in driver (take i40e for example)
> -----------------------------------------------------
> static struct eth_dev_ops i40e_eth_dev_ops = {
>          . filter_ctl = i40e_filter_ctl,
> };
> -----------------------------------------------------
> Then the functions in drivers can be implemented separately.
> 
> 4.   Use case In test-pmd/cmdline.c
> -----------------------------------------------------
> #include <rte_eth_filter.h>
> /* add or change commands e.g. fdir_set (arg1) (arg2) …… */
> 
> static void
> cmd_fdir_parsed()
> {
> 	……
> 　　/* take setting fdir mask for example*/
> 　　struct rte_eth_filter_fdir_cfg cfg;
> 
> 　　if (rte_eth_filter_supported(port, RTE_ETH_FILTER_FDIR)) {
> 　　	cfg.cfg_type = RTE_ETH_FILTER_FDIR_SET_MASK;
> 　　	/* fill the corresponding fields in cfg*/
> 　　	……
> 　　	rte_eth_filter_ctl(port, RTE_ETH_FILTER_FDIR, RTE_ETH_FILTER_OP_SET, &cfg);
> 　　}
> 	……
> }
> -----------------------------------------------------
> 
> 
> Any comments are welcome!
> 
> At the time being, only Intel PMD is only available on dpdk.org. We are lack of understanding
> on the other non-Intel PMD, the current design did not take them into account. But we are
> looking for the inputs from those PMD developers, we strongly look forward to those PMD
> are released as open source.
> 
> Thanks!
> Jingjing

Re: [dpdk-dev] filter_ctl PMD API idea

[Thomas Monjalon]

2014-09-08 15:06, Wu, Jingjing:
> Any comments or advises? 
> 
> Thanks!
> 
> Fortville Filter features' development will be started based on this design this week.

Thanks Jingjing for explaining your plan before working on it.
There were no comment for 1 month so we'll assume everybody is OK.
Now your work is done and it's time to integrate it.

This design is used in many pending patchsets.
Now I wait for an unique patch out of any patchset in order to do some
comments about implementation.
Then it will be applied with i40e filters using this API.
So we'll have a new API implemented only for i40e.
But when DPDK 1.8 will be out, I expect to receive patches replacing old API
with this new one for igb and ixgbe.

Last request, please could you write a brief email summarizing all filters
of Intel NICs from an user perspective, and which ones are implemented in
DPDK, with which API?

Thanks


> > -----Original Message-----
> > Hi, all
> > 
> > When we develop filters feature in i40e driver for Intel® Ethernet Controller XL710/X710
> > [Fortville] (For both 10G/40G), we found that there are lots of new filters, there are also
> > some changes on the existing filters, comparing to ixgbe.
> > If we keep the way to add new ops in rte_eth_dev for each new filter, it can work.
> > But we suggest to use a more generic API for all filters to avoid a superset dev_ops. It needs
> > to be cleaner and easy-to-use. There is a need for technical discussion.
> > 
> > Here is the early design idea we are looking for comments.
> > 
> > 1.   Create two new APIs
> > -----------------------------------------------------
> > rte_eth_filter_supported(uint8_t port, uint16_t filter_type);
> > /* check whether this filter type is supported for the queried port */
> > rte_eth_filter_ctl(uint8_t port, uint16_t filter_type, uint16_t filter_op, void *arg);
> > /* configure filters, will call new ops eth_filter_ctl in eth_dev_ops */
> > -----------------------------------------------------
> > 
> > 2.   Define filter types, operations, and structures in new header file
> > lib/librte_eth/rte_eth_filter.h.
> > -----------------------------------------------------
> > #define RTE_ETH_FILTER_RSS		1
> > #define RTE_ETH_FILTER_SYN	    2
> > #define RTE_ETH_FILTER_5TUPLE	    3
> > #define RTE_ETH_FILTER_FDIR		4
> > .... <all other filter types we support>
> > 
> > #define RTE_ETH_FILTER_OP_GET      	1
> > #define RTE_ETH_FILTER_OP_ADD    	2
> > #define RTE_ETH_FILTER_OP_DELETE		3
> > #define RTE_ETH_FILTER_OP_SET			4
> > ....< other operations if want to define>...
> > 
> > /* structures defined for corresponding filter type and operation */
> > /* take RTE_ETH_FILTER_FDIR and OP_SET for example*/
> > 
> > struct rte_eth_filter_fdir_cfg {
> > #define RTE_ETH_FILTER_FDIR_SET_MASK   0
> > #define RTE_ETH_FILTER_FDIR_SET_OFFSET  1
> > …… <other sub operations in this structure>
> > 	uint16_t cfg_type;
> > 　　/* sub operation to defined what specific configuration it will take,
> > 　　　and which following fields are meaningful*/
> > 　　……
> > 　　/* fields, can be a union or combine of required specific items*/
> > 　　……
> > 
> > };
> > 
> > -----------------------------------------------------
> > By this way, It is easy to add more filter types or operation in future.
> > And the difference among the same filter and operation can be distinguish by sub command
> > in defined structure, e.g. ”cfg_type” in above rte_eth_filter_fdir_cfg structure.
> > 
> > 3.   Define ops in driver (take i40e for example)
> > -----------------------------------------------------
> > static struct eth_dev_ops i40e_eth_dev_ops = {
> >          . filter_ctl = i40e_filter_ctl,
> > };
> > -----------------------------------------------------
> > Then the functions in drivers can be implemented separately.
> > 
> > 4.   Use case In test-pmd/cmdline.c
> > -----------------------------------------------------
> > #include <rte_eth_filter.h>
> > /* add or change commands e.g. fdir_set (arg1) (arg2) …… */
> > 
> > static void
> > cmd_fdir_parsed()
> > {
> > 	……
> > 　　/* take setting fdir mask for example*/
> > 　　struct rte_eth_filter_fdir_cfg cfg;
> > 
> > 　　if (rte_eth_filter_supported(port, RTE_ETH_FILTER_FDIR)) {
> > 　　	cfg.cfg_type = RTE_ETH_FILTER_FDIR_SET_MASK;
> > 　　	/* fill the corresponding fields in cfg*/
> > 　　	……
> > 　　	rte_eth_filter_ctl(port, RTE_ETH_FILTER_FDIR, RTE_ETH_FILTER_OP_SET, &cfg);
> > 　　}
> > 	……
> > }
> > -----------------------------------------------------
> > 
> > 
> > Any comments are welcome!
> > 
> > At the time being, only Intel PMD is only available on dpdk.org. We are lack of understanding
> > on the other non-Intel PMD, the current design did not take them into account. But we are
> > looking for the inputs from those PMD developers, we strongly look forward to those PMD
> > are released as open source.
> > 
> > Thanks!
> > Jingjing

Re: [dpdk-dev] filter_ctl PMD API idea

[Wu, Jingjing]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, October 17, 2014 12:07 AM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] filter_ctl PMD API idea
> 
> 2014-09-08 15:06, Wu, Jingjing:
> > Any comments or advises?
> >
> > Thanks!
> >
> > Fortville Filter features' development will be started based on this design this week.
> 
> Thanks Jingjing for explaining your plan before working on it.
> There were no comment for 1 month so we'll assume everybody is OK.
> Now your work is done and it's time to integrate it.
> 
It's great, thanks.

> This design is used in many pending patchsets.
> Now I wait for an unique patch out of any patchset in order to do some
> comments about implementation.

OK, an unique patch of this new API definition will be sent soon. And I hope it can be reviewed
With high priority, due to many pending patchsets we need to rework.

> Then it will be applied with i40e filters using this API.
> So we'll have a new API implemented only for i40e.
> But when DPDK 1.8 will be out, I expect to receive patches replacing old API
> with this new one for igb and ixgbe.

Fine, now I am working on integrating ixgbe's flow director to the new APIs.

> Last request, please could you write a brief email summarizing all filters
> of Intel NICs from an user perspective, and which ones are implemented in
> DPDK, with which API?
> 
OK.

> Thanks
> 
> 
> > > -----Original Message-----
> > > Hi, all
> > >
> > > When we develop filters feature in i40e driver for Intel® Ethernet Controller XL710/X710
> > > [Fortville] (For both 10G/40G), we found that there are lots of new filters, there are also
> > > some changes on the existing filters, comparing to ixgbe.
> > > If we keep the way to add new ops in rte_eth_dev for each new filter, it can work.
> > > But we suggest to use a more generic API for all filters to avoid a superset dev_ops. It
> needs
> > > to be cleaner and easy-to-use. There is a need for technical discussion.
> > >
> > > Here is the early design idea we are looking for comments.
> > >
> > > 1.   Create two new APIs
> > > -----------------------------------------------------
> > > rte_eth_filter_supported(uint8_t port, uint16_t filter_type);
> > > /* check whether this filter type is supported for the queried port */
> > > rte_eth_filter_ctl(uint8_t port, uint16_t filter_type, uint16_t filter_op, void *arg);
> > > /* configure filters, will call new ops eth_filter_ctl in eth_dev_ops */
> > > -----------------------------------------------------
> > >
> > > 2.   Define filter types, operations, and structures in new header file
> > > lib/librte_eth/rte_eth_filter.h.
> > > -----------------------------------------------------
> > > #define RTE_ETH_FILTER_RSS		1
> > > #define RTE_ETH_FILTER_SYN	    2
> > > #define RTE_ETH_FILTER_5TUPLE	    3
> > > #define RTE_ETH_FILTER_FDIR		4
> > > .... <all other filter types we support>
> > >
> > > #define RTE_ETH_FILTER_OP_GET      	1
> > > #define RTE_ETH_FILTER_OP_ADD    	2
> > > #define RTE_ETH_FILTER_OP_DELETE		3
> > > #define RTE_ETH_FILTER_OP_SET			4
> > > ....< other operations if want to define>...
> > >
> > > /* structures defined for corresponding filter type and operation */
> > > /* take RTE_ETH_FILTER_FDIR and OP_SET for example*/
> > >
> > > struct rte_eth_filter_fdir_cfg {
> > > #define RTE_ETH_FILTER_FDIR_SET_MASK   0
> > > #define RTE_ETH_FILTER_FDIR_SET_OFFSET  1
> > > …… <other sub operations in this structure>
> > > 	uint16_t cfg_type;
> > > 　　/* sub operation to defined what specific configuration it will take,
> > > 　　　and which following fields are meaningful*/
> > > 　　……
> > > 　　/* fields, can be a union or combine of required specific items*/
> > > 　　……
> > >
> > > };
> > >
> > > -----------------------------------------------------
> > > By this way, It is easy to add more filter types or operation in future.
> > > And the difference among the same filter and operation can be distinguish by sub
> command
> > > in defined structure, e.g. ”cfg_type” in above rte_eth_filter_fdir_cfg structure.
> > >
> > > 3.   Define ops in driver (take i40e for example)
> > > -----------------------------------------------------
> > > static struct eth_dev_ops i40e_eth_dev_ops = {
> > >          . filter_ctl = i40e_filter_ctl,
> > > };
> > > -----------------------------------------------------
> > > Then the functions in drivers can be implemented separately.
> > >
> > > 4.   Use case In test-pmd/cmdline.c
> > > -----------------------------------------------------
> > > #include <rte_eth_filter.h>
> > > /* add or change commands e.g. fdir_set (arg1) (arg2) …… */
> > >
> > > static void
> > > cmd_fdir_parsed()
> > > {
> > > 	……
> > > 　　/* take setting fdir mask for example*/
> > > 　　struct rte_eth_filter_fdir_cfg cfg;
> > >
> > > 　　if (rte_eth_filter_supported(port, RTE_ETH_FILTER_FDIR)) {
> > > 　　	cfg.cfg_type = RTE_ETH_FILTER_FDIR_SET_MASK;
> > > 　　	/* fill the corresponding fields in cfg*/
> > > 　　	……
> > > 　　	rte_eth_filter_ctl(port, RTE_ETH_FILTER_FDIR, RTE_ETH_FILTER_OP_SET, &cfg);
> > > 　　}
> > > 	……
> > > }
> > > -----------------------------------------------------
> > >
> > >
> > > Any comments are welcome!
> > >
> > > At the time being, only Intel PMD is only available on dpdk.org. We are lack of
> understanding
> > > on the other non-Intel PMD, the current design did not take them into account. But we
> are
> > > looking for the inputs from those PMD developers, we strongly look forward to those
> PMD
> > > are released as open source.
> > >
> > > Thanks!
> > > Jingjing