[01/14] ethdev: remove legacy MACVLAN filter type support

  RTE flow API should be used for filtering.

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

On Sun, Oct 18, 2020 at 4:10 PM Andrew Rybchenko
<arybchenko@solarflare.com> wrote:
>
> RTE flow API should be used for filtering.

- We still have some trace in testpmd documentation.
$ git grep set.port.*vf.*mac
doc/guides/testpmd_app_ug/testpmd_funcs.rst:   testpmd> set port
(port_id) vf (vf_id) (mac_addr) \


- Do we have some documentation describing how to do the same with
rte_flow + testpmd?
  

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

On 10/21/20 6:31 AM, Guo, Jia wrote:
> 
>> -----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.

Sorry, don't understand. Isn't MACVLAN in summary sufficient?
  

On 10/20/20 2:07 PM, David Marchand wrote:
> On Sun, Oct 18, 2020 at 4:10 PM Andrew Rybchenko
> <arybchenko@solarflare.com> wrote:
>> RTE flow API should be used for filtering.
> - We still have some trace in testpmd documentation.
> $ git grep set.port.*vf.*mac
> doc/guides/testpmd_app_ug/testpmd_funcs.rst:   testpmd> set port
> (port_id) vf (vf_id) (mac_addr) \

Thanks a lot for catching it. Will cleanup in v2.

> - Do we have some documentation describing how to do the same with
> rte_flow + testpmd?

I don't think so. I guess just generic rte_flow documentation
in testpmd.
  

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Thursday, October 22, 2020 12:05 AM
> To: Guo, Jia <jia.guo@intel.com>; 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>; 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: Re: [PATCH 01/14] ethdev: remove legacy MACVLAN filter type
> support
> 
> On 10/21/20 6:31 AM, Guo, Jia wrote:
> >
> >> -----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.
> 
> Sorry, don't understand. Isn't MACVLAN in summary sufficient?

Sorry, maybe I did't say clear. Basically we concentrate the context in title and then detail more in description,
for this description, maybe some confuse that if this patch is wholly for all filtering but eventually it just specific for one filtering.