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

["Wu, Jingjing" <jingjing.wu@intel.com>]

> -----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