[v5] ethdev: add sanity checks in control APIs

Message ID 1618484959-4360-1-git-send-email-humin29@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v5] ethdev: add sanity checks in control APIs |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success travis build: passed
ci/github-robot success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues

Commit Message

humin (Q) April 15, 2021, 11:09 a.m. UTC
  This patch adds more sanity checks in control path APIs.

Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
v5:
* Keep "RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV)"
  and "dev = &rte_eth_devices[port_id]" together.

v4:
* add error logging.
* delete check in control path API.

v3:
* set port_id checked first.
* add error logging.

v2:
* Removed unnecessary checks.
* Deleted checks in internal API.
* Added documentation in the header file.
---
 lib/librte_ethdev/rte_ethdev.c | 313 ++++++++++++++++++++++++++++++++++++-----
 lib/librte_ethdev/rte_ethdev.h |  20 ++-
 2 files changed, 293 insertions(+), 40 deletions(-)
  

Comments

Ferruh Yigit April 15, 2021, 3:38 p.m. UTC | #1
On 4/15/2021 12:09 PM, Min Hu (Connor) wrote:
> This patch adds more sanity checks in control path APIs.
> 
> Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
> Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
> Fixes: 0366137722a0 ("ethdev: check for invalid device name")
> Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process model")
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> Fixes: f8244c6399d9 ("ethdev: increase port id range")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Hi Connor,

Please find a few comments below, with they are addressed:

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

> @@ -738,23 +769,30 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
>   int
>   rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
>   {
> -	int ret = 0;
> -	struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
> -
> -	eth_dev_shared_data_prepare();
> +	struct rte_eth_dev *ethdev;
>   
> -	rte_spinlock_lock(&eth_dev_shared_data->ownership_lock);
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
>   

Not sure '-1' is a good return value, what do you think sending '-ENODEV' as it 
is mostly done in the file?

Also emptly line can be removed as suggested by Andrew.

<...>

> @@ -2138,8 +2181,14 @@ rte_eth_rx_hairpin_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
>   	int count;
>   
>   	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> -
>   	dev = &rte_eth_devices[port_id];
> +
> +	if (conf == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Failed to setup ethdev port %u Rx hairpin queue to NULL\n",
> +			port_id);

When the line is long, can you break the message part to next line, to reduce 
the length at least a few columns, like:

RTE_ETHDEV_LOG(ERR,
	"Failed to setup ethdev port %u Rx hairpin queue to NULL\n",
	port_id);

Same for all long messages.

<...>

> @@ -5619,6 +5853,11 @@ rte_eth_representor_id_get(const struct rte_eth_dev *ethdev,
>   	struct rte_eth_representor_info *info = NULL;
>   	size_t size;
>   
> +	if (ethdev == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Failed to get device representor info from NULL\n");
> +		return -EINVAL;
> +	}
> +

This is also an internal function, not sure if we should verify the 'ehtdev' here.
  
Ferruh Yigit April 15, 2021, 3:45 p.m. UTC | #2
On 4/15/2021 12:09 PM, Min Hu (Connor) wrote:
> This patch adds more sanity checks in control path APIs.
> 
> Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
> Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
> Fixes: 0366137722a0 ("ethdev: check for invalid device name")
> Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process model")
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> Fixes: f8244c6399d9 ("ethdev: increase port id range")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

<...>

> @@ -2684,8 +2767,14 @@ rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats)
>   	struct rte_eth_dev *dev;
>   
>   	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> -
>   	dev = &rte_eth_devices[port_id];
> +
> +	if (stats == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u stats by NULL\n",
> +			port_id);
> +		return -EINVAL;
> +	}
> +
>   	memset(stats, 0, sizeof(*stats));
>   
>   	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->stats_get, -ENOTSUP);

Not exactly related to this patch, but related to the API input, some input that 
is filled by API is memset first before passing it to the PMD, which makes 
sense. And for this we have two different ordering with dev_ops check:

