From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <helin.zhang@intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
 by dpdk.org (Postfix) with ESMTP id 105894A63
 for <dev@dpdk.org>; Mon, 12 Jan 2015 02:32:15 +0100 (CET)
Received: from orsmga001.jf.intel.com ([10.7.209.18])
 by fmsmga102.fm.intel.com with ESMTP; 11 Jan 2015 17:32:14 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.07,740,1413270000"; d="scan'208";a="635805767"
Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91])
 by orsmga001.jf.intel.com with ESMTP; 11 Jan 2015 17:32:13 -0800
Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by
 PGSMSX104.gar.corp.intel.com (10.221.44.91) with Microsoft SMTP Server (TLS)
 id 14.3.195.1; Mon, 12 Jan 2015 09:32:11 +0800
Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.182]) by
 SHSMSX101.ccr.corp.intel.com ([169.254.1.110]) with mapi id 14.03.0195.001;
 Mon, 12 Jan 2015 09:32:10 +0800
From: "Zhang, Helin" <helin.zhang@intel.com>
To: "Wu, Jingjing" <jingjing.wu@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [PATCH 2/4] e1000: new functions replace old ones for
 ethertype filters
Thread-Index: AQHQH/DwbdZCZSD0NkOuRap05J7F65y7zoVQ
Date: Mon, 12 Jan 2015 01:32:10 +0000
Message-ID: <F35DEAC7BCE34641BA9FAC6BCA4A12E70A7DDF65@SHSMSX104.ccr.corp.intel.com>
References: <1419477270-14611-1-git-send-email-jingjing.wu@intel.com>
 <1419477270-14611-3-git-send-email-jingjing.wu@intel.com>
In-Reply-To: <1419477270-14611-3-git-send-email-jingjing.wu@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [10.239.127.40]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 2/4] e1000: new functions replace old ones
 for ethertype filters
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 12 Jan 2015 01:32:17 -0000



> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, December 25, 2014 11:14 AM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Zhang, Helin; Cao, Waterman
> Subject: [PATCH 2/4] e1000: new functions replace old ones for ethertype =
filters
>=20
> This patch removes old functions which deal with ethertype filters in ixg=
be
> driver.
> It also defines eth_igb_filter_ctrl which is binding to filter_ctrl API, =
and
> ethertype filter can be dealt with through this new entrance.
>=20
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  lib/librte_pmd_e1000/e1000_ethdev.h |  13 ++
>  lib/librte_pmd_e1000/igb_ethdev.c   | 332
> +++++++++++++++++++++++-------------
>  2 files changed, 228 insertions(+), 117 deletions(-)
>=20
> diff --git a/lib/librte_pmd_e1000/e1000_ethdev.h
> b/lib/librte_pmd_e1000/e1000_ethdev.h
> index 71eb5fb..d155e77 100644
> --- a/lib/librte_pmd_e1000/e1000_ethdev.h
> +++ b/lib/librte_pmd_e1000/e1000_ethdev.h
> @@ -132,6 +132,15 @@ struct e1000_vf_info {  };
>=20
>  /*
> + * Structure to store filters' info.
> + */
> +struct e1000_filter_info {
> +	uint8_t ethertype_mask; /* Bit mask for every used ethertype filter */
> +	/* store used ethertype filters*/
> +	uint16_t ethertype_filters[E1000_MAX_ETQF_FILTERS];
> +};
> +
> +/*
>   * Structure to store private data for each driver instance (for each po=
rt).
>   */
>  struct e1000_adapter {
> @@ -140,6 +149,7 @@ struct e1000_adapter {
>  	struct e1000_interrupt  intr;
>  	struct e1000_vfta       shadow_vfta;
>  	struct e1000_vf_info    *vfdata;
> +	struct e1000_filter_info filter;
>  };
>=20
>  #define E1000_DEV_PRIVATE_TO_HW(adapter) \ @@ -157,6 +167,9 @@
> struct e1000_adapter {  #define E1000_DEV_PRIVATE_TO_P_VFDATA(adapter)
> \
>          (&((struct e1000_adapter *)adapter)->vfdata)
>=20
> +#define E1000_DEV_PRIVATE_TO_FILTER_INFO(adapter) \
> +	(&((struct e1000_adapter *)adapter)->filter)
> +
>  /*
>   * RX/TX IGB function prototypes
>   */
> diff --git a/lib/librte_pmd_e1000/igb_ethdev.c
> b/lib/librte_pmd_e1000/igb_ethdev.c
> index 873d65e..3c549aa 100644
> --- a/lib/librte_pmd_e1000/igb_ethdev.c
> +++ b/lib/librte_pmd_e1000/igb_ethdev.c
> @@ -154,14 +154,6 @@ static int eth_igb_add_syn_filter(struct rte_eth_dev
> *dev,  static int eth_igb_remove_syn_filter(struct rte_eth_dev *dev);  st=
atic
> int eth_igb_get_syn_filter(struct rte_eth_dev *dev,
>  			struct rte_syn_filter *filter, uint16_t *rx_queue); -static int
> eth_igb_add_ethertype_filter(struct rte_eth_dev *dev,
> -			uint16_t index,
> -			struct rte_ethertype_filter *filter, uint16_t rx_queue);
> -static int eth_igb_remove_ethertype_filter(struct rte_eth_dev *dev,
> -			uint16_t index);
> -static int eth_igb_get_ethertype_filter(struct rte_eth_dev *dev,
> -			uint16_t index,
> -			struct rte_ethertype_filter *filter, uint16_t *rx_queue);
>  static int eth_igb_add_2tuple_filter(struct rte_eth_dev *dev,
>  			uint16_t index,
>  			struct rte_2tuple_filter *filter, uint16_t rx_queue); @@ -186,6
> +178,18 @@ static int eth_igb_remove_5tuple_filter(struct rte_eth_dev *de=
v,
> static int eth_igb_get_5tuple_filter(struct rte_eth_dev *dev,
>  			uint16_t index,
>  			struct rte_5tuple_filter *filter, uint16_t *rx_queue);
> +static int igb_add_del_ethertype_filter(struct rte_eth_dev *dev,
> +			struct rte_eth_ethertype_filter *filter,
> +			bool add);
> +static int igb_ethertype_filter_handle(struct rte_eth_dev *dev,
> +				enum rte_filter_op filter_op,
> +				void *arg);
> +static int igb_get_ethertype_filter(struct rte_eth_dev *dev,
> +			struct rte_eth_ethertype_filter *filter); static int
> +eth_igb_filter_ctrl(struct rte_eth_dev *dev,
> +		     enum rte_filter_type filter_type,
> +		     enum rte_filter_op filter_op,
> +		     void *arg);
>=20
>  /*
>   * Define VF Stats MACRO for Non "cleared on read" register @@ -264,9
> +268,6 @@ static struct eth_dev_ops eth_igb_ops =3D {
>  	.add_syn_filter          =3D eth_igb_add_syn_filter,
>  	.remove_syn_filter       =3D eth_igb_remove_syn_filter,
>  	.get_syn_filter          =3D eth_igb_get_syn_filter,
> -	.add_ethertype_filter    =3D eth_igb_add_ethertype_filter,
> -	.remove_ethertype_filter =3D eth_igb_remove_ethertype_filter,
> -	.get_ethertype_filter    =3D eth_igb_get_ethertype_filter,
>  	.add_2tuple_filter       =3D eth_igb_add_2tuple_filter,
>  	.remove_2tuple_filter    =3D eth_igb_remove_2tuple_filter,
>  	.get_2tuple_filter       =3D eth_igb_get_2tuple_filter,
> @@ -276,6 +277,7 @@ static struct eth_dev_ops eth_igb_ops =3D {
>  	.add_5tuple_filter       =3D eth_igb_add_5tuple_filter,
>  	.remove_5tuple_filter    =3D eth_igb_remove_5tuple_filter,
>  	.get_5tuple_filter       =3D eth_igb_get_5tuple_filter,
> +	.filter_ctrl             =3D eth_igb_filter_ctrl,
>  };
>=20
>  /*
> @@ -2388,7 +2390,7 @@ eth_igb_rss_reta_query(struct rte_eth_dev *dev,
>  #define MAC_TYPE_FILTER_SUP(type)    do {\
>  	if ((type) !=3D e1000_82580 && (type) !=3D e1000_i350 &&\
>  		(type) !=3D e1000_82576)\
> -		return -ENOSYS;\
> +		return -ENOTSUP;\
>  } while (0)
>=20
>  /*
> @@ -2485,111 +2487,6 @@ eth_igb_get_syn_filter(struct rte_eth_dev *dev,
>  	return -ENOENT;
>  }
>=20
> -/*
> - * add an ethertype filter
> - *
> - * @param
> - * dev: Pointer to struct rte_eth_dev.
> - * index: the index the filter allocates.
> - * filter: ponter to the filter that will be added.
> - * rx_queue: the queue id the filter assigned to.
> - *
> - * @return
> - *    - On success, zero.
> - *    - On failure, a negative value.
> - */
> -static int
> -eth_igb_add_ethertype_filter(struct rte_eth_dev *dev, uint16_t index,
> -			struct rte_ethertype_filter *filter, uint16_t rx_queue)
> -{
> -	struct e1000_hw *hw =3D
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> -	uint32_t etqf;
> -
> -	MAC_TYPE_FILTER_SUP(hw->mac.type);
> -
> -	if (index >=3D E1000_MAX_ETQF_FILTERS || rx_queue >=3D
> IGB_MAX_RX_QUEUE_NUM)
> -		return -EINVAL;
> -
> -	etqf =3D E1000_READ_REG(hw, E1000_ETQF(index));
> -	if (etqf & E1000_ETQF_FILTER_ENABLE)
> -		return -EINVAL;  /* filter index is in use. */
> -	else
> -		etqf =3D 0;
> -
> -	etqf |=3D E1000_ETQF_FILTER_ENABLE | E1000_ETQF_QUEUE_ENABLE;
> -	etqf |=3D (uint32_t)(filter->ethertype & E1000_ETQF_ETHERTYPE);
> -	etqf |=3D rx_queue << E1000_ETQF_QUEUE_SHIFT;
> -
> -	if (filter->priority_en) {
> -		PMD_INIT_LOG(ERR, "vlan and priority (%d) is not supported"
> -			" in E1000.", filter->priority);
> -		return -EINVAL;
> -	}
> -
> -	E1000_WRITE_REG(hw, E1000_ETQF(index), etqf);
> -	return 0;
> -}
> -
> -/*
> - * remove an ethertype filter
> - *
> - * @param
> - * dev: Pointer to struct rte_eth_dev.
> - * index: the index the filter allocates.
> - *
> - * @return
> - *    - On success, zero.
> - *    - On failure, a negative value.
> - */
> -static int
> -eth_igb_remove_ethertype_filter(struct rte_eth_dev *dev, uint16_t index)=
 -{
> -	struct e1000_hw *hw =3D
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> -
> -	MAC_TYPE_FILTER_SUP(hw->mac.type);
> -
> -	if (index >=3D E1000_MAX_ETQF_FILTERS)
> -		return -EINVAL;
> -
> -	E1000_WRITE_REG(hw, E1000_ETQF(index), 0);
> -	return 0;
> -}
> -
> -/*
> - * get an ethertype filter
> - *
> - * @param
> - * dev: Pointer to struct rte_eth_dev.
> - * index: the index the filter allocates.
> - * filter: ponter to the filter that will be gotten.
> - * *rx_queue: the ponited of the queue id the filter assigned to.
> - *
> - * @return
> - *    - On success, zero.
> - *    - On failure, a negative value.
> - */
> -static int
> -eth_igb_get_ethertype_filter(struct rte_eth_dev *dev, uint16_t index,
> -			struct rte_ethertype_filter *filter, uint16_t *rx_queue)
> -{
> -	struct e1000_hw *hw =3D
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> -	uint32_t etqf;
> -
> -	MAC_TYPE_FILTER_SUP(hw->mac.type);
> -
> -	if (index >=3D E1000_MAX_ETQF_FILTERS)
> -		return -EINVAL;
> -
> -	etqf =3D E1000_READ_REG(hw, E1000_ETQF(index));
> -	if (etqf & E1000_ETQF_FILTER_ENABLE) {
> -		filter->ethertype =3D etqf & E1000_ETQF_ETHERTYPE;
> -		filter->priority_en =3D 0;
> -		*rx_queue =3D (etqf & E1000_ETQF_QUEUE) >>
> E1000_ETQF_QUEUE_SHIFT;
> -		return 0;
> -	}
> -	return -ENOENT;
> -}
> -
>  #define MAC_TYPE_FILTER_SUP_EXT(type)    do {\
>  	if ((type) !=3D e1000_82580 && (type) !=3D e1000_i350)\
>  		return -ENOSYS; \
> @@ -3149,6 +3046,207 @@ eth_igb_mtu_set(struct rte_eth_dev *dev,
> uint16_t mtu)
>  	return 0;
>  }
>=20
> +static inline int
> +igb_ethertype_filter_lookup(struct e1000_filter_info *filter_info,
> +			uint16_t ethertype)
> +{
> +	int i;
> +
> +	for (i =3D 0; i < E1000_MAX_ETQF_FILTERS; i++) {
> +		if (filter_info->ethertype_filters[i] =3D=3D ethertype &&
> +		    (filter_info->ethertype_mask & (1 << i)))
> +			return i;
> +	}
> +	return -1;
> +}
> +
> +static inline int
> +igb_ethertype_filter_insert(struct e1000_filter_info *filter_info,
> +			uint16_t ethertype)
> +{
> +	int i;
> +
> +	for (i =3D 0; i < E1000_MAX_ETQF_FILTERS; i++) {
> +		if (!(filter_info->ethertype_mask & (1 << i))) {
> +			filter_info->ethertype_mask |=3D 1 << i;
> +			filter_info->ethertype_filters[i] =3D ethertype;
> +			return i;
> +		}
> +	}
> +	return -1;
> +}
> +
> +static inline int
> +igb_ethertype_filter_remove(struct e1000_filter_info *filter_info,
> +			uint8_t idx)
> +{
> +	if (idx >=3D E1000_MAX_ETQF_FILTERS)
> +		return -1;
> +	filter_info->ethertype_mask &=3D ~(1 << idx);
> +	filter_info->ethertype_filters[idx] =3D 0;
> +	return idx;
> +}
> +
> +
> +static int
> +igb_add_del_ethertype_filter(struct rte_eth_dev *dev,
> +			struct rte_eth_ethertype_filter *filter,
> +			bool add)
> +{
> +	struct e1000_hw *hw =3D
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct e1000_filter_info *filter_info =3D
> +		E1000_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
> +	uint32_t etqf =3D 0;
> +	int ret;
> +
> +	if (filter->ether_type =3D=3D ETHER_TYPE_IPv4 ||
> +		filter->ether_type =3D=3D ETHER_TYPE_IPv6) {
> +		PMD_DRV_LOG(ERR, "unsupported ether_type(0x%04x) in"
> +			" ethertype filter.", filter->ether_type);
> +		return -EINVAL;
> +	}
> +
> +	if (filter->flags & RTE_ETHTYPE_FLAGS_MAC) {
> +		PMD_DRV_LOG(ERR, "mac compare is unsupported.");
> +		return -EINVAL;
> +	}
> +	if (filter->flags & RTE_ETHTYPE_FLAGS_DROP) {
> +		PMD_DRV_LOG(ERR, "drop option is unsupported.");
> +		return -EINVAL;
> +	}
> +
> +	ret =3D igb_ethertype_filter_lookup(filter_info, filter->ether_type);
> +	if (ret >=3D 0 && add) {
> +		PMD_DRV_LOG(WARNING, "ethertype (0x%04x) filter exists.",
> +			    filter->ether_type);
> +		return ret;
Will the value be returned to the top-most user application? If yes, what's=
 the
meaning of the return value for application?
It might be better to just return 0 or negative values to the top-most inte=
rfaces.

> +	}
> +	if (ret < 0 && !add) {
> +		PMD_DRV_LOG(ERR, "ethertype (0x%04x) filter doesn't exist.",
> +			    filter->ether_type);
> +		return -ENOENT;
> +	}
> +
> +	if (add) {
> +		ret =3D igb_ethertype_filter_insert(filter_info,
> +			filter->ether_type);
> +		if (ret < 0) {
> +			PMD_DRV_LOG(ERR, "ethertype filters are full.");
> +			return -ENOSYS;
> +		}
> +
> +		etqf |=3D E1000_ETQF_FILTER_ENABLE |
> E1000_ETQF_QUEUE_ENABLE;
> +		etqf |=3D (uint32_t)(filter->ether_type & E1000_ETQF_ETHERTYPE);
> +		etqf |=3D filter->queue << E1000_ETQF_QUEUE_SHIFT;
> +	} else {
> +		ret =3D igb_ethertype_filter_remove(filter_info, (uint8_t)ret);
> +		if (ret < 0)
> +			return -ENOSYS;
> +	}
> +	E1000_WRITE_REG(hw, E1000_ETQF(ret), etqf);
> +	E1000_WRITE_FLUSH(hw);
> +
> +	return 0;
> +}
> +
> +static int
> +igb_get_ethertype_filter(struct rte_eth_dev *dev,
> +			struct rte_eth_ethertype_filter *filter) {
> +	struct e1000_hw *hw =3D
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct e1000_filter_info *filter_info =3D
> +		E1000_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
> +	uint32_t etqf;
> +	int ret;
> +
> +	ret =3D igb_ethertype_filter_lookup(filter_info, filter->ether_type);
> +	if (ret < 0) {
> +		PMD_DRV_LOG(ERR, "ethertype (0x%04x) filter doesn't exist.",
> +			    filter->ether_type);
> +		return -ENOENT;
> +	}
> +
> +	etqf =3D E1000_READ_REG(hw, E1000_ETQF(ret));
> +	if (etqf & E1000_ETQF_FILTER_ENABLE) {
> +		filter->ether_type =3D etqf & E1000_ETQF_ETHERTYPE;
> +		filter->flags =3D 0;
> +		filter->queue =3D (etqf & E1000_ETQF_QUEUE) >>
> +				E1000_ETQF_QUEUE_SHIFT;
> +		return 0;
> +	}
> +
> +	return -ENOENT;
> +}
> +
> +/*
> + * igb_ethertype_filter_handle - Handle operations for ethertype filter.
> + * @dev: pointer to rte_eth_dev structure
> + * @filter_op:operation will be taken.
> + * @arg: a pointer to specific structure corresponding to the filter_op
> +*/ static int igb_ethertype_filter_handle(struct rte_eth_dev *dev,
> +				enum rte_filter_op filter_op,
> +				void *arg)
> +{
> +	struct e1000_hw *hw =3D
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	int ret;
> +
> +	MAC_TYPE_FILTER_SUP(hw->mac.type);
> +
> +	if (filter_op =3D=3D RTE_ETH_FILTER_NOP)
> +		return 0;
> +
> +	if (arg =3D=3D NULL) {
> +		PMD_DRV_LOG(ERR, "arg shouldn't be NULL for operation %u.",
> +			    filter_op);
> +		return -EINVAL;
> +	}
> +
> +	switch (filter_op) {
> +	case RTE_ETH_FILTER_ADD:
> +		ret =3D igb_add_del_ethertype_filter(dev,
> +			(struct rte_eth_ethertype_filter *)arg,
> +			TRUE);
> +		break;
> +	case RTE_ETH_FILTER_DELETE:
> +		ret =3D igb_add_del_ethertype_filter(dev,
> +			(struct rte_eth_ethertype_filter *)arg,
> +			FALSE);
> +		break;
> +	case RTE_ETH_FILTER_GET:
> +		ret =3D igb_get_ethertype_filter(dev,
> +			(struct rte_eth_ethertype_filter *)arg);
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "unsupported operation %u.", filter_op);
> +		ret =3D -EINVAL;
> +		break;
> +	}
> +	return ret;
> +}
> +
> +static int
> +eth_igb_filter_ctrl(struct rte_eth_dev *dev,
> +		     enum rte_filter_type filter_type,
> +		     enum rte_filter_op filter_op,
> +		     void *arg)
> +{
> +	int ret =3D -EINVAL;
> +
> +	switch (filter_type) {
> +	case RTE_ETH_FILTER_ETHERTYPE:
> +		ret =3D igb_ethertype_filter_handle(dev, filter_op, arg);
> +		break;
> +	default:
> +		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
> +							filter_type);
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
>  static struct rte_driver pmd_igb_drv =3D {
>  	.type =3D PMD_PDEV,
>  	.init =3D rte_igb_pmd_init,
> --
> 1.9.3