[dpdk-dev,1/4] lib/librte_ether: new filter APIs definition

Message ID 1411628369-29532-2-git-send-email-jingjing.wu@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Jingjing Wu Sept. 25, 2014, 6:59 a.m. UTC
Define new APIs to support configure multi-kind filters using same APIs
 - rte_eth_dev_filter_supported
 - rte_eth_dev_filter_ctrl

As to the implemetation discussion, please refer to http://dpdk.org/ml/archives/dev/2014-September/005179.html, and control packet filter implementation is based on it.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 lib/librte_ether/Makefile       |  1 +
 lib/librte_ether/rte_eth_ctrl.h | 78 +++++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.c   | 32 +++++++++++++++++
 lib/librte_ether/rte_ethdev.h   | 44 +++++++++++++++++++++++
 4 files changed, 155 insertions(+)
 create mode 100644 lib/librte_ether/rte_eth_ctrl.h
  

Comments

De Lara Guarch, Pablo Oct. 9, 2014, 3:34 p.m. UTC | #1
Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> Sent: Thursday, September 25, 2014 7:59 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs definition
> 
> Define new APIs to support configure multi-kind filters using same APIs
>  - rte_eth_dev_filter_supported
>  - rte_eth_dev_filter_ctrl
> 
> As to the implemetation discussion, please refer to
> http://dpdk.org/ml/archives/dev/2014-September/005179.html, and control
> packet filter implementation is based on it.

This patch is also present on the patchset Support flow director programming on Fortville.
Should this patchset be rejected then or just this patch? In second case, could you send a v2 without this patch?

Thanks,
Pablo
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  lib/librte_ether/Makefile       |  1 +
>  lib/librte_ether/rte_eth_ctrl.h | 78
> +++++++++++++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ethdev.c   | 32 +++++++++++++++++
>  lib/librte_ether/rte_ethdev.h   | 44 +++++++++++++++++++++++
>  4 files changed, 155 insertions(+)
>  create mode 100644 lib/librte_ether/rte_eth_ctrl.h
> 
> diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
> index b310f8b..a461c31 100644
> --- a/lib/librte_ether/Makefile
> +++ b/lib/librte_ether/Makefile
> @@ -46,6 +46,7 @@ SRCS-y += rte_ethdev.c
>  #
>  SYMLINK-y-include += rte_ether.h
>  SYMLINK-y-include += rte_ethdev.h
> +SYMLINK-y-include += rte_eth_ctrl.h
> 
>  # this lib depends upon:
>  DEPDIRS-y += lib/librte_eal lib/librte_mempool lib/librte_ring
> lib/librte_mbuf
> diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
> new file mode 100644
> index 0000000..34ab278
> --- /dev/null
> +++ b/lib/librte_ether/rte_eth_ctrl.h
> @@ -0,0 +1,78 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> + */
> +
> +#ifndef _RTE_ETH_CTRL_H_
> +#define _RTE_ETH_CTRL_H_
> +
> +/**
> + * @file
> + *
> + * Ethernet device features and related data structures used
> + * by control APIs should be defined in this file.
> + *
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * Feature filter types
> + */
> +enum rte_filter_type {
> +	RTE_ETH_FILTER_NONE = 0,
> +	RTE_ETH_FILTER_RSS,
> +	RTE_ETH_FILTER_FDIR,
> +	RTE_ETH_FILTER_MAX,
> +};
> +
> +/**
> + * All generic operations to filters
> + */
> +enum rte_filter_op {
> +	RTE_ETH_FILTER_OP_NONE = 0, /**< used to check whether the
> type filter is supported */
> +	RTE_ETH_FILTER_OP_ADD,      /**< add filter entry */
> +	RTE_ETH_FILTER_OP_UPDATE,   /**< update filter entry */
> +	RTE_ETH_FILTER_OP_DELETE,   /**< delete filter entry */
> +	RTE_ETH_FILTER_OP_FLUSH,    /**< flush all entries */
> +	RTE_ETH_FILTER_OP_GET,      /**< get filter entry */
> +	RTE_ETH_FILTER_OP_SET,      /**< configurations */
> +	RTE_ETH_FILTER_OP_GET_INFO, /**< get information of filter, such
> as status or statistics */
> +	RTE_ETH_FILTER_OP_MAX,
> +};
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_ETH_CTRL_H_ */
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index b71b679..fdafb15 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -3139,3 +3139,35 @@ rte_eth_dev_get_flex_filter(uint8_t port_id,
> uint16_t index,
>  	return (*dev->dev_ops->get_flex_filter)(dev, index, filter,
>  						rx_queue);
>  }
> +
> +int
> +rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type
> filter_type)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	if (port_id >= nb_ports) {
> +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> +		return -ENODEV;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
> +	return (*dev->dev_ops->filter_ctrl)(dev, filter_type,
> +				RTE_ETH_FILTER_OP_NONE, NULL);
> +}
> +
> +int
> +rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type filter_type,
> +		       enum rte_filter_op filter_op, void *arg)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	if (port_id >= nb_ports) {
> +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> +		return -ENODEV;
> +	}
> +
> +	dev = &rte_eth_devices[port_id];
> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
> +	return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op, arg);
> +}
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 60b24c5..e2ea84a 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -177,6 +177,7 @@ extern "C" {
>  #include <rte_pci.h>
>  #include <rte_mbuf.h>
>  #include "rte_ether.h"
> +#include "rte_eth_ctrl.h"
> 
>  /**
>   * A structure used to retrieve statistics for an Ethernet port.
> @@ -1383,6 +1384,12 @@ typedef int (*eth_get_flex_filter_t)(struct
> rte_eth_dev *dev,
>  			uint16_t *rx_queue);
>  /**< @internal Get a flex filter rule on an Ethernet device */
> 
> +typedef int (*eth_filter_ctrl_t)(struct rte_eth_dev *dev,
> +				 enum rte_filter_type filter_type,
> +				 enum rte_filter_op filter_op,
> +				 void *arg);
> +/**< @internal Take operations to assigned filter type on an Ethernet
> device */
> +
>  /**
>   * @internal A structure containing the functions exported by an Ethernet
> driver.
>   */
> @@ -1491,6 +1498,7 @@ struct eth_dev_ops {
>  	eth_add_flex_filter_t          add_flex_filter;      /**< add flex filter. */
>  	eth_remove_flex_filter_t       remove_flex_filter;   /**< remove flex
> filter. */
>  	eth_get_flex_filter_t          get_flex_filter;      /**< get flex filter. */
> +	eth_filter_ctrl_t              filter_ctrl;          /**< common filter control*/
>  };
> 
>  /**
> @@ -3613,6 +3621,42 @@ int rte_eth_dev_remove_flex_filter(uint8_t
> port_id, uint16_t index);
>  int rte_eth_dev_get_flex_filter(uint8_t port_id, uint16_t index,
>  			struct rte_flex_filter *filter, uint16_t *rx_queue);
> 
> +/**
> + * Check whether the filter type is supported on an Ethernet device.
> + * All the supported filter types are defined in 'rte_eth_ctrl.h'.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param filter_type
> + *   filter type.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if hardware doesn't support this filter type.
> + *   - (-ENODEV) if *port_id* invalid.
> + */
> +int rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type
> filter_type);
> +
> +/**
> + * Take operations to assigned filter type on an Ethernet device.
> + * All the supported operations and filter types are defined in
> 'rte_eth_ctrl.h'.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param filter_type
> + *   filter type.
> +  * @param filter_op
> + *   The operation taken to assigned filter.
> + * @param arg
> + *   A pointer to arguments defined specifically for the operation.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if hardware doesn't support.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - others depends on the specific operations implementation.
> + */
> +int rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type
> filter_type,
> +			enum rte_filter_op filter_op, void *arg);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 1.8.1.4
  
