diff mbox series

[v4,3/3] ethdev: eliminate interim variable

Message ID 20181113111238.80107-3-ferruh.yigit@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v4,1/3] ethdev: fix invalid device configuration after failure |

Checks

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

Commit Message

Ferruh Yigit Nov. 13, 2018, 11:12 a.m. UTC 
  `local_conf` variable was needed for offload conversions but no more
required. No functional difference, only interim variable eliminated.

Fixes: ab3ce1e0c193 ("ethdev: remove old offload API")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 lib/librte_ethdev/rte_ethdev.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Comments

Andrew Rybchenko Nov. 13, 2018, 11:22 a.m. UTC | #1 
On 11/13/18 2:12 PM, Ferruh Yigit wrote:
> `local_conf` variable was needed for offload conversions but no more
> required. No functional difference, only interim variable eliminated.
>
> Fixes: ab3ce1e0c193 ("ethdev: remove old offload API")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

I guess it will be easier to backport the fix if it is the first fix in 
the series.

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Ferruh Yigit Nov. 13, 2018, 11:51 a.m. UTC | #2 
On 11/13/2018 11:22 AM, Andrew Rybchenko wrote:
> On 11/13/18 2:12 PM, Ferruh Yigit wrote:
>> `local_conf` variable was needed for offload conversions but no more
>> required. No functional difference, only interim variable eliminated.
>>
>> Fixes: ab3ce1e0c193 ("ethdev: remove old offload API")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> I guess it will be easier to backport the fix if it is the first fix in the series.

I thought this is the less important one and that is why put it as last. Didn't
want this patch cause any conflict while getting other fixes.

Which patch do you mention to be easier to backport if this is the first, this
one or others?

> 
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>
Andrew Rybchenko Nov. 13, 2018, 11:56 a.m. UTC | #3 
On 11/13/18 2:51 PM, Ferruh Yigit wrote:
> On 11/13/2018 11:22 AM, Andrew Rybchenko wrote:
>> On 11/13/18 2:12 PM, Ferruh Yigit wrote:
>>> `local_conf` variable was needed for offload conversions but no more
>>> required. No functional difference, only interim variable eliminated.
>>>
>>> Fixes: ab3ce1e0c193 ("ethdev: remove old offload API")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> I guess it will be easier to backport the fix if it is the first fix in the series.
> I thought this is the less important one and that is why put it as last. Didn't
> want this patch cause any conflict while getting other fixes.
>
> Which patch do you mention to be easier to backport if this is the first, this
> one or others?

This one. It touches lines which are moved in the previous patch.
Not really that important.

>> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 0f01138ea..5f858174b 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1093,7 +1093,6 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 	struct rte_eth_dev *dev;
 	struct rte_eth_dev_info dev_info;
 	struct rte_eth_conf orig_conf;
-	struct rte_eth_conf local_conf = *dev_conf;
 	int diag;
 	int ret;
 
@@ -1118,7 +1117,7 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 	 * Copy the dev_conf parameter into the dev structure.
 	 * rte_eth_dev_info_get() requires dev_conf, copy it before dev_info get
 	 */
-	memcpy(&dev->data->dev_conf, &local_conf, sizeof(dev->data->dev_conf));
+	memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
 
 	rte_eth_dev_info_get(port_id, &dev_info);
 
@@ -1192,7 +1191,7 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 	 * If jumbo frames are enabled, check that the maximum RX packet
 	 * length is supported by the configured device.
 	 */
-	if (local_conf.rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
+	if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
 		if (dev_conf->rxmode.max_rx_pkt_len > dev_info.max_rx_pktlen) {
 			RTE_ETHDEV_LOG(ERR,
 				"Ethdev port_id=%u max_rx_pkt_len %u > max valid value %u\n",
@@ -1217,23 +1216,23 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 	}
 
 	/* Any requested offloading must be within its device capabilities */
-	if ((local_conf.rxmode.offloads & dev_info.rx_offload_capa) !=
-	     local_conf.rxmode.offloads) {
+	if ((dev_conf->rxmode.offloads & dev_info.rx_offload_capa) !=
+	     dev_conf->rxmode.offloads) {
 		RTE_ETHDEV_LOG(ERR,
 			"Ethdev port_id=%u requested Rx offloads 0x%"PRIx64" doesn't match Rx offloads "
 			"capabilities 0x%"PRIx64" in %s()\n",
-			port_id, local_conf.rxmode.offloads,
+			port_id, dev_conf->rxmode.offloads,
 			dev_info.rx_offload_capa,
 			__func__);
 		ret = -EINVAL;
 		goto rollback;
 	}
-	if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) !=
-	     local_conf.txmode.offloads) {
+	if ((dev_conf->txmode.offloads & dev_info.tx_offload_capa) !=
+	     dev_conf->txmode.offloads) {
 		RTE_ETHDEV_LOG(ERR,
 			"Ethdev port_id=%u requested Tx offloads 0x%"PRIx64" doesn't match Tx offloads "
 			"capabilities 0x%"PRIx64" in %s()\n",
-			port_id, local_conf.txmode.offloads,
+			port_id, dev_conf->txmode.offloads,
 			dev_info.tx_offload_capa,
 			__func__);
 		ret = -EINVAL;