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"?

> 
> 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

Reply via email to