1.
memset(input)
check dev_ops, return error if not supported
call dev_ops

2.
check dev_ops, return error if not supported
memset(input)
call dev_ops

Connor, Thomas, Andrew,
Do you think does it have any benifit to unify it, and is one better than other?
  
Thomas Monjalon April 15, 2021, 4:21 p.m. UTC | #3
15/04/2021 17:45, Ferruh Yigit:
> Not exactly related to this patch, but related to the API input, some input that 
> is filled by API is memset first before passing it to the PMD, which makes 
> sense. And for this we have two different ordering with dev_ops check:
> 
> 1.
> memset(input)
> check dev_ops, return error if not supported
> call dev_ops
> 
> 2.
> check dev_ops, return error if not supported
> memset(input)
> call dev_ops
> 
> Connor, Thomas, Andrew,
> Do you think does it have any benifit to unify it, and is one better than other?

Yes good catch.
It deserves a separate patch to not memset if not needed.
We must check if supported before memset.
  
humin (Q) April 16, 2021, 7:02 a.m. UTC | #4
Hi, Ferruh,
	v6 has been sent to fix it, please check it out, thanks.

在 2021/4/15 23:38, Ferruh Yigit 写道:
> On 4/15/2021 12:09 PM, Min Hu (Connor) wrote:
>> This patch adds more sanity checks in control path APIs.
>>
>> Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
>> Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and 
>> variables")
>> Fixes: 0366137722a0 ("ethdev: check for invalid device name")
>> Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple 
>> process model")
>> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
>> Fixes: f8244c6399d9 ("ethdev: increase port id range")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> Hi Connor,
> 
> Please find a few comments below, with they are addressed:
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
>> @@ -738,23 +769,30 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
>>   int
>>   rte_eth_dev_owner_get(const uint16_t port_id, struct 
>> rte_eth_dev_owner *owner)
>>   {
>> -    int ret = 0;
>> -    struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
>> -
>> -    eth_dev_shared_data_prepare();
>> +    struct rte_eth_dev *ethdev;
>> -    rte_spinlock_lock(&eth_dev_shared_data->ownership_lock);
>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
> 
> Not sure '-1' is a good return value, what do you think sending 
> '-ENODEV' as it is mostly done in the file?
> 
> Also emptly line can be removed as suggested by Andrew.
> 
> <...>
> 
>> @@ -2138,8 +2181,14 @@ rte_eth_rx_hairpin_queue_setup(uint16_t 
>> port_id, uint16_t rx_queue_id,
>>       int count;
>>       RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> -
>>       dev = &rte_eth_devices[port_id];
>> +
>> +    if (conf == NULL) {
>> +        RTE_ETHDEV_LOG(ERR, "Failed to setup ethdev port %u Rx 
>> hairpin queue to NULL\n",
>> +            port_id);
> 
> When the line is long, can you break the message part to next line, to 
> reduce the length at least a few columns, like:
> 
> RTE_ETHDEV_LOG(ERR,
>      "Failed to setup ethdev port %u Rx hairpin queue to NULL\n",
>      port_id);
> 
> Same for all long messages.
> 
> <...>
> 
>> @@ -5619,6 +5853,11 @@ rte_eth_representor_id_get(const struct 
>> rte_eth_dev *ethdev,
>>       struct rte_eth_representor_info *info = NULL;
>>       size_t size;
>> +    if (ethdev == NULL) {
>> +        RTE_ETHDEV_LOG(ERR, "Failed to get device representor info 
>> from NULL\n");
>> +        return -EINVAL;
>> +    }
>> +
> 
> This is also an internal function, not sure if we should verify the 
> 'ehtdev' here.
> .
  
humin (Q) April 16, 2021, 7:04 a.m. UTC | #5
在 2021/4/16 0:21, Thomas Monjalon 写道:
> 15/04/2021 17:45, Ferruh Yigit:
>> Not exactly related to this patch, but related to the API input, some input that
>> is filled by API is memset first before passing it to the PMD, which makes
>> sense. And for this we have two different ordering with dev_ops check:
>>
>> 1.
>> memset(input)
>> check dev_ops, return error if not supported
>> call dev_ops
>>
>> 2.
>> check dev_ops, return error if not supported
>> memset(input)
>> call dev_ops
>>
>> Connor, Thomas, Andrew,
>> Do you think does it have any benifit to unify it, and is one better than other?
> 
> Yes good catch.
> It deserves a separate patch to not memset if not needed.
> We must check if supported before memset.
>
Agreed, I will send next patch to fix it.

>
> .
>
  
Stephen Hemminger April 16, 2021, 4:19 p.m. UTC | #6
On Thu, 15 Apr 2021 16:38:16 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> When the line is long, can you break the message part to next line, to reduce 
> the length at least a few columns, like:
> 
> RTE_ETHDEV_LOG(ERR,
> 	"Failed to setup ethdev port %u Rx hairpin queue to NULL\n",
> 	port_id);
> 
> Same for all long messages.

Why does message have to be so long?
   "Port %u failed: NULL Rx hairpin queue"
  
Stephen Hemminger April 16, 2021, 4:25 p.m. UTC | #7
On Thu, 15 Apr 2021 19:09:19 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> +		RTE_ETHDEV_LOG(ERR, "Failed to get owner id by NULL\n");

This message is awkward wording.

It would be good time to fix something that has causes problems.
Redefine RTE_ETHDEV_LOG to include the function name.
There are several places where same log message can come from different
ethdev functions.

Then change the error you added to:
	RTE_ETHDEV_LOG(ERR, "NULL owner id");

Which would result in:
	eth_dev_owner_set: NULL owner id
  

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 3059aa5..95115a5 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,6 +199,16 @@  rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 	char *cls_str = NULL;
 	int str_size;
 
+	if (iter == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to iterator init for NULL\n");
+		return -EINVAL;
+	}
+
+	if (devargs_str == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to iterate matching NULL\n");
+		return -EINVAL;
+	}
+
 	memset(iter, 0, sizeof(*iter));
 
 	/*
@@ -293,6 +303,11 @@  rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 uint16_t
 rte_eth_iterator_next(struct rte_dev_iterator *iter)
 {
+	if (iter == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to iterate next for NULL\n");
+		return RTE_MAX_ETHPORTS;
+	}
+
 	if (iter->cls == NULL) /* invalid ethdev iterator */
 		return RTE_MAX_ETHPORTS;
 
@@ -322,6 +337,11 @@  rte_eth_iterator_next(struct rte_dev_iterator *iter)
 void
 rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
 {
+	if (iter == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to iterator clear up for NULL\n");
+		return;
+	}
+
 	if (iter->bus_str == NULL)
 		return; /* nothing to free in pure class filter */
 	free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
@@ -622,6 +642,11 @@  rte_eth_find_next_owned_by(uint16_t port_id, const uint64_t owner_id)
 int
 rte_eth_dev_owner_new(uint64_t *owner_id)
 {
+	if (owner_id == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get owner id by NULL\n");
+		return -EINVAL;
+	}
+
 	eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&eth_dev_shared_data->ownership_lock);
@@ -645,6 +670,12 @@  eth_dev_owner_set(const uint16_t port_id, const uint64_t old_owner_id,
 		return -ENODEV;
 	}
 
+	if (new_owner == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u owner to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if (!eth_is_valid_owner_id(new_owner->id) &&
 	    !eth_is_valid_owner_id(old_owner_id)) {
 		RTE_ETHDEV_LOG(ERR,
@@ -738,23 +769,30 @@  rte_eth_dev_owner_delete(const uint64_t owner_id)
 int
 rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
 {
-	int ret = 0;
-	struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
-
-	eth_dev_shared_data_prepare();
+	struct rte_eth_dev *ethdev;
 
-	rte_spinlock_lock(&eth_dev_shared_data->ownership_lock);
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
 
-	if (port_id >= RTE_MAX_ETHPORTS || !eth_dev_is_allocated(ethdev)) {
+	ethdev = &rte_eth_devices[port_id];
+	if (!eth_dev_is_allocated(ethdev)) {
 		RTE_ETHDEV_LOG(ERR, "Port id %"PRIu16" is not allocated\n",
 			port_id);
-		ret = -ENODEV;
-	} else {
-		rte_memcpy(owner, &ethdev->data->owner, sizeof(*owner));
+		return -ENODEV;
 	}
 
+	if (owner == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u owner by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	eth_dev_shared_data_prepare();
+
+	rte_spinlock_lock(&eth_dev_shared_data->ownership_lock);
+	rte_memcpy(owner, &ethdev->data->owner, sizeof(*owner));
 	rte_spinlock_unlock(&eth_dev_shared_data->ownership_lock);
-	return ret;
+
+	return 0;
 }
 
 int
@@ -820,7 +858,7 @@  rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id)
 {
 	uint16_t pid;
 
-	if (name == NULL) {
+	if (name == NULL || port_id == NULL) {
 		RTE_ETHDEV_LOG(ERR, "Null pointer is specified\n");
 		return -EINVAL;
 	}
@@ -1298,9 +1336,14 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 	uint16_t old_mtu;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
 	dev = &rte_eth_devices[port_id];
 
+	if (dev_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to configure ethdev port %u to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP);
 
 	if (dev->data->dev_started) {
@@ -2138,8 +2181,14 @@  rte_eth_rx_hairpin_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 	int count;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
 	dev = &rte_eth_devices[port_id];
+
+	if (conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to setup ethdev port %u Rx hairpin queue to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if (rx_queue_id >= dev->data->nb_rx_queues) {
 		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", rx_queue_id);
 		return -EINVAL;
@@ -2311,6 +2360,13 @@  rte_eth_tx_hairpin_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+
+	if (conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to setup ethdev port %u Tx hairpin queue to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if (tx_queue_id >= dev->data->nb_tx_queues) {
 		RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", tx_queue_id);
 		return -EINVAL;
@@ -2459,6 +2515,11 @@  int
 rte_eth_tx_buffer_set_err_callback(struct rte_eth_dev_tx_buffer *buffer,
 		buffer_tx_error_fn cbfn, void *userdata)
 {
+	if (buffer == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to set error callback for NULL\n");
+		return -EINVAL;
+	}
+
 	buffer->error_callback = cbfn;
 	buffer->error_userdata = userdata;
 	return 0;
@@ -2609,6 +2670,12 @@  rte_eth_link_get(uint16_t port_id, struct rte_eth_link *eth_link)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (eth_link == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u link by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if (dev->data->dev_conf.intr_conf.lsc &&
 	    dev->data->dev_started)
 		rte_eth_linkstatus_get(dev, eth_link);
@@ -2629,6 +2696,12 @@  rte_eth_link_get_nowait(uint16_t port_id, struct rte_eth_link *eth_link)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (eth_link == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u nowait link by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if (dev->data->dev_conf.intr_conf.lsc &&
 	    dev->data->dev_started)
 		rte_eth_linkstatus_get(dev, eth_link);
@@ -2667,6 +2740,16 @@  rte_eth_link_speed_to_str(uint32_t link_speed)
 int
 rte_eth_link_to_str(char *str, size_t len, const struct rte_eth_link *eth_link)
 {
+	if (str == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to convert link to NULL\n");
+		return -EINVAL;
+	}
+
+	if (eth_link == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to convert NULL to string\n");
+		return -EINVAL;
+	}
+
 	if (eth_link->link_status == ETH_LINK_DOWN)
 		return snprintf(str, len, "Link down");
 	else
@@ -2684,8 +2767,14 @@  rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats)
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
 	dev = &rte_eth_devices[port_id];
+
+	if (stats == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u stats by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	memset(stats, 0, sizeof(*stats));
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->stats_get, -ENOTSUP);
@@ -3260,6 +3349,12 @@  rte_eth_dev_fw_version_get(uint16_t port_id, char *fw_version, size_t fw_size)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (fw_version == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u fw version by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fw_version_get, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->fw_version_get)(dev,
 							fw_version, fw_size));
@@ -3278,16 +3373,21 @@  rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	};
 	int diag;
 
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_info == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u info by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	/*
 	 * Init dev_info before port_id check since caller does not have
 	 * return status and does not know if get is successful or not.
 	 */
 	memset(dev_info, 0, sizeof(struct rte_eth_dev_info));
 	dev_info->switch_info.domain_id = RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID;
-
-	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-	dev = &rte_eth_devices[port_id];
-
 	dev_info->rx_desc_lim = lim;
 	dev_info->tx_desc_lim = lim;
 	dev_info->device = dev->device;
@@ -3327,6 +3427,13 @@  rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+
+	if (ptypes == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u supported ptypes by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_supported_ptypes_get, 0);
 	all_ptypes = (*dev->dev_ops->dev_supported_ptypes_get)(dev);
 
@@ -3436,6 +3543,13 @@  rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+
+	if (mac_addr == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u MAC address by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	rte_ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
 
 	return 0;
@@ -3447,8 +3561,14 @@  rte_eth_dev_get_mtu(uint16_t port_id, uint16_t *mtu)
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
 	dev = &rte_eth_devices[port_id];
+
+	if (mtu == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u MTU by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	*mtu = dev->data->mtu;
 	return 0;
 }
@@ -3697,6 +3817,13 @@  rte_eth_dev_flow_ctrl_get(uint16_t port_id, struct rte_eth_fc_conf *fc_conf)
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+
+	if (fc_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u flow conf by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->flow_ctrl_get, -ENOTSUP);
 	memset(fc_conf, 0, sizeof(*fc_conf));
 	return eth_err(port_id, (*dev->dev_ops->flow_ctrl_get)(dev, fc_conf));
@@ -3708,12 +3835,19 @@  rte_eth_dev_flow_ctrl_set(uint16_t port_id, struct rte_eth_fc_conf *fc_conf)
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (fc_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u flow conf to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if ((fc_conf->send_xon != 0) && (fc_conf->send_xon != 1)) {
 		RTE_ETHDEV_LOG(ERR, "Invalid send_xon, only 0/1 allowed\n");
 		return -EINVAL;
 	}
 
-	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->flow_ctrl_set, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->flow_ctrl_set)(dev, fc_conf));
 }
@@ -3725,12 +3859,19 @@  rte_eth_dev_priority_flow_ctrl_set(uint16_t port_id,
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (pfc_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u priority flow conf to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if (pfc_conf->priority > (ETH_DCB_NUM_USER_PRIORITIES - 1)) {
 		RTE_ETHDEV_LOG(ERR, "Invalid priority, only 0-7 allowed\n");
 		return -EINVAL;
 	}
 
-	dev = &rte_eth_devices[port_id];
 	/* High water, low water validation are device specific */
 	if  (*dev->dev_ops->priority_flow_ctrl_set)
 		return eth_err(port_id, (*dev->dev_ops->priority_flow_ctrl_set)
@@ -3744,9 +3885,6 @@  eth_check_reta_mask(struct rte_eth_rss_reta_entry64 *reta_conf,
 {
 	uint16_t i, num;
 
-	if (!reta_conf)
-		return -EINVAL;
-
 	num = (reta_size + RTE_RETA_GROUP_SIZE - 1) / RTE_RETA_GROUP_SIZE;
 	for (i = 0; i < num; i++) {
 		if (reta_conf[i].mask)
@@ -3763,9 +3901,6 @@  eth_check_reta_entry(struct rte_eth_rss_reta_entry64 *reta_conf,
 {
 	uint16_t i, idx, shift;
 
-	if (!reta_conf)
-		return -EINVAL;
-
 	if (max_rxq == 0) {
 		RTE_ETHDEV_LOG(ERR, "No receive queue is available\n");
 		return -EINVAL;
@@ -3796,13 +3931,19 @@  rte_eth_dev_rss_reta_update(uint16_t port_id,
 	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (reta_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to update ethdev port %u rss reta to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	/* Check mask bits */
 	ret = eth_check_reta_mask(reta_conf, reta_size);
 	if (ret < 0)
 		return ret;
 
-	dev = &rte_eth_devices[port_id];
-
 	/* Check entry value */
 	ret = eth_check_reta_entry(reta_conf, reta_size,
 				dev->data->nb_rx_queues);
@@ -3823,13 +3964,19 @@  rte_eth_dev_rss_reta_query(uint16_t port_id,
 	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (reta_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to query ethdev port %u rss reta by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
 
 	/* Check mask bits */
 	ret = eth_check_reta_mask(reta_conf, reta_size);
 	if (ret < 0)
 		return ret;
 
-	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->reta_query, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->reta_query)(dev, reta_conf,
 							    reta_size));
@@ -3844,14 +3991,19 @@  rte_eth_dev_rss_hash_update(uint16_t port_id,
 	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (rss_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to update ethdev port %u rss hash to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
 
 	ret = rte_eth_dev_info_get(port_id, &dev_info);
 	if (ret != 0)
 		return ret;
 
 	rss_conf->rss_hf = rte_eth_rss_hf_refine(rss_conf->rss_hf);
-
-	dev = &rte_eth_devices[port_id];
 	if ((dev_info.flow_type_rss_offloads | rss_conf->rss_hf) !=
 	    dev_info.flow_type_rss_offloads) {
 		RTE_ETHDEV_LOG(ERR,
@@ -3873,6 +4025,13 @@  rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+
+	if (rss_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u rss hash conf by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_conf_get, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
 								   rss_conf));
@@ -4028,6 +4187,13 @@  rte_eth_dev_mac_addr_add(uint16_t port_id, struct rte_ether_addr *addr,
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+
+	if (addr == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to add ethdev port %u MAC address to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_add, -ENOTSUP);
 
 	if (rte_is_zero_ether_addr(addr)) {
@@ -4078,6 +4244,13 @@  rte_eth_dev_mac_addr_remove(uint16_t port_id, struct rte_ether_addr *addr)
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+
+	if (addr == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to remove ethdev port %u MAC address by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_remove, -ENOTSUP);
 
 	index = eth_dev_get_mac_addr_index(port_id, addr);
@@ -4108,11 +4281,17 @@  rte_eth_dev_default_mac_addr_set(uint16_t port_id, struct rte_ether_addr *addr)
 	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (addr == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u default MAC address to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
 
 	if (!rte_is_valid_assigned_ether_addr(addr))
 		return -EINVAL;
 
-	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_set, -ENOTSUP);
 
 	ret = (*dev->dev_ops->mac_addr_set)(dev, addr);
@@ -4163,8 +4342,14 @@  rte_eth_dev_uc_hash_table_set(uint16_t port_id, struct rte_ether_addr *addr,
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
 	dev = &rte_eth_devices[port_id];
+
+	if (addr == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u uc hash table to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if (rte_is_zero_ether_addr(addr)) {
 		RTE_ETHDEV_LOG(ERR, "Port %u: Cannot add NULL MAC address\n",
 			port_id);
@@ -4265,6 +4450,14 @@  rte_eth_mirror_rule_set(uint16_t port_id,
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (mirror_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u mirror rule to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	if (mirror_conf->rule_type == 0) {
 		RTE_ETHDEV_LOG(ERR, "Mirror rule type can not be 0\n");
 		return -EINVAL;
@@ -4291,7 +4484,6 @@  rte_eth_mirror_rule_set(uint16_t port_id,
 		return -EINVAL;
 	}
 
-	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
 
 	return eth_err(port_id, (*dev->dev_ops->mirror_rule_set)(dev,
@@ -5210,6 +5402,12 @@  rte_eth_timesync_read_rx_timestamp(uint16_t port_id, struct timespec *timestamp,
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (timestamp == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to read ethdev port %u Rx timestamp by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_rx_timestamp, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->timesync_read_rx_timestamp)
 				(dev, timestamp, flags));
@@ -5224,6 +5422,12 @@  rte_eth_timesync_read_tx_timestamp(uint16_t port_id,
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (timestamp == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to read ethdev port %u Tx timestamp by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_tx_timestamp, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->timesync_read_tx_timestamp)
 				(dev, timestamp));
@@ -5250,6 +5454,12 @@  rte_eth_timesync_read_time(uint16_t port_id, struct timespec *timestamp)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (timestamp == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to read ethdev port %u time by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_time, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->timesync_read_time)(dev,
 								timestamp));
@@ -5263,6 +5473,12 @@  rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *timestamp)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (timestamp == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to write ethdev port %u time to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_write_time, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->timesync_write_time)(dev,
 								timestamp));
@@ -5276,6 +5492,12 @@  rte_eth_read_clock(uint16_t port_id, uint64_t *clock)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (clock == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to read ethdev port %u clock by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->read_clock, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->read_clock)(dev, clock));
 }
@@ -5361,8 +5583,14 @@  rte_eth_dev_get_dcb_info(uint16_t port_id,
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
 	dev = &rte_eth_devices[port_id];
+
+	if (dcb_info == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u dcb info by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	memset(dcb_info, 0, sizeof(struct rte_eth_dcb_info));
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_dcb_info, -ENOTSUP);
@@ -5412,8 +5640,14 @@  rte_eth_dev_hairpin_capability_get(uint16_t port_id,
 	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
 	dev = &rte_eth_devices[port_id];
+
+	if (cap == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u hairpin capability by NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->hairpin_cap_get, -ENOTSUP);
 	memset(cap, 0, sizeof(*cap));
 	return eth_err(port_id, (*dev->dev_ops->hairpin_cap_get)(dev, cap));
@@ -5619,6 +5853,11 @@  rte_eth_representor_id_get(const struct rte_eth_dev *ethdev,
 	struct rte_eth_representor_info *info = NULL;
 	size_t size;
 
+	if (ethdev == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Failed to get device representor info from NULL\n");
+		return -EINVAL;
+	}
+
 	if (type == RTE_ETH_REPRESENTOR_NONE)
 		return 0;
 	if (repr_id == NULL)
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 6c7c728..29f69ed 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2702,6 +2702,7 @@  int rte_eth_allmulticast_get(uint16_t port_id);
  *   - (0) if successful.
  *   - (-ENOTSUP) if the function is not supported in PMD driver.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_link_get(uint16_t port_id, struct rte_eth_link *link);
 
@@ -2717,6 +2718,7 @@  int rte_eth_link_get(uint16_t port_id, struct rte_eth_link *link);
  *   - (0) if successful.
  *   - (-ENOTSUP) if the function is not supported in PMD driver.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_link_get_nowait(uint16_t port_id, struct rte_eth_link *link);
 
@@ -2752,7 +2754,7 @@  const char *rte_eth_link_speed_to_str(uint32_t link_speed);
  * @param eth_link
  *   Link status returned by rte_eth_link_get function
  * @return
- *   Number of bytes written to str array.
+ *   Number of bytes written to str array or -EINVAL if bad parameter.
  */
 __rte_experimental
 int rte_eth_link_to_str(char *str, size_t len,
@@ -2997,6 +2999,7 @@  int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id,
  * @return
  *   - (0) if successful
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
 
@@ -3041,6 +3044,7 @@  int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  *   - (0) if successful.
  *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
@@ -3060,6 +3064,7 @@  int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
  *   - (-ENOTSUP) if operation is not supported.
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EIO) if device is removed.
+ *   - (-EINVAL) if bad parameter.
  *   - (>0) if *fw_size* is not enough to store firmware version, return
  *          the size of the non truncated string.
  */
@@ -3103,6 +3108,7 @@  int rte_eth_dev_fw_version_get(uint16_t port_id,
  *           only num entries will be filled into the ptypes array, but the full
  *           count of supported ptypes will be returned.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				     uint32_t *ptypes, int num);
@@ -3153,6 +3159,7 @@  int rte_eth_dev_set_ptypes(uint16_t port_id, uint32_t ptype_mask,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_dev_get_mtu(uint16_t port_id, uint16_t *mtu);
 
@@ -3347,7 +3354,7 @@  rte_eth_tx_buffer_init(struct rte_eth_dev_tx_buffer *buffer, uint16_t size);
  * @param userdata
  *   Arbitrary parameter to be passed to the callback function
  * @return
- *   0 on success, or -1 on error with rte_errno set appropriately
+ *   0 on success, or -EINVAL if bad parameter
  */
 int
 rte_eth_tx_buffer_set_err_callback(struct rte_eth_dev_tx_buffer *buffer,
@@ -3774,6 +3781,7 @@  int rte_eth_fec_set(uint16_t port_id, uint32_t fec_capa);
  *   - (-ENOTSUP) if hardware doesn't support flow control.
  *   - (-ENODEV)  if *port_id* invalid.
  *   - (-EIO)  if device is removed.
+ *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_dev_flow_ctrl_get(uint16_t port_id,
 			      struct rte_eth_fc_conf *fc_conf);
@@ -3845,7 +3853,8 @@  int rte_eth_dev_mac_addr_add(uint16_t port_id, struct rte_ether_addr *mac_addr,
  *   - (0) if successful, or *mac_addr* didn't exist.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-ENODEV) if *port* invalid.
- *   - (-EADDRINUSE) if attempting to remove the default MAC address
+ *   - (-EADDRINUSE) if attempting to remove the default MAC address.
+ *   - (-EINVAL) if MAC address is invalid.
  */
 int rte_eth_dev_mac_addr_remove(uint16_t port_id,
 				struct rte_ether_addr *mac_addr);
@@ -4044,6 +4053,7 @@  int rte_eth_dev_rss_hash_update(uint16_t port_id,
  *   - (-ENODEV) if port identifier is invalid.
  *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support RSS.
+ *   - (-EINVAL) if bad parameter.
  */
 int
 rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
@@ -4112,6 +4122,7 @@  rte_eth_dev_udp_tunnel_port_delete(uint16_t port_id,
  *   - (-ENODEV) if port identifier is invalid.
  *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support.
+ *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_dev_get_dcb_info(uint16_t port_id,
 			     struct rte_eth_dcb_info *dcb_info);
@@ -4623,6 +4634,7 @@  int rte_eth_timesync_adjust_time(uint16_t port_id, int64_t delta);
  *
  * @return
  *   - 0: Success.
+ *   - -EINVAL: Bad parameter.
  */
 int rte_eth_timesync_read_time(uint16_t port_id, struct timespec *time);
 
@@ -4689,6 +4701,7 @@  int rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *time);
  *   - 0: Success.
  *   - -ENODEV: The port ID is invalid.
  *   - -ENOTSUP: The function is not supported by the Ethernet driver.
+ *   - -EINVAL: if bad parameter.
  */
 __rte_experimental
 int
@@ -4792,6 +4805,7 @@  rte_eth_dev_get_sec_ctx(uint16_t port_id);
  * @return
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
+ *   - (-EINVAL) if bad parameter.
  */
 __rte_experimental
 int rte_eth_dev_hairpin_capability_get(uint16_t port_id,