diff mbox

[dpdk-dev,v2] ethdev: check Rx/Tx offloads

Message ID 20180328085709.28310-1-wei.dai@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Wei Dai March 28, 2018, 8:57 a.m. UTC 
  This patch check if a requested offloading
is supported in the device capability.
A per port offloading feature should be enabled or
disabled at same time in both rte_eth_dev_configure( )
and rte_eth_rx_queue_setup( )/rte_eth_tx_queue_setup( ).
This patch check if a per port offloading flag has
same configuration in rte_eth_dev_configure( ) and
rte_eth_rx_queue_setup( )/rte_eth_tx_queue_setup( ).
This patch can make such checking in a common way in
rte_ethdev layer to avoid same checking in underlying PMD.

Signed-off-by: Wei Dai <wei.dai@intel.com>

---
v2: add offlaods checking in rte_eth_dev_configure( ).
    check if a requested offloading is supported
---
 lib/librte_ether/rte_ethdev.c | 100 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

Comments

Ferruh Yigit April 13, 2018, 5:31 p.m. UTC | #1 
On 3/28/2018 9:57 AM, Wei Dai wrote:
> This patch check if a requested offloading
> is supported in the device capability.
> A per port offloading feature should be enabled or
> disabled at same time in both rte_eth_dev_configure( )
> and rte_eth_rx_queue_setup( )/rte_eth_tx_queue_setup( ).
> This patch check if a per port offloading flag has
> same configuration in rte_eth_dev_configure( ) and
> rte_eth_rx_queue_setup( )/rte_eth_tx_queue_setup( ).
> This patch can make such checking in a common way in
> rte_ethdev layer to avoid same checking in underlying PMD.
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> 

Hi Wei,

I think it is good idea to move common check to the abstraction layer as much as
possible.

But for this case we are targeting an API change in rc2, I believe better wait
that API change for this update.

Thanks,
ferruh
Thomas Monjalon April 15, 2018, 10:37 a.m. UTC | #2 
13/04/2018 19:31, Ferruh Yigit:
> On 3/28/2018 9:57 AM, Wei Dai wrote:
> > This patch check if a requested offloading
> > is supported in the device capability.
> > A per port offloading feature should be enabled or
> > disabled at same time in both rte_eth_dev_configure( )
> > and rte_eth_rx_queue_setup( )/rte_eth_tx_queue_setup( ).
> > This patch check if a per port offloading flag has
> > same configuration in rte_eth_dev_configure( ) and
> > rte_eth_rx_queue_setup( )/rte_eth_tx_queue_setup( ).
> > This patch can make such checking in a common way in
> > rte_ethdev layer to avoid same checking in underlying PMD.
> 
> I think it is good idea to move common check to the abstraction layer as much as
> possible.
> 
> But for this case we are targeting an API change in rc2, I believe better wait
> that API change for this update.

I think Wei could implement some filtering of offload flags:
If an offload is already enabled at port level, we can filter out them
when enabling again at queue level.
By removing such repetition in ethdev, before calling the PMD op,
the PMD does not need to bother for offloads enabled twice.
Wei Dai April 16, 2018, 3:06 a.m. UTC | #3 
Thanks, Thomas and Ferruh
I think I can implement v3 for this.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Sunday, April 15, 2018 6:37 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Dai, Wei <wei.dai@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] ethdev: check Rx/Tx offloads
> 
> 13/04/2018 19:31, Ferruh Yigit:
> > On 3/28/2018 9:57 AM, Wei Dai wrote:
> > > This patch check if a requested offloading is supported in the
> > > device capability.
> > > A per port offloading feature should be enabled or disabled at same
> > > time in both rte_eth_dev_configure( ) and rte_eth_rx_queue_setup(
> > > )/rte_eth_tx_queue_setup( ).
> > > This patch check if a per port offloading flag has same
> > > configuration in rte_eth_dev_configure( ) and
> > > rte_eth_rx_queue_setup( )/rte_eth_tx_queue_setup( ).
> > > This patch can make such checking in a common way in rte_ethdev
> > > layer to avoid same checking in underlying PMD.
> >
> > I think it is good idea to move common check to the abstraction layer
> > as much as possible.
> >
> > But for this case we are targeting an API change in rc2, I believe
> > better wait that API change for this update.
> 
> I think Wei could implement some filtering of offload flags:
> If an offload is already enabled at port level, we can filter out them when
> enabling again at queue level.
> By removing such repetition in ethdev, before calling the PMD op, the PMD
> does not need to bother for offloads enabled twice.
>
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 0590f0c..a04a705 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1152,6 +1152,27 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 							ETHER_MAX_LEN;
 	}
 