Jingjing Wu Oct. 10, 2014, 1:19 a.m. UTC | #2
Hi

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Thursday, October 09, 2014 11:35 PM
> To: Wu, Jingjing; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> definition
> 
> Hi,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> > Sent: Thursday, September 25, 2014 7:59 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> > definition
> >
> > Define new APIs to support configure multi-kind filters using same
> > APIs
> >  - rte_eth_dev_filter_supported
> >  - rte_eth_dev_filter_ctrl
> >
> > As to the implemetation discussion, please refer to
> > http://dpdk.org/ml/archives/dev/2014-September/005179.html, and
> > control packet filter implementation is based on it.
> 
> This patch is also present on the patchset Support flow director programming
> on Fortville.
> Should this patchset be rejected then or just this patch? In second case,
> could you send a v2 without this patch?

I think this patch does not only present on the flow director patchset, but also on mac vlan support patchset, vxlan patchset, and so on. All of them are using the same new filter APIs. If any patchset is applied, others may require some modification (just as you said to remove this pacth).

> 
> Thanks,
> Pablo
> >
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > ---
> >  lib/librte_ether/Makefile       |  1 +
> >  lib/librte_ether/rte_eth_ctrl.h | 78
> > +++++++++++++++++++++++++++++++++++++++++
> >  lib/librte_ether/rte_ethdev.c   | 32 +++++++++++++++++
> >  lib/librte_ether/rte_ethdev.h   | 44 +++++++++++++++++++++++
> >  4 files changed, 155 insertions(+)
> >  create mode 100644 lib/librte_ether/rte_eth_ctrl.h
> >
> > diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
> > index b310f8b..a461c31 100644
> > --- a/lib/librte_ether/Makefile
> > +++ b/lib/librte_ether/Makefile
> > @@ -46,6 +46,7 @@ SRCS-y += rte_ethdev.c  #  SYMLINK-y-include +=
> > rte_ether.h  SYMLINK-y-include += rte_ethdev.h
> > +SYMLINK-y-include += rte_eth_ctrl.h
> >
> >  # this lib depends upon:
> >  DEPDIRS-y += lib/librte_eal lib/librte_mempool lib/librte_ring
> > lib/librte_mbuf
> > diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
> > new file mode 100644
> > index 0000000..34ab278
> > --- /dev/null
> > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > @@ -0,0 +1,78 @@
> > +/*-
> > + *   BSD LICENSE
> > + *
> > + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> > + *   All rights reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copyright
> > + *       notice, this list of conditions and the following disclaimer in
> > + *       the documentation and/or other materials provided with the
> > + *       distribution.
> > + *     * Neither the name of Intel Corporation nor the names of its
> > + *       contributors may be used to endorse or promote products derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> > CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> > NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > FITNESS FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> > COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> > INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> BUT
> > NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> LOSS
> > OF USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> > AND ON ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> OF
> > THE USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > + */
> > +
> > +#ifndef _RTE_ETH_CTRL_H_
> > +#define _RTE_ETH_CTRL_H_
> > +
> > +/**
> > + * @file
> > + *
> > + * Ethernet device features and related data structures used
> > + * by control APIs should be defined in this file.
> > + *
> > + */
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * Feature filter types
> > + */
> > +enum rte_filter_type {
> > +	RTE_ETH_FILTER_NONE = 0,
> > +	RTE_ETH_FILTER_RSS,
> > +	RTE_ETH_FILTER_FDIR,
> > +	RTE_ETH_FILTER_MAX,
> > +};
> > +
> > +/**
> > + * All generic operations to filters
> > + */
> > +enum rte_filter_op {
> > +	RTE_ETH_FILTER_OP_NONE = 0, /**< used to check whether the
> > type filter is supported */
> > +	RTE_ETH_FILTER_OP_ADD,      /**< add filter entry */
> > +	RTE_ETH_FILTER_OP_UPDATE,   /**< update filter entry */
> > +	RTE_ETH_FILTER_OP_DELETE,   /**< delete filter entry */
> > +	RTE_ETH_FILTER_OP_FLUSH,    /**< flush all entries */
> > +	RTE_ETH_FILTER_OP_GET,      /**< get filter entry */
> > +	RTE_ETH_FILTER_OP_SET,      /**< configurations */
> > +	RTE_ETH_FILTER_OP_GET_INFO, /**< get information of filter, such
> > as status or statistics */
> > +	RTE_ETH_FILTER_OP_MAX,
> > +};
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* _RTE_ETH_CTRL_H_ */
> > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> > index b71b679..fdafb15 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -3139,3 +3139,35 @@ rte_eth_dev_get_flex_filter(uint8_t port_id,
> > uint16_t index,
> >  	return (*dev->dev_ops->get_flex_filter)(dev, index, filter,
> >  						rx_queue);
> >  }
> > +
> > +int
> > +rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type
> > filter_type)
> > +{
> > +	struct rte_eth_dev *dev;
> > +
> > +	if (port_id >= nb_ports) {
> > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > +		return -ENODEV;
> > +	}
> > +
> > +	dev = &rte_eth_devices[port_id];
> > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
> > +	return (*dev->dev_ops->filter_ctrl)(dev, filter_type,
> > +				RTE_ETH_FILTER_OP_NONE, NULL);
> > +}
> > +
> > +int
> > +rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type filter_type,
> > +		       enum rte_filter_op filter_op, void *arg)
> > +{
> > +	struct rte_eth_dev *dev;
> > +
> > +	if (port_id >= nb_ports) {
> > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > +		return -ENODEV;
> > +	}
> > +
> > +	dev = &rte_eth_devices[port_id];
> > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
> > +	return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op, arg);
> > +}
> > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > index 60b24c5..e2ea84a 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -177,6 +177,7 @@ extern "C" {
> >  #include <rte_pci.h>
> >  #include <rte_mbuf.h>
> >  #include "rte_ether.h"
> > +#include "rte_eth_ctrl.h"
> >
> >  /**
> >   * A structure used to retrieve statistics for an Ethernet port.
> > @@ -1383,6 +1384,12 @@ typedef int (*eth_get_flex_filter_t)(struct
> > rte_eth_dev *dev,
> >  			uint16_t *rx_queue);
> >  /**< @internal Get a flex filter rule on an Ethernet device */
> >
> > +typedef int (*eth_filter_ctrl_t)(struct rte_eth_dev *dev,
> > +				 enum rte_filter_type filter_type,
> > +				 enum rte_filter_op filter_op,
> > +				 void *arg);
> > +/**< @internal Take operations to assigned filter type on an Ethernet
> > device */
> > +
> >  /**
> >   * @internal A structure containing the functions exported by an Ethernet
> > driver.
> >   */
> > @@ -1491,6 +1498,7 @@ struct eth_dev_ops {
> >  	eth_add_flex_filter_t          add_flex_filter;      /**< add flex filter. */
> >  	eth_remove_flex_filter_t       remove_flex_filter;   /**< remove flex
> > filter. */
> >  	eth_get_flex_filter_t          get_flex_filter;      /**< get flex filter. */
> > +	eth_filter_ctrl_t              filter_ctrl;          /**< common filter control*/
> >  };
> >
> >  /**
> > @@ -3613,6 +3621,42 @@ int rte_eth_dev_remove_flex_filter(uint8_t
> > port_id, uint16_t index);
> >  int rte_eth_dev_get_flex_filter(uint8_t port_id, uint16_t index,
> >  			struct rte_flex_filter *filter, uint16_t *rx_queue);
> >
> > +/**
> > + * Check whether the filter type is supported on an Ethernet device.
> > + * All the supported filter types are defined in 'rte_eth_ctrl.h'.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param filter_type
> > + *   filter type.
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENOTSUP) if hardware doesn't support this filter type.
> > + *   - (-ENODEV) if *port_id* invalid.
> > + */
> > +int rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type
> > filter_type);
> > +
> > +/**
> > + * Take operations to assigned filter type on an Ethernet device.
> > + * All the supported operations and filter types are defined in
> > 'rte_eth_ctrl.h'.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param filter_type
> > + *   filter type.
> > +  * @param filter_op
> > + *   The operation taken to assigned filter.
> > + * @param arg
> > + *   A pointer to arguments defined specifically for the operation.
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENOTSUP) if hardware doesn't support.
> > + *   - (-ENODEV) if *port_id* invalid.
> > + *   - others depends on the specific operations implementation.
> > + */
> > +int rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type
> > filter_type,
> > +			enum rte_filter_op filter_op, void *arg);
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> > --
> > 1.8.1.4
  
