This looks good to me.
Thanks,
Jay
On Fri, Apr 4, 2025 at 10:02 AM Kevin Traynor <ktray...@redhat.com> wrote:
> 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);
>
>
>
>
>
>
>
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
