> -----Original Message-----
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 30, 2016 3:53 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Zhao1, Wei <wei.zh...@intel.com>
> Subject: [dpdk-dev] [PATCH v2 03/18] net/ixgbe: store L2 tunnel filter
>
> Add support for storing L2 tunnel filter in SW.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo...@intel.com>
> Signed-off-by: Wei Zhao <wei.zh...@intel.com>
> ---
>
>
> +static inline struct ixgbe_l2_tn_filter *
> +ixgbe_l2_tn_filter_lookup(struct ixgbe_l2_tn_info *l2_tn_info,
> + struct ixgbe_l2_tn_key *key)
> +{
> + int ret = 0;
Needles initialization since ret will be set bellow, it can be removed.
And this initialization can be removed in other filter lookup/insert/delete
functions, either.
> +
> + ret = rte_hash_lookup(l2_tn_info->hash_handle, (const void *)key);
> + if (ret < 0)
> + return NULL;
> +
> + return l2_tn_info->hash_map[ret];
> +}
> +
> +static inline int
> +ixgbe_insert_l2_tn_filter(struct ixgbe_l2_tn_info *l2_tn_info,
> + struct ixgbe_l2_tn_filter *l2_tn_filter) {
> + int ret = 0;
> +
> + ret = rte_hash_add_key(l2_tn_info->hash_handle,
> + &l2_tn_filter->key);
> +
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR,
> + "Failed to insert L2 tunnel filter"
> + " to hash table %d!",
> + ret);
> + return ret;
> + }
> +
> + l2_tn_info->hash_map[ret] = l2_tn_filter;
> +
> + TAILQ_INSERT_TAIL(&l2_tn_info->l2_tn_list, l2_tn_filter, entries);
> +
> + return 0;
> +}
> /* Add l2 tunnel filter */
> static int
> ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
> struct rte_eth_l2_tunnel_conf *l2_tunnel) {
> int ret = 0;
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
> + struct ixgbe_l2_tn_key key;
> + struct ixgbe_l2_tn_filter *node;
> +
> + key.l2_tn_type = l2_tunnel->l2_tunnel_type;
> + key.tn_id = l2_tunnel->tunnel_id;
> +
> + node = ixgbe_l2_tn_filter_lookup(l2_tn_info, &key);
> +
> + if (node) {
> + PMD_DRV_LOG(ERR, "The L2 tunnel filter already exists!");
> + return -EINVAL;
> + }
> +
> + node = rte_zmalloc("ixgbe_l2_tn",
> + sizeof(struct ixgbe_l2_tn_filter),
> + 0);
> + if (!node)
> + return -ENOMEM;
> +
> + (void)rte_memcpy(&node->key,
> + &key,
> + sizeof(struct ixgbe_l2_tn_key));
> + node->pool = l2_tunnel->pool;
> + ret = ixgbe_insert_l2_tn_filter(l2_tn_info, node);
> + if (ret < 0) {
> + rte_free(node);
> + return ret;
> + }
>
> switch (l2_tunnel->l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> @@ -7195,6 +7340,9 @@ ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev
> *dev,
> break;
> }
>
> + if (ret < 0)
> + (void)ixgbe_remove_l2_tn_filter(l2_tn_info, &key);
Some confusion, why remove l2_tn_filter here? Can this check be moved before
ixgbe_insert_l2_tn_filter?
> +
> return ret;
> }
>
> @@ -7204,6 +7352,15 @@ ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev
> *dev,
> struct rte_eth_l2_tunnel_conf *l2_tunnel) {
> int ret = 0;
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
> + struct ixgbe_l2_tn_key key;
> +
> + key.l2_tn_type = l2_tunnel->l2_tunnel_type;
> + key.tn_id = l2_tunnel->tunnel_id;
> + ret = ixgbe_remove_l2_tn_filter(l2_tn_info, &key);
> + if (ret < 0)
> + return ret;
>
> switch (l2_tunnel->l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 8310220..6663fc9 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -132,6 +132,7 @@
> #define IXGBE_RX_VEC_START
> RTE_INTR_VEC_RXTX_OFFSET
>
> #define IXGBE_MAX_FDIR_FILTER_NUM (1024 * 32)
> +#define IXGBE_MAX_L2_TN_FILTER_NUM 128
>
> /*
> * Information about the fdir mode.
> @@ -283,6 +284,25 @@ struct ixgbe_filter_info {
> uint32_t syn_info;
> };
>
> +struct ixgbe_l2_tn_key {
> + enum rte_eth_tunnel_type l2_tn_type;
> + uint32_t tn_id;
> +};
> +
> +struct ixgbe_l2_tn_filter {
> + TAILQ_ENTRY(ixgbe_l2_tn_filter) entries;
> + struct ixgbe_l2_tn_key key;
> + uint32_t pool;
Maybe more comments here will be helpful.
> +};
> +