Jingjing Wu Oct. 10, 2014, 3:34 a.m. UTC | #3
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, October 10, 2014 9:20 AM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> definition
> 
> Hi
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Thursday, October 09, 2014 11:35 PM
> > To: Wu, Jingjing; dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> > definition
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> > > Sent: Thursday, September 25, 2014 7:59 AM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> > > definition
> > >
> > > Define new APIs to support configure multi-kind filters using same
> > > APIs
> > >  - rte_eth_dev_filter_supported
> > >  - rte_eth_dev_filter_ctrl
> > >
> > > As to the implemetation discussion, please refer to
> > > http://dpdk.org/ml/archives/dev/2014-September/005179.html, and
> > > control packet filter implementation is based on it.
> >
> > This patch is also present on the patchset Support flow director
> > programming on Fortville.
> > Should this patchset be rejected then or just this patch? In second
> > case, could you send a v2 without this patch?
> 
> I think this patch does not only present on the flow director patchset, but
> also on mac vlan support patchset, vxlan patchset, and so on. All of them are
> using the same new filter APIs. If any patchset is applied, others may require
> some modification (just as you said to remove this pacth).
> 
Additional, without the patch, this patchset cannot work separately. More than one features depend on the new filter APIs, but none patchset contains the new filter APIs is applied currently.  That's why each patchset has such patch.

