[PATCH 30/62] net/txgbe: use kvargs numeric helpers

[Stephen Hemminger]

txgbe_handle_devarg() truncates the strtoul() result into a uint16_t and
then infers overflow from the truncated value being USHRT_MAX, so a
value such as "65536" is silently accepted as zero. It also never
checked the end pointer, and read errno without clearing it first.

All thirteen rte_kvargs_process() results were discarded, so a bad
value was ignored and the default used instead. Collect them and fail
the probe, which makes a malformed devarg visible rather than silently
changing the configuration the user asked for.

auto_neg, poll, present, sgmii, tx_headwb and rx_desc_merge are
booleans, so parse them with rte_kvargs_handle_bool() into a bool. The
struct txgbe_devargs fields stay u16, since they live in base code
shared with the vendor tree; a bool assigned into them still yields 0
or 1. A bare key now enables the option, and the usual spellings are
accepted.

Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
---
 drivers/net/txgbe/txgbe_ethdev.c | 91 +++++++++++++++-----------------
 1 file changed, 42 insertions(+), 49 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c
index 814840b3f5..b99e1ad1bc 100644
--- a/drivers/net/txgbe/txgbe_ethdev.c
+++ b/drivers/net/txgbe/txgbe_ethdev.c
@@ -501,22 +501,6 @@ txgbe_swfw_lock_reset(struct txgbe_hw *hw)
 }
 
 static int
