| Message ID | 20240801162112.62537-1-roriorden@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] controller, controller-vtep: Add --unixctl option. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Recheck-request: github-robot-_ovn-kubernetes On 8/1/24 12:21 PM, Rosemarie O'Riorden wrote: > All other OVN daemons have the --unixctl option to specify the control > socket file. It was missing from ovn-controller and ovn-controller-vtep, > which this patch fixes. > > Reported-at: https://issues.redhat.com/browse/FDP-714 > Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com> > --- > controller-vtep/ovn-controller-vtep.8.xml | 3 +++ > controller-vtep/ovn-controller-vtep.c | 11 ++++++++++- > controller/ovn-controller.8.xml | 3 ++- > controller/ovn-controller.c | 11 ++++++++++- > 4 files changed, 25 insertions(+), 3 deletions(-) > > diff --git a/controller-vtep/ovn-controller-vtep.8.xml b/controller-vtep/ovn-controller-vtep.8.xml > index 89acae7ed..026e18d97 100644 > --- a/controller-vtep/ovn-controller-vtep.8.xml > +++ b/controller-vtep/ovn-controller-vtep.8.xml > @@ -28,6 +28,9 @@ > <xi:include href="lib/ssl-bootstrap.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > <xi:include href="lib/ssl-peer-ca-cert.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > + <h2>Other Options</h2> > + <xi:include href="lib/unixctl.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > + > <h1>Configuration</h1> > <p> > <code>ovn-controller-vtep</code> retrieves its configuration > diff --git a/controller-vtep/ovn-controller-vtep.c b/controller-vtep/ovn-controller-vtep.c > index 4472e5285..698511482 100644 > --- a/controller-vtep/ovn-controller-vtep.c > +++ b/controller-vtep/ovn-controller-vtep.c > @@ -58,6 +58,9 @@ static char *vtep_remote; > static char *ovnsb_remote; > static char *default_db_; > > +/* --unixctl-path: Path to use for unixctl server socket. */ > +static char *unixctl_path; > + > /* Returns true if the northd internal version stored in SB_Global > * and ovn-controller-vtep internal version match. > */ > @@ -118,7 +121,7 @@ main(int argc, char *argv[]) > > daemonize_start(false, false); > > - char *abs_unixctl_path = get_abs_unix_ctl_path(NULL); > + char *abs_unixctl_path = get_abs_unix_ctl_path(unixctl_path); > retval = unixctl_server_create(abs_unixctl_path, &unixctl); > free(abs_unixctl_path); > > @@ -287,6 +290,7 @@ parse_options(int argc, char *argv[]) > {"vtep-db", required_argument, NULL, 'D'}, > {"help", no_argument, NULL, 'h'}, > {"version", no_argument, NULL, 'V'}, > + {"unixctl", required_argument, NULL, 'u'}, > VLOG_LONG_OPTIONS, > OVN_DAEMON_LONG_OPTIONS, > STREAM_SSL_LONG_OPTIONS, > @@ -322,6 +326,10 @@ parse_options(int argc, char *argv[]) > ovn_print_version(OFP13_VERSION, OFP13_VERSION); > exit(EXIT_SUCCESS); > > + case 'u': > + unixctl_path = optarg; > + break; > + > VLOG_OPTION_HANDLERS > OVN_DAEMON_OPTION_HANDLERS > STREAM_SSL_OPTION_HANDLERS > @@ -364,6 +372,7 @@ Options:\n\ > (default: %s)\n\ > --ovnsb-db=DATABASE connect to ovn-sb database at DATABASE\n\ > (default: %s)\n\ > + -u, --unixctl=SOCKET set control socket name\n\ > -h, --help display this help message\n\ > -o, --options list available options\n\ > -V, --version display version information\n\ > diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml > index faefa77b9..bcae4617e 100644 > --- a/controller/ovn-controller.8.xml > +++ b/controller/ovn-controller.8.xml > @@ -47,7 +47,8 @@ > <xi:include href="lib/ssl-peer-ca-cert.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > <h2>Other Options</h2> > - > + <xi:include href="lib/unixctl.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > + <h3></h3> > <xi:include href="lib/common.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index f5674184a..936b42636 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -137,6 +137,9 @@ static const char *ssl_private_key_file; > static const char *ssl_certificate_file; > static const char *ssl_ca_cert_file; > > +/* --unixctl-path: Path to use for unixctl server socket. */ > +static char *unixctl_path; > + > /* By default don't set an upper bound for the lflow cache and enable auto > * trimming above 10K logical flows when reducing cache size by 50%. > */ > @@ -4835,7 +4838,7 @@ main(int argc, char *argv[]) > > daemonize_start(true, false); > > - char *abs_unixctl_path = get_abs_unix_ctl_path(NULL); > + char *abs_unixctl_path = get_abs_unix_ctl_path(unixctl_path); > retval = unixctl_server_create(abs_unixctl_path, &unixctl); > free(abs_unixctl_path); > if (retval) { > @@ -5957,6 +5960,7 @@ parse_options(int argc, char *argv[]) > static struct option long_options[] = { > {"help", no_argument, NULL, 'h'}, > {"version", no_argument, NULL, 'V'}, > + {"unixctl", required_argument, NULL, 'u'}, > VLOG_LONG_OPTIONS, > OVN_DAEMON_LONG_OPTIONS, > STREAM_SSL_LONG_OPTIONS, > @@ -5986,6 +5990,10 @@ parse_options(int argc, char *argv[]) > printf("SB DB Schema %s\n", sbrec_get_db_version()); > exit(EXIT_SUCCESS); > > + case 'u': > + unixctl_path = optarg; > + break; > + > VLOG_OPTION_HANDLERS > OVN_DAEMON_OPTION_HANDLERS > > @@ -6061,6 +6069,7 @@ usage(void) > daemon_usage(); > vlog_usage(); > printf("\nOther options:\n" > + " -u, --unixctl=SOCKET set control socket name\n" > " -n custom chassis name\n" > " -h, --help display this help message\n" > " -V, --version display version information\n");
On 8/1/24 18:21, Rosemarie O'Riorden wrote: > All other OVN daemons have the --unixctl option to specify the control > socket file. It was missing from ovn-controller and ovn-controller-vtep, > which this patch fixes. > > Reported-at: https://issues.redhat.com/browse/FDP-714 > Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com> > --- Looks good to me, thanks! Acked-by: Dumitru Ceara <dceara@redhat.com> Regards, Dumitru
On Wed, Aug 7, 2024 at 5:02 PM Dumitru Ceara <dceara@redhat.com> wrote: > > On 8/1/24 18:21, Rosemarie O'Riorden wrote: > > All other OVN daemons have the --unixctl option to specify the control > > socket file. It was missing from ovn-controller and ovn-controller-vtep, > > which this patch fixes. > > > > Reported-at: https://issues.redhat.com/browse/FDP-714 > > Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com> > > --- > > Looks good to me, thanks! > > Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks for the patch. I added your name to AUTHORS list and applied this patch to main. Numan > > Regards, > Dumitru > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 8/8/24 05:47, Numan Siddique wrote: > On Wed, Aug 7, 2024 at 5:02 PM Dumitru Ceara <dceara@redhat.com> wrote: >> >> On 8/1/24 18:21, Rosemarie O'Riorden wrote: >>> All other OVN daemons have the --unixctl option to specify the control >>> socket file. It was missing from ovn-controller and ovn-controller-vtep, >>> which this patch fixes. >>> >>> Reported-at: https://issues.redhat.com/browse/FDP-714 >>> Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com> >>> --- >> >> Looks good to me, thanks! >> >> Acked-by: Dumitru Ceara <dceara@redhat.com> > > Thanks for the patch. I added your name to AUTHORS list and applied > this patch to main. > Thanks, Numan, but did you actually mean to reply to the other patch from Rosemarie, the one that adds --help and --version? https://patchwork.ozlabs.org/project/ovn/patch/20240802145404.75625-1-roriorden@redhat.com/ The --unixctl patch should, in theory, be accepted only after branching. Regards, Dumitru
On Thu, Aug 8, 2024 at 3:31 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 8/8/24 05:47, Numan Siddique wrote: > > On Wed, Aug 7, 2024 at 5:02 PM Dumitru Ceara <dceara@redhat.com> wrote: > >> > >> On 8/1/24 18:21, Rosemarie O'Riorden wrote: > >>> All other OVN daemons have the --unixctl option to specify the control > >>> socket file. It was missing from ovn-controller and ovn-controller-vtep, > >>> which this patch fixes. > >>> > >>> Reported-at: https://issues.redhat.com/browse/FDP-714 > >>> Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com> > >>> --- > >> > >> Looks good to me, thanks! > >> > >> Acked-by: Dumitru Ceara <dceara@redhat.com> > > > > Thanks for the patch. I added your name to AUTHORS list and applied > > this patch to main. > > > > Thanks, Numan, but did you actually mean to reply to the other patch > from Rosemarie, the one that adds --help and --version? > > https://patchwork.ozlabs.org/project/ovn/patch/20240802145404.75625-1-roriorden@redhat.com/ You're right. I actually applied the doc patch. Thanks Numan > > The --unixctl patch should, in theory, be accepted only after branching. > > Regards, > Dumitru > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 8/1/24 18:21, Rosemarie O'Riorden wrote: > All other OVN daemons have the --unixctl option to specify the control > socket file. It was missing from ovn-controller and ovn-controller-vtep, > which this patch fixes. > > Reported-at: https://issues.redhat.com/browse/FDP-714 > Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com> > --- > controller-vtep/ovn-controller-vtep.8.xml | 3 +++ > controller-vtep/ovn-controller-vtep.c | 11 ++++++++++- > controller/ovn-controller.8.xml | 3 ++- > controller/ovn-controller.c | 11 ++++++++++- > 4 files changed, 25 insertions(+), 3 deletions(-) > > diff --git a/controller-vtep/ovn-controller-vtep.8.xml b/controller-vtep/ovn-controller-vtep.8.xml > index 89acae7ed..026e18d97 100644 > --- a/controller-vtep/ovn-controller-vtep.8.xml > +++ b/controller-vtep/ovn-controller-vtep.8.xml > @@ -28,6 +28,9 @@ > <xi:include href="lib/ssl-bootstrap.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > <xi:include href="lib/ssl-peer-ca-cert.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > + <h2>Other Options</h2> > + <xi:include href="lib/unixctl.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > + This part needs a minor rebase to look the same as a page for ovn-controller (h3 there is important): diff --git a/controller-vtep/ovn-controller-vtep.8.xml b/controller-vtep/ovn-controller-vtep.8.xml index ab9af63b1..6aec43fa7 100644 --- a/controller-vtep/ovn-controller-vtep.8.xml +++ b/controller-vtep/ovn-controller-vtep.8.xml @@ -29,6 +29,8 @@ <xi:include href="lib/ssl-peer-ca-cert.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> <h2>Other Options</h2> + <xi:include href="lib/unixctl.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> + <h3></h3> <xi:include href="lib/common.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> <h1>Configuration</h1> -- I guess, maintainers may do that while applying. Acked-by: Ilya Maximets <i.maximets@ovn.org>
On Tue, Aug 27, 2024 at 11:30 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 8/1/24 18:21, Rosemarie O'Riorden wrote: > > All other OVN daemons have the --unixctl option to specify the control > > socket file. It was missing from ovn-controller and ovn-controller-vtep, > > which this patch fixes. > > > > Reported-at: https://issues.redhat.com/browse/FDP-714 > > Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com> > > --- > > controller-vtep/ovn-controller-vtep.8.xml | 3 +++ > > controller-vtep/ovn-controller-vtep.c | 11 ++++++++++- > > controller/ovn-controller.8.xml | 3 ++- > > controller/ovn-controller.c | 11 ++++++++++- > > 4 files changed, 25 insertions(+), 3 deletions(-) > > > > diff --git a/controller-vtep/ovn-controller-vtep.8.xml b/controller-vtep/ovn-controller-vtep.8.xml > > index 89acae7ed..026e18d97 100644 > > --- a/controller-vtep/ovn-controller-vtep.8.xml > > +++ b/controller-vtep/ovn-controller-vtep.8.xml > > @@ -28,6 +28,9 @@ > > <xi:include href="lib/ssl-bootstrap.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > <xi:include href="lib/ssl-peer-ca-cert.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > > > + <h2>Other Options</h2> > > + <xi:include href="lib/unixctl.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > + > > This part needs a minor rebase to look the same as a page for ovn-controller > (h3 there is important): > > diff --git a/controller-vtep/ovn-controller-vtep.8.xml b/controller-vtep/ovn-controller-vtep.8.xml > index ab9af63b1..6aec43fa7 100644 > --- a/controller-vtep/ovn-controller-vtep.8.xml > +++ b/controller-vtep/ovn-controller-vtep.8.xml > @@ -29,6 +29,8 @@ > <xi:include href="lib/ssl-peer-ca-cert.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > <h2>Other Options</h2> > + <xi:include href="lib/unixctl.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > + <h3></h3> > <xi:include href="lib/common.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > <h1>Configuration</h1> > -- > > I guess, maintainers may do that while applying. > > Acked-by: Ilya Maximets <i.maximets@ovn.org> Thanks. Applied after rebasing. Numan > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller-vtep/ovn-controller-vtep.8.xml b/controller-vtep/ovn-controller-vtep.8.xml index 89acae7ed..026e18d97 100644 --- a/controller-vtep/ovn-controller-vtep.8.xml +++ b/controller-vtep/ovn-controller-vtep.8.xml @@ -28,6 +28,9 @@ <xi:include href="lib/ssl-bootstrap.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> <xi:include href="lib/ssl-peer-ca-cert.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> + <h2>Other Options</h2> + <xi:include href="lib/unixctl.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> + <h1>Configuration</h1> <p> <code>ovn-controller-vtep</code> retrieves its configuration diff --git a/controller-vtep/ovn-controller-vtep.c b/controller-vtep/ovn-controller-vtep.c index 4472e5285..698511482 100644 --- a/controller-vtep/ovn-controller-vtep.c +++ b/controller-vtep/ovn-controller-vtep.c @@ -58,6 +58,9 @@ static char *vtep_remote; static char *ovnsb_remote; static char *default_db_; +/* --unixctl-path: Path to use for unixctl server socket. */ +static char *unixctl_path; + /* Returns true if the northd internal version stored in SB_Global * and ovn-controller-vtep internal version match. */ @@ -118,7 +121,7 @@ main(int argc, char *argv[]) daemonize_start(false, false); - char *abs_unixctl_path = get_abs_unix_ctl_path(NULL); + char *abs_unixctl_path = get_abs_unix_ctl_path(unixctl_path); retval = unixctl_server_create(abs_unixctl_path, &unixctl); free(abs_unixctl_path); @@ -287,6 +290,7 @@ parse_options(int argc, char *argv[]) {"vtep-db", required_argument, NULL, 'D'}, {"help", no_argument, NULL, 'h'}, {"version", no_argument, NULL, 'V'}, + {"unixctl", required_argument, NULL, 'u'}, VLOG_LONG_OPTIONS, OVN_DAEMON_LONG_OPTIONS, STREAM_SSL_LONG_OPTIONS, @@ -322,6 +326,10 @@ parse_options(int argc, char *argv[]) ovn_print_version(OFP13_VERSION, OFP13_VERSION); exit(EXIT_SUCCESS); + case 'u': + unixctl_path = optarg; + break; + VLOG_OPTION_HANDLERS OVN_DAEMON_OPTION_HANDLERS STREAM_SSL_OPTION_HANDLERS @@ -364,6 +372,7 @@ Options:\n\ (default: %s)\n\ --ovnsb-db=DATABASE connect to ovn-sb database at DATABASE\n\ (default: %s)\n\ + -u, --unixctl=SOCKET set control socket name\n\ -h, --help display this help message\n\ -o, --options list available options\n\ -V, --version display version information\n\ diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml index faefa77b9..bcae4617e 100644 --- a/controller/ovn-controller.8.xml +++ b/controller/ovn-controller.8.xml @@ -47,7 +47,8 @@ <xi:include href="lib/ssl-peer-ca-cert.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> <h2>Other Options</h2> - + <xi:include href="lib/unixctl.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> + <h3></h3> <xi:include href="lib/common.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index f5674184a..936b42636 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -137,6 +137,9 @@ static const char *ssl_private_key_file; static const char *ssl_certificate_file; static const char *ssl_ca_cert_file; +/* --unixctl-path: Path to use for unixctl server socket. */ +static char *unixctl_path; + /* By default don't set an upper bound for the lflow cache and enable auto * trimming above 10K logical flows when reducing cache size by 50%. */ @@ -4835,7 +4838,7 @@ main(int argc, char *argv[]) daemonize_start(true, false); - char *abs_unixctl_path = get_abs_unix_ctl_path(NULL); + char *abs_unixctl_path = get_abs_unix_ctl_path(unixctl_path); retval = unixctl_server_create(abs_unixctl_path, &unixctl); free(abs_unixctl_path); if (retval) { @@ -5957,6 +5960,7 @@ parse_options(int argc, char *argv[]) static struct option long_options[] = { {"help", no_argument, NULL, 'h'}, {"version", no_argument, NULL, 'V'}, + {"unixctl", required_argument, NULL, 'u'}, VLOG_LONG_OPTIONS, OVN_DAEMON_LONG_OPTIONS, STREAM_SSL_LONG_OPTIONS, @@ -5986,6 +5990,10 @@ parse_options(int argc, char *argv[]) printf("SB DB Schema %s\n", sbrec_get_db_version()); exit(EXIT_SUCCESS); + case 'u': + unixctl_path = optarg; + break; + VLOG_OPTION_HANDLERS OVN_DAEMON_OPTION_HANDLERS @@ -6061,6 +6069,7 @@ usage(void) daemon_usage(); vlog_usage(); printf("\nOther options:\n" + " -u, --unixctl=SOCKET set control socket name\n" " -n custom chassis name\n" " -h, --help display this help message\n" " -V, --version display version information\n");
All other OVN daemons have the --unixctl option to specify the control socket file. It was missing from ovn-controller and ovn-controller-vtep, which this patch fixes. Reported-at: https://issues.redhat.com/browse/FDP-714 Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com> --- controller-vtep/ovn-controller-vtep.8.xml | 3 +++ controller-vtep/ovn-controller-vtep.c | 11 ++++++++++- controller/ovn-controller.8.xml | 3 ++- controller/ovn-controller.c | 11 ++++++++++- 4 files changed, 25 insertions(+), 3 deletions(-)