Thanks 
JIngjing
> >
> > Thanks,
> > Pablo
> > >
> > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > ---
> > >  lib/librte_ether/Makefile       |  1 +
> > >  lib/librte_ether/rte_eth_ctrl.h | 78
> > > +++++++++++++++++++++++++++++++++++++++++
> > >  lib/librte_ether/rte_ethdev.c   | 32 +++++++++++++++++
> > >  lib/librte_ether/rte_ethdev.h   | 44 +++++++++++++++++++++++
> > >  4 files changed, 155 insertions(+)
> > >  create mode 100644 lib/librte_ether/rte_eth_ctrl.h
> > >
> > > diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
> > > index b310f8b..a461c31 100644
> > > --- a/lib/librte_ether/Makefile
> > > +++ b/lib/librte_ether/Makefile
> > > @@ -46,6 +46,7 @@ SRCS-y += rte_ethdev.c  #  SYMLINK-y-include +=
> > > rte_ether.h  SYMLINK-y-include += rte_ethdev.h
> > > +SYMLINK-y-include += rte_eth_ctrl.h
> > >
> > >  # this lib depends upon:
> > >  DEPDIRS-y += lib/librte_eal lib/librte_mempool lib/librte_ring
> > > lib/librte_mbuf diff --git a/lib/librte_ether/rte_eth_ctrl.h
> > > b/lib/librte_ether/rte_eth_ctrl.h new file mode 100644 index
> > > 0000000..34ab278
> > > --- /dev/null
> > > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > > @@ -0,0 +1,78 @@
> > > +/*-
> > > + *   BSD LICENSE
> > > + *
> > > + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> > > + *   All rights reserved.
> > > + *
> > > + *   Redistribution and use in source and binary forms, with or without
> > > + *   modification, are permitted provided that the following conditions
> > > + *   are met:
> > > + *
> > > + *     * Redistributions of source code must retain the above copyright
> > > + *       notice, this list of conditions and the following disclaimer.
> > > + *     * Redistributions in binary form must reproduce the above
> copyright
> > > + *       notice, this list of conditions and the following disclaimer in
> > > + *       the documentation and/or other materials provided with the
> > > + *       distribution.
> > > + *     * Neither the name of Intel Corporation nor the names of its
> > > + *       contributors may be used to endorse or promote products derived
> > > + *       from this software without specific prior written permission.
> > > + *
> > > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> > > CONTRIBUTORS
> > > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
> BUT
> > > NOT
> > > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > > FITNESS FOR
> > > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> > > COPYRIGHT
> > > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> > > INCIDENTAL,
> > > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> > BUT
> > > NOT
> > > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > LOSS
> > > OF USE,
> > > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
> CAUSED
> > > AND ON ANY
> > > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > > TORT
> > > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> OUT
> > OF
> > > THE USE
> > > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > > DAMAGE.
> > > + */
> > > +
> > > +#ifndef _RTE_ETH_CTRL_H_
> > > +#define _RTE_ETH_CTRL_H_
> > > +
> > > +/**
> > > + * @file
> > > + *
> > > + * Ethernet device features and related data structures used
> > > + * by control APIs should be defined in this file.
> > > + *
> > > + */
> > > +
> > > +#ifdef __cplusplus
> > > +extern "C" {
> > > +#endif
> > > +
> > > +/**
> > > + * Feature filter types
> > > + */
> > > +enum rte_filter_type {
> > > +	RTE_ETH_FILTER_NONE = 0,
> > > +	RTE_ETH_FILTER_RSS,
> > > +	RTE_ETH_FILTER_FDIR,
> > > +	RTE_ETH_FILTER_MAX,
> > > +};
> > > +
> > > +/**
> > > + * All generic operations to filters  */ enum rte_filter_op {
> > > +	RTE_ETH_FILTER_OP_NONE = 0, /**< used to check whether the
> > > type filter is supported */
> > > +	RTE_ETH_FILTER_OP_ADD,      /**< add filter entry */
> > > +	RTE_ETH_FILTER_OP_UPDATE,   /**< update filter entry */
> > > +	RTE_ETH_FILTER_OP_DELETE,   /**< delete filter entry */
> > > +	RTE_ETH_FILTER_OP_FLUSH,    /**< flush all entries */
> > > +	RTE_ETH_FILTER_OP_GET,      /**< get filter entry */
> > > +	RTE_ETH_FILTER_OP_SET,      /**< configurations */
> > > +	RTE_ETH_FILTER_OP_GET_INFO, /**< get information of filter, such
> > > as status or statistics */
> > > +	RTE_ETH_FILTER_OP_MAX,
> > > +};
> > > +
> > > +#ifdef __cplusplus
> > > +}
> > > +#endif
> > > +
> > > +#endif /* _RTE_ETH_CTRL_H_ */
> > > diff --git a/lib/librte_ether/rte_ethdev.c
> > > b/lib/librte_ether/rte_ethdev.c index b71b679..fdafb15 100644
> > > --- a/lib/librte_ether/rte_ethdev.c
> > > +++ b/lib/librte_ether/rte_ethdev.c
> > > @@ -3139,3 +3139,35 @@ rte_eth_dev_get_flex_filter(uint8_t port_id,
> > > uint16_t index,
> > >  	return (*dev->dev_ops->get_flex_filter)(dev, index, filter,
> > >  						rx_queue);
> > >  }
> > > +
> > > +int
> > > +rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type
> > > filter_type)
> > > +{
> > > +	struct rte_eth_dev *dev;
> > > +
> > > +	if (port_id >= nb_ports) {
> > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	dev = &rte_eth_devices[port_id];
> > > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
> > > +	return (*dev->dev_ops->filter_ctrl)(dev, filter_type,
> > > +				RTE_ETH_FILTER_OP_NONE, NULL);
> > > +}
> > > +
> > > +int
> > > +rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type
> filter_type,
> > > +		       enum rte_filter_op filter_op, void *arg) {
> > > +	struct rte_eth_dev *dev;
> > > +
> > > +	if (port_id >= nb_ports) {
> > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	dev = &rte_eth_devices[port_id];
> > > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
> > > +	return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op,
> > > +arg); }
> > > diff --git a/lib/librte_ether/rte_ethdev.h
> > > b/lib/librte_ether/rte_ethdev.h index 60b24c5..e2ea84a 100644
> > > --- a/lib/librte_ether/rte_ethdev.h
> > > +++ b/lib/librte_ether/rte_ethdev.h
> > > @@ -177,6 +177,7 @@ extern "C" {
> > >  #include <rte_pci.h>
> > >  #include <rte_mbuf.h>
> > >  #include "rte_ether.h"
> > > +#include "rte_eth_ctrl.h"
> > >
> > >  /**
> > >   * A structure used to retrieve statistics for an Ethernet port.
> > > @@ -1383,6 +1384,12 @@ typedef int (*eth_get_flex_filter_t)(struct
> > > rte_eth_dev *dev,
> > >  			uint16_t *rx_queue);
> > >  /**< @internal Get a flex filter rule on an Ethernet device */
> > >
> > > +typedef int (*eth_filter_ctrl_t)(struct rte_eth_dev *dev,
> > > +				 enum rte_filter_type filter_type,
> > > +				 enum rte_filter_op filter_op,
> > > +				 void *arg);
> > > +/**< @internal Take operations to assigned filter type on an
> > > +Ethernet
> > > device */
> > > +
> > >  /**
> > >   * @internal A structure containing the functions exported by an
> > > Ethernet driver.
> > >   */
> > > @@ -1491,6 +1498,7 @@ struct eth_dev_ops {
> > >  	eth_add_flex_filter_t          add_flex_filter;      /**< add flex filter. */
> > >  	eth_remove_flex_filter_t       remove_flex_filter;   /**< remove flex
> > > filter. */
> > >  	eth_get_flex_filter_t          get_flex_filter;      /**< get flex filter. */
> > > +	eth_filter_ctrl_t              filter_ctrl;          /**< common filter control*/
> > >  };
> > >
> > >  /**
> > > @@ -3613,6 +3621,42 @@ int rte_eth_dev_remove_flex_filter(uint8_t
> > > port_id, uint16_t index);
> > >  int rte_eth_dev_get_flex_filter(uint8_t port_id, uint16_t index,
> > >  			struct rte_flex_filter *filter, uint16_t *rx_queue);
> > >
> > > +/**
> > > + * Check whether the filter type is supported on an Ethernet device.
> > > + * All the supported filter types are defined in 'rte_eth_ctrl.h'.
> > > + *
> > > + * @param port_id
> > > + *   The port identifier of the Ethernet device.
> > > + * @param filter_type
> > > + *   filter type.
> > > + * @return
> > > + *   - (0) if successful.
> > > + *   - (-ENOTSUP) if hardware doesn't support this filter type.
> > > + *   - (-ENODEV) if *port_id* invalid.
> > > + */
> > > +int rte_eth_dev_filter_supported(uint8_t port_id, enum
> > > +rte_filter_type
> > > filter_type);
> > > +
> > > +/**
> > > + * Take operations to assigned filter type on an Ethernet device.
> > > + * All the supported operations and filter types are defined in
> > > 'rte_eth_ctrl.h'.
> > > + *
> > > + * @param port_id
> > > + *   The port identifier of the Ethernet device.
> > > + * @param filter_type
> > > + *   filter type.
> > > +  * @param filter_op
> > > + *   The operation taken to assigned filter.
> > > + * @param arg
> > > + *   A pointer to arguments defined specifically for the operation.
> > > + * @return
> > > + *   - (0) if successful.
> > > + *   - (-ENOTSUP) if hardware doesn't support.
> > > + *   - (-ENODEV) if *port_id* invalid.
> > > + *   - others depends on the specific operations implementation.
> > > + */
> > > +int rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type
> > > filter_type,
> > > +			enum rte_filter_op filter_op, void *arg);
> > > +
> > >  #ifdef __cplusplus
> > >  }
> > >  #endif
> > > --
> > > 1.8.1.4
  
