[dpdk-dev] [PATCH v5] ethdev: add sanity checks in control APIs
Min Hu (Connor)
humin29 at huawei.com
Fri Apr 16 09:02:27 CEST 2021
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 at dpdk.org
>>
>> Signed-off-by: Min Hu (Connor) <humin29 at huawei.com>
>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
>
> Hi Connor,
>
> Please find a few comments below, with they are addressed:
>
> Reviewed-by: Ferruh Yigit <ferruh.yigit at 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(ð_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.
> .
More information about the dev
mailing list