On 5/21/22 05:55, lic121 wrote:
> Max allowed userspace dp conntrack entries is configurable with
> 'ovs-appctl dpctl/ct-set-maxconns' command. In real scenarios,
> this configuration is expected to survive from host reboot, from
> ovs service restart.
>
> Signed-off-by: lic121 <[email protected]>
> ---
>
> Notes:
> v4:
> - add '\n' for warning msg
> v3:
> - add a warning to dpctl_ct_set_maxconns
> - add NEWS entry
> v2:
> - rename "ct-maxconns" to "userspace-ct-maxconns"
>
> NEWS | 5 +++++
> lib/dpctl.c | 3 +++
> lib/dpctl.man | 4 +++-
> lib/dpif-netdev.c | 11 +++++++++++
> tests/system-traffic.at | 10 ++++++++++
> vswitchd/vswitch.xml | 7 +++++++
> 6 files changed, 39 insertions(+), 1 deletion(-)
Hi. Sorry for the late reply.
The ct configuration in general seems to be a complete mess at the
moment and I agree that this kind of configuration should be
persistent, i.e. stored in the database. I'm not sure about the
exact implementation though.
First of all, It doesn't seem great to have some options configured
via database and some still via dpctl. I think, it's better to move
all options at once and deprecate all the old APIs to avoid confusion
in the future.
Secondly, while most of the options are strictly for userspace, some
are not, e.g. ct-set-limits. And dpctl provides a unified interface
for all of them, so we should probably have unified database
configuration knobs as well. Userspace-only options will need to
have a note in the documentation and also trigger a warning in the
log if configured for a system datapath type.
Suggesting a schema change like this:
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 4873cfde7..4dad41d4c 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -656,6 +656,28 @@
"value": {"type": "uuid",
"refTable": "CT_Zone"},
"min": 0, "max": "unlimited"}},
+ "ct_maxconns": {
+ "type": {"key": {"type": "integer",
+ "minInteger": 0,
+ "maxInteger" : 4294967295},
+ "min": 0, "max": 1}},
+ "ct_tcp_seq_check": {
+ "type": {"key": {"type": "boolean"},
+ "min": 0, "max": 1}},
+ "ct_ipf_enabled": {
+ "type": {"key": {"type": "string", "enum": ["set", ["v4", "v6"]]},
+ "value": "boolean",
+ "min": 0, "max": 2}},
+ "ct_ipf_min_frag_size": {
+ "type": {"key": {"type": "string", "enum": ["set", ["v4", "v6"]]},
+ "value": {"type": "integer",
+ "minInteger": 400, "maxInteger": 5000},
+ "min": 0, "max": 2}},
+ "ct_ipf_max_nfrag": {
+ "type": {"key": {"type": "integer",
+ "minInteger": 0,
+ "maxInteger" : 4294967295},
+ "min": 0, "max": 1}},
"capabilities": {
"type": {"key": "string", "value": "string",
"min": 0, "max": "unlimited"}},
@@ -668,6 +690,10 @@
"type": {"key": {"type": "uuid",
"refTable": "CT_Timeout_Policy"},
"min": 0, "max": 1}},
+ "limit": {"key": {"type": "integer",
+ "minInteger": 0,
+ "maxInteger" : 4294967295},
+ "min": 0, "max": 1}},
"external_ids": {
"type": {"key": "string", "value": "string",
"min": 0, "max": "unlimited"}}}},
---
All the configs are optional, but if they are specified, datapath_reconfigure()
should take the values and update the corresponding configuration via
ofproto_ct_* API calls. Failures to do so, either due to no support from the
underlying datapath or for other reasons should be logged at WARN level.
Corresponding ovs-vsctl commands should be added to set these values.
All the dpctl commands should be deprecated, at least 'set' ones. We may
also deprecate all the 'get' commands as well and replace all of them with
a single 'show'-type of a command, i.e. conntrack/show or something like that.
What do you think? Aaron, Paolo?
One caveat here is that entries in the 'Datapath' table are not created
automatically, which is also weird. Though, should not be very hard to create
them once all the datapath types are registered, maybe inside the same
datapath_reconfigure() function.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