+	if ((local_conf.rxmode.offloads & dev_info.rx_offload_capa) !=
+	     local_conf.rxmode.offloads) {
+		RTE_PMD_DEBUG_TRACE("ethdev port_id=%d requested Rx offloads "
+				    "0x%" PRIx64 " doesn't match Rx offloads "
+				    "capability 0x%" PRIx64 "\n",
+				    port_id,
+				    local_conf.rxmode.offloads,
+				    dev_info.rx_offload_capa);
+		return -EINVAL;
+	}
+	if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) !=
+	     local_conf.txmode.offloads) {
+		RTE_PMD_DEBUG_TRACE("ethdev port_id=%d requested Tx offloads "
+				    "0x%" PRIx64 " doesn't match Tx offloads "
+				    "capability 0x%" PRIx64 "\n",
+				    port_id,
+				    local_conf.txmode.offloads,
+				    dev_info.tx_offload_capa);
+		return -EINVAL;
+	}
+
 	/*
 	 * Setup new number of RX/TX queues and reconfigure device.
 	 */
@@ -1404,6 +1425,50 @@  rte_eth_dev_is_removed(uint16_t port_id)
 	return ret;
 }
 
+/**
+* Check if the Rx/Tx queue offloading settings is valid
+* @param queue_offloads
+*   offloads input to rte_eth_rx_queue_setup( ) or rte_eth_tx_queue_setup( )
+* @param port_offloads
+*   Rx or Tx offloads input to rte_eth_dev_configure( )
+* @param queue_offload_capa
+*   rx_queue_offload_capa or tx_queue_offload_capa in struct rte_eth_dev_ifnfo
+*   got from rte_eth_dev_info_get( )
+* @param all_offload_capa
+*   rx_offload_capa or tx_offload_capa in struct rte_eth_dev_info
+*   got from rte_eth_dev_info_get( )
+*
+* @return
+*   Nonzero when per-queue offloading setting is valid
+*/
+static int
+rte_eth_check_queue_offloads(uint64_t queue_offloads,
+			     uint64_t port_offloads,
+			     uint64_t queue_offload_capa,
+			     uint64_t all_offload_capa)
+{
+	uint64_t pure_port_capa = all_offload_capa ^ queue_offload_capa;
+
+	if ((queue_offloads & all_offload_capa) != queue_offloads)
+		return 0;
+
+	if ((port_offloads ^ queue_offloads) & pure_port_capa)
+		return 0;
+
+	return 1;
+}
+
+static int
+rte_eth_check_rx_queue_offloads(uint64_t rx_queue_offloads,
+				const struct rte_eth_rxmode *rxmode,
+				const struct rte_eth_dev_info *dev_info)
+{
+	return rte_eth_check_queue_offloads(rx_queue_offloads,
+					    rxmode->offloads,
+					    dev_info->rx_queue_offload_capa,
+					    dev_info->rx_offload_capa);
+}
+
 int
 rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 		       uint16_t nb_rx_desc, unsigned int socket_id,
@@ -1495,6 +1560,18 @@  rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 						    &local_conf.offloads);
 	}
 
+	if (!rte_eth_check_rx_queue_offloads(local_conf.offloads,
+		&dev->data->dev_conf.rxmode, &dev_info)) {
+		RTE_PMD_DEBUG_TRACE("Ethdev port %d : Rx queue offloads ox%"
+			PRIx64 " don't match port offloads 0x%" PRIx64
+			" or supported offloads 0x%" PRIx64 "\n",
+			port_id,
+			local_conf.offloads,
+			dev->data->dev_conf.rxmode.offloads,
+			dev_info.rx_offload_capa);
+		return -ENOTSUP;
+	}
+
 	ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
 					      socket_id, &local_conf, mp);
 	if (!ret) {
@@ -1555,6 +1632,17 @@  rte_eth_convert_txq_offloads(const uint64_t tx_offloads, uint32_t *txq_flags)
 	*txq_flags = flags;
 }
 
+static int
+rte_eth_check_tx_queue_offloads(uint64_t tx_queue_offloads,
+				const struct rte_eth_txmode *txmode,
+				const struct rte_eth_dev_info *dev_info)
+{
+	return rte_eth_check_queue_offloads(tx_queue_offloads,
+					    txmode->offloads,
+					    dev_info->tx_queue_offload_capa,
+					    dev_info->tx_offload_capa);
+}
+
 int
 rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
 		       uint16_t nb_tx_desc, unsigned int socket_id,
@@ -1622,6 +1710,18 @@  rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
 					  &local_conf.offloads);
 	}
 
+	if (!rte_eth_check_tx_queue_offloads(local_conf.offloads,
+		&dev->data->dev_conf.txmode, &dev_info)) {
+		RTE_PMD_DEBUG_TRACE("Ethdev port %d : Tx queue offloads ox%"
+			PRIx64 " don't match port offloads 0x%" PRIx64
+			" or supported offloads 0x%" PRIx64 "\n",
+			port_id,
+			local_conf.offloads,
+			dev->data->dev_conf.txmode.offloads,
+			dev_info.tx_offload_capa);
+		return -ENOTSUP;
+	}
+
 	return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,
 		       tx_queue_id, nb_tx_desc, socket_id, &local_conf));
 }