On 06/03/2025 19:38, Mike Pattrick wrote:
> On Thu, Mar 6, 2025 at 9:53 AM Kevin Traynor <ktray...@redhat.com> 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>
>> ---
>> lib/netdev-dpdk.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 549887b31..35cd2ae17 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -2424,5 +2424,6 @@ 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 (lsc_interrupt_mode && !ret &&
>> + !(*info.dev_flags & RTE_ETH_DEV_INTR_LSC)) {
>
> Hello,
>
> With this, if rte_eth_dev_info_get() returns an error and the card
> does not support lsc_interrupt_mode, we will always set
> requested_lsc_interrupt_mode.
>
> Maybe it would be preferable to wrap all the lines related to
> lsc_interrupt_mode in an "if (!ret)" ?
>
Hi Jay, did you see this comment ^^^ from Mike on your patch - what do
you think ?
If we can't get the device info and the user has requested
"dpdk-lsc-interrupt" == true, then perhaps we should continue to
configure what they requested.
OTOH, we could change the default if the user has not explicitly
requested anything and it is a safer option to prevent a configuration
failure later.
Something like below..?
- lsc_interrupt_mode = smap_get_bool(args, "dpdk-lsc-interrupt", true);
+ lsc_interrupt_mode = smap_get_bool(args, "dpdk-lsc-interrupt",
+ !ret ? true : false);
if (lsc_interrupt_mode && !ret &&
!(*info.dev_flags & RTE_ETH_DEV_INTR_LSC)) {
thanks,
Kevin.
> Cheers,
> M
>
>> if (smap_get(args, "dpdk-lsc-interrupt")) {
>> VLOG_WARN_BUF(errp, "'%s': link status interrupt is not "
>> --
>> 2.48.1
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
