[ovs-dev] [ovs-dev, ovs-dev, v4] netdev-dpdk: Configurable Link State Change (LSC) detection mode

[Ilya Maximets]

On 05.02.2018 19:29, Eelco Chaudron wrote:
> On 02/02/18 17:17, Ilya Maximets wrote:
>> On 02.02.2018 17:05, Róbert Mulik wrote:
>>> It is possible to change LSC detection mode to polling or interrupt mode
>>> for DPDK interfaces. The default is polling mode. To set interrupt mode,
>>> option dpdk-lsc-interrupt has to be set to true.
>>>
>>> In polling mode more processor time is needed, since the OVS repeatedly reads
>>> the link state with a short period. It can lead to packet loss for certain
>>> systems.
>>>
>>> In interrupt mode the hardware itself triggers an interrupt when link state
>>> change happens, so less processing time needs for the OVS. It is not possible
>>> to enable the interrupt mode on all hardware.
>>>
>>> For detailed description and usage see the dpdk install documentation.
>>>
>>> Signed-off-by: Robert Mulik <robert.mulik at ericsson.com>
>>> Reviewed-by: Ilya Maximets <i.maximets at samsung.com>
>> 8<---- SNIP ---->8
>>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>>> index f44f950..87608a5 100644
>>> --- a/vswitchd/bridge.c
>>> +++ b/vswitchd/bridge.c
>>> @@ -68,6 +68,9 @@
>>>   #include "lib/vswitch-idl.h"
>>>   #include "xenserver.h"
>>>   #include "vlan-bitmap.h"
>>> +#ifdef DPDK_NETDEV
>>> +#include "./lib/netdev-provider.h"
>>> +#endif
>>>
>>>   VLOG_DEFINE_THIS_MODULE(bridge);
>>>
>>> @@ -601,6 +604,11 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
>>>       ofproto_set_vlan_limit(smap_get_int(&ovs_cfg->other_config, "vlan-limit",
>>>                                          LEGACY_MAX_VLAN_HEADERS));
>>>
>>> +#ifdef DPDK_NETDEV
>>> +    netdev_dpdk_set_def_lsc_int_mode_enabled(
>>> +        smap_get_bool(&ovs_cfg->other_config, "dpdk-lsc-interrupt", false));
>>> +#endif
>>> +
>> I'm still thinking that this is not necessary.
>> Eelco, why do you think we need to change the default global value in run-time?
>> IMHO, this only complicates the code. (I didn't check, but it looks like current
>> implementation will not work anyway.)
>>
> I do feel like the global option should be applied directly for the following reasons:
> 
> - There is no way to see what is the running configuration
> - If not done this way the LSC mode might change for some devices, for example if you decide to change another port related configuration.
> 


I feel that we have some kind of misunderstanding.
I'll try to explain my position:

1. I'm not against the global config option. I just want it to be static.
   i.e. OVS restart will be required to change it.

2. Per-port option will allow overriding of the global default configuration.

3. You're saying that the running configuration is not visible, but It'll be
   visible through global default value in other_config and per-port value in
   options of the particular interface. If we'll not set any of these configs,
   the value will not be visible in any case.

   Suggestion: We definitely need to export the current configuration in
               netdev_dpdk_get_config(). And it'll be always visible.

   Maybe you're saying that we can update global default in database and this
   will hide real default value? But it's the default behaviour for many other
   'restart required' options like iommu support of vhost_sock_dir.
   We need to inform about the global default value in VLOG message while
   initializing inside dpdk_init() like it's done for all other options.

4. I didn't understand your second reason. There is no way to change the current
   port by changing config of the another one. DPDK initialized only once, so
   default will be fixed on boot and never changed. Any database changes for a
   global other_config after the initialization phase will be ignored.

Best regards, Ilya Maximets.