diff mbox series

[4/9] net/mlx5: add validation for Direct Rule E-Switch

Message ID 1555276357-4892-5-git-send-email-orika@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Shahaf Shuler
Headers
Series net/mlx5: add Direct Verbs E-Switch support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ori Kam April 14, 2019, 9:12 p.m. UTC 
  Add validation logic for E-Switch using Direct Rules.

Signed-off-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5.h         |   2 +
 drivers/net/mlx5/mlx5_ethdev.c  |  39 +++++++
 drivers/net/mlx5/mlx5_flow.h    |   5 +
 drivers/net/mlx5/mlx5_flow_dv.c | 252 ++++++++++++++++++++++++++++++++++++++--
 4 files changed, 287 insertions(+), 11 deletions(-)

Comments

Yongseok Koh April 17, 2019, 11:59 p.m. UTC | #1 
On Sun, Apr 14, 2019 at 09:12:32PM +0000, Ori Kam wrote:
> Add validation logic for E-Switch using Direct Rules.
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5.h         |   2 +
>  drivers/net/mlx5/mlx5_ethdev.c  |  39 +++++++
>  drivers/net/mlx5/mlx5_flow.h    |   5 +
>  drivers/net/mlx5/mlx5_flow_dv.c | 252 ++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 287 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index 33a4127..8d63575 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -412,6 +412,8 @@ int mlx5_ibv_device_to_pci_addr(const struct ibv_device *device,
>  unsigned int mlx5_dev_to_port_id(const struct rte_device *dev,
>  				 uint16_t *port_list,
>  				 unsigned int port_list_n);
> +int mlx5_port_to_eswitch_info(uint16_t port, uint16_t *es_domain_id,
> +			      uint16_t *es_port_id);
>  int mlx5_sysfs_switch_info(unsigned int ifindex,
>  			   struct mlx5_switch_info *info);
>  bool mlx5_translate_port_name(const char *port_name_in,
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 3992918..c821772 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1376,6 +1376,45 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size)
>  }
>  
>  /**
> + * Get the e-switch domain id this port belongs to.

E-Switch

> + *
> + * @param[in] port
> + *   Device port id.
> + * @param[out] es_domain_id
> + *   e-switch domain id.

E-Switch
Please correct in the entire patchset.

> + * @param[out] es_port_id
> + *   The port id of the port in the switch.
> + *
> + * @return
> + *   0 on success, Negative error otherwise.

From looking at the use-cases below, rte_errno must be set.

> + */
> +int
> +mlx5_port_to_eswitch_info(uint16_t port,
> +			  uint16_t *es_domain_id, uint16_t *es_port_id)
> +{
> +	struct rte_eth_dev *dev;
> +	struct mlx5_priv *priv;
> +
> +	if (port >= RTE_MAX_ETHPORTS)
> +		return -EINVAL;
> +	dev = &rte_eth_devices[port];
> +	if (dev == NULL)
> +		return -ENODEV;

dev is an l-value, it cannot be null.
The above two checks can be replaced with rte_eth_dev_is_valid_port().

> +	if (!dev->device ||
> +	    !dev->device->driver ||
> +	    strcmp(dev->device->driver->name, MLX5_DRIVER_NAME))
> +		return -EINVAL;

Looks too paranoid. The function is just PMD-internal.

> +	priv = dev->data->dev_private;
> +	if (!(priv->representor || priv->master))
> +		return -EINVAL;
> +	if (es_domain_id)
> +		*es_domain_id = priv->domain_id;
> +	if (es_port_id)
> +		*es_port_id = priv->vport_id;

It is okay for now but we need to define a new struct like esw_info next time.
This info should be grouped in the priv.

> +	return 0;
> +}
> +
> +/**
>   * Get switch information associated with network interface.
>   *
>   * @param ifindex
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 9f47fd4..85954c2 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -48,6 +48,7 @@
>  
>  /* General pattern items bits. */
>  #define MLX5_FLOW_ITEM_METADATA (1u << 16)
> +#define MLX5_FLOW_ITEM_PORT_ID (1u << 17)
>  
>  /* Outer Masks. */
>  #define MLX5_FLOW_LAYER_OUTER_L3 \
> @@ -118,6 +119,10 @@
>  	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | \
>  	 MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_JUMP)
>  
> +#define MLX5_FLOW_FATE_ESWITCH_ACTIONS \
> +	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
> +	 MLX5_FLOW_ACTION_JUMP)
> +
>  #define MLX5_FLOW_ENCAP_ACTIONS	(MLX5_FLOW_ACTION_VXLAN_ENCAP | \
>  				 MLX5_FLOW_ACTION_NVGRE_ENCAP | \
>  				 MLX5_FLOW_ACTION_RAW_ENCAP)
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 7b582f0..fedc6cb 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -613,6 +613,92 @@ struct field_modify_info modify_tcp[] = {
>  }
>  
>  /**
> + * Validate vport item.
> + *
> + * @param[in] dev
> + *   Pointer to the rte_eth_dev structure.
> + * @param[in] item
> + *   Item specification.
> + * @param[in] attr
> + *   Attributes of flow that includes this item.
> + * @param[in] item_flags
> + *   Bit-fields that holds the items detected until now.
> + * @param[out] error
> + *   Pointer to error structure.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + */
> +static int
> +flow_dv_validate_item_port_id(struct rte_eth_dev *dev,
> +			      const struct rte_flow_item *item,
> +			      const struct rte_flow_attr *attr,
> +			      uint64_t item_flags,
> +			      struct rte_flow_error *error)
> +{
> +	const struct rte_flow_item_port_id *spec = item->spec;
> +	const struct rte_flow_item_port_id *mask = item->mask;
> +	const struct rte_flow_item_port_id switch_mask = {
> +			.id = 0xffffffff,
> +	};
> +	uint16_t esw_domain_id;
> +	uint16_t item_port_esw_domain_id;
> +	uint16_t item_port_esw_port_id;
> +	uint16_t port;
> +	int ret;
> +
> +	if (!attr->transfer)
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ITEM,
> +					  NULL,
> +					  "match on port id is valid for"
> +					  " eswitch only");

Need to mention about 'transfer' flag here instead of esw. BTW, is it okay to
speak in PMD specific language for error messages? Even if so, 'eswitch' should
be fixed. Please re-visit all the error messages again.

> +	if (item_flags & MLX5_FLOW_ITEM_PORT_ID)
> +		return rte_flow_error_set(error, ENOTSUP,
> +					  RTE_FLOW_ERROR_TYPE_ITEM, item,
> +					  "multiple source vport are not"
> +					  " supported");

Same here. 'vport' doesn't look appropriate.

> +	if (!mask)
> +		mask = &switch_mask;
> +	if (mask->id && mask->id != 0xffffffff)
> +		return rte_flow_error_set(error, ENOTSUP,
> +					   RTE_FLOW_ERROR_TYPE_ITEM_MASK,
> +					   mask,
> +					   "no support for partial mask on"
> +					   " \"id\" field");
> +	ret = mlx5_flow_item_acceptable
> +				(item, (const uint8_t *)mask,
> +				 (const uint8_t *)&rte_flow_item_port_id_mask,
> +				 sizeof(struct rte_flow_item_port_id),
> +				 error);
> +	if (ret)
> +		return ret;
> +	if (!spec)
> +		return 0;
> +	port = mask->id ? spec->id : 0;

Non-masked value means 'any' value. Is it correct to set port 0 in
this case?

> +	ret = mlx5_port_to_eswitch_info(port, &item_port_esw_domain_id,
> +					&item_port_esw_port_id);

item_port_esw_port_id is of no use here;

