Re: [dpdk-dev] [PATCH 01/14] ethdev: remove legacy MACVLAN filter type support

["Guo, Jia" <jia.guo@intel.com>]

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Sunday, October 18, 2020 10:09 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil
> Horman <nhorman@tuxdriver.com>; Guo, Jia <jia.guo@intel.com>; Rasesh
> Mody <rmody@marvell.com>; Shahed Shaikh <shshaikh@marvell.com>;
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Thomas Monjalon
> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH 01/14] ethdev: remove legacy MACVLAN filter type support
> 
> RTE flow API should be used for filtering.

Look like each patch in the patch set remove one specific legacy filter,  so I think the removing
filter info is need to show in the commit log to make it more clear, please add in the next version.

> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  app/test-pmd/cmdline.c               | 110 -------------------------
>  doc/guides/rel_notes/deprecation.rst |   2 +-
>  drivers/net/i40e/i40e_ethdev.c       | 116 ---------------------------
>  drivers/net/qede/qede_filter.c       |   1 -
>  drivers/net/sfc/sfc_ethdev.c         |   3 -
>  lib/librte_ethdev/rte_eth_ctrl.h     |  11 ---
>  6 files changed, 1 insertion(+), 242 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 07ee4e4e13..bb0be8cf42 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -559,11 +559,6 @@ static void cmd_help_long_parsed(void
> *parsed_result,
>  			"set port (port_id) vf (vf_id) rx|tx on|off\n"
>  			"    Enable/Disable a VF receive/tranmit from a
> port\n\n"
> 
> -			"set port (port_id) vf (vf_id) (mac_addr)"
> -			" (exact-mac#exact-mac-vlan#hashmac|hashmac-
> vlan) on|off\n"
> -			"   Add/Remove unicast or multicast MAC addr filter"
> -			" for a VF.\n\n"
> -
>  			"set port (port_id) vf (vf_id) rxmode
> (AUPE|ROPE|BAM"
>  			"|MPE) (on|off)\n"
>  			"    AUPE:accepts untagged VLAN;"
> @@ -8757,110 +8752,6 @@ cmdline_parse_inst_t
> cmd_set_uc_all_hash_filter = {
>  	},
>  };
> 
> -/* *** CONFIGURE MACVLAN FILTER FOR VF(s) *** */ -struct
> cmd_set_vf_macvlan_filter {
> -	cmdline_fixed_string_t set;
> -	cmdline_fixed_string_t port;
> -	portid_t port_id;
> -	cmdline_fixed_string_t vf;
> -	uint8_t vf_id;
> -	struct rte_ether_addr address;
> -	cmdline_fixed_string_t filter_type;
> -	cmdline_fixed_string_t mode;
> -};
> -
> -static void
> -cmd_set_vf_macvlan_parsed(void *parsed_result,
> -		       __rte_unused struct cmdline *cl,
> -		       __rte_unused void *data)
> -{
> -	int is_on, ret = 0;
> -	struct cmd_set_vf_macvlan_filter *res = parsed_result;
> -	struct rte_eth_mac_filter filter;
> -
> -	memset(&filter, 0, sizeof(struct rte_eth_mac_filter));
> -
> -	rte_memcpy(&filter.mac_addr, &res->address,
> RTE_ETHER_ADDR_LEN);
> -
> -	/* set VF MAC filter */
> -	filter.is_vf = 1;
> -
> -	/* set VF ID */
> -	filter.dst_id = res->vf_id;
> -
> -	if (!strcmp(res->filter_type, "exact-mac"))
> -		filter.filter_type = RTE_MAC_PERFECT_MATCH;
> -	else if (!strcmp(res->filter_type, "exact-mac-vlan"))
> -		filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> -	else if (!strcmp(res->filter_type, "hashmac"))
> -		filter.filter_type = RTE_MAC_HASH_MATCH;
> -	else if (!strcmp(res->filter_type, "hashmac-vlan"))
> -		filter.filter_type = RTE_MACVLAN_HASH_MATCH;
> -
> -	is_on = (strcmp(res->mode, "on") == 0) ? 1 : 0;
> -
> -	if (is_on)
> -		ret = rte_eth_dev_filter_ctrl(res->port_id,
> -					RTE_ETH_FILTER_MACVLAN,
> -					RTE_ETH_FILTER_ADD,
> -					 &filter);
> -	else
> -		ret = rte_eth_dev_filter_ctrl(res->port_id,
> -					RTE_ETH_FILTER_MACVLAN,
> -					RTE_ETH_FILTER_DELETE,
> -					&filter);
> -
> -	if (ret < 0)
> -		printf("bad set MAC hash parameter, return code = %d\n",
> ret);
> -
> -}
> -
> -cmdline_parse_token_string_t cmd_set_vf_macvlan_set =
> -	TOKEN_STRING_INITIALIZER(struct cmd_set_vf_macvlan_filter,
> -				 set, "set");
> -cmdline_parse_token_string_t cmd_set_vf_macvlan_port =
> -	TOKEN_STRING_INITIALIZER(struct cmd_set_vf_macvlan_filter,
> -				 port, "port");
> -cmdline_parse_token_num_t cmd_set_vf_macvlan_portid =
> -	TOKEN_NUM_INITIALIZER(struct cmd_set_vf_macvlan_filter,
> -			      port_id, UINT16);
> -cmdline_parse_token_string_t cmd_set_vf_macvlan_vf =
> -	TOKEN_STRING_INITIALIZER(struct cmd_set_vf_macvlan_filter,
> -				 vf, "vf");
> -cmdline_parse_token_num_t cmd_set_vf_macvlan_vf_id =
> -	TOKEN_NUM_INITIALIZER(struct cmd_set_vf_macvlan_filter,
> -				vf_id, UINT8);
> -cmdline_parse_token_etheraddr_t cmd_set_vf_macvlan_mac =
> -	TOKEN_ETHERADDR_INITIALIZER(struct cmd_set_vf_macvlan_filter,
> -				address);
> -cmdline_parse_token_string_t cmd_set_vf_macvlan_filter_type =
> -	TOKEN_STRING_INITIALIZER(struct cmd_set_vf_macvlan_filter,
> -				filter_type, "exact-mac#exact-mac-vlan"
> -				"#hashmac#hashmac-vlan");
> -cmdline_parse_token_string_t cmd_set_vf_macvlan_mode =
> -	TOKEN_STRING_INITIALIZER(struct cmd_set_vf_macvlan_filter,
> -				 mode, "on#off");
> -
> -cmdline_parse_inst_t cmd_set_vf_macvlan_filter = {
> -	.f = cmd_set_vf_macvlan_parsed,
> -	.data = NULL,
> -	.help_str = "set port <port_id> vf <vf_id> <mac_addr> "
> -		"exact-mac|exact-mac-vlan|hashmac|hashmac-vlan on|off:
> "
> -		"Exact match rule: exact match of MAC or MAC and VLAN; "
> -		"hash match rule: hash match of MAC and exact match of
> VLAN",
> -	.tokens = {
> -		(void *)&cmd_set_vf_macvlan_set,
> -		(void *)&cmd_set_vf_macvlan_port,
> -		(void *)&cmd_set_vf_macvlan_portid,
> -		(void *)&cmd_set_vf_macvlan_vf,
> -		(void *)&cmd_set_vf_macvlan_vf_id,
> -		(void *)&cmd_set_vf_macvlan_mac,
> -		(void *)&cmd_set_vf_macvlan_filter_type,
> -		(void *)&cmd_set_vf_macvlan_mode,
> -		NULL,
> -	},
> -};
> -
>  /* *** CONFIGURE VF TRAFFIC CONTROL *** */  struct cmd_set_vf_traffic {
>  	cmdline_fixed_string_t set;
> @@ -20041,7 +19932,6 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_set_uc_hash_filter,
>  	(cmdline_parse_inst_t *)&cmd_set_uc_all_hash_filter,
>  	(cmdline_parse_inst_t *)&cmd_vf_mac_addr_filter,
> -	(cmdline_parse_inst_t *)&cmd_set_vf_macvlan_filter,
>  	(cmdline_parse_inst_t *)&cmd_queue_rate_limit,
>  	(cmdline_parse_inst_t *)&cmd_tunnel_filter,
>  	(cmdline_parse_inst_t *)&cmd_tunnel_udp_config, diff --git
> a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index ff39243f32..223ff7661f 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -108,7 +108,7 @@ Deprecation Notices
> 
>  * ethdev: the legacy filter API, including
>    ``rte_eth_dev_filter_supported()``, ``rte_eth_dev_filter_ctrl()`` as well
> -  as filter types MACVLAN, ETHERTYPE, FLEXIBLE, SYN, NTUPLE, TUNNEL,
> FDIR,
> +  as filter types ETHERTYPE, FLEXIBLE, SYN, NTUPLE, TUNNEL, FDIR,
>    HASH and L2_TUNNEL, is superseded by the generic flow API (rte_flow) in
>    PMDs that implement the latter.
>    The legacy API will be removed in DPDK 20.11.
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 4778aaf299..217a7bbbd8 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -4386,119 +4386,6 @@ i40e_macaddr_remove(struct rte_eth_dev *dev,
> uint32_t index)
>  	}
>  }
> 
> -/* Set perfect match or hash match of MAC and VLAN for a VF */ -static int -
> i40e_vf_mac_filter_set(struct i40e_pf *pf,
> -		 struct rte_eth_mac_filter *filter,
> -		 bool add)
> -{
> -	struct i40e_hw *hw;
> -	struct i40e_mac_filter_info mac_filter;
> -	struct rte_ether_addr old_mac;
> -	struct rte_ether_addr *new_mac;
> -	struct i40e_pf_vf *vf = NULL;
> -	uint16_t vf_id;
> -	int ret;
> -
> -	if (pf == NULL) {
> -		PMD_DRV_LOG(ERR, "Invalid PF argument.");
> -		return -EINVAL;
> -	}
> -	hw = I40E_PF_TO_HW(pf);
> -
> -	if (filter == NULL) {
> -		PMD_DRV_LOG(ERR, "Invalid mac filter argument.");
> -		return -EINVAL;
> -	}
> -
> -	new_mac = &filter->mac_addr;
> -
> -	if (rte_is_zero_ether_addr(new_mac)) {
> -		PMD_DRV_LOG(ERR, "Invalid ethernet address.");
> -		return -EINVAL;
> -	}
> -
> -	vf_id = filter->dst_id;
> -
> -	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> -		PMD_DRV_LOG(ERR, "Invalid argument.");
> -		return -EINVAL;
> -	}
> -	vf = &pf->vfs[vf_id];
> -
> -	if (add && rte_is_same_ether_addr(new_mac, &pf->dev_addr)) {
> -		PMD_DRV_LOG(INFO, "Ignore adding permanent MAC
> address.");
> -		return -EINVAL;
> -	}
> -
> -	if (add) {
> -		rte_memcpy(&old_mac, hw->mac.addr,
> RTE_ETHER_ADDR_LEN);
> -		rte_memcpy(hw->mac.addr, new_mac->addr_bytes,
> -				RTE_ETHER_ADDR_LEN);
> -		rte_memcpy(&mac_filter.mac_addr, &filter->mac_addr,
> -				 RTE_ETHER_ADDR_LEN);
> -
> -		mac_filter.filter_type = filter->filter_type;
> -		ret = i40e_vsi_add_mac(vf->vsi, &mac_filter);
> -		if (ret != I40E_SUCCESS) {
> -			PMD_DRV_LOG(ERR, "Failed to add MAC filter.");
> -			return -1;
> -		}
> -		rte_ether_addr_copy(new_mac, &pf->dev_addr);
> -	} else {
> -		rte_memcpy(hw->mac.addr, hw->mac.perm_addr,
> -				RTE_ETHER_ADDR_LEN);
> -		ret = i40e_vsi_delete_mac(vf->vsi, &filter->mac_addr);
> -		if (ret != I40E_SUCCESS) {
> -			PMD_DRV_LOG(ERR, "Failed to delete MAC filter.");
> -			return -1;
> -		}
> -
> -		/* Clear device address as it has been removed */
> -		if (rte_is_same_ether_addr(&pf->dev_addr, new_mac))
> -			memset(&pf->dev_addr, 0, sizeof(struct
> rte_ether_addr));
> -	}
> -
> -	return 0;
> -}
> -
> -/* MAC filter handle */
> -static int
> -i40e_mac_filter_handle(struct rte_eth_dev *dev, enum rte_filter_op
> filter_op,
> -		void *arg)
> -{
> -	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> >dev_private);
> -	struct rte_eth_mac_filter *filter;
> -	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> -	int ret = I40E_NOT_SUPPORTED;
> -
> -	filter = (struct rte_eth_mac_filter *)(arg);
> -
> -	switch (filter_op) {
> -	case RTE_ETH_FILTER_NOP:
> -		ret = I40E_SUCCESS;
> -		break;
> -	case RTE_ETH_FILTER_ADD:
> -		i40e_pf_disable_irq0(hw);
> -		if (filter->is_vf)
> -			ret = i40e_vf_mac_filter_set(pf, filter, 1);
> -		i40e_pf_enable_irq0(hw);
> -		break;
> -	case RTE_ETH_FILTER_DELETE:
> -		i40e_pf_disable_irq0(hw);
> -		if (filter->is_vf)
> -			ret = i40e_vf_mac_filter_set(pf, filter, 0);
> -		i40e_pf_enable_irq0(hw);
> -		break;
> -	default:
> -		PMD_DRV_LOG(ERR, "unknown operation %u", filter_op);
> -		ret = I40E_ERR_PARAM;
> -		break;
> -	}
> -
> -	return ret;
> -}
> -
>  static int
>  i40e_get_rss_lut(struct i40e_vsi *vsi, uint8_t *lut, uint16_t lut_size)  { @@ -
> 10620,9 +10507,6 @@ i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
>  	case RTE_ETH_FILTER_HASH:
>  		ret = i40e_hash_filter_ctrl(dev, filter_op, arg);
>  		break;
> -	case RTE_ETH_FILTER_MACVLAN:
> -		ret = i40e_mac_filter_handle(dev, filter_op, arg);
> -		break;
>  	case RTE_ETH_FILTER_ETHERTYPE:
>  		ret = i40e_ethertype_filter_handle(dev, filter_op, arg);
>  		break;
> diff --git a/drivers/net/qede/qede_filter.c b/drivers/net/qede/qede_filter.c
> index 86a2e0dc9a..2e1646fe89 100644
> --- a/drivers/net/qede/qede_filter.c
> +++ b/drivers/net/qede/qede_filter.c
> @@ -1561,7 +1561,6 @@ int qede_dev_filter_ctrl(struct rte_eth_dev
> *eth_dev,
> 
>  		*(const void **)arg = &qede_flow_ops;
>  		return 0;
> -	case RTE_ETH_FILTER_MACVLAN:
>  	case RTE_ETH_FILTER_ETHERTYPE:
>  	case RTE_ETH_FILTER_FLEXIBLE:
>  	case RTE_ETH_FILTER_SYN:
> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
> index c0672083ec..1abf05a80c 100644
> --- a/drivers/net/sfc/sfc_ethdev.c
> +++ b/drivers/net/sfc/sfc_ethdev.c
> @@ -1748,9 +1748,6 @@ sfc_dev_filter_ctrl(struct rte_eth_dev *dev, enum
> rte_filter_type filter_type,
>  	case RTE_ETH_FILTER_NONE:
>  		sfc_err(sa, "Global filters configuration not supported");
>  		break;
> -	case RTE_ETH_FILTER_MACVLAN:
> -		sfc_err(sa, "MACVLAN filters not supported");
> -		break;
>  	case RTE_ETH_FILTER_ETHERTYPE:
>  		sfc_err(sa, "EtherType filters not supported");
>  		break;
> diff --git a/lib/librte_ethdev/rte_eth_ctrl.h
> b/lib/librte_ethdev/rte_eth_ctrl.h
> index 1416c371fb..bbb94eccce 100644
> --- a/lib/librte_ethdev/rte_eth_ctrl.h
> +++ b/lib/librte_ethdev/rte_eth_ctrl.h
> @@ -27,7 +27,6 @@ extern "C" {
>   */
>  enum rte_filter_type {
>  	RTE_ETH_FILTER_NONE = 0,
> -	RTE_ETH_FILTER_MACVLAN,
>  	RTE_ETH_FILTER_ETHERTYPE,
>  	RTE_ETH_FILTER_FLEXIBLE,
>  	RTE_ETH_FILTER_SYN,
> @@ -68,16 +67,6 @@ enum rte_mac_filter_type {
>  	RTE_MACVLAN_HASH_MATCH,
>  };
> 
> -/**
> - * MAC filter info
> - */
> -struct rte_eth_mac_filter {
> -	uint8_t is_vf; /**< 1 for VF, 0 for port dev */
> -	uint16_t dst_id; /**< VF ID, available when is_vf is 1*/
> -	enum rte_mac_filter_type filter_type; /**< MAC filter type */
> -	struct rte_ether_addr mac_addr;
> -};
> -
>  /**
>   * Define all structures for Ethertype Filter type.
>   */
> --
> 2.17.1