Re: [ovs-dev] [PATCH v2] dpif-netdev: ct maxconns config persistence

[lic121]

On Tue, May 17, 2022 at 09:06:24AM -0400, Aaron Conole wrote:
> lic121  writes:
> 
> > 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 
> > ---
> 
> Sorry - just one last comment, and then I think this can go in.
> 
> Please add a warning to dpctl_ct_set_maxconns that also flags the
> deprecation, and add a NEWS entry as well.  That way end users are aware
> of the change.  If you want to add NEWS entry as a separate patch
> (because those can generate git-am related issues) that's probably okay.

No worry, and thanks for your careful review. I will add warning and add NEWS in
the next version.

> 
> > Notes:
> > v2:
> >   - rename "ct-maxconns" to "userspace-ct-maxconns"
> >
> >  lib/dpctl.man   |  4 +++-
> >  lib/dpif-netdev.c   | 11 +++
> >  tests/system-traffic.at | 10 ++
> >  vswitchd/vswitch.xml|  7 +++
> >  4 files changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/dpctl.man b/lib/dpctl.man
> > index c100d0a..4f08a3f 100644
> > --- a/lib/dpctl.man
> > +++ b/lib/dpctl.man
> > @@ -343,7 +343,9 @@ 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:userspace-ct-maxconns
> > +because of persistence capability.
> >  .
> >  .TP
> >  \*(DX\fBct\-get\-maxconns\fR [\fIdp\fR]
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 6764343..e2348a1 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -4827,6 +4827,17 @@ 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, "userspace-ct-maxconns",
> > +   UINT32_MAX);
> > +/* Leave runtime value as it is when cfg is removed. */
> > +if (ct_maxconns < UINT32_MAX) {
> > +conntrack_get_maxconns(dp->conntrack, _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(>smc_enable_db, _smc);
> > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> > index 4a7fa49..82ea992 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:userspace-ct-maxconns=20], [0])
> > +AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
> > +20
> > +])
> > +
> > +AT_CHECK([ovs-vsctl remove Open_vswitch . other_config 
> > userspace-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..48f05d7 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -183,6 +183,13 @@
> >  
> >
> >  
> > +   > +  type='{"type": "integer", "minInteger": 0}'>
> > +The maximum number of connection tracker entries allowed in the
> > +userspace datapath.  This deprecates "ovs-appctl 
> > dpctl/ct-set-maxconns"
> > +command.
> > +  
> > +
> > >type='{"type": "integer", "minInteger": 500}'>
> >  
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] dpif-netdev: ct maxconns config persistence

[Aaron Conole]

lic121  writes:

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

Sorry - just one last comment, and then I think this can go in.

Please add a warning to dpctl_ct_set_maxconns that also flags the
deprecation, and add a NEWS entry as well.  That way end users are aware
of the change.  If you want to add NEWS entry as a separate patch
(because those can generate git-am related issues) that's probably okay.

> Notes:
> v2:
>   - rename "ct-maxconns" to "userspace-ct-maxconns"
>
>  lib/dpctl.man   |  4 +++-
>  lib/dpif-netdev.c   | 11 +++
>  tests/system-traffic.at | 10 ++
>  vswitchd/vswitch.xml|  7 +++
>  4 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpctl.man b/lib/dpctl.man
> index c100d0a..4f08a3f 100644
> --- a/lib/dpctl.man
> +++ b/lib/dpctl.man
> @@ -343,7 +343,9 @@ 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:userspace-ct-maxconns
> +because of persistence capability.
>  .
>  .TP
>  \*(DX\fBct\-get\-maxconns\fR [\fIdp\fR]
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 6764343..e2348a1 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4827,6 +4827,17 @@ 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, "userspace-ct-maxconns",
> +   UINT32_MAX);
> +/* Leave runtime value as it is when cfg is removed. */
> +if (ct_maxconns < UINT32_MAX) {
> +conntrack_get_maxconns(dp->conntrack, _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(>smc_enable_db, _smc);
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 4a7fa49..82ea992 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:userspace-ct-maxconns=20], [0])
> +AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
> +20
> +])
> +
> +AT_CHECK([ovs-vsctl remove Open_vswitch . other_config 
> userspace-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..48f05d7 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -183,6 +183,13 @@
>  
>
>  
> +   +  type='{"type": "integer", "minInteger": 0}'>
> +The maximum number of connection tracker entries allowed in the
> +userspace datapath.  This deprecates "ovs-appctl 
> dpctl/ct-set-maxconns"
> +command.
> +  
> +
>type='{"type": "integer", "minInteger": 500}'>
>  

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] dpif-netdev: ct maxconns config persistence

[lic121]

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

Notes:
v2:
  - rename "ct-maxconns" to "userspace-ct-maxconns"

 lib/dpctl.man   |  4 +++-
 lib/dpif-netdev.c   | 11 +++
 tests/system-traffic.at | 10 ++
 vswitchd/vswitch.xml|  7 +++
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/dpctl.man b/lib/dpctl.man
index c100d0a..4f08a3f 100644
--- a/lib/dpctl.man
+++ b/lib/dpctl.man
@@ -343,7 +343,9 @@ 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:userspace-ct-maxconns
+because of persistence capability.
 .
 .TP
 \*(DX\fBct\-get\-maxconns\fR [\fIdp\fR]
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 6764343..e2348a1 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4827,6 +4827,17 @@ 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, "userspace-ct-maxconns",
+   UINT32_MAX);
+/* Leave runtime value as it is when cfg is removed. */
+if (ct_maxconns < UINT32_MAX) {
+conntrack_get_maxconns(dp->conntrack, _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(>smc_enable_db, _smc);
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 4a7fa49..82ea992 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:userspace-ct-maxconns=20], 
[0])
+AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
+20
+])
+
+AT_CHECK([ovs-vsctl remove Open_vswitch . other_config userspace-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..48f05d7 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -183,6 +183,13 @@
 
   
 
+  
+The maximum number of connection tracker entries allowed in the
+userspace datapath.  This deprecates "ovs-appctl dpctl/ct-set-maxconns"
+command.
+  
+
   
 
-- 
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev