[v2,2/4] ethdev: add dev op to set/get IP reassembly configuration

Message ID 20220120162627.4155695-3-gakhil@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: introduce IP reassembly offload |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Akhil Goyal Jan. 20, 2022, 4:26 p.m. UTC
  A new ethernet device op is added to give application control over
the IP reassembly configuration. This operation is an optional
call from the application, default values are set by PMD and
exposed via rte_eth_dev_info.
Application should always first retrieve the capabilities from
rte_eth_dev_info and then set the fields accordingly.
User can get the currently set values using the get API.

Signed-off-by: Akhil Goyal <gakhil@marvell.com>
---
 lib/ethdev/ethdev_driver.h | 37 +++++++++++++++++
 lib/ethdev/rte_ethdev.c    | 81 ++++++++++++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h    | 51 ++++++++++++++++++++++++
 lib/ethdev/version.map     |  4 ++
 4 files changed, 173 insertions(+)
  

Comments

Andrew Rybchenko Jan. 22, 2022, 8:17 a.m. UTC | #1
On 1/20/22 19:26, Akhil Goyal wrote:
> A new ethernet device op is added to give application control over

ethernet -> Ethernet

> the IP reassembly configuration. This operation is an optional
> call from the application, default values are set by PMD and
> exposed via rte_eth_dev_info.

Are defaults or maximum support values exposed via rte_eth_dev_info?
I guess it should be maximum. Defaults can be obtained using
get without set.

> Application should always first retrieve the capabilities from
> rte_eth_dev_info and then set the fields accordingly.
> User can get the currently set values using the get API.
> 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>

[snip]


> +/**
> + * @internal
> + * Set configuration parameters for enabling IP reassembly offload in hardware.
> + *
> + * @param dev
> + *   Port (ethdev) handle
> + *
> + * @param[in] conf
> + *   Configuration parameters for IP reassembly.
> + *
> + * @return
> + *   Negative errno value on error, zero otherwise
> + */
> +typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
> +				       struct rte_eth_ip_reass_params *conf);

const

[snip]

> +int
> +rte_eth_ip_reassembly_conf_get(uint16_t port_id,
> +			       struct rte_eth_ip_reass_params *conf)

Please, preserve order everywhere. If get comes first, it must be first
everywhere.

> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (conf == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Cannot get reassembly info to NULL");
> +		return -EINVAL;
> +	}

Why is order of check different in set and get?

> +
> +	if (dev->data->dev_configured == 0) {
> +		RTE_ETHDEV_LOG(ERR,
> +			"Device with port_id=%"PRIu16" is not configured.\n",
> +			port_id);
> +		return -EINVAL;
> +	}
> +
> +	if ((dev->data->dev_conf.rxmode.offloads &
> +			RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) == 0) {
> +		RTE_ETHDEV_LOG(ERR,
> +			"The port (ID=%"PRIu16") is not configured for IP reassembly\n",
> +			port_id);
> +		return -EINVAL;
> +	}
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->ip_reassembly_conf_get,
> +				-ENOTSUP);
> +	memset(conf, 0, sizeof(struct rte_eth_ip_reass_params));
> +	return eth_err(port_id,
> +		       (*dev->dev_ops->ip_reassembly_conf_get)(dev, conf));
> +}
> +
>   RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
>   
>   RTE_INIT(ethdev_init_telemetry)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 11427b2e4d..53af158bcb 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5218,6 +5218,57 @@ int rte_eth_representor_info_get(uint16_t port_id,
>   __rte_experimental
>   int rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features);
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Get IP reassembly configuration parameters currently set in PMD,
> + * if device rx offload flag (RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) is

rx -> Rx

> + * enabled and the PMD supports IP reassembly offload.
> + *
> + * @param port_id
> + *   The port identifier of the device.
> + * @param conf
> + *   A pointer to rte_eth_ip_reass_params structure.
> + * @return
> + *   - (-ENOTSUP) if offload configuration is not supported by device.
> + *   - (-EINVAL) if offload is not enabled in rte_eth_conf.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EIO) if device is removed.
> + *   - (0) on success.
> + */
> +__rte_experimental
> +int rte_eth_ip_reassembly_conf_get(uint16_t port_id,
> +				   struct rte_eth_ip_reass_params *conf);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Set IP reassembly configuration parameters if device rx offload

rx -> Rx

> + * flag (RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) is enabled and the PMD
> + * supports IP reassembly offload. User should first check the
> + * reass_capa in rte_eth_dev_info before setting the configuration.
> + * The values of configuration parameters must not exceed the device
> + * capabilities.

It sounds like set API should retrieve dev_info and check set
values vs maximums.

> The use of this API is optional and if called, it
> + * should be called before rte_eth_dev_start().

It should be highlighted that the device must be already configured.

> + *
> + * @param port_id
> + *   The port identifier of the device.
> + * @param conf
> + *   A pointer to rte_eth_ip_reass_params structure.
> + * @return
> + *   - (-ENOTSUP) if offload configuration is not supported by device.
> + *   - (-EINVAL) if offload is not enabled in rte_eth_conf.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EIO) if device is removed.
> + *   - (0) on success.
> + */
> +__rte_experimental
> +int rte_eth_ip_reassembly_conf_set(uint16_t port_id,
> +				   struct rte_eth_ip_reass_params *conf);
> +
> +
>   #include <rte_ethdev_core.h>
>   
>   /**
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index c2fb0669a4..ad829dd47e 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -256,6 +256,10 @@ EXPERIMENTAL {
>   	rte_flow_flex_item_create;
>   	rte_flow_flex_item_release;
>   	rte_flow_pick_transfer_proxy;
> +
> +	#added in 22.03
> +	rte_eth_ip_reassembly_conf_get;
> +	rte_eth_ip_reassembly_conf_set;
>   };
>   
>   INTERNAL {
  
Akhil Goyal Jan. 30, 2022, 4:30 p.m. UTC | #2
Hi Andrew,

> On 1/20/22 19:26, Akhil Goyal wrote:
> > A new ethernet device op is added to give application control over
> 
> ethernet -> Ethernet

Ok
> 
> > the IP reassembly configuration. This operation is an optional
> > call from the application, default values are set by PMD and
> > exposed via rte_eth_dev_info.
> 
> Are defaults or maximum support values exposed via rte_eth_dev_info?
> I guess it should be maximum. Defaults can be obtained using
> get without set.
> 

Rte_eth_dev_info gives the maximum values/capabilities that a device can support
And also the default values set if user does not call set() API.

And get() op will give the currently set values.

> > Application should always first retrieve the capabilities from
> > rte_eth_dev_info and then set the fields accordingly.
> > User can get the currently set values using the get API.
> >
> > Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> 
> [snip]
> 
> 
> > +/**
> > + * @internal
> > + * Set configuration parameters for enabling IP reassembly offload in
> hardware.
> > + *
> > + * @param dev
> > + *   Port (ethdev) handle
> > + *
> > + * @param[in] conf
> > + *   Configuration parameters for IP reassembly.
> > + *
> > + * @return
> > + *   Negative errno value on error, zero otherwise
> > + */
> > +typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
> > +				       struct rte_eth_ip_reass_params *conf);
> 
> const
> 
> [snip]
> 
> > +int
> > +rte_eth_ip_reassembly_conf_get(uint16_t port_id,
> > +			       struct rte_eth_ip_reass_params *conf)
> 
> Please, preserve order everywhere. If get comes first, it must be first
> everywhere.
ok
> 
> > +{
> > +	struct rte_eth_dev *dev;
> > +
> > +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +	dev = &rte_eth_devices[port_id];
> > +
> > +	if (conf == NULL) {
> > +		RTE_ETHDEV_LOG(ERR, "Cannot get reassembly info to NULL");
> > +		return -EINVAL;
> > +	}
> 
> Why is order of check different in set and get?
Ok will correct it.
> 
> > +
> > +	if (dev->data->dev_configured == 0) {
> > +		RTE_ETHDEV_LOG(ERR,
> > +			"Device with port_id=%"PRIu16" is not configured.\n",
> > +			port_id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if ((dev->data->dev_conf.rxmode.offloads &
> > +			RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) == 0) {
> > +		RTE_ETHDEV_LOG(ERR,
> > +			"The port (ID=%"PRIu16") is not configured for IP
> reassembly\n",
> > +			port_id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >ip_reassembly_conf_get,
> > +				-ENOTSUP);
> > +	memset(conf, 0, sizeof(struct rte_eth_ip_reass_params));
> > +	return eth_err(port_id,
> > +		       (*dev->dev_ops->ip_reassembly_conf_get)(dev, conf));
> > +}
> > +
> >   RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
> >
> >   RTE_INIT(ethdev_init_telemetry)
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> > index 11427b2e4d..53af158bcb 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -5218,6 +5218,57 @@ int rte_eth_representor_info_get(uint16_t port_id,
> >   __rte_experimental
> >   int rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Get IP reassembly configuration parameters currently set in PMD,
> > + * if device rx offload flag (RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) is
> 
> rx -> Rx
> 
> > + * enabled and the PMD supports IP reassembly offload.
> > + *
> > + * @param port_id
> > + *   The port identifier of the device.
> > + * @param conf
> > + *   A pointer to rte_eth_ip_reass_params structure.
> > + * @return
> > + *   - (-ENOTSUP) if offload configuration is not supported by device.
> > + *   - (-EINVAL) if offload is not enabled in rte_eth_conf.
> > + *   - (-ENODEV) if *port_id* invalid.
> > + *   - (-EIO) if device is removed.
> > + *   - (0) on success.
> > + */
> > +__rte_experimental
> > +int rte_eth_ip_reassembly_conf_get(uint16_t port_id,
> > +				   struct rte_eth_ip_reass_params *conf);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Set IP reassembly configuration parameters if device rx offload
> 
> rx -> Rx
> 
Ok

> > + * flag (RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) is enabled and the PMD
> > + * supports IP reassembly offload. User should first check the
> > + * reass_capa in rte_eth_dev_info before setting the configuration.
> > + * The values of configuration parameters must not exceed the device
> > + * capabilities.
> 
> It sounds like set API should retrieve dev_info and check set
> values vs maximums.

Yes.

> 
> > The use of this API is optional and if called, it
> > + * should be called before rte_eth_dev_start().
> 
> It should be highlighted that the device must be already configured.

Where should this be highlighted?
  

Patch

diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index d95605a355..a310001648 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -990,6 +990,38 @@  typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
 				       uint64_t *features);
 
+/**
+ * @internal
+ * Get IP reassembly offload configuration parameters set in PMD.
+ *
+ * @param dev
+ *   Port (ethdev) handle
+ *
+ * @param[out] conf
+ *   Configuration parameters for IP reassembly.
+ *
+ * @return
+ *   Negative errno value on error, zero otherwise
+ */
+typedef int (*eth_ip_reassembly_conf_get_t)(struct rte_eth_dev *dev,
+				       struct rte_eth_ip_reass_params *conf);
+
+/**
+ * @internal
+ * Set configuration parameters for enabling IP reassembly offload in hardware.
+ *
+ * @param dev
+ *   Port (ethdev) handle
+ *
+ * @param[in] conf
+ *   Configuration parameters for IP reassembly.
+ *
+ * @return
+ *   Negative errno value on error, zero otherwise
+ */
+typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
+				       struct rte_eth_ip_reass_params *conf);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1186,6 +1218,11 @@  struct eth_dev_ops {
 	 * kinds of metadata to the PMD
 	 */
 	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
+
+	/** Get IP reassembly configuration */
+	eth_ip_reassembly_conf_get_t ip_reassembly_conf_get;
+	/** Set IP reassembly configuration */
+	eth_ip_reassembly_conf_set_t ip_reassembly_conf_set;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index d9a03f12f9..4bd31034a6 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6473,6 +6473,87 @@  rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
 		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
 }
 
+int
+rte_eth_ip_reassembly_conf_set(uint16_t port_id,
+			       struct rte_eth_ip_reass_params *conf)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev->data->dev_configured == 0) {
+		RTE_ETHDEV_LOG(ERR,
+			"Device with port_id=%"PRIu16" is not configured.\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	if (dev->data->dev_started != 0) {
+		RTE_ETHDEV_LOG(ERR,
+			"Device with port_id=%"PRIu16" started,\n"
+			"cannot configure IP reassembly params.\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	if ((dev->data->dev_conf.rxmode.offloads &
+			RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) == 0) {
+		RTE_ETHDEV_LOG(ERR,
+			"The port (ID=%"PRIu16") is not configured for IP reassembly\n",
+			port_id);
+		return -EINVAL;
+	}
+
+
+	if (conf == NULL) {
+		RTE_ETHDEV_LOG(ERR,
+				"Invalid IP reassembly configuration (NULL)\n");
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->ip_reassembly_conf_set,
+				-ENOTSUP);
+	return eth_err(port_id,
+		       (*dev->dev_ops->ip_reassembly_conf_set)(dev, conf));
+}
+
+int
+rte_eth_ip_reassembly_conf_get(uint16_t port_id,
+			       struct rte_eth_ip_reass_params *conf)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Cannot get reassembly info to NULL");
+		return -EINVAL;
+	}
+
+	if (dev->data->dev_configured == 0) {
+		RTE_ETHDEV_LOG(ERR,
+			"Device with port_id=%"PRIu16" is not configured.\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	if ((dev->data->dev_conf.rxmode.offloads &
+			RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) == 0) {
+		RTE_ETHDEV_LOG(ERR,
+			"The port (ID=%"PRIu16") is not configured for IP reassembly\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->ip_reassembly_conf_get,
+				-ENOTSUP);
+	memset(conf, 0, sizeof(struct rte_eth_ip_reass_params));
+	return eth_err(port_id,
+		       (*dev->dev_ops->ip_reassembly_conf_get)(dev, conf));
+}
+
 RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
 
 RTE_INIT(ethdev_init_telemetry)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 11427b2e4d..53af158bcb 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5218,6 +5218,57 @@  int rte_eth_representor_info_get(uint16_t port_id,
 __rte_experimental
 int rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Get IP reassembly configuration parameters currently set in PMD,
+ * if device rx offload flag (RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) is
+ * enabled and the PMD supports IP reassembly offload.
+ *
+ * @param port_id
+ *   The port identifier of the device.
+ * @param conf
+ *   A pointer to rte_eth_ip_reass_params structure.
+ * @return
+ *   - (-ENOTSUP) if offload configuration is not supported by device.
+ *   - (-EINVAL) if offload is not enabled in rte_eth_conf.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
+ *   - (0) on success.
+ */
+__rte_experimental
+int rte_eth_ip_reassembly_conf_get(uint16_t port_id,
+				   struct rte_eth_ip_reass_params *conf);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Set IP reassembly configuration parameters if device rx offload
+ * flag (RTE_ETH_RX_OFFLOAD_IP_REASSEMBLY) is enabled and the PMD
+ * supports IP reassembly offload. User should first check the
+ * reass_capa in rte_eth_dev_info before setting the configuration.
+ * The values of configuration parameters must not exceed the device
+ * capabilities. The use of this API is optional and if called, it
+ * should be called before rte_eth_dev_start().
+ *
+ * @param port_id
+ *   The port identifier of the device.
+ * @param conf
+ *   A pointer to rte_eth_ip_reass_params structure.
+ * @return
+ *   - (-ENOTSUP) if offload configuration is not supported by device.
+ *   - (-EINVAL) if offload is not enabled in rte_eth_conf.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
+ *   - (0) on success.
+ */
+__rte_experimental
+int rte_eth_ip_reassembly_conf_set(uint16_t port_id,
+				   struct rte_eth_ip_reass_params *conf);
+
+
 #include <rte_ethdev_core.h>
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index c2fb0669a4..ad829dd47e 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -256,6 +256,10 @@  EXPERIMENTAL {
 	rte_flow_flex_item_create;
 	rte_flow_flex_item_release;
 	rte_flow_pick_transfer_proxy;
+
+	#added in 22.03
+	rte_eth_ip_reassembly_conf_get;
+	rte_eth_ip_reassembly_conf_set;
 };
 
 INTERNAL {