| Message ID | 20201216120435.3453365-4-mark.d.gray@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ipsec: Various fixes for ovs-monitor-ipsec | expand |
On 16 Dec 2020, at 13:04, Mark Gray wrote: > When 'ovs-monitor-ipsec' exits, it clears all persistent state (i.e. > active ipsec connections, /etc/ipsec.conf, certs/keys). In some > use-cases, we may want to exit and maintain state so that ipsec > connectivity is maintained. One example of this is during an > upgrade. This will require the caller to clear this persistent > state when appropriate (e.g. before 'ovs-monitor-ipsec') is restarted. > > Signed-off-by: Mark Gray <mark.d.gray@redhat.com> > --- > ipsec/ovs-monitor-ipsec.in | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in > index 1793088d9be1..cac42d7b2b31 100755 > --- a/ipsec/ovs-monitor-ipsec.in > +++ b/ipsec/ovs-monitor-ipsec.in > @@ -1146,6 +1146,11 @@ def unixctl_refresh(conn, unused_argv, > unused_aux): > monitor.ike_helper.refresh(monitor) > conn.reply(None) Add extra line before and after this function: ipsec/ovs-monitor-ipsec.in:1151:1: E302 expected 2 blank lines, found 1 ipsec/ovs-monitor-ipsec.in:1157:1: E302 expected 2 blank lines, found 1 > +def unixctl_exit_noflush(conn, unused_argv, unused_aux): > + global exiting > + # Do not clear persistent state > + exiting = True > + conn.reply(None) > > def unixctl_exit(conn, unused_argv, unused_aux): > global monitor > @@ -1205,6 +1210,7 @@ def main(): > ovs.unixctl.command_register("tunnels/show", "", 0, 0, > unixctl_show, None) > ovs.unixctl.command_register("refresh", "", 0, 0, > unixctl_refresh, None) > + ovs.unixctl.command_register("exit/noflush", "", 0, 0, > unixctl_exit_noflush, None) ipsec/ovs-monitor-ipsec.in:1196:80: E501 line too long (88 > 79 characters) > ovs.unixctl.command_register("exit", "", 0, 0, unixctl_exit, > None) > > error, unixctl_server = > ovs.unixctl.server.UnixctlServer.create(None) The patch itself looks fine, so you can add my ack to a v2 if you fix the style issues.
On Wed, Dec 16, 2020 at 07:04:34AM -0500, Mark Gray wrote: > When 'ovs-monitor-ipsec' exits, it clears all persistent state (i.e. > active ipsec connections, /etc/ipsec.conf, certs/keys). In some > use-cases, we may want to exit and maintain state so that ipsec > connectivity is maintained. One example of this is during an > upgrade. This will require the caller to clear this persistent > state when appropriate (e.g. before 'ovs-monitor-ipsec') is restarted. > > Signed-off-by: Mark Gray <mark.d.gray@redhat.com> > --- > ipsec/ovs-monitor-ipsec.in | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in > index 1793088d9be1..cac42d7b2b31 100755 > --- a/ipsec/ovs-monitor-ipsec.in > +++ b/ipsec/ovs-monitor-ipsec.in > @@ -1146,6 +1146,11 @@ def unixctl_refresh(conn, unused_argv, unused_aux): > monitor.ike_helper.refresh(monitor) > conn.reply(None) > > +def unixctl_exit_noflush(conn, unused_argv, unused_aux): > + global exiting > + # Do not clear persistent state > + exiting = True > + conn.reply(None) > > def unixctl_exit(conn, unused_argv, unused_aux): > global monitor > @@ -1205,6 +1210,7 @@ def main(): > ovs.unixctl.command_register("tunnels/show", "", 0, 0, > unixctl_show, None) > ovs.unixctl.command_register("refresh", "", 0, 0, unixctl_refresh, None) > + ovs.unixctl.command_register("exit/noflush", "", 0, 0, unixctl_exit_noflush, None) This is a nice idea. However, ovs-vswitchd(8) implements 'exit --cleanup' to actually release all resources. It would be great to follow the same wording. In this case would be 'exit --no-cleanup'? Does that make sense? fbl > ovs.unixctl.command_register("exit", "", 0, 0, unixctl_exit, None) > > error, unixctl_server = ovs.unixctl.server.UnixctlServer.create(None) > -- > 2.26.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 23/12/2020 19:09, Flavio Leitner wrote: > On Wed, Dec 16, 2020 at 07:04:34AM -0500, Mark Gray wrote: >> When 'ovs-monitor-ipsec' exits, it clears all persistent state (i.e. >> active ipsec connections, /etc/ipsec.conf, certs/keys). In some >> use-cases, we may want to exit and maintain state so that ipsec >> connectivity is maintained. One example of this is during an >> upgrade. This will require the caller to clear this persistent >> state when appropriate (e.g. before 'ovs-monitor-ipsec') is restarted. >> >> Signed-off-by: Mark Gray <mark.d.gray@redhat.com> >> --- >> ipsec/ovs-monitor-ipsec.in | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in >> index 1793088d9be1..cac42d7b2b31 100755 >> --- a/ipsec/ovs-monitor-ipsec.in >> +++ b/ipsec/ovs-monitor-ipsec.in >> @@ -1146,6 +1146,11 @@ def unixctl_refresh(conn, unused_argv, unused_aux): >> monitor.ike_helper.refresh(monitor) >> conn.reply(None) >> >> +def unixctl_exit_noflush(conn, unused_argv, unused_aux): >> + global exiting >> + # Do not clear persistent state >> + exiting = True >> + conn.reply(None) >> >> def unixctl_exit(conn, unused_argv, unused_aux): >> global monitor >> @@ -1205,6 +1210,7 @@ def main(): >> ovs.unixctl.command_register("tunnels/show", "", 0, 0, >> unixctl_show, None) >> ovs.unixctl.command_register("refresh", "", 0, 0, unixctl_refresh, None) >> + ovs.unixctl.command_register("exit/noflush", "", 0, 0, unixctl_exit_noflush, None) > > > This is a nice idea. However, ovs-vswitchd(8) implements 'exit --cleanup' > to actually release all resources. It would be great to follow the same > wording. In this case would be 'exit --no-cleanup'? Does that make sense? I wasn't aware of that and it would be good to follow the convention. > > fbl > >> ovs.unixctl.command_register("exit", "", 0, 0, unixctl_exit, None) >> >> error, unixctl_server = ovs.unixctl.server.UnixctlServer.create(None) >> -- >> 2.26.2 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in index 1793088d9be1..cac42d7b2b31 100755 --- a/ipsec/ovs-monitor-ipsec.in +++ b/ipsec/ovs-monitor-ipsec.in @@ -1146,6 +1146,11 @@ def unixctl_refresh(conn, unused_argv, unused_aux): monitor.ike_helper.refresh(monitor) conn.reply(None) +def unixctl_exit_noflush(conn, unused_argv, unused_aux): + global exiting + # Do not clear persistent state + exiting = True + conn.reply(None) def unixctl_exit(conn, unused_argv, unused_aux): global monitor @@ -1205,6 +1210,7 @@ def main(): ovs.unixctl.command_register("tunnels/show", "", 0, 0, unixctl_show, None) ovs.unixctl.command_register("refresh", "", 0, 0, unixctl_refresh, None) + ovs.unixctl.command_register("exit/noflush", "", 0, 0, unixctl_exit_noflush, None) ovs.unixctl.command_register("exit", "", 0, 0, unixctl_exit, None) error, unixctl_server = ovs.unixctl.server.UnixctlServer.create(None)
When 'ovs-monitor-ipsec' exits, it clears all persistent state (i.e. active ipsec connections, /etc/ipsec.conf, certs/keys). In some use-cases, we may want to exit and maintain state so that ipsec connectivity is maintained. One example of this is during an upgrade. This will require the caller to clear this persistent state when appropriate (e.g. before 'ovs-monitor-ipsec') is restarted. Signed-off-by: Mark Gray <mark.d.gray@redhat.com> --- ipsec/ovs-monitor-ipsec.in | 6 ++++++ 1 file changed, 6 insertions(+)