> +	if (ret)
> +		return rte_flow_error_set(error, -ret,
> +					  RTE_FLOW_ERROR_TYPE_ITEM_SPEC, spec,
> +					  "failed to obtain eswitch info for"
> +					  " port");
> +	ret = mlx5_port_to_eswitch_info(dev->data->port_id,
> +					&esw_domain_id, NULL);
> +	if (ret < 0)
> +		return rte_flow_error_set(error, -ret,
> +					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					  NULL,
> +					  "failed to obtain eswitch info");
> +	if (item_port_esw_domain_id != esw_domain_id)
> +		return rte_flow_error_set(error, -ret,
> +					  RTE_FLOW_ERROR_TYPE_ITEM_SPEC, spec,
> +					  "cannot match on a port from a"
> +					  " different eswitch");
> +	return 0;
> +}
> +
> +/**
>   * Validate count action.
>   *
>   * @param[in] dev
> @@ -622,6 +708,7 @@ struct field_modify_info modify_tcp[] = {
>   *
>   * @return
>   *   0 on success, a negative errno value otherwise and rte_errno is set.
> + *   w

What is this change?

>   */
>  static int
>  flow_dv_validate_action_count(struct rte_eth_dev *dev,
> @@ -676,7 +763,7 @@ struct field_modify_info modify_tcp[] = {
>  					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
>  					  "can only have a single encap or"
>  					  " decap action in a flow");
> -	if (attr->ingress)
> +	if (!attr->transfer && attr->ingress)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
>  					  NULL,
> @@ -761,7 +848,8 @@ struct field_modify_info modify_tcp[] = {
>  					  "can only have a single encap"
>  					  " action in a flow");
>  	/* encap without preceding decap is not supported for ingress */
> -	if (attr->ingress && !(action_flags & MLX5_FLOW_ACTION_RAW_DECAP))
> +	if (!attr->transfer &&  attr->ingress &&
> +	    !(action_flags & MLX5_FLOW_ACTION_RAW_DECAP))
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
>  					  NULL,
> @@ -1561,6 +1649,77 @@ struct field_modify_info modify_tcp[] = {
>  	return 0;
>  }
>  
> +/*
> + * Validate the port_id action.
> + *
> + * @param[in] dev
> + *   Pointer to rte_eth_dev structure.
> + * @param[in] action_flags
> + *   Bit-fields that holds the actions detected until now.
> + * @param[in] action
> + *   Port_id RTE action structure.
> + * @param[in] attr
> + *   Attributes of flow that includes this action.
> + * @param[out] error
> + *   Pointer to error structure.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + */
> +static int
> +flow_dv_validate_action_port_id(struct rte_eth_dev *dev,
> +				uint64_t action_flags,
> +				const struct rte_flow_action *action,
> +				const struct rte_flow_attr *attr,
> +				struct rte_flow_error *error)
> +{
> +	const struct rte_flow_action_port_id *port_id;
> +	uint16_t port;
> +	uint16_t esw_domain_id;
> +	uint16_t act_port_domain_id;
> +	int ret;
> +
> +	if (!attr->transfer)
> +		return rte_flow_error_set(error, ENOTSUP,
> +					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					  NULL,
> +					  "port id action is valid in transfer"
> +					  " mode only");
> +	if (!action || !action->conf)
> +		return rte_flow_error_set(error, ENOTSUP,
> +					  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> +					  NULL,
> +					  "port id action parameters must be"
> +					  " specified");
> +	if (action_flags & (MLX5_FLOW_FATE_ACTIONS |
> +			    MLX5_FLOW_FATE_ESWITCH_ACTIONS))
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> +					  "can have only one fate actions in"
> +					  " a flow");
> +	ret = mlx5_port_to_eswitch_info(dev->data->port_id,
> +					&esw_domain_id, NULL);
> +	if (ret < 0)
> +		return rte_flow_error_set(error, -ret,
> +					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					  NULL,
> +					  "failed to obtain eswitch info");
> +	port_id = action->conf;
> +	port = port_id->original ? dev->data->port_id : port_id->id;
> +	ret = mlx5_port_to_eswitch_info(port, &act_port_domain_id, NULL);
> +	if (ret)
> +		return rte_flow_error_set
> +				(error, -ret,
> +				 RTE_FLOW_ERROR_TYPE_ACTION_CONF, port_id,
> +				 "failed to obtain eswitch port-id for port");
> +	if (act_port_domain_id != esw_domain_id)
> +		return rte_flow_error_set
> +				(error, -ret,
> +				 RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> +				 "port does not belong to"
> +				 " eswitch being configured");
> +	return 0;
> +}
>  
>  /**
>   * Find existing modify-header resource or create and register a new one.
> @@ -1759,11 +1918,34 @@ struct field_modify_info modify_tcp[] = {
>  					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
>  					  NULL,
>  					  "priority out of range");
> -	if (attributes->transfer)
> -		return rte_flow_error_set(error, ENOTSUP,
> -					  RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
> -					  NULL,
> -					  "transfer is not supported");
> +	if (attributes->transfer) {
> +		if (!priv->config.dv_eswitch_en)
> +			return rte_flow_error_set
> +						(error, ENOTSUP,
> +						RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +						NULL,
> +						"eswitch dr is not supported");

If you open a parenthesis in a new line, you should indent by a tab.

> +		if (!(priv->representor || priv->master))
> +			return rte_flow_error_set
> +					(error, EINVAL,
> +					 RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					 NULL,
> +					 "eswitch configurationd can only be"
> +					 " done by a master or a representor"
> +					 " device");
> +		if (attributes->egress)
> +			return rte_flow_error_set
> +					(error, ENOTSUP,
> +					 RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
> +					 attributes, "egress is not supported");
> +		if (attributes->group >= MLX5_MAX_TABLES_FDB)
> +			return rte_flow_error_set
> +				       (error, EINVAL,
> +					RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
> +					NULL,
> +					"group must be smaller than "
> +					RTE_STR(MLX5_MAX_FDB_TABLES));
> +	}
>  	if (!(attributes->egress ^ attributes->ingress))
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ATTR, NULL,
> @@ -1812,6 +1994,13 @@ struct field_modify_info modify_tcp[] = {
>  		switch (items->type) {
>  		case RTE_FLOW_ITEM_TYPE_VOID:
>  			break;
> +		case RTE_FLOW_ITEM_TYPE_PORT_ID:
> +			ret = flow_dv_validate_item_port_id
> +					(dev, items, attr, item_flags, error);
> +			if (ret < 0)
> +				return ret;
> +			item_flags |= MLX5_FLOW_ITEM_PORT_ID;
> +			break;

Shouldn't it use last_item?

>  		case RTE_FLOW_ITEM_TYPE_ETH:
>  			ret = mlx5_flow_validate_item_eth(items, item_flags,
>  							  error);
> @@ -1943,6 +2132,17 @@ struct field_modify_info modify_tcp[] = {
>  		switch (actions->type) {
>  		case RTE_FLOW_ACTION_TYPE_VOID:
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_PORT_ID:
> +			ret = flow_dv_validate_action_port_id(dev,
> +							      action_flags,
> +							      actions,
> +							      attr,
> +							      error);
> +			if (ret)
> +				return ret;
> +			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
> +			++actions_n;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_FLAG:
>  			ret = mlx5_flow_validate_action_flag(action_flags,
>  							     attr, error);
> @@ -2133,10 +2333,40 @@ struct field_modify_info modify_tcp[] = {
>  						  "action not supported");
>  		}
>  	}
> -	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr->ingress)
> -		return rte_flow_error_set(error, EINVAL,
> -					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
> -					  "no fate action is found");
> +	/* Eswitch has few restrictions on using items and actions */
> +	if (attr->transfer) {
> +		if (action_flags & MLX5_FLOW_ACTION_FLAG)
> +			return rte_flow_error_set(error, ENOTSUP,
> +						  RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL,
> +						  "unsupported action FLAG");
> +		if (action_flags & MLX5_FLOW_ACTION_MARK)
> +			return rte_flow_error_set(error, ENOTSUP,
> +						  RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL,
> +						  "unsupported action MARK");
> +		if (action_flags & MLX5_FLOW_ACTION_QUEUE)
> +			return rte_flow_error_set(error, ENOTSUP,
> +						  RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL,
> +						  "unsupported action QUEUE");
> +		if (action_flags & MLX5_FLOW_ACTION_RSS)
> +			return rte_flow_error_set(error, ENOTSUP,
> +						  RTE_FLOW_ERROR_TYPE_ACTION,
> +						  NULL,
> +						  "unsupported action RSS");
> +		if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS))
> +			return rte_flow_error_set(error, EINVAL,
> +						  RTE_FLOW_ERROR_TYPE_ACTION,
> +						  actions,
> +						  "no fate action is found");
> +	} else {
> +		if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr->ingress)
> +			return rte_flow_error_set(error, EINVAL,
> +						  RTE_FLOW_ERROR_TYPE_ACTION,
> +						  actions,
> +						  "no fate action is found");
> +	}
>  	return 0;
>  }
>  
> -- 
> 1.8.3.1
>
Ori Kam April 18, 2019, 4:40 a.m. UTC | #2 
Hi Koh,
PSB

> -----Original Message-----
> From: Yongseok Koh
> Sent: Thursday, April 18, 2019 3:00 AM
> To: Ori Kam <orika@mellanox.com>
> Cc: Shahaf Shuler <shahafs@mellanox.com>; Matan Azrad
> <matan@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>; Moti
> Haimovsky <motih@mellanox.com>; dev@dpdk.org
> Subject: Re: [PATCH 4/9] net/mlx5: add validation for Direct Rule E-Switch
> 
> On Sun, Apr 14, 2019 at 09:12:32PM +0000, Ori Kam wrote:
> > Add validation logic for E-Switch using Direct Rules.
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5.h         |   2 +
> >  drivers/net/mlx5/mlx5_ethdev.c  |  39 +++++++
> >  drivers/net/mlx5/mlx5_flow.h    |   5 +
> >  drivers/net/mlx5/mlx5_flow_dv.c | 252
> ++++++++++++++++++++++++++++++++++++++--
> >  4 files changed, 287 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> > index 33a4127..8d63575 100644
> > --- a/drivers/net/mlx5/mlx5.h
> > +++ b/drivers/net/mlx5/mlx5.h
> > @@ -412,6 +412,8 @@ int mlx5_ibv_device_to_pci_addr(const struct
> ibv_device *device,
> >  unsigned int mlx5_dev_to_port_id(const struct rte_device *dev,
> >  				 uint16_t *port_list,
> >  				 unsigned int port_list_n);
> > +int mlx5_port_to_eswitch_info(uint16_t port, uint16_t *es_domain_id,
> > +			      uint16_t *es_port_id);
> >  int mlx5_sysfs_switch_info(unsigned int ifindex,
> >  			   struct mlx5_switch_info *info);
> >  bool mlx5_translate_port_name(const char *port_name_in,
> > diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> b/drivers/net/mlx5/mlx5_ethdev.c
> > index 3992918..c821772 100644
> > --- a/drivers/net/mlx5/mlx5_ethdev.c
> > +++ b/drivers/net/mlx5/mlx5_ethdev.c
> > @@ -1376,6 +1376,45 @@ int mlx5_fw_version_get(struct rte_eth_dev *dev,
> char *fw_ver, size_t fw_size)
> >  }
> >
> >  /**
> > + * Get the e-switch domain id this port belongs to.
> 
> E-Switch

Will fix.
> 
> > + *
> > + * @param[in] port
> > + *   Device port id.
> > + * @param[out] es_domain_id
> > + *   e-switch domain id.
> 
> E-Switch
> Please correct in the entire patchset.
> 
> > + * @param[out] es_port_id
> > + *   The port id of the port in the switch.
> > + *
> > + * @return
> > + *   0 on success, Negative error otherwise.
> 
> From looking at the use-cases below, rte_errno must be set.
> 


Will fix.

> > + */
> > +int
> > +mlx5_port_to_eswitch_info(uint16_t port,
> > +			  uint16_t *es_domain_id, uint16_t *es_port_id)
> > +{
> > +	struct rte_eth_dev *dev;
> > +	struct mlx5_priv *priv;
> > +
> > +	if (port >= RTE_MAX_ETHPORTS)
> > +		return -EINVAL;
> > +	dev = &rte_eth_devices[port];
> > +	if (dev == NULL)
> > +		return -ENODEV;
> 
> dev is an l-value, it cannot be null.
> The above two checks can be replaced with rte_eth_dev_is_valid_port().
> 

Will  fix.

> > +	if (!dev->device ||
> > +	    !dev->device->driver ||
> > +	    strcmp(dev->device->driver->name, MLX5_DRIVER_NAME))
> > +		return -EINVAL;
> 
> Looks too paranoid. The function is just PMD-internal.
> 

Will delete.

> > +	priv = dev->data->dev_private;
> > +	if (!(priv->representor || priv->master))
> > +		return -EINVAL;
> > +	if (es_domain_id)
> > +		*es_domain_id = priv->domain_id;
> > +	if (es_port_id)
> > +		*es_port_id = priv->vport_id;
> 
> It is okay for now but we need to define a new struct like esw_info next time.
> This info should be grouped in the priv.
> 

I think I need a new member to mark it is pf or vd, can we in this patch set add it as is?
I will create a patch to move all of the E-Switch variables to dedicated structure for next version.

> > +	return 0;
> > +}
> > +
> > +/**
> >   * Get switch information associated with network interface.
> >   *
> >   * @param ifindex
> > diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> > index 9f47fd4..85954c2 100644
> > --- a/drivers/net/mlx5/mlx5_flow.h
> > +++ b/drivers/net/mlx5/mlx5_flow.h
> > @@ -48,6 +48,7 @@
> >
> >  /* General pattern items bits. */
> >  #define MLX5_FLOW_ITEM_METADATA (1u << 16)
> > +#define MLX5_FLOW_ITEM_PORT_ID (1u << 17)
> >
> >  /* Outer Masks. */
> >  #define MLX5_FLOW_LAYER_OUTER_L3 \
> > @@ -118,6 +119,10 @@
> >  	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | \
> >  	 MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_JUMP)
> >
> > +#define MLX5_FLOW_FATE_ESWITCH_ACTIONS \
> > +	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
> > +	 MLX5_FLOW_ACTION_JUMP)
> > +
> >  #define MLX5_FLOW_ENCAP_ACTIONS
> 	(MLX5_FLOW_ACTION_VXLAN_ENCAP | \
> >  				 MLX5_FLOW_ACTION_NVGRE_ENCAP | \
> >  				 MLX5_FLOW_ACTION_RAW_ENCAP)
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> > index 7b582f0..fedc6cb 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -613,6 +613,92 @@ struct field_modify_info modify_tcp[] = {
> >  }
> >
> >  /**
> > + * Validate vport item.
> > + *
> > + * @param[in] dev
> > + *   Pointer to the rte_eth_dev structure.
> > + * @param[in] item
> > + *   Item specification.
> > + * @param[in] attr
> > + *   Attributes of flow that includes this item.
> > + * @param[in] item_flags
> > + *   Bit-fields that holds the items detected until now.
> > + * @param[out] error
> > + *   Pointer to error structure.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +static int
> > +flow_dv_validate_item_port_id(struct rte_eth_dev *dev,
> > +			      const struct rte_flow_item *item,
> > +			      const struct rte_flow_attr *attr,
> > +			      uint64_t item_flags,
> > +			      struct rte_flow_error *error)
> > +{
> > +	const struct rte_flow_item_port_id *spec = item->spec;
> > +	const struct rte_flow_item_port_id *mask = item->mask;
> > +	const struct rte_flow_item_port_id switch_mask = {
> > +			.id = 0xffffffff,
> > +	};
> > +	uint16_t esw_domain_id;
> > +	uint16_t item_port_esw_domain_id;
> > +	uint16_t item_port_esw_port_id;
> > +	uint16_t port;
> > +	int ret;
> > +
> > +	if (!attr->transfer)
> > +		return rte_flow_error_set(error, EINVAL,
> > +					  RTE_FLOW_ERROR_TYPE_ITEM,
> > +					  NULL,
> > +					  "match on port id is valid for"
> > +					  " eswitch only");
> 
> Need to mention about 'transfer' flag here instead of esw. BTW, is it okay to
> speak in PMD specific language for error messages? Even if so, 'eswitch' should
> be fixed. Please re-visit all the error messages again.
> 

I think the correct message should be that the eswitch  should be replaced by E-Switch
Other option is to say something like this "matching on port id must have the transfer attribute"
Which do you prefer?

> > +	if (item_flags & MLX5_FLOW_ITEM_PORT_ID)
> > +		return rte_flow_error_set(error, ENOTSUP,
> > +					  RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> > +					  "multiple source vport are not"
> > +					  " supported");
> 
> Same here. 'vport' doesn't look appropriate.
>

Will change from vport to ports.
 
> > +	if (!mask)
> > +		mask = &switch_mask;
> > +	if (mask->id && mask->id != 0xffffffff)
> > +		return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM_MASK,
> > +					   mask,
> > +					   "no support for partial mask on"
> > +					   " \"id\" field");
> > +	ret = mlx5_flow_item_acceptable
> > +				(item, (const uint8_t *)mask,
> > +				 (const uint8_t
> *)&rte_flow_item_port_id_mask,
> > +				 sizeof(struct rte_flow_item_port_id),
> > +				 error);
> > +	if (ret)
> > +		return ret;
> > +	if (!spec)
> > +		return 0;
> > +	port = mask->id ? spec->id : 0;
> 
> Non-masked value means 'any' value. Is it correct to set port 0 in
> this case?
> 

Will check again.

> > +	ret = mlx5_port_to_eswitch_info(port, &item_port_esw_domain_id,
> > +					&item_port_esw_port_id);
> 
> item_port_esw_port_id is of no use here;
> 

Will remove the variable.

> > +	if (ret)
> > +		return rte_flow_error_set(error, -ret,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM_SPEC, spec,
> > +					  "failed to obtain eswitch info for"
> > +					  " port");
> > +	ret = mlx5_port_to_eswitch_info(dev->data->port_id,
> > +					&esw_domain_id, NULL);
> > +	if (ret < 0)
> > +		return rte_flow_error_set(error, -ret,
> > +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > +					  NULL,
> > +					  "failed to obtain eswitch info");
> > +	if (item_port_esw_domain_id != esw_domain_id)
> > +		return rte_flow_error_set(error, -ret,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM_SPEC, spec,
> > +					  "cannot match on a port from a"
> > +					  " different eswitch");
> > +	return 0;
> > +}
> > +
> > +/**
> >   * Validate count action.
> >   *
> >   * @param[in] dev
> > @@ -622,6 +708,7 @@ struct field_modify_info modify_tcp[] = {
> >   *
> >   * @return
> >   *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + *   w
> 
> What is this change?
> 

Will fix

> >   */
> >  static int
> >  flow_dv_validate_action_count(struct rte_eth_dev *dev,
> > @@ -676,7 +763,7 @@ struct field_modify_info modify_tcp[] = {
> >  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> >  					  "can only have a single encap or"
> >  					  " decap action in a flow");
> > -	if (attr->ingress)
> > +	if (!attr->transfer && attr->ingress)
> >  		return rte_flow_error_set(error, ENOTSUP,
> >
> RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
> >  					  NULL,
> > @@ -761,7 +848,8 @@ struct field_modify_info modify_tcp[] = {
> >  					  "can only have a single encap"
> >  					  " action in a flow");
> >  	/* encap without preceding decap is not supported for ingress */
> > -	if (attr->ingress && !(action_flags &
> MLX5_FLOW_ACTION_RAW_DECAP))
> > +	if (!attr->transfer &&  attr->ingress &&
> > +	    !(action_flags & MLX5_FLOW_ACTION_RAW_DECAP))
> >  		return rte_flow_error_set(error, ENOTSUP,
> >
> RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
> >  					  NULL,
> > @@ -1561,6 +1649,77 @@ struct field_modify_info modify_tcp[] = {
> >  	return 0;
> >  }
> >
> > +/*
> > + * Validate the port_id action.
> > + *
> > + * @param[in] dev
> > + *   Pointer to rte_eth_dev structure.
> > + * @param[in] action_flags
> > + *   Bit-fields that holds the actions detected until now.
> > + * @param[in] action
> > + *   Port_id RTE action structure.
> > + * @param[in] attr
> > + *   Attributes of flow that includes this action.
> > + * @param[out] error
> > + *   Pointer to error structure.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +static int
> > +flow_dv_validate_action_port_id(struct rte_eth_dev *dev,
> > +				uint64_t action_flags,
> > +				const struct rte_flow_action *action,
> > +				const struct rte_flow_attr *attr,
> > +				struct rte_flow_error *error)
> > +{
> > +	const struct rte_flow_action_port_id *port_id;
> > +	uint16_t port;
> > +	uint16_t esw_domain_id;
> > +	uint16_t act_port_domain_id;
> > +	int ret;
> > +
> > +	if (!attr->transfer)
> > +		return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > +					  NULL,
> > +					  "port id action is valid in transfer"
> > +					  " mode only");
> > +	if (!action || !action->conf)
> > +		return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> > +					  NULL,
> > +					  "port id action parameters must be"
> > +					  " specified");
> > +	if (action_flags & (MLX5_FLOW_FATE_ACTIONS |
> > +			    MLX5_FLOW_FATE_ESWITCH_ACTIONS))
> > +		return rte_flow_error_set(error, EINVAL,
> > +					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> > +					  "can have only one fate actions in"
> > +					  " a flow");
> > +	ret = mlx5_port_to_eswitch_info(dev->data->port_id,
> > +					&esw_domain_id, NULL);
> > +	if (ret < 0)
> > +		return rte_flow_error_set(error, -ret,
> > +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > +					  NULL,
> > +					  "failed to obtain eswitch info");
> > +	port_id = action->conf;
> > +	port = port_id->original ? dev->data->port_id : port_id->id;
> > +	ret = mlx5_port_to_eswitch_info(port, &act_port_domain_id, NULL);
> > +	if (ret)
> > +		return rte_flow_error_set
> > +				(error, -ret,
> > +				 RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> port_id,
> > +				 "failed to obtain eswitch port-id for port");
> > +	if (act_port_domain_id != esw_domain_id)
> > +		return rte_flow_error_set
> > +				(error, -ret,
> > +				 RTE_FLOW_ERROR_TYPE_ACTION, NULL,
> > +				 "port does not belong to"
> > +				 " eswitch being configured");
> > +	return 0;
> > +}
> >
> >  /**
> >   * Find existing modify-header resource or create and register a new one.
> > @@ -1759,11 +1918,34 @@ struct field_modify_info modify_tcp[] = {
> >
> RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> >  					  NULL,
> >  					  "priority out of range");
> > -	if (attributes->transfer)
> > -		return rte_flow_error_set(error, ENOTSUP,
> > -
> RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
> > -					  NULL,
> > -					  "transfer is not supported");
> > +	if (attributes->transfer) {
> > +		if (!priv->config.dv_eswitch_en)
> > +			return rte_flow_error_set
> > +						(error, ENOTSUP,
> > +
> 	RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > +						NULL,
> > +						"eswitch dr is not supported");
> 
> If you open a parenthesis in a new line, you should indent by a tab.
> 

Missed that, fill fix.

> > +		if (!(priv->representor || priv->master))
> > +			return rte_flow_error_set
> > +					(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > +					 NULL,
> > +					 "eswitch configurationd can only be"
> > +					 " done by a master or a representor"
> > +					 " device");
> > +		if (attributes->egress)
> > +			return rte_flow_error_set
> > +					(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
> > +					 attributes, "egress is not supported");
> > +		if (attributes->group >= MLX5_MAX_TABLES_FDB)
> > +			return rte_flow_error_set
> > +				       (error, EINVAL,
> > +
> 	RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
> > +					NULL,
> > +					"group must be smaller than "
> > +					RTE_STR(MLX5_MAX_FDB_TABLES));
> > +	}
> >  	if (!(attributes->egress ^ attributes->ingress))
> >  		return rte_flow_error_set(error, ENOTSUP,
> >  					  RTE_FLOW_ERROR_TYPE_ATTR,
> NULL,
> > @@ -1812,6 +1994,13 @@ struct field_modify_info modify_tcp[] = {
> >  		switch (items->type) {
> >  		case RTE_FLOW_ITEM_TYPE_VOID:
> >  			break;
> > +		case RTE_FLOW_ITEM_TYPE_PORT_ID:
> > +			ret = flow_dv_validate_item_port_id
> > +					(dev, items, attr, item_flags, error);
> > +			if (ret < 0)
> > +				return ret;
> > +			item_flags |= MLX5_FLOW_ITEM_PORT_ID;
> > +			break;
> 
> Shouldn't it use last_item?
> 

Good question, should the port id be in some specific order? 
We can force it to be first or last the which makes sense.
I will change to last_item.

 
> >  		case RTE_FLOW_ITEM_TYPE_ETH:
> >  			ret = mlx5_flow_validate_item_eth(items, item_flags,
> >  							  error);
> > @@ -1943,6 +2132,17 @@ struct field_modify_info modify_tcp[] = {
> >  		switch (actions->type) {
> >  		case RTE_FLOW_ACTION_TYPE_VOID:
> >  			break;
> > +		case RTE_FLOW_ACTION_TYPE_PORT_ID:
> > +			ret = flow_dv_validate_action_port_id(dev,
> > +							      action_flags,
> > +							      actions,
> > +							      attr,
> > +							      error);
> > +			if (ret)
> > +				return ret;
> > +			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
> > +			++actions_n;
> > +			break;
> >  		case RTE_FLOW_ACTION_TYPE_FLAG:
> >  			ret = mlx5_flow_validate_action_flag(action_flags,
> >  							     attr, error);
> > @@ -2133,10 +2333,40 @@ struct field_modify_info modify_tcp[] = {
> >  						  "action not supported");
> >  		}
> >  	}
> > -	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr->ingress)
> > -		return rte_flow_error_set(error, EINVAL,
> > -					  RTE_FLOW_ERROR_TYPE_ACTION,
> actions,
> > -					  "no fate action is found");
> > +	/* Eswitch has few restrictions on using items and actions */
> > +	if (attr->transfer) {
> > +		if (action_flags & MLX5_FLOW_ACTION_FLAG)
> > +			return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_ACTION,
> > +						  NULL,
> > +						  "unsupported action FLAG");
> > +		if (action_flags & MLX5_FLOW_ACTION_MARK)
> > +			return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_ACTION,
> > +						  NULL,
> > +						  "unsupported action MARK");
> > +		if (action_flags & MLX5_FLOW_ACTION_QUEUE)
> > +			return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_ACTION,
> > +						  NULL,
> > +						  "unsupported action
> QUEUE");
> > +		if (action_flags & MLX5_FLOW_ACTION_RSS)
> > +			return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_ACTION,
> > +						  NULL,
> > +						  "unsupported action RSS");
> > +		if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS))
> > +			return rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_ACTION,
> > +						  actions,
> > +						  "no fate action is found");
> > +	} else {
> > +		if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr-
> >ingress)
> > +			return rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_ACTION,
> > +						  actions,
> > +						  "no fate action is found");
> > +	}
> >  	return 0;
> >  }
> >
> > --
> > 1.8.3.1
> >
diff mbox series