De Lara Guarch, Pablo Oct. 10, 2014, 7:28 a.m. UTC | #4
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, October 10, 2014 4:34 AM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> definition
> 
> 
> 
> > -----Original Message-----
> > From: Wu, Jingjing
> > Sent: Friday, October 10, 2014 9:20 AM
> > To: De Lara Guarch, Pablo; dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> > definition
> >
> > Hi
> >
> > > -----Original Message-----
> > > From: De Lara Guarch, Pablo
> > > Sent: Thursday, October 09, 2014 11:35 PM
> > > To: Wu, Jingjing; dev@dpdk.org
> > > Subject: RE: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> > > definition
> > >
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> > > > Sent: Thursday, September 25, 2014 7:59 AM
> > > > To: dev@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs
> > > > definition
> > > >
> > > > Define new APIs to support configure multi-kind filters using same
> > > > APIs
> > > >  - rte_eth_dev_filter_supported
> > > >  - rte_eth_dev_filter_ctrl
> > > >
> > > > As to the implemetation discussion, please refer to
> > > > http://dpdk.org/ml/archives/dev/2014-September/005179.html, and
> > > > control packet filter implementation is based on it.
> > >
> > > This patch is also present on the patchset Support flow director
> > > programming on Fortville.
> > > Should this patchset be rejected then or just this patch? In second
> > > case, could you send a v2 without this patch?
> >
> > I think this patch does not only present on the flow director patchset, but
> > also on mac vlan support patchset, vxlan patchset, and so on. All of them
> are
> > using the same new filter APIs. If any patchset is applied, others may
> require
> > some modification (just as you said to remove this pacth).
> >
> Additional, without the patch, this patchset cannot work separately. More
> than one features depend on the new filter APIs, but none patchset contains
> the new filter APIs is applied currently.  That's why each patchset has such
> patch.

I see, then probably the best idea would have been send this patch separately,
 and just say that these patchsets depend on this patch, basically because
 if you try to apply all these patches, you are going to get failures.

