| Message ID | 20240717160337.660920-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovs: Bump submodule to branch-3.4. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Wed, Jul 17, 2024 at 6:04 PM Ilya Maximets <i.maximets@ovn.org> wrote: > Updating to OVS 3.4 to get enhanced sample action and other latest > features and fixes in general. > > Unixctl library was changed to provide JSON-formatted output, so > clients has to be adjusted. Clients also need to handle extra line > breaks at the end, since those are no longer prvided by the server. > > We could define a common reply_to_string() function for both dbctl > and appctl, but these programs beahve differently in regerads to exit > codes and we'll need to do more refactoring to handle the difference. > > Will move to v3.4.0 once it is available. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > controller/physical.c | 4 ++-- > lib/actions.c | 6 +++--- > ovs | 2 +- > utilities/ovn-appctl.c | 42 ++++++++++++++++++++++++++++++++++++------ > utilities/ovn-dbctl.c | 40 +++++++++++++++++++++++++++++++++------- > 5 files changed, 75 insertions(+), 19 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index 22756810f..0255a1bbd 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -889,8 +889,8 @@ put_drop(const struct physical_debug *debug, uint8_t > table_id, > struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts); > os->probability = UINT16_MAX; > os->collector_set_id = debug->collector_set_id; > - os->obs_domain_id = (debug->obs_domain_id << 24); > - os->obs_point_id = table_id; > + os->obs_domain_imm = (debug->obs_domain_id << 24); > + os->obs_point_imm = table_id; > os->sampling_port = OFPP_NONE; > } > } > diff --git a/lib/actions.c b/lib/actions.c > index e8cc0994d..37676ef81 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -4546,13 +4546,13 @@ encode_SAMPLE(const struct ovnact_sample *sample, > os = ofpact_put_SAMPLE(ofpacts); > os->probability = sample->probability; > os->collector_set_id = sample->collector_set_id; > - os->obs_domain_id = > + os->obs_domain_imm = > (sample->obs_domain_id << 24) | (ep->dp_key & 0xFFFFFF); > > if (sample->use_cookie) { > - os->obs_point_id = ep->lflow_uuid.parts[0]; > + os->obs_point_imm = ep->lflow_uuid.parts[0]; > } else { > - os->obs_point_id = sample->obs_point_id; > + os->obs_point_imm = sample->obs_point_id; > } > os->sampling_port = OFPP_NONE; > } > diff --git a/ovs b/ovs > index bf1b16364..0aa14d912 160000 > --- a/ovs > +++ b/ovs > @@ -1 +1 @@ > -Subproject commit bf1b16364b3f01b0ff5f2f6e76842e666226a17b > +Subproject commit 0aa14d912d9a29d07ebc727007a1f21e3639eea5 > diff --git a/utilities/ovn-appctl.c b/utilities/ovn-appctl.c > index 059492972..dff7d1295 100644 > --- a/utilities/ovn-appctl.c > +++ b/utilities/ovn-appctl.c > @@ -27,6 +27,7 @@ > #include "lib/ovn-dirs.h" > #include "lib/ovn-util.h" > #include "openvswitch/dynamic-string.h" > +#include "openvswitch/json.h" > #include "jsonrpc.h" > #include "process.h" > #include "timeval.h" > @@ -37,14 +38,16 @@ > static void usage(void); > static const char *parse_command_line(int argc, char *argv[]); > static struct jsonrpc *connect_to_target(const char *target); > +static char *reply_to_string(struct json *); > > int > main(int argc, char *argv[]) > { > - char *cmd_result, *cmd_error; > + struct json *cmd_result, *cmd_error; > struct jsonrpc *client; > char *cmd, **cmd_argv; > const char *target; > + char *msg = NULL; > int cmd_argc; > int error; > > @@ -66,19 +69,23 @@ main(int argc, char *argv[]) > > if (cmd_error) { > jsonrpc_close(client); > - fputs(cmd_error, stderr); > + msg = reply_to_string(cmd_error); > + fputs(msg, stderr); > + free(msg); > ovs_error(0, "%s: server returned an error", target); > - free(cmd_error); > + json_destroy(cmd_error); > error ? exit(error) : exit(2); > } else if (cmd_result) { > - fputs(cmd_result, stdout); > + msg = reply_to_string(cmd_result); > + fputs(msg, stdout); > + free(msg); > } else { > OVS_NOT_REACHED(); > } > > jsonrpc_close(client); > - free(cmd_result); > - free(cmd_error); > + json_destroy(cmd_result); > + json_destroy(cmd_error); > return 0; > } > > @@ -239,3 +246,26 @@ connect_to_target(const char *target) > return client; > } > > +/* The caller is responsible for freeing the returned string, with > free(), when > + * it is no longer needed. */ > +static char * > +reply_to_string(struct json *reply) > +{ > + ovs_assert(reply); > + > + if (reply->type != JSON_STRING) { > + ovs_error(0, "Unexpected reply type in JSON rpc reply: %s", > + json_type_to_string(reply->type)); > + exit(2); > + } > + > + struct ds ds = DS_EMPTY_INITIALIZER; > + > + ds_put_cstr(&ds, json_string(reply)); > + > + if (ds_last(&ds) != EOF && ds_last(&ds) != '\n') { > + ds_put_char(&ds, '\n'); > + } > + > + return ds_steal_cstr(&ds); > +} > diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c > index 92be27b2c..4024261ef 100644 > --- a/utilities/ovn-dbctl.c > +++ b/utilities/ovn-dbctl.c > @@ -1181,6 +1181,28 @@ server_loop(const struct ovn_dbctl_options > *dbctl_options, > unixctl_server_destroy(server); > } > > +static char * > +reply_to_string(struct json *reply) > +{ > + ovs_assert(reply); > + > + if (reply->type != JSON_STRING) { > + ovs_error(0, "Unexpected reply type in JSON rpc reply: %s", > + json_type_to_string(reply->type)); > + exit(EXIT_FAILURE); > + } > + > + struct ds ds = DS_EMPTY_INITIALIZER; > + > + ds_put_cstr(&ds, json_string(reply)); > + > + if (ds_last(&ds) != EOF && ds_last(&ds) != '\n') { > + ds_put_char(&ds, '\n'); > + } > + > + return ds_steal_cstr(&ds); > +} > + > static void > dbctl_client(const struct ovn_dbctl_options *dbctl_options, > const char *socket_name, > @@ -1264,11 +1286,11 @@ dbctl_client(const struct ovn_dbctl_options > *dbctl_options, > > ctl_timeout_setup(timeout); > > - char *cmd_result = NULL; > - char *cmd_error = NULL; > + struct json *cmd_result = NULL; > + struct json *cmd_error = NULL; > struct jsonrpc *client; > int exit_status; > - char *error_str; > + char *error_str, *str; > > int error = unixctl_client_create(socket_name, &client); > if (error) { > @@ -1289,12 +1311,16 @@ dbctl_client(const struct ovn_dbctl_options > *dbctl_options, > } > > if (cmd_error) { > - fprintf(stderr, "%s: %s", program_name, cmd_error); > + str = reply_to_string(cmd_error); > + fprintf(stderr, "%s: %s", program_name, str); > + free(str); > goto error; > } > > exit_status = EXIT_SUCCESS; > - fputs(cmd_result, stdout); > + str = reply_to_string(cmd_result); > + fputs(str, stdout); > + free(str); > goto cleanup; > > log_error: > @@ -1306,8 +1332,8 @@ error: > exit_status = EXIT_FAILURE; > > cleanup: > - free(cmd_result); > - free(cmd_error); > + json_destroy(cmd_result); > + json_destroy(cmd_error); > jsonrpc_close(client); > svec_destroy(&args); > destroy_argv(argc, argv); > -- > 2.45.2 > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
Thanks Ilya and Ales. I merged this to main. On 7/18/24 01:50, Ales Musil wrote: > On Wed, Jul 17, 2024 at 6:04 PM Ilya Maximets <i.maximets@ovn.org> wrote: > >> Updating to OVS 3.4 to get enhanced sample action and other latest >> features and fixes in general. >> >> Unixctl library was changed to provide JSON-formatted output, so >> clients has to be adjusted. Clients also need to handle extra line >> breaks at the end, since those are no longer prvided by the server. >> >> We could define a common reply_to_string() function for both dbctl >> and appctl, but these programs beahve differently in regerads to exit >> codes and we'll need to do more refactoring to handle the difference. >> >> Will move to v3.4.0 once it is available. >> >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >> --- >> controller/physical.c | 4 ++-- >> lib/actions.c | 6 +++--- >> ovs | 2 +- >> utilities/ovn-appctl.c | 42 ++++++++++++++++++++++++++++++++++++------ >> utilities/ovn-dbctl.c | 40 +++++++++++++++++++++++++++++++++------- >> 5 files changed, 75 insertions(+), 19 deletions(-) >> >> diff --git a/controller/physical.c b/controller/physical.c >> index 22756810f..0255a1bbd 100644 >> --- a/controller/physical.c >> +++ b/controller/physical.c >> @@ -889,8 +889,8 @@ put_drop(const struct physical_debug *debug, uint8_t >> table_id, >> struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts); >> os->probability = UINT16_MAX; >> os->collector_set_id = debug->collector_set_id; >> - os->obs_domain_id = (debug->obs_domain_id << 24); >> - os->obs_point_id = table_id; >> + os->obs_domain_imm = (debug->obs_domain_id << 24); >> + os->obs_point_imm = table_id; >> os->sampling_port = OFPP_NONE; >> } >> } >> diff --git a/lib/actions.c b/lib/actions.c >> index e8cc0994d..37676ef81 100644 >> --- a/lib/actions.c >> +++ b/lib/actions.c >> @@ -4546,13 +4546,13 @@ encode_SAMPLE(const struct ovnact_sample *sample, >> os = ofpact_put_SAMPLE(ofpacts); >> os->probability = sample->probability; >> os->collector_set_id = sample->collector_set_id; >> - os->obs_domain_id = >> + os->obs_domain_imm = >> (sample->obs_domain_id << 24) | (ep->dp_key & 0xFFFFFF); >> >> if (sample->use_cookie) { >> - os->obs_point_id = ep->lflow_uuid.parts[0]; >> + os->obs_point_imm = ep->lflow_uuid.parts[0]; >> } else { >> - os->obs_point_id = sample->obs_point_id; >> + os->obs_point_imm = sample->obs_point_id; >> } >> os->sampling_port = OFPP_NONE; >> } >> diff --git a/ovs b/ovs >> index bf1b16364..0aa14d912 160000 >> --- a/ovs >> +++ b/ovs >> @@ -1 +1 @@ >> -Subproject commit bf1b16364b3f01b0ff5f2f6e76842e666226a17b >> +Subproject commit 0aa14d912d9a29d07ebc727007a1f21e3639eea5 >> diff --git a/utilities/ovn-appctl.c b/utilities/ovn-appctl.c >> index 059492972..dff7d1295 100644 >> --- a/utilities/ovn-appctl.c >> +++ b/utilities/ovn-appctl.c >> @@ -27,6 +27,7 @@ >> #include "lib/ovn-dirs.h" >> #include "lib/ovn-util.h" >> #include "openvswitch/dynamic-string.h" >> +#include "openvswitch/json.h" >> #include "jsonrpc.h" >> #include "process.h" >> #include "timeval.h" >> @@ -37,14 +38,16 @@ >> static void usage(void); >> static const char *parse_command_line(int argc, char *argv[]); >> static struct jsonrpc *connect_to_target(const char *target); >> +static char *reply_to_string(struct json *); >> >> int >> main(int argc, char *argv[]) >> { >> - char *cmd_result, *cmd_error; >> + struct json *cmd_result, *cmd_error; >> struct jsonrpc *client; >> char *cmd, **cmd_argv; >> const char *target; >> + char *msg = NULL; >> int cmd_argc; >> int error; >> >> @@ -66,19 +69,23 @@ main(int argc, char *argv[]) >> >> if (cmd_error) { >> jsonrpc_close(client); >> - fputs(cmd_error, stderr); >> + msg = reply_to_string(cmd_error); >> + fputs(msg, stderr); >> + free(msg); >> ovs_error(0, "%s: server returned an error", target); >> - free(cmd_error); >> + json_destroy(cmd_error); >> error ? exit(error) : exit(2); >> } else if (cmd_result) { >> - fputs(cmd_result, stdout); >> + msg = reply_to_string(cmd_result); >> + fputs(msg, stdout); >> + free(msg); >> } else { >> OVS_NOT_REACHED(); >> } >> >> jsonrpc_close(client); >> - free(cmd_result); >> - free(cmd_error); >> + json_destroy(cmd_result); >> + json_destroy(cmd_error); >> return 0; >> } >> >> @@ -239,3 +246,26 @@ connect_to_target(const char *target) >> return client; >> } >> >> +/* The caller is responsible for freeing the returned string, with >> free(), when >> + * it is no longer needed. */ >> +static char * >> +reply_to_string(struct json *reply) >> +{ >> + ovs_assert(reply); >> + >> + if (reply->type != JSON_STRING) { >> + ovs_error(0, "Unexpected reply type in JSON rpc reply: %s", >> + json_type_to_string(reply->type)); >> + exit(2); >> + } >> + >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + >> + ds_put_cstr(&ds, json_string(reply)); >> + >> + if (ds_last(&ds) != EOF && ds_last(&ds) != '\n') { >> + ds_put_char(&ds, '\n'); >> + } >> + >> + return ds_steal_cstr(&ds); >> +} >> diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c >> index 92be27b2c..4024261ef 100644 >> --- a/utilities/ovn-dbctl.c >> +++ b/utilities/ovn-dbctl.c >> @@ -1181,6 +1181,28 @@ server_loop(const struct ovn_dbctl_options >> *dbctl_options, >> unixctl_server_destroy(server); >> } >> >> +static char * >> +reply_to_string(struct json *reply) >> +{ >> + ovs_assert(reply); >> + >> + if (reply->type != JSON_STRING) { >> + ovs_error(0, "Unexpected reply type in JSON rpc reply: %s", >> + json_type_to_string(reply->type)); >> + exit(EXIT_FAILURE); >> + } >> + >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + >> + ds_put_cstr(&ds, json_string(reply)); >> + >> + if (ds_last(&ds) != EOF && ds_last(&ds) != '\n') { >> + ds_put_char(&ds, '\n'); >> + } >> + >> + return ds_steal_cstr(&ds); >> +} >> + >> static void >> dbctl_client(const struct ovn_dbctl_options *dbctl_options, >> const char *socket_name, >> @@ -1264,11 +1286,11 @@ dbctl_client(const struct ovn_dbctl_options >> *dbctl_options, >> >> ctl_timeout_setup(timeout); >> >> - char *cmd_result = NULL; >> - char *cmd_error = NULL; >> + struct json *cmd_result = NULL; >> + struct json *cmd_error = NULL; >> struct jsonrpc *client; >> int exit_status; >> - char *error_str; >> + char *error_str, *str; >> >> int error = unixctl_client_create(socket_name, &client); >> if (error) { >> @@ -1289,12 +1311,16 @@ dbctl_client(const struct ovn_dbctl_options >> *dbctl_options, >> } >> >> if (cmd_error) { >> - fprintf(stderr, "%s: %s", program_name, cmd_error); >> + str = reply_to_string(cmd_error); >> + fprintf(stderr, "%s: %s", program_name, str); >> + free(str); >> goto error; >> } >> >> exit_status = EXIT_SUCCESS; >> - fputs(cmd_result, stdout); >> + str = reply_to_string(cmd_result); >> + fputs(str, stdout); >> + free(str); >> goto cleanup; >> >> log_error: >> @@ -1306,8 +1332,8 @@ error: >> exit_status = EXIT_FAILURE; >> >> cleanup: >> - free(cmd_result); >> - free(cmd_error); >> + json_destroy(cmd_result); >> + json_destroy(cmd_error); >> jsonrpc_close(client); >> svec_destroy(&args); >> destroy_argv(argc, argv); >> -- >> 2.45.2 >> >> > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com> >
diff --git a/controller/physical.c b/controller/physical.c index 22756810f..0255a1bbd 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -889,8 +889,8 @@ put_drop(const struct physical_debug *debug, uint8_t table_id, struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts); os->probability = UINT16_MAX; os->collector_set_id = debug->collector_set_id; - os->obs_domain_id = (debug->obs_domain_id << 24); - os->obs_point_id = table_id; + os->obs_domain_imm = (debug->obs_domain_id << 24); + os->obs_point_imm = table_id; os->sampling_port = OFPP_NONE; } } diff --git a/lib/actions.c b/lib/actions.c index e8cc0994d..37676ef81 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -4546,13 +4546,13 @@ encode_SAMPLE(const struct ovnact_sample *sample, os = ofpact_put_SAMPLE(ofpacts); os->probability = sample->probability; os->collector_set_id = sample->collector_set_id; - os->obs_domain_id = + os->obs_domain_imm = (sample->obs_domain_id << 24) | (ep->dp_key & 0xFFFFFF); if (sample->use_cookie) { - os->obs_point_id = ep->lflow_uuid.parts[0]; + os->obs_point_imm = ep->lflow_uuid.parts[0]; } else { - os->obs_point_id = sample->obs_point_id; + os->obs_point_imm = sample->obs_point_id; } os->sampling_port = OFPP_NONE; } diff --git a/ovs b/ovs index bf1b16364..0aa14d912 160000 --- a/ovs +++ b/ovs @@ -1 +1 @@ -Subproject commit bf1b16364b3f01b0ff5f2f6e76842e666226a17b +Subproject commit 0aa14d912d9a29d07ebc727007a1f21e3639eea5 diff --git a/utilities/ovn-appctl.c b/utilities/ovn-appctl.c index 059492972..dff7d1295 100644 --- a/utilities/ovn-appctl.c +++ b/utilities/ovn-appctl.c @@ -27,6 +27,7 @@ #include "lib/ovn-dirs.h" #include "lib/ovn-util.h" #include "openvswitch/dynamic-string.h" +#include "openvswitch/json.h" #include "jsonrpc.h" #include "process.h" #include "timeval.h" @@ -37,14 +38,16 @@ static void usage(void); static const char *parse_command_line(int argc, char *argv[]); static struct jsonrpc *connect_to_target(const char *target); +static char *reply_to_string(struct json *); int main(int argc, char *argv[]) { - char *cmd_result, *cmd_error; + struct json *cmd_result, *cmd_error; struct jsonrpc *client; char *cmd, **cmd_argv; const char *target; + char *msg = NULL; int cmd_argc; int error; @@ -66,19 +69,23 @@ main(int argc, char *argv[]) if (cmd_error) { jsonrpc_close(client); - fputs(cmd_error, stderr); + msg = reply_to_string(cmd_error); + fputs(msg, stderr); + free(msg); ovs_error(0, "%s: server returned an error", target); - free(cmd_error); + json_destroy(cmd_error); error ? exit(error) : exit(2); } else if (cmd_result) { - fputs(cmd_result, stdout); + msg = reply_to_string(cmd_result); + fputs(msg, stdout); + free(msg); } else { OVS_NOT_REACHED(); } jsonrpc_close(client); - free(cmd_result); - free(cmd_error); + json_destroy(cmd_result); + json_destroy(cmd_error); return 0; } @@ -239,3 +246,26 @@ connect_to_target(const char *target) return client; } +/* The caller is responsible for freeing the returned string, with free(), when + * it is no longer needed. */ +static char * +reply_to_string(struct json *reply) +{ + ovs_assert(reply); + + if (reply->type != JSON_STRING) { + ovs_error(0, "Unexpected reply type in JSON rpc reply: %s", + json_type_to_string(reply->type)); + exit(2); + } + + struct ds ds = DS_EMPTY_INITIALIZER; + + ds_put_cstr(&ds, json_string(reply)); + + if (ds_last(&ds) != EOF && ds_last(&ds) != '\n') { + ds_put_char(&ds, '\n'); + } + + return ds_steal_cstr(&ds); +} diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c index 92be27b2c..4024261ef 100644 --- a/utilities/ovn-dbctl.c +++ b/utilities/ovn-dbctl.c @@ -1181,6 +1181,28 @@ server_loop(const struct ovn_dbctl_options *dbctl_options, unixctl_server_destroy(server); } +static char * +reply_to_string(struct json *reply) +{ + ovs_assert(reply); + + if (reply->type != JSON_STRING) { + ovs_error(0, "Unexpected reply type in JSON rpc reply: %s", + json_type_to_string(reply->type)); + exit(EXIT_FAILURE); + } + + struct ds ds = DS_EMPTY_INITIALIZER; + + ds_put_cstr(&ds, json_string(reply)); + + if (ds_last(&ds) != EOF && ds_last(&ds) != '\n') { + ds_put_char(&ds, '\n'); + } + + return ds_steal_cstr(&ds); +} + static void dbctl_client(const struct ovn_dbctl_options *dbctl_options, const char *socket_name, @@ -1264,11 +1286,11 @@ dbctl_client(const struct ovn_dbctl_options *dbctl_options, ctl_timeout_setup(timeout); - char *cmd_result = NULL; - char *cmd_error = NULL; + struct json *cmd_result = NULL; + struct json *cmd_error = NULL; struct jsonrpc *client; int exit_status; - char *error_str; + char *error_str, *str; int error = unixctl_client_create(socket_name, &client); if (error) { @@ -1289,12 +1311,16 @@ dbctl_client(const struct ovn_dbctl_options *dbctl_options, } if (cmd_error) { - fprintf(stderr, "%s: %s", program_name, cmd_error); + str = reply_to_string(cmd_error); + fprintf(stderr, "%s: %s", program_name, str); + free(str); goto error; } exit_status = EXIT_SUCCESS; - fputs(cmd_result, stdout); + str = reply_to_string(cmd_result); + fputs(str, stdout); + free(str); goto cleanup; log_error: @@ -1306,8 +1332,8 @@ error: exit_status = EXIT_FAILURE; cleanup: - free(cmd_result); - free(cmd_error); + json_destroy(cmd_result); + json_destroy(cmd_error); jsonrpc_close(client); svec_destroy(&args); destroy_argv(argc, argv);
Updating to OVS 3.4 to get enhanced sample action and other latest features and fixes in general. Unixctl library was changed to provide JSON-formatted output, so clients has to be adjusted. Clients also need to handle extra line breaks at the end, since those are no longer prvided by the server. We could define a common reply_to_string() function for both dbctl and appctl, but these programs beahve differently in regerads to exit codes and we'll need to do more refactoring to handle the difference. Will move to v3.4.0 once it is available. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- controller/physical.c | 4 ++-- lib/actions.c | 6 +++--- ovs | 2 +- utilities/ovn-appctl.c | 42 ++++++++++++++++++++++++++++++++++++------ utilities/ovn-dbctl.c | 40 +++++++++++++++++++++++++++++++++------- 5 files changed, 75 insertions(+), 19 deletions(-)