On 06/02/18 16:01, Ilya Maximets wrote:
On 06.02.2018 16:47, Eelco Chaudron wrote:
On 06/02/18 13:12, Ilya Maximets wrote:
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 <[email protected]>
Reviewed-by: Ilya Maximets <[email protected]>
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.
Got your point, but why do you want it to be static? I'm arguing against it
as other options, like other_config:pmd-cpu-mask, are also not static.
If it can be done dynamic, I prefer doing it that way.
'pmd-cpu-mask' change is the only way to configure the number of PMD threads,
and this is important to have this dynamic for run-time scaling. Also, this is
the datapath parameter, not the parameter of any ports. We, actually, have no
dynamic global parameters for ports. All the ports related parameters like
iommu-support, vhost_sock_dir are static or hardcoded.
other_config:dpdk-lsc-interrupt is just the default value that could be
overridden by the dynamic per-port configuration. We're not frequently
adding new physical NICs to servers. At this point user likely knows, which
devices supports LSC interrupts and which are not, and it's possible to decide
which value for default other_config:dpdk-lsc-interrupt is preferable.
I personally prefer to not have global option. Only per-port configurable
with 'false' as default value. This will additionally help with NICs that
doesn't support LSC interrupts, and with mismatch between output of ovs-vsctl
and real configured values. Someday in the future we could change default
to 'true' when most NICs will support this, and if this feature will be
in demand.
Having all this complexity, I would not mind also removing the global
setting.
Any others in favor of keeping it? If not Robert might be able to remove
it in V5?
However if we do go with a static option, it should not effect any existing
NIC configuration, see 4.
2. Per-port option will allow overriding of the global default configuration.
No problem here... However it's not clear to me how we would handle PMDs not
supporting RTE_ETH_DEV_INTR_LSC?For individual interfaces, we might want to
check this flag, and report an error at configuration time, rather than wait
for re-initialization and add something to the log file, EINVAL?
Robert, can we add this in V5?
But how do we deal with the global values and unsupported devices? Currently
looking at the code it will fail initialization after the restart with no
sensible error message.
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.
I agree we need some way to show the current config because if you set the
global
value (and not restart) you will have no way of knowing the current operational
state.The best way would be to have some ovs-appctl command to do this, as logs
might rotate...
Current configuration could be checked by 'ovs-appctl dpif/show'. We just need
to add new value like 'lsc_interrupts_enabled' to netdev_dpdk_get_config().
Robert can you update this function in V5?
See also my general review of V4 before you submit a v5.
Also currently all port settings are on the port, there are no global defaults.
So people need to be aware that doing an "ovs-vsctl show" does not show all the
actual port configuration.
Yes. That is not so elegant. But users should know that ovs-vsctl reports only
the
status of the database, not the status of the daemon. 'dpif/show' and other
'dp'-commands are intended to check the actual status of the working switch.
It's the historical behaviour of the OVS.
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.
Yes, and this makes it some times hard to trouble shoot.
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.
Let me explain, this is with the current implementation if we would have a
global
static option (i.e. v2 of this patch).Assume you configure the following:
ovs-vsctl set Open_vSwitch . other_config:dpdk-lsc-interrupt=true
ovs-vsctl set interface dpdk1 options:n_rxq=2
The last command will trigger a re-configure of dpdk1, and causes now interrupts
to be enabled.
Just want to indicate its a problem with the patch as implemented, if we want to
go with a static global configuration it needs to be fixed.
With v2 of this series first command will update only database. Configuration
inside daemon will not be updated. After the second command dpdk1 will be
reconfigured with the old value of global dpdk-lsc-interrupt, i.e. 'false'.
(I didn't test real v2, but it should work this way if implemented right).
There is no issues here. The only issue is that value in database will mismatch
with actual value used by daemon. After restart of ovs-vswitch, global
dpdk-lsc-interrupt will be updated from the database and dpdk1 will be
configured
with enabled interrupts.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