Patch

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 33a4127..8d63575 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -412,6 +412,8 @@  int mlx5_ibv_device_to_pci_addr(const struct ibv_device *device,
 unsigned int mlx5_dev_to_port_id(const struct rte_device *dev,
 				 uint16_t *port_list,
 				 unsigned int port_list_n);
+int mlx5_port_to_eswitch_info(uint16_t port, uint16_t *es_domain_id,
+			      uint16_t *es_port_id);
 int mlx5_sysfs_switch_info(unsigned int ifindex,
 			   struct mlx5_switch_info *info);
 bool mlx5_translate_port_name(const char *port_name_in,
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 3992918..c821772 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1376,6 +1376,45 @@  int mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size)
 }
 
 /**
+ * Get the e-switch domain id this port belongs to.
+ *
+ * @param[in] port
+ *   Device port id.
+ * @param[out] es_domain_id
+ *   e-switch domain id.
+ * @param[out] es_port_id
+ *   The port id of the port in the switch.
+ *
+ * @return
+ *   0 on success, Negative error otherwise.
+ */
+int
+mlx5_port_to_eswitch_info(uint16_t port,
+			  uint16_t *es_domain_id, uint16_t *es_port_id)
+{
+	struct rte_eth_dev *dev;
+	struct mlx5_priv *priv;
+
+	if (port >= RTE_MAX_ETHPORTS)
+		return -EINVAL;
+	dev = &rte_eth_devices[port];
+	if (dev == NULL)
+		return -ENODEV;
+	if (!dev->device ||
+	    !dev->device->driver ||
+	    strcmp(dev->device->driver->name, MLX5_DRIVER_NAME))
+		return -EINVAL;
+	priv = dev->data->dev_private;
+	if (!(priv->representor || priv->master))
+		return -EINVAL;
+	if (es_domain_id)
+		*es_domain_id = priv->domain_id;
+	if (es_port_id)
+		*es_port_id = priv->vport_id;
+	return 0;
+}
+
+/**
  * Get switch information associated with network interface.
  *
  * @param ifindex
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 9f47fd4..85954c2 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -48,6 +48,7 @@ 
 
 /* General pattern items bits. */
 #define MLX5_FLOW_ITEM_METADATA (1u << 16)