-txgbe_handle_devarg(__rte_unused const char *key, const char *value,
-		  void *extra_args)
-{
-	uint16_t *n = extra_args;
-
-	if (value == NULL || extra_args == NULL)
-		return -EINVAL;
-
-	*n = (uint16_t)strtoul(value, NULL, 10);
-	if (*n == USHRT_MAX && errno == ERANGE)
-		return -1;
-
-	return 0;
-}
-
-static void
 txgbe_parse_devargs(struct rte_eth_dev *dev)
 {
 	struct rte_eth_fdir_conf *fdir_conf = TXGBE_DEV_FDIR_CONF(dev);
@@ -524,10 +508,10 @@ txgbe_parse_devargs(struct rte_eth_dev *dev)
 	struct rte_devargs *devargs = pci_dev->device.devargs;
 	struct txgbe_hw *hw = TXGBE_DEV_HW(dev);
 	struct rte_kvargs *kvlist;
-	u16 auto_neg = 1;
-	u16 poll = 0;
-	u16 present = 0;
-	u16 sgmii = 0;
+	bool auto_neg = true;
+	bool poll = false;
+	bool present = false;
+	bool sgmii = false;
 	u16 ffe_set = 0;
 	u16 ffe_main = 27;
 	u16 ffe_pre = 8;
@@ -536,9 +520,10 @@ txgbe_parse_devargs(struct rte_eth_dev *dev)
 	u16 pballoc = 0;
 	u16 drop_queue = 127;
 	/* New devargs for amberlite config */
-	u16 tx_headwb = 1;
+	bool tx_headwb = true;
 	u16 tx_headwb_size = 16;
-	u16 rx_desc_merge = 1;
+	bool rx_desc_merge = true;
+	int ret;
 
 	if (devargs == NULL)
 		goto null;
@@ -547,34 +532,37 @@ txgbe_parse_devargs(struct rte_eth_dev *dev)
 	if (kvlist == NULL)
 		goto null;
 
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_BP_AUTO,
-			   &txgbe_handle_devarg, &auto_neg);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_KR_POLL,
-			   &txgbe_handle_devarg, &poll);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_KR_PRESENT,
-			   &txgbe_handle_devarg, &present);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_KX_SGMII,
-			   &txgbe_handle_devarg, &sgmii);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_FFE_SET,
-			   &txgbe_handle_devarg, &ffe_set);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_FFE_MAIN,
-			   &txgbe_handle_devarg, &ffe_main);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_FFE_PRE,
-			   &txgbe_handle_devarg, &ffe_pre);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_FFE_POST,
-			   &txgbe_handle_devarg, &ffe_post);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_FDIR_PBALLOC,
-			   &txgbe_handle_devarg, &pballoc);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_FDIR_DROP_QUEUE,
-			   &txgbe_handle_devarg, &drop_queue);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_TX_HEAD_WB,
-			   &txgbe_handle_devarg, &tx_headwb);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_TX_HEAD_WB_SIZE,
-			   &txgbe_handle_devarg, &tx_headwb_size);
-	rte_kvargs_process(kvlist, TXGBE_DEVARG_RX_DESC_MERGE,
-			   &txgbe_handle_devarg, &rx_desc_merge);
+	ret = rte_kvargs_process_opt(kvlist, TXGBE_DEVARG_BP_AUTO,
+				     rte_kvargs_handle_bool, &auto_neg);
+	ret |= rte_kvargs_process_opt(kvlist, TXGBE_DEVARG_KR_POLL,
+				      rte_kvargs_handle_bool, &poll);
+	ret |= rte_kvargs_process_opt(kvlist, TXGBE_DEVARG_KR_PRESENT,
+				      rte_kvargs_handle_bool, &present);
+	ret |= rte_kvargs_process_opt(kvlist, TXGBE_DEVARG_KX_SGMII,
+				      rte_kvargs_handle_bool, &sgmii);
+	ret |= rte_kvargs_process(kvlist, TXGBE_DEVARG_FFE_SET,
+				  rte_kvargs_handle_u16, &ffe_set);
+	ret |= rte_kvargs_process(kvlist, TXGBE_DEVARG_FFE_MAIN,
+				  rte_kvargs_handle_u16, &ffe_main);
+	ret |= rte_kvargs_process(kvlist, TXGBE_DEVARG_FFE_PRE,
+				  rte_kvargs_handle_u16, &ffe_pre);
+	ret |= rte_kvargs_process(kvlist, TXGBE_DEVARG_FFE_POST,
+				  rte_kvargs_handle_u16, &ffe_post);
+	ret |= rte_kvargs_process(kvlist, TXGBE_DEVARG_FDIR_PBALLOC,
+				  rte_kvargs_handle_u16, &pballoc);
+	ret |= rte_kvargs_process(kvlist, TXGBE_DEVARG_FDIR_DROP_QUEUE,
+				  rte_kvargs_handle_u16, &drop_queue);
+	ret |= rte_kvargs_process_opt(kvlist, TXGBE_DEVARG_TX_HEAD_WB,
+				      rte_kvargs_handle_bool, &tx_headwb);
+	ret |= rte_kvargs_process(kvlist, TXGBE_DEVARG_TX_HEAD_WB_SIZE,
+				  rte_kvargs_handle_u16, &tx_headwb_size);
+	ret |= rte_kvargs_process_opt(kvlist, TXGBE_DEVARG_RX_DESC_MERGE,
+				      rte_kvargs_handle_bool, &rx_desc_merge);
 	rte_kvargs_free(kvlist);
 
+	if (ret != 0)
+		return -EINVAL;
+
 null:
 	hw->devarg.auto_neg = auto_neg;
 	hw->devarg.poll = poll;
@@ -590,6 +578,8 @@ txgbe_parse_devargs(struct rte_eth_dev *dev)
 
 	fdir_conf->pballoc = pballoc;
 	fdir_conf->drop_queue = drop_queue;
+
+	return 0;
 }
 
 static void
@@ -690,7 +680,10 @@ eth_txgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	hw->isb_dma = TMZ_PADDR(mz);
 	hw->isb_mem = TMZ_VADDR(mz);
 
-	txgbe_parse_devargs(eth_dev);
+	err = txgbe_parse_devargs(eth_dev);
+	if (err != 0)
+		return err;
+
 	/* Initialize the shared code (base driver) */
 	err = txgbe_init_shared_code(hw);
 	if (err != 0) {
-- 
2.53.0