[ovs-dev] [PATCH 1/2] dpif-netdev: auto load balance log state on user request.

[Kevin Traynor]

On 12/02/2021 19:17, Ilya Maximets wrote:
> On 2/12/21 6:39 PM, Stokes, Ian wrote:
>>> At present the log displays the auto load balance state
>>> everytime it is changed.
>>>
>>> There are some cases where the user will try to enable
>>> auto load balance, but it cannot be enabled because not
>>> enough PMDs or RxQs. As the state does not change, there
>>> is no new log of the state.
>>>
>>> While the the last log report of state is still correct,
>>> it is better to log the state again at this point so the
>>> user can explicitly confirm the outcome of their request.
>>>
>>> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
>>
>> Thanks for the patch Kevin.
>>
>> This is a nice improvement to be fair. Have tested a number of cases and works as expected.
>>
>> LGTM.
>>
>> Question is should we include this and patch 2 of the series in OVS 2.15?
>>
>> Personally I think it would be worth it. It's not a big change as it's only a log trigger and makes the auto-lb more user intuitive.
>>
>> @Ilya Maximets what are your thoughts on this?
> 
> I'm OK with applying to 2.15.  Seems like a good improvement.
> Didn't test it myself, though.
> 
> Looking forward for auto-lb unit tests. :)
> 

Ah, you don't need to look forward - you can look back to yesterday :-)

https://mail.openvswitch.org/pipermail/ovs-dev/2021-February/380315.html

In fact, as I mentioned, it was writing and running the unit tests that
identified these gaps in the logs. Who knew unit tests could be helpful.

>>
>> Regards
>> Ian
>>> ---
>>>  lib/dpif-netdev.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index e3fd0a07f..4381c618f 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -4347,6 +4347,10 @@ dpif_netdev_set_config(struct dpif *dpif, const
>>> struct smap *other_config)
>>>
>>>      struct pmd_auto_lb *pmd_alb = &dp->pmd_alb;
>>> +    bool cur_rebalance_requested = pmd_alb->auto_lb_requested;
>>>      pmd_alb->auto_lb_requested = smap_get_bool(other_config, "pmd-auto-lb",
>>>                                false);
>>> +    if (cur_rebalance_requested != pmd_alb->auto_lb_requested) {
>>> +        log_autolb = true;
>>> +    }
>>>
>>>      rebalance_intvl = smap_get_int(other_config, "pmd-auto-lb-rebal-interval",
>>> --
>>> 2.26.2
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev at openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>