| Message ID | 1489440092-13945-1-git-send-email-azhou@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On 13 March 2017 at 14:21, Andy Zhou <azhou@ovn.org> wrote: > Exporting Datapath runtime detected features can be useful for > both debugging and for writing system unit testing easier. > > Signed-off-by: Andy Zhou <azhou@ovn.org> Can we perform some kind of build-time check how many fields are in 'support' and ensure that this function is extended when people add new probes? Looks otherwise trivial and straightforward (and useful!), thanks. Couple of minor comments below. Acked-by: Joe Stringer <joe@ovn.org> > --- > ofproto/ofproto-dpif.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 52 insertions(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index df779c2..af70ab3 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -4891,6 +4891,38 @@ ofproto_unixctl_dpif_dump_dps(struct unixctl_conn *conn, int argc OVS_UNUSED, > } > > static void > +show_dp_feature_b(struct ds *ds, const char *feature, bool b) > +{ > + ds_put_format(ds, "%s: %s\n", feature, b ? "Yes" : "No"); > +} > + > +static void > +show_dp_feature_s(struct ds *ds, const char *feature, size_t s) > +{ > + ds_put_format(ds, "%s: %"PRIuSIZE"\n", feature, s); > +} > + > +static void > +dpif_show_support(const struct dpif_backer_support *support, struct ds *ds) > +{ > + show_dp_feature_b(ds, "Variable length userdata", > + support->variable_length_userdata); > + show_dp_feature_b(ds, "Masked set action", support->masked_set_action); > + show_dp_feature_b(ds, "Tunnel push pop", support->tnl_push_pop); > + show_dp_feature_b(ds, "Ufid", support->ufid); > + show_dp_feature_b(ds, "Trunc action", support->trunc); > + show_dp_feature_b(ds, "Clone action", support->clone); > + show_dp_feature_s(ds, "Max MPLS depth", support->odp.max_mpls_depth); > + show_dp_feature_b(ds, "Recirc", support->odp.recirc); > + show_dp_feature_b(ds, "CT state", support->odp.ct_state); > + show_dp_feature_b(ds, "CT zone", support->odp.ct_zone); > + show_dp_feature_b(ds, "CT mark", support->odp.ct_mark); > + show_dp_feature_b(ds, "CT label", support->odp.ct_label); > + show_dp_feature_b(ds, "CT State NAT", support->odp.ct_state_nat); > + show_dp_feature_s(ds, "Max sample nesting",support->sample_nesting); Needs an extra whitespace on this last line, before support->sample_nesting > +} > + > +static void > dpif_show_backer(const struct dpif_backer *backer, struct ds *ds) > { > const struct shash_node **ofprotos; > @@ -4899,7 +4931,6 @@ dpif_show_backer(const struct dpif_backer *backer, struct ds *ds) > size_t i; > > dpif_get_dp_stats(backer->dpif, &dp_stats); > - > ds_put_format(ds, "%s: hit:%"PRIu64" missed:%"PRIu64"\n", > dpif_name(backer->dpif), dp_stats.n_hit, dp_stats.n_missed); > > @@ -5119,6 +5150,24 @@ disable_datapath_clone(struct unixctl_conn *conn OVS_UNUSED, > } > > static void > +ofproto_unixctl_dpif_show_dp_features(struct unixctl_conn *conn, > + int argc, const char *argv[], > + void *aux OVS_UNUSED) > +{ > + struct ds ds = DS_EMPTY_INITIALIZER; > + const char *br = argv[argc -1]; > + struct ofproto_dpif *ofproto = ofproto_dpif_lookup(br); > + > + if (!ofproto) { > + unixctl_command_reply_error(conn, "no such bridge"); > + return; > + } > + > + dpif_show_support(&ofproto->backer->support, &ds); > + unixctl_command_reply(conn, ds_cstr(&ds)); > +} > + > +static void > ofproto_unixctl_init(void) > { > static bool registered; > @@ -5139,6 +5188,8 @@ ofproto_unixctl_init(void) > ofproto_unixctl_dpif_dump_dps, NULL); > unixctl_command_register("dpif/show", "", 0, 0, ofproto_unixctl_dpif_show, > NULL); > + unixctl_command_register("dpif/show-dp-features", "bridge", 1, 1, > + ofproto_unixctl_dpif_show_dp_features, NULL); > unixctl_command_register("dpif/dump-flows", "[-m] bridge", 1, 2, > ofproto_unixctl_dpif_dump_flows, NULL); > > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Tue, Mar 14, 2017 at 6:19 PM, Joe Stringer <joe@ovn.org> wrote: > On 13 March 2017 at 14:21, Andy Zhou <azhou@ovn.org> wrote: >> Exporting Datapath runtime detected features can be useful for >> both debugging and for writing system unit testing easier. >> >> Signed-off-by: Andy Zhou <azhou@ovn.org> > > Can we perform some kind of build-time check how many fields are in > 'support' and ensure that this function is extended when people add > new probes? > That would be nice. I don't have any concrete at the moment. > Looks otherwise trivial and straightforward (and useful!), thanks. > Couple of minor comments below. > > Acked-by: Joe Stringer <joe@ovn.org> Thanks for the review. Pushed. > >> --- >> ofproto/ofproto-dpif.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 52 insertions(+), 1 deletion(-) >> >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >> index df779c2..af70ab3 100644 >> --- a/ofproto/ofproto-dpif.c >> +++ b/ofproto/ofproto-dpif.c >> @@ -4891,6 +4891,38 @@ ofproto_unixctl_dpif_dump_dps(struct unixctl_conn *conn, int argc OVS_UNUSED, >> } >> >> static void >> +show_dp_feature_b(struct ds *ds, const char *feature, bool b) >> +{ >> + ds_put_format(ds, "%s: %s\n", feature, b ? "Yes" : "No"); >> +} >> + >> +static void >> +show_dp_feature_s(struct ds *ds, const char *feature, size_t s) >> +{ >> + ds_put_format(ds, "%s: %"PRIuSIZE"\n", feature, s); >> +} >> + >> +static void >> +dpif_show_support(const struct dpif_backer_support *support, struct ds *ds) >> +{ >> + show_dp_feature_b(ds, "Variable length userdata", >> + support->variable_length_userdata); >> + show_dp_feature_b(ds, "Masked set action", support->masked_set_action); >> + show_dp_feature_b(ds, "Tunnel push pop", support->tnl_push_pop); >> + show_dp_feature_b(ds, "Ufid", support->ufid); >> + show_dp_feature_b(ds, "Trunc action", support->trunc); >> + show_dp_feature_b(ds, "Clone action", support->clone); >> + show_dp_feature_s(ds, "Max MPLS depth", support->odp.max_mpls_depth); >> + show_dp_feature_b(ds, "Recirc", support->odp.recirc); >> + show_dp_feature_b(ds, "CT state", support->odp.ct_state); >> + show_dp_feature_b(ds, "CT zone", support->odp.ct_zone); >> + show_dp_feature_b(ds, "CT mark", support->odp.ct_mark); >> + show_dp_feature_b(ds, "CT label", support->odp.ct_label); >> + show_dp_feature_b(ds, "CT State NAT", support->odp.ct_state_nat); >> + show_dp_feature_s(ds, "Max sample nesting",support->sample_nesting); > > Needs an extra whitespace on this last line, before support->sample_nesting > O.K. Fixed. >> +} >> + >> +static void >> dpif_show_backer(const struct dpif_backer *backer, struct ds *ds) >> { >> const struct shash_node **ofprotos; >> @@ -4899,7 +4931,6 @@ dpif_show_backer(const struct dpif_backer *backer, struct ds *ds) >> size_t i; >> >> dpif_get_dp_stats(backer->dpif, &dp_stats); >> - >> ds_put_format(ds, "%s: hit:%"PRIu64" missed:%"PRIu64"\n", >> dpif_name(backer->dpif), dp_stats.n_hit, dp_stats.n_missed); >> >> @@ -5119,6 +5150,24 @@ disable_datapath_clone(struct unixctl_conn *conn OVS_UNUSED, >> } >> >> static void >> +ofproto_unixctl_dpif_show_dp_features(struct unixctl_conn *conn, >> + int argc, const char *argv[], >> + void *aux OVS_UNUSED) >> +{ >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + const char *br = argv[argc -1]; >> + struct ofproto_dpif *ofproto = ofproto_dpif_lookup(br); >> + >> + if (!ofproto) { >> + unixctl_command_reply_error(conn, "no such bridge"); >> + return; >> + } >> + >> + dpif_show_support(&ofproto->backer->support, &ds); >> + unixctl_command_reply(conn, ds_cstr(&ds)); >> +} >> + >> +static void >> ofproto_unixctl_init(void) >> { >> static bool registered; >> @@ -5139,6 +5188,8 @@ ofproto_unixctl_init(void) >> ofproto_unixctl_dpif_dump_dps, NULL); >> unixctl_command_register("dpif/show", "", 0, 0, ofproto_unixctl_dpif_show, >> NULL); >> + unixctl_command_register("dpif/show-dp-features", "bridge", 1, 1, >> + ofproto_unixctl_dpif_show_dp_features, NULL); >> unixctl_command_register("dpif/dump-flows", "[-m] bridge", 1, 2, >> ofproto_unixctl_dpif_dump_flows, NULL); >> >> -- >> 1.8.3.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index df779c2..af70ab3 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4891,6 +4891,38 @@ ofproto_unixctl_dpif_dump_dps(struct unixctl_conn *conn, int argc OVS_UNUSED, } static void +show_dp_feature_b(struct ds *ds, const char *feature, bool b) +{ + ds_put_format(ds, "%s: %s\n", feature, b ? "Yes" : "No"); +} + +static void +show_dp_feature_s(struct ds *ds, const char *feature, size_t s) +{ + ds_put_format(ds, "%s: %"PRIuSIZE"\n", feature, s); +} + +static void +dpif_show_support(const struct dpif_backer_support *support, struct ds *ds) +{ + show_dp_feature_b(ds, "Variable length userdata", + support->variable_length_userdata); + show_dp_feature_b(ds, "Masked set action", support->masked_set_action); + show_dp_feature_b(ds, "Tunnel push pop", support->tnl_push_pop); + show_dp_feature_b(ds, "Ufid", support->ufid); + show_dp_feature_b(ds, "Trunc action", support->trunc); + show_dp_feature_b(ds, "Clone action", support->clone); + show_dp_feature_s(ds, "Max MPLS depth", support->odp.max_mpls_depth); + show_dp_feature_b(ds, "Recirc", support->odp.recirc); + show_dp_feature_b(ds, "CT state", support->odp.ct_state); + show_dp_feature_b(ds, "CT zone", support->odp.ct_zone); + show_dp_feature_b(ds, "CT mark", support->odp.ct_mark); + show_dp_feature_b(ds, "CT label", support->odp.ct_label); + show_dp_feature_b(ds, "CT State NAT", support->odp.ct_state_nat); + show_dp_feature_s(ds, "Max sample nesting",support->sample_nesting); +} + +static void dpif_show_backer(const struct dpif_backer *backer, struct ds *ds) { const struct shash_node **ofprotos; @@ -4899,7 +4931,6 @@ dpif_show_backer(const struct dpif_backer *backer, struct ds *ds) size_t i; dpif_get_dp_stats(backer->dpif, &dp_stats); - ds_put_format(ds, "%s: hit:%"PRIu64" missed:%"PRIu64"\n", dpif_name(backer->dpif), dp_stats.n_hit, dp_stats.n_missed); @@ -5119,6 +5150,24 @@ disable_datapath_clone(struct unixctl_conn *conn OVS_UNUSED, } static void +ofproto_unixctl_dpif_show_dp_features(struct unixctl_conn *conn, + int argc, const char *argv[], + void *aux OVS_UNUSED) +{ + struct ds ds = DS_EMPTY_INITIALIZER; + const char *br = argv[argc -1]; + struct ofproto_dpif *ofproto = ofproto_dpif_lookup(br); + + if (!ofproto) { + unixctl_command_reply_error(conn, "no such bridge"); + return; + } + + dpif_show_support(&ofproto->backer->support, &ds); + unixctl_command_reply(conn, ds_cstr(&ds)); +} + +static void ofproto_unixctl_init(void) { static bool registered; @@ -5139,6 +5188,8 @@ ofproto_unixctl_init(void) ofproto_unixctl_dpif_dump_dps, NULL); unixctl_command_register("dpif/show", "", 0, 0, ofproto_unixctl_dpif_show, NULL); + unixctl_command_register("dpif/show-dp-features", "bridge", 1, 1, + ofproto_unixctl_dpif_show_dp_features, NULL); unixctl_command_register("dpif/dump-flows", "[-m] bridge", 1, 2, ofproto_unixctl_dpif_dump_flows, NULL);
Exporting Datapath runtime detected features can be useful for both debugging and for writing system unit testing easier. Signed-off-by: Andy Zhou <azhou@ovn.org> --- ofproto/ofproto-dpif.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-)