Anyway, good to know. Thanks,
Pablo
> 
> Thanks
> JIngjing
> > >
> > > Thanks,
> > > Pablo
> > > >
> > > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > > ---
> > > >  lib/librte_ether/Makefile       |  1 +
> > > >  lib/librte_ether/rte_eth_ctrl.h | 78
> > > > +++++++++++++++++++++++++++++++++++++++++
> > > >  lib/librte_ether/rte_ethdev.c   | 32 +++++++++++++++++
> > > >  lib/librte_ether/rte_ethdev.h   | 44 +++++++++++++++++++++++
> > > >  4 files changed, 155 insertions(+)
> > > >  create mode 100644 lib/librte_ether/rte_eth_ctrl.h
> > > >
> > > > diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
> > > > index b310f8b..a461c31 100644
> > > > --- a/lib/librte_ether/Makefile
> > > > +++ b/lib/librte_ether/Makefile
> > > > @@ -46,6 +46,7 @@ SRCS-y += rte_ethdev.c  #  SYMLINK-y-include +=
> > > > rte_ether.h  SYMLINK-y-include += rte_ethdev.h
> > > > +SYMLINK-y-include += rte_eth_ctrl.h
> > > >
> > > >  # this lib depends upon:
> > > >  DEPDIRS-y += lib/librte_eal lib/librte_mempool lib/librte_ring
> > > > lib/librte_mbuf diff --git a/lib/librte_ether/rte_eth_ctrl.h
> > > > b/lib/librte_ether/rte_eth_ctrl.h new file mode 100644 index
> > > > 0000000..34ab278
> > > > --- /dev/null
> > > > +++ b/lib/librte_ether/rte_eth_ctrl.h
> > > > @@ -0,0 +1,78 @@
> > > > +/*-
> > > > + *   BSD LICENSE
> > > > + *
> > > > + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> > > > + *   All rights reserved.
> > > > + *
> > > > + *   Redistribution and use in source and binary forms, with or without
> > > > + *   modification, are permitted provided that the following conditions
> > > > + *   are met:
> > > > + *
> > > > + *     * Redistributions of source code must retain the above copyright
> > > > + *       notice, this list of conditions and the following disclaimer.
> > > > + *     * Redistributions in binary form must reproduce the above
> > copyright
> > > > + *       notice, this list of conditions and the following disclaimer in
> > > > + *       the documentation and/or other materials provided with the
> > > > + *       distribution.
> > > > + *     * Neither the name of Intel Corporation nor the names of its
> > > > + *       contributors may be used to endorse or promote products
> derived
> > > > + *       from this software without specific prior written permission.
> > > > + *
> > > > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> > > > CONTRIBUTORS
> > > > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
> > BUT
> > > > NOT
> > > > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > > > FITNESS FOR
> > > > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> > > > COPYRIGHT
> > > > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> > > > INCIDENTAL,
> > > > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> (INCLUDING,
> > > BUT
> > > > NOT
> > > > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > > LOSS
> > > > OF USE,
> > > > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
> > CAUSED
> > > > AND ON ANY
> > > > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> OR
> > > > TORT
> > > > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > OUT
> > > OF
> > > > THE USE
> > > > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > > > DAMAGE.
> > > > + */
> > > > +
> > > > +#ifndef _RTE_ETH_CTRL_H_
> > > > +#define _RTE_ETH_CTRL_H_
> > > > +
> > > > +/**
> > > > + * @file
> > > > + *
> > > > + * Ethernet device features and related data structures used
> > > > + * by control APIs should be defined in this file.
> > > > + *
> > > > + */
> > > > +
> > > > +#ifdef __cplusplus
> > > > +extern "C" {
> > > > +#endif
> > > > +
> > > > +/**
> > > > + * Feature filter types
> > > > + */
> > > > +enum rte_filter_type {
> > > > +	RTE_ETH_FILTER_NONE = 0,
> > > > +	RTE_ETH_FILTER_RSS,
> > > > +	RTE_ETH_FILTER_FDIR,
> > > > +	RTE_ETH_FILTER_MAX,
> > > > +};
> > > > +
> > > > +/**
> > > > + * All generic operations to filters  */ enum rte_filter_op {
> > > > +	RTE_ETH_FILTER_OP_NONE = 0, /**< used to check whether the
> > > > type filter is supported */
> > > > +	RTE_ETH_FILTER_OP_ADD,      /**< add filter entry */
> > > > +	RTE_ETH_FILTER_OP_UPDATE,   /**< update filter entry */
> > > > +	RTE_ETH_FILTER_OP_DELETE,   /**< delete filter entry */
> > > > +	RTE_ETH_FILTER_OP_FLUSH,    /**< flush all entries */
> > > > +	RTE_ETH_FILTER_OP_GET,      /**< get filter entry */
> > > > +	RTE_ETH_FILTER_OP_SET,      /**< configurations */
> > > > +	RTE_ETH_FILTER_OP_GET_INFO, /**< get information of filter, such
> > > > as status or statistics */
> > > > +	RTE_ETH_FILTER_OP_MAX,
> > > > +};
> > > > +
> > > > +#ifdef __cplusplus
> > > > +}
> > > > +#endif
> > > > +
> > > > +#endif /* _RTE_ETH_CTRL_H_ */
> > > > diff --git a/lib/librte_ether/rte_ethdev.c
> > > > b/lib/librte_ether/rte_ethdev.c index b71b679..fdafb15 100644
> > > > --- a/lib/librte_ether/rte_ethdev.c
> > > > +++ b/lib/librte_ether/rte_ethdev.c
> > > > @@ -3139,3 +3139,35 @@ rte_eth_dev_get_flex_filter(uint8_t port_id,
> > > > uint16_t index,
> > > >  	return (*dev->dev_ops->get_flex_filter)(dev, index, filter,
> > > >  						rx_queue);
> > > >  }
> > > > +
> > > > +int
> > > > +rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type
> > > > filter_type)
> > > > +{
> > > > +	struct rte_eth_dev *dev;
> > > > +
> > > > +	if (port_id >= nb_ports) {
> > > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > > +		return -ENODEV;
> > > > +	}
> > > > +
> > > > +	dev = &rte_eth_devices[port_id];
> > > > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
> > > > +	return (*dev->dev_ops->filter_ctrl)(dev, filter_type,
> > > > +				RTE_ETH_FILTER_OP_NONE, NULL);
> > > > +}
> > > > +
> > > > +int
> > > > +rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type
> > filter_type,
> > > > +		       enum rte_filter_op filter_op, void *arg) {
> > > > +	struct rte_eth_dev *dev;
> > > > +
> > > > +	if (port_id >= nb_ports) {
> > > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > > +		return -ENODEV;
> > > > +	}
> > > > +
> > > > +	dev = &rte_eth_devices[port_id];
> > > > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
> > > > +	return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op,
> > > > +arg); }
> > > > diff --git a/lib/librte_ether/rte_ethdev.h
> > > > b/lib/librte_ether/rte_ethdev.h index 60b24c5..e2ea84a 100644
> > > > --- a/lib/librte_ether/rte_ethdev.h
> > > > +++ b/lib/librte_ether/rte_ethdev.h
> > > > @@ -177,6 +177,7 @@ extern "C" {
> > > >  #include <rte_pci.h>
> > > >  #include <rte_mbuf.h>
> > > >  #include "rte_ether.h"
> > > > +#include "rte_eth_ctrl.h"
> > > >
> > > >  /**
> > > >   * A structure used to retrieve statistics for an Ethernet port.
> > > > @@ -1383,6 +1384,12 @@ typedef int (*eth_get_flex_filter_t)(struct
> > > > rte_eth_dev *dev,
> > > >  			uint16_t *rx_queue);
> > > >  /**< @internal Get a flex filter rule on an Ethernet device */
> > > >
> > > > +typedef int (*eth_filter_ctrl_t)(struct rte_eth_dev *dev,
> > > > +				 enum rte_filter_type filter_type,
> > > > +				 enum rte_filter_op filter_op,
> > > > +				 void *arg);
> > > > +/**< @internal Take operations to assigned filter type on an
> > > > +Ethernet
> > > > device */
> > > > +
> > > >  /**
> > > >   * @internal A structure containing the functions exported by an
> > > > Ethernet driver.
> > > >   */
> > > > @@ -1491,6 +1498,7 @@ struct eth_dev_ops {
> > > >  	eth_add_flex_filter_t          add_flex_filter;      /**< add flex filter. */
> > > >  	eth_remove_flex_filter_t       remove_flex_filter;   /**< remove flex
> > > > filter. */
> > > >  	eth_get_flex_filter_t          get_flex_filter;      /**< get flex filter. */
> > > > +	eth_filter_ctrl_t              filter_ctrl;          /**< common filter control*/
> > > >  };
> > > >
> > > >  /**
> > > > @@ -3613,6 +3621,42 @@ int rte_eth_dev_remove_flex_filter(uint8_t
> > > > port_id, uint16_t index);
> > > >  int rte_eth_dev_get_flex_filter(uint8_t port_id, uint16_t index,
> > > >  			struct rte_flex_filter *filter, uint16_t *rx_queue);
> > > >
> > > > +/**
> > > > + * Check whether the filter type is supported on an Ethernet device.
> > > > + * All the supported filter types are defined in 'rte_eth_ctrl.h'.
> > > > + *
> > > > + * @param port_id
> > > > + *   The port identifier of the Ethernet device.
> > > > + * @param filter_type
> > > > + *   filter type.
> > > > + * @return
> > > > + *   - (0) if successful.
> > > > + *   - (-ENOTSUP) if hardware doesn't support this filter type.
> > > > + *   - (-ENODEV) if *port_id* invalid.
> > > > + */
> > > > +int rte_eth_dev_filter_supported(uint8_t port_id, enum
> > > > +rte_filter_type
> > > > filter_type);
> > > > +
> > > > +/**
> > > > + * Take operations to assigned filter type on an Ethernet device.
> > > > + * All the supported operations and filter types are defined in
> > > > 'rte_eth_ctrl.h'.
> > > > + *
> > > > + * @param port_id
> > > > + *   The port identifier of the Ethernet device.
> > > > + * @param filter_type
> > > > + *   filter type.
> > > > +  * @param filter_op
> > > > + *   The operation taken to assigned filter.
> > > > + * @param arg
> > > > + *   A pointer to arguments defined specifically for the operation.
> > > > + * @return
> > > > + *   - (0) if successful.
> > > > + *   - (-ENOTSUP) if hardware doesn't support.
> > > > + *   - (-ENODEV) if *port_id* invalid.
> > > > + *   - others depends on the specific operations implementation.
> > > > + */
> > > > +int rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type
> > > > filter_type,
> > > > +			enum rte_filter_op filter_op, void *arg);
> > > > +
> > > >  #ifdef __cplusplus
> > > >  }
> > > >  #endif
> > > > --
> > > > 1.8.1.4
  
