On Tue, Dec 27, 2016 at 02:26:09PM +0800, Beilei Xing wrote:
> Currently there's no tunnel filter stored in SW.
> This patch stores tunnel filter in SW with cuckoo
> hash, also adds protection if a tunnel filter has
> been added.
>
> Signed-off-by: Beilei Xing <beilei.x...@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 167
> ++++++++++++++++++++++++++++++++++++++++-
> drivers/net/i40e/i40e_ethdev.h | 27 +++++++
> 2 files changed, 191 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 80dd8d7..c012d5d 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
[...]
> @@ -1267,6 +1314,7 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
> hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> pci_dev = dev->pci_dev;
> ethertype_rule = &pf->ethertype;
> + tunnel_rule = &pf->tunnel;
>
> if (hw->adapter_stopped == 0)
> i40e_dev_close(dev);
> @@ -1283,6 +1331,17 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
> rte_free(p_ethertype);
> }
>
> + /* Remove all tunnel director rules and hash */
> + if (tunnel_rule->hash_map)
> + rte_free(tunnel_rule->hash_map);
> + if (tunnel_rule->hash_table)
> + rte_hash_free(tunnel_rule->hash_table);
> +
> + while ((p_tunnel = TAILQ_FIRST(&tunnel_rule->tunnel_list))) {
There is a redundant pair of parentheses, or you should compare with
NULL.
> + TAILQ_REMOVE(&tunnel_rule->tunnel_list, p_tunnel, rules);
> + rte_free(p_tunnel);
> + }
> +
> dev->dev_ops = NULL;
> dev->rx_pkt_burst = NULL;
> dev->tx_pkt_burst = NULL;
> @@ -6482,6 +6541,81 @@ i40e_dev_get_filter_type(uint16_t filter_type,
> uint16_t *flag)
> return 0;
> }
>
> +/* Convert tunnel filter structure */
> +static int
> +i40e_tunnel_filter_convert(struct
> i40e_aqc_add_remove_cloud_filters_element_data
> + *cld_filter,
> + struct i40e_tunnel_filter *tunnel_filter)
> +{
> + ether_addr_copy((struct ether_addr *)&cld_filter->outer_mac,
> + (struct ether_addr *)&tunnel_filter->input.outer_mac);
> + ether_addr_copy((struct ether_addr *)&cld_filter->inner_mac,
> + (struct ether_addr *)&tunnel_filter->input.inner_mac);
> + tunnel_filter->input.inner_vlan = cld_filter->inner_vlan;
> + tunnel_filter->input.flags = cld_filter->flags;
> + tunnel_filter->input.tenant_id = cld_filter->tenant_id;
> + tunnel_filter->queue = cld_filter->queue_number;
> +
> + return 0;
> +}
> +
> +/* Check if there exists the tunnel filter */
> +static struct i40e_tunnel_filter *
> +i40e_sw_tunnel_filter_lookup(struct i40e_tunnel_rule *tunnel_rule,
> + const struct i40e_tunnel_filter_input *input)
> +{
> + int ret = 0;
> +
The initialization is meaningless, as it will be written by the below
assignment unconditionally.
> + ret = rte_hash_lookup(tunnel_rule->hash_table, (const void *)input);
> + if (ret < 0)
> + return NULL;
> +
> + return tunnel_rule->hash_map[ret];
> +}
> +
> +/* Add a tunnel filter into the SW list */
> +static int
> +i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
> + struct i40e_tunnel_filter *tunnel_filter)
> +{
> + struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
> + int ret = 0;
> +
Same here.
> + ret = rte_hash_add_key(tunnel_rule->hash_table,
> + &tunnel_filter->input);
> + if (ret < 0)
> + PMD_DRV_LOG(ERR,
> + "Failed to insert tunnel filter to hash table %d!",
> + ret);
Function should return when ret < 0.
> + tunnel_rule->hash_map[ret] = tunnel_filter;
> +
> + TAILQ_INSERT_TAIL(&tunnel_rule->tunnel_list, tunnel_filter, rules);
> +
> + return 0;
> +}
> +
> +/* Delete a tunnel filter from the SW list */
> +static int
> +i40e_sw_tunnel_filter_del(struct i40e_pf *pf,
> + struct i40e_tunnel_filter *tunnel_filter)
> +{
> + struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
> + int ret = 0;
> +
Same here.
> + ret = rte_hash_del_key(tunnel_rule->hash_table,
> + &tunnel_filter->input);
> + if (ret < 0)
> + PMD_DRV_LOG(ERR,
> + "Failed to delete tunnel filter to hash table %d!",
> + ret);
Function should return when ret < 0.
> + tunnel_rule->hash_map[ret] = NULL;
> +
> + TAILQ_REMOVE(&tunnel_rule->tunnel_list, tunnel_filter, rules);
> + rte_free(tunnel_filter);
> +
> + return 0;
> +}
> +
> static int
> i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
> struct rte_eth_tunnel_filter_conf *tunnel_filter,
> @@ -6497,6 +6631,8 @@ i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
> struct i40e_vsi *vsi = pf->main_vsi;
> struct i40e_aqc_add_remove_cloud_filters_element_data *cld_filter;
> struct i40e_aqc_add_remove_cloud_filters_element_data *pfilter;
> + struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
> + struct i40e_tunnel_filter *tunnel, *node;
>
> cld_filter = rte_zmalloc("tunnel_filter",
> sizeof(struct i40e_aqc_add_remove_cloud_filters_element_data),
> @@ -6559,11 +6695,36 @@ i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
> pfilter->tenant_id = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> pfilter->queue_number = rte_cpu_to_le_16(tunnel_filter->queue_id);
>
> - if (add)
> + tunnel = rte_zmalloc("tunnel_filter", sizeof(*tunnel), 0);
> + i40e_tunnel_filter_convert(cld_filter, tunnel);
> + node = i40e_sw_tunnel_filter_lookup(tunnel_rule, &tunnel->input);
> + if (add && node) {
> + PMD_DRV_LOG(ERR, "Conflict with existing tunnel rules!");
> + rte_free(tunnel);
> + return -EINVAL;
> + } else if (!add && !node) {
When `if (add && node)' is true, function will return. There is no need
to use `else' here.
Best regards,
Tiwei Bie
