lic121 <[email protected]> writes:
> On Mon, Apr 25, 2022 at 08:47:32AM -0400, Aaron Conole wrote:
>> lic121 <[email protected]> writes:
>>
>> > Max allowed conntrack entries is configurable with
>> > 'ovs-appctl dpctl/ct-set-maxconns' command. In real scenarios,
>> > this configuration is expected to survive from host reboot.
>> >
>> > Signed-off-by: lic121 <[email protected]>
>> > ---
>>
>> One complication is that there are 2 other conntrack implementations
>> that OvS supports - one for the Windows Netlink, and the other for
>> Linux kernel netlink. How do you deal with this parameter there?
>
> As linux kernel datapath leverages the kernel implementent of conntrack.
> Besides ovs, the kernel conntrack is used by kernel network stack
> itself. A proper way to set linux kernel datapath conntrack max conns
> could be sysctl interface (nf_conntrack_max).
> But I am not sure if a similar system level configuration exists for windows
> datapath.
>
>>
>> Something in the datapath for this needs to accommodate these. Maybe
>> the DB should store the datapath name as well - that way each dp can
>> lookup the configuration if supported (and if not we can either log the
>> error, etc). That does start to look a bit unfriendly, but at least it
>> prevents user confusion with this knob.
>
> Agree, to indiates the datapath name in configuration item name is
> better than explanation in configuration detail description.
> How about the name "userspace-ct-maxconns"?
I guess that should be fine. I might prefer 'netdev-conntrack-maxconns'
or something to that effect. "userspace" makes sense to developers, but
we usually call the userspace side the "netdev" datapath.
Ilya, might have some suggestion.
>>
>> Each CT dp interface does something slightly differently for
>> configuration, and I'm not sure this knob as proposed is the best
>> solution.
>>
>> The 'deprecated' command was only implemented for the userspace DP, but
>> in theory it could work independently for all. I'm not sure this one
>> can work though (for example, if we use AF_XDP for some bridge, but
>> kernel DP for another, we would want to have two different max entry
>> values, but this interface doesn't allow that).
>>
>> > lib/dpctl.man | 3 ++-
>> > lib/dpif-netdev.c | 10 ++++++++++
>> > tests/system-traffic.at | 10 ++++++++++
>> > vswitchd/vswitch.xml | 7 +++++++
>> > 4 files changed, 29 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/lib/dpctl.man b/lib/dpctl.man
>> > index c100d0a..2cfc4f2 100644
>> > --- a/lib/dpctl.man
>> > +++ b/lib/dpctl.man
>> > @@ -343,7 +343,8 @@ system due to connection tracking or simply limiting
>> > connection
>> > tracking. If the number of connections is already over the new maximum
>> > limit request then the new maximum limit will be enforced when the
>> > number of connections decreases to that limit, which normally happens
>> > -due to connection expiry. Only supported for userspace datapath.
>> > +due to connection expiry. Only supported for userspace datapath. This
>> > +command is deprecated by ovsdb cfg other_config:ct-maxconns.
>> > .
>> > .TP
>> > \*(DX\fBct\-get\-maxconns\fR [\fIdp\fR]
>> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> > index 6764343..c518d30 100644
>> > --- a/lib/dpif-netdev.c
>> > +++ b/lib/dpif-netdev.c
>> > @@ -4827,6 +4827,16 @@ dpif_netdev_set_config(struct dpif *dpif, const
>> > struct smap *other_config)
>> > }
>> > }
>> >
>> > + uint32_t ct_maxconns, cur_maxconns;
>> > + ct_maxconns = smap_get_int(other_config, "ct-maxconns", UINT32_MAX);
>> > + /* Leave runtime value as it is when cfg is removed. */
>> > + if (ct_maxconns < UINT32_MAX) {
>> > + conntrack_get_maxconns(dp->conntrack, &cur_maxconns);
>> > + if (ct_maxconns != cur_maxconns) {
>> > + conntrack_set_maxconns(dp->conntrack, ct_maxconns);
>> > + }
>> > + }
>> > +
>> > bool smc_enable = smap_get_bool(other_config, "smc-enable", false);
>> > bool cur_smc;
>> > atomic_read_relaxed(&dp->smc_enable_db, &cur_smc);
>> > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> > index 4a7fa49..00fefc5 100644
>> > --- a/tests/system-traffic.at
>> > +++ b/tests/system-traffic.at
>> > @@ -2258,6 +2258,16 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [],
>> > [dnl
>> > 10
>> > ])
>> >
>> > +AT_CHECK([ovs-vsctl set Open_vswitch . other_config:ct-maxconns=20], [0])
>> > +AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
>> > +20
>> > +])
>> > +
>> > +AT_CHECK([ovs-vsctl remove Open_vswitch . other_config ct-maxconns], [0])
>> > +AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
>> > +20
>> > +])
>> > +
>> > OVS_TRAFFIC_VSWITCHD_STOP
>> > AT_CLEANUP
>> >
>> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> > index 0c66326..ec634d5 100644
>> > --- a/vswitchd/vswitch.xml
>> > +++ b/vswitchd/vswitch.xml
>> > @@ -183,6 +183,13 @@
>> > </p>
>> > </column>
>> >
>> > + <column name="other_config" key="ct-maxconns"
>> > + type='{"type": "integer", "minInteger": 0}'>
>> > + The maximum number of connection tracker entries allowed in the
>> > + datapath. Only supported for userspace datapath. This deprecates
>> > + "ovs-appctl dpctl/ct-set-maxconns" command.
>> > + </column>
>> > +
>> > <column name="other_config" key="max-idle"
>> > type='{"type": "integer", "minInteger": 500}'>
>> > <p>
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