Thomas Monjalon Oct. 16, 2014, 3:11 p.m. UTC | #5
2014-10-10 07:28, De Lara Guarch, Pablo:
> > > > > Define new APIs to support configure multi-kind filters using same
> > > > > APIs
> > > > >  - rte_eth_dev_filter_supported
> > > > >  - rte_eth_dev_filter_ctrl
> > > > >
> > > > > As to the implemetation discussion, please refer to
> > > > > http://dpdk.org/ml/archives/dev/2014-September/005179.html, and
> > > > > control packet filter implementation is based on it.
> > > >
> > > > This patch is also present on the patchset Support flow director
> > > > programming on Fortville.
> > > > Should this patchset be rejected then or just this patch? In second
> > > > case, could you send a v2 without this patch?
> > >
> > > I think this patch does not only present on the flow director patchset, but
> > > also on mac vlan support patchset, vxlan patchset, and so on. All of them
> > are
> > > using the same new filter APIs. If any patchset is applied, others may
> > require
> > > some modification (just as you said to remove this pacth).
> > >
> > Additional, without the patch, this patchset cannot work separately. More
> > than one features depend on the new filter APIs, but none patchset contains
> > the new filter APIs is applied currently.  That's why each patchset has such
> > patch.
> 
> I see, then probably the best idea would have been send this patch separately,
>  and just say that these patchsets depend on this patch, basically because
>  if you try to apply all these patches, you are going to get failures.

