On 04/04/2025 14:52, Kevin Traynor via dev wrote:
> From: Jay Ding <jay.d...@broadcom.com>
>
> rte_eth_dev_info_get() could fail due to device reset, etc.
>
> The return value should be checked before the device info
> pointer is dereferenced.
>
> Fixes: 2f196c80e716 ("netdev-dpdk: Use LSC interrupt mode.")
> Signed-off-by: Jay Ding <jay.d...@broadcom.com>
>
> ---
> Sending for Jay for visibility on mailing list, I will send comments next [kt]
> ---
> lib/netdev-dpdk.c | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 549887b31..9d9feb88a 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2423,19 +2423,22 @@ netdev_dpdk_set_config(struct netdev *netdev, const
> struct smap *args,
> }
>
> - lsc_interrupt_mode = smap_get_bool(args, "dpdk-lsc-interrupt", true);
> - if (lsc_interrupt_mode && !(*info.dev_flags & RTE_ETH_DEV_INTR_LSC)) {
> - if (smap_get(args, "dpdk-lsc-interrupt")) {
> - VLOG_WARN_BUF(errp, "'%s': link status interrupt is not "
> - "supported.", netdev_get_name(netdev));
> - err = EINVAL;
> - goto out;
> + if (!ret) {
> + lsc_interrupt_mode = smap_get_bool(args, "dpdk-lsc-interrupt", true);
> + if (lsc_interrupt_mode && !(*info.dev_flags & RTE_ETH_DEV_INTR_LSC))
> {
> + if (smap_get(args, "dpdk-lsc-interrupt")) {
> + VLOG_WARN_BUF(errp, "'%s': link status interrupt is not "
> + "supported.", netdev_get_name(netdev));
> + err = EINVAL;
> + goto out;
> + }
> + VLOG_DBG("'%s': not enabling link status interrupt.",
> + netdev_get_name(netdev));
> + lsc_interrupt_mode = false;
> + }
> +
> + if (dev->requested_lsc_interrupt_mode != lsc_interrupt_mode) {
> + dev->requested_lsc_interrupt_mode = lsc_interrupt_mode;
> + netdev_request_reconfigure(netdev);
> }
> - VLOG_DBG("'%s': not enabling link status interrupt.",
> - netdev_get_name(netdev));
> - lsc_interrupt_mode = false;
> - }
> - if (dev->requested_lsc_interrupt_mode != lsc_interrupt_mode) {
> - dev->requested_lsc_interrupt_mode = lsc_interrupt_mode;
> - netdev_request_reconfigure(netdev);
> }
>
Hi Jay,
This will just skip the lsc config setting. The comment was to not
continue the config if we can't get the device status.
Also, if we are doing that, then there is no need to keep the fallback
for queue size as we will have already returned. So maybe something like
below (untested) ?
thanks,
Kevin.
@@ -2333,5 +2333,4 @@ netdev_dpdk_set_config(struct netdev *netdev,
const struct smap *args,
const char *vf_mac;
int err = 0;
- int ret;
ovs_mutex_lock(&dpdk_mutex);
@@ -2397,8 +2396,13 @@ netdev_dpdk_set_config(struct netdev *netdev,
const struct smap *args,
}
- ret = rte_eth_dev_info_get(dev->port_id, &info);
+ err = -rte_eth_dev_info_get(dev->port_id, &info);
+ if (!err) {
+ VLOG_ERR("Interface %s rte_eth_dev_info_get error: %s",
+ dev->up.name, rte_strerror(err));
+ goto out;
+ }
- dpdk_process_queue_size(netdev, args, !ret ? &info : NULL, true);
- dpdk_process_queue_size(netdev, args, !ret ? &info : NULL, false);
+ dpdk_process_queue_size(netdev, args, &info, true);
+ dpdk_process_queue_size(netdev, args, &info, false);
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
