On Thu, Apr 28, 2022 at 10:04:17AM -0400, Aaron Conole wrote:
> lic121 <lic...@chinatelecom.cn> writes:
>
> > On Mon, Apr 25, 2022 at 08:47:32AM -0400, Aaron Conole wrote:
> >> lic121 <lic...@chinatelecom.cn> 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 <lic...@chinatelecom.cn>
> >> > ---
> >>
> >> 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.
When we say 'netdev', one may think it as 'netdev datapath'. This is what
we expect. But I wonder if someone else may think it as ovs 'netdev
library' that abstracts interacting with network devices.
>
> 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