+#define MLX5_FLOW_ITEM_PORT_ID (1u << 17)
 
 /* Outer Masks. */
 #define MLX5_FLOW_LAYER_OUTER_L3 \
@@ -118,6 +119,10 @@ 
 	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | \
 	 MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_JUMP)
 
+#define MLX5_FLOW_FATE_ESWITCH_ACTIONS \
+	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
+	 MLX5_FLOW_ACTION_JUMP)
+
 #define MLX5_FLOW_ENCAP_ACTIONS	(MLX5_FLOW_ACTION_VXLAN_ENCAP | \
 				 MLX5_FLOW_ACTION_NVGRE_ENCAP | \
 				 MLX5_FLOW_ACTION_RAW_ENCAP)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 7b582f0..fedc6cb 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -613,6 +613,92 @@  struct field_modify_info modify_tcp[] = {
 }
 
 /**
+ * Validate vport item.
+ *
+ * @param[in] dev
+ *   Pointer to the rte_eth_dev structure.
+ * @param[in] item
+ *   Item specification.
+ * @param[in] attr
+ *   Attributes of flow that includes this item.
+ * @param[in] item_flags
+ *   Bit-fields that holds the items detected until now.
+ * @param[out] error
+ *   Pointer to error structure.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+static int
+flow_dv_validate_item_port_id(struct rte_eth_dev *dev,
+			      const struct rte_flow_item *item,
+			      const struct rte_flow_attr *attr,
+			      uint64_t item_flags,
+			      struct rte_flow_error *error)
+{
+	const struct rte_flow_item_port_id *spec = item->spec;
+	const struct rte_flow_item_port_id *mask = item->mask;
+	const struct rte_flow_item_port_id switch_mask = {
+			.id = 0xffffffff,
+	};
+	uint16_t esw_domain_id;
+	uint16_t item_port_esw_domain_id;
+	uint16_t item_port_esw_port_id;
+	uint16_t port;
+	int ret;
+
+	if (!attr->transfer)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ITEM,
+					  NULL,
+					  "match on port id is valid for"
+					  " eswitch only");
+	if (item_flags & MLX5_FLOW_ITEM_PORT_ID)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ITEM, item,
+					  "multiple source vport are not"
+					  " supported");
+	if (!mask)
+		mask = &switch_mask;
+	if (mask->id && mask->id != 0xffffffff)
+		return rte_flow_error_set(error, ENOTSUP,
+					   RTE_FLOW_ERROR_TYPE_ITEM_MASK,
+					   mask,
+					   "no support for partial mask on"
+					   " \"id\" field");
+	ret = mlx5_flow_item_acceptable
+				(item, (const uint8_t *)mask,
+				 (const uint8_t *)&rte_flow_item_port_id_mask,
+				 sizeof(struct rte_flow_item_port_id),
+				 error);
+	if (ret)
+		return ret;
+	if (!spec)
+		return 0;
+	port = mask->id ? spec->id : 0;
+	ret = mlx5_port_to_eswitch_info(port, &item_port_esw_domain_id,
+					&item_port_esw_port_id);
+	if (ret)
+		return rte_flow_error_set(error, -ret,
+					  RTE_FLOW_ERROR_TYPE_ITEM_SPEC, spec,
+					  "failed to obtain eswitch info for"
+					  " port");
+	ret = mlx5_port_to_eswitch_info(dev->data->port_id,
+					&esw_domain_id, NULL);
+	if (ret < 0)
+		return rte_flow_error_set(error, -ret,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL,
+					  "failed to obtain eswitch info");
+	if (item_port_esw_domain_id != esw_domain_id)
+		return rte_flow_error_set(error, -ret,
+					  RTE_FLOW_ERROR_TYPE_ITEM_SPEC, spec,
+					  "cannot match on a port from a"
+					  " different eswitch");
+	return 0;
+}
+
+/**
  * Validate count action.
  *
  * @param[in] dev
@@ -622,6 +708,7 @@  struct field_modify_info modify_tcp[] = {
  *
  * @return
  *   0 on success, a negative errno value otherwise and rte_errno is set.
+ *   w
  */
 static int
 flow_dv_validate_action_count(struct rte_eth_dev *dev,
@@ -676,7 +763,7 @@  struct field_modify_info modify_tcp[] = {
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can only have a single encap or"
 					  " decap action in a flow");
-	if (attr->ingress)
+	if (!attr->transfer && attr->ingress)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
 					  NULL,
@@ -761,7 +848,8 @@  struct field_modify_info modify_tcp[] = {
 					  "can only have a single encap"
 					  " action in a flow");
 	/* encap without preceding decap is not supported for ingress */
-	if (attr->ingress && !(action_flags & MLX5_FLOW_ACTION_RAW_DECAP))
+	if (!attr->transfer &&  attr->ingress &&
+	    !(action_flags & MLX5_FLOW_ACTION_RAW_DECAP))
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
 					  NULL,
@@ -1561,6 +1649,77 @@  struct field_modify_info modify_tcp[] = {
 	return 0;
 }
 
+/*
+ * Validate the port_id action.
+ *
+ * @param[in] dev
+ *   Pointer to rte_eth_dev structure.
+ * @param[in] action_flags
+ *   Bit-fields that holds the actions detected until now.
+ * @param[in] action
+ *   Port_id RTE action structure.
+ * @param[in] attr
+ *   Attributes of flow that includes this action.
+ * @param[out] error
+ *   Pointer to error structure.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+static int
+flow_dv_validate_action_port_id(struct rte_eth_dev *dev,
+				uint64_t action_flags,
+				const struct rte_flow_action *action,
+				const struct rte_flow_attr *attr,
+				struct rte_flow_error *error)
+{
+	const struct rte_flow_action_port_id *port_id;
+	uint16_t port;
+	uint16_t esw_domain_id;
+	uint16_t act_port_domain_id;
+	int ret;
+
+	if (!attr->transfer)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL,
+					  "port id action is valid in transfer"
+					  " mode only");
+	if (!action || !action->conf)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+					  NULL,
+					  "port id action parameters must be"
+					  " specified");
+	if (action_flags & (MLX5_FLOW_FATE_ACTIONS |
+			    MLX5_FLOW_FATE_ESWITCH_ACTIONS))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "can have only one fate actions in"
+					  " a flow");
+	ret = mlx5_port_to_eswitch_info(dev->data->port_id,
+					&esw_domain_id, NULL);
+	if (ret < 0)
+		return rte_flow_error_set(error, -ret,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL,
+					  "failed to obtain eswitch info");
+	port_id = action->conf;
+	port = port_id->original ? dev->data->port_id : port_id->id;
+	ret = mlx5_port_to_eswitch_info(port, &act_port_domain_id, NULL);
+	if (ret)
+		return rte_flow_error_set
+				(error, -ret,
+				 RTE_FLOW_ERROR_TYPE_ACTION_CONF, port_id,
+				 "failed to obtain eswitch port-id for port");
+	if (act_port_domain_id != esw_domain_id)
+		return rte_flow_error_set
+				(error, -ret,
+				 RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+				 "port does not belong to"
+				 " eswitch being configured");
+	return 0;
+}
 
 /**
  * Find existing modify-header resource or create and register a new one.
@@ -1759,11 +1918,34 @@  struct field_modify_info modify_tcp[] = {
 					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
 					  NULL,
 					  "priority out of range");
-	if (attributes->transfer)
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
-					  NULL,
-					  "transfer is not supported");
+	if (attributes->transfer) {
+		if (!priv->config.dv_eswitch_en)
+			return rte_flow_error_set
+						(error, ENOTSUP,
+						RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+						NULL,
+						"eswitch dr is not supported");
+		if (!(priv->representor || priv->master))
+			return rte_flow_error_set
+					(error, EINVAL,
+					 RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					 NULL,
+					 "eswitch configurationd can only be"
+					 " done by a master or a representor"
+					 " device");
+		if (attributes->egress)
+			return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+					 attributes, "egress is not supported");
+		if (attributes->group >= MLX5_MAX_TABLES_FDB)
+			return rte_flow_error_set
+				       (error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
+					NULL,
+					"group must be smaller than "
+					RTE_STR(MLX5_MAX_FDB_TABLES));
+	}
 	if (!(attributes->egress ^ attributes->ingress))
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR, NULL,
@@ -1812,6 +1994,13 @@  struct field_modify_info modify_tcp[] = {
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
 			break;
+		case RTE_FLOW_ITEM_TYPE_PORT_ID:
+			ret = flow_dv_validate_item_port_id
+					(dev, items, attr, item_flags, error);
+			if (ret < 0)
+				return ret;
+			item_flags |= MLX5_FLOW_ITEM_PORT_ID;
+			break;
 		case RTE_FLOW_ITEM_TYPE_ETH:
 			ret = mlx5_flow_validate_item_eth(items, item_flags,
 							  error);
@@ -1943,6 +2132,17 @@  struct field_modify_info modify_tcp[] = {
 		switch (actions->type) {
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
+		case RTE_FLOW_ACTION_TYPE_PORT_ID:
+			ret = flow_dv_validate_action_port_id(dev,
+							      action_flags,
+							      actions,
+							      attr,
+							      error);
+			if (ret)
+				return ret;
+			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
+			++actions_n;
+			break;
 		case RTE_FLOW_ACTION_TYPE_FLAG:
 			ret = mlx5_flow_validate_action_flag(action_flags,
 							     attr, error);
@@ -2133,10 +2333,40 @@  struct field_modify_info modify_tcp[] = {
 						  "action not supported");
 		}
 	}
-	if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr->ingress)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ACTION, actions,
-					  "no fate action is found");
+	/* Eswitch has few restrictions on using items and actions */
+	if (attr->transfer) {
+		if (action_flags & MLX5_FLOW_ACTION_FLAG)
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "unsupported action FLAG");
+		if (action_flags & MLX5_FLOW_ACTION_MARK)
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "unsupported action MARK");
+		if (action_flags & MLX5_FLOW_ACTION_QUEUE)
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "unsupported action QUEUE");
+		if (action_flags & MLX5_FLOW_ACTION_RSS)
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "unsupported action RSS");
+		if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS))
+			return rte_flow_error_set(error, EINVAL,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  actions,
+						  "no fate action is found");
+	} else {
+		if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr->ingress)
+			return rte_flow_error_set(error, EINVAL,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  actions,
+						  "no fate action is found");
+	}
 	return 0;
 }