Yes, sending a separated patch and explicitly base your patch on this one
would be really easier to understand. And more generally, it's easier when
things are explained. You won't have to pay for the extra words you put in
your cover letter ;)

There is another problem with this patch: there are many versions around with
different logs and even different authors!
  

Patch

diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
index b310f8b..a461c31 100644
--- a/lib/librte_ether/Makefile
+++ b/lib/librte_ether/Makefile
@@ -46,6 +46,7 @@  SRCS-y += rte_ethdev.c
 #
 SYMLINK-y-include += rte_ether.h
 SYMLINK-y-include += rte_ethdev.h
+SYMLINK-y-include += rte_eth_ctrl.h
 
 # this lib depends upon:
 DEPDIRS-y += lib/librte_eal lib/librte_mempool lib/librte_ring lib/librte_mbuf
diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
new file mode 100644
index 0000000..34ab278
--- /dev/null
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -0,0 +1,78 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_ETH_CTRL_H_
+#define _RTE_ETH_CTRL_H_
+
+/**
+ * @file
+ *
+ * Ethernet device features and related data structures used
+ * by control APIs should be defined in this file.
+ *
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Feature filter types
+ */
+enum rte_filter_type {
+	RTE_ETH_FILTER_NONE = 0,
+	RTE_ETH_FILTER_RSS,
+	RTE_ETH_FILTER_FDIR,
+	RTE_ETH_FILTER_MAX,
+};
+
+/**
+ * All generic operations to filters
+ */
+enum rte_filter_op {
+	RTE_ETH_FILTER_OP_NONE = 0, /**< used to check whether the type filter is supported */
+	RTE_ETH_FILTER_OP_ADD,      /**< add filter entry */
+	RTE_ETH_FILTER_OP_UPDATE,   /**< update filter entry */
+	RTE_ETH_FILTER_OP_DELETE,   /**< delete filter entry */
+	RTE_ETH_FILTER_OP_FLUSH,    /**< flush all entries */
+	RTE_ETH_FILTER_OP_GET,      /**< get filter entry */
+	RTE_ETH_FILTER_OP_SET,      /**< configurations */
+	RTE_ETH_FILTER_OP_GET_INFO, /**< get information of filter, such as status or statistics */
+	RTE_ETH_FILTER_OP_MAX,
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ETH_CTRL_H_ */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index b71b679..fdafb15 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3139,3 +3139,35 @@  rte_eth_dev_get_flex_filter(uint8_t port_id, uint16_t index,
 	return (*dev->dev_ops->get_flex_filter)(dev, index, filter,
 						rx_queue);
 }
+
+int
+rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type filter_type)
+{
+	struct rte_eth_dev *dev;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -ENODEV;
+	}
+
+	dev = &rte_eth_devices[port_id];
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
+	return (*dev->dev_ops->filter_ctrl)(dev, filter_type,
+				RTE_ETH_FILTER_OP_NONE, NULL);
+}
+
+int
+rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type filter_type,
+		       enum rte_filter_op filter_op, void *arg)
+{
+	struct rte_eth_dev *dev;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -ENODEV;
+	}
+
+	dev = &rte_eth_devices[port_id];
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
+	return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op, arg);
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 60b24c5..e2ea84a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -177,6 +177,7 @@  extern "C" {
 #include <rte_pci.h>
 #include <rte_mbuf.h>
 #include "rte_ether.h"
+#include "rte_eth_ctrl.h"
 
 /**
  * A structure used to retrieve statistics for an Ethernet port.
@@ -1383,6 +1384,12 @@  typedef int (*eth_get_flex_filter_t)(struct rte_eth_dev *dev,
 			uint16_t *rx_queue);
 /**< @internal Get a flex filter rule on an Ethernet device */
 
+typedef int (*eth_filter_ctrl_t)(struct rte_eth_dev *dev,
+				 enum rte_filter_type filter_type,
+				 enum rte_filter_op filter_op,
+				 void *arg);
+/**< @internal Take operations to assigned filter type on an Ethernet device */
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1491,6 +1498,7 @@  struct eth_dev_ops {
 	eth_add_flex_filter_t          add_flex_filter;      /**< add flex filter. */
 	eth_remove_flex_filter_t       remove_flex_filter;   /**< remove flex filter. */
 	eth_get_flex_filter_t          get_flex_filter;      /**< get flex filter. */
+	eth_filter_ctrl_t              filter_ctrl;          /**< common filter control*/
 };
 
 /**
@@ -3613,6 +3621,42 @@  int rte_eth_dev_remove_flex_filter(uint8_t port_id, uint16_t index);
 int rte_eth_dev_get_flex_filter(uint8_t port_id, uint16_t index,
 			struct rte_flex_filter *filter, uint16_t *rx_queue);
 
+/**
+ * Check whether the filter type is supported on an Ethernet device.
+ * All the supported filter types are defined in 'rte_eth_ctrl.h'.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param filter_type
+ *   filter type.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if hardware doesn't support this filter type.
+ *   - (-ENODEV) if *port_id* invalid.
+ */
+int rte_eth_dev_filter_supported(uint8_t port_id, enum rte_filter_type filter_type);
+
+/**
+ * Take operations to assigned filter type on an Ethernet device.
+ * All the supported operations and filter types are defined in 'rte_eth_ctrl.h'.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param filter_type
+ *   filter type.
+  * @param filter_op
+ *   The operation taken to assigned filter.
+ * @param arg
+ *   A pointer to arguments defined specifically for the operation.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if hardware doesn't support.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - others depends on the specific operations implementation.
+ */
+int rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type filter_type,
+			enum rte_filter_op filter_op, void *arg);
+
 #ifdef __cplusplus
 }
 #endif