Re: [ovs-dev] [PATCH] ofproto: Add appctl command to show Datapath features

[Andy Zhou]

On Tue, Mar 14, 2017 at 6:19 PM, Joe Stringer  wrote:
> On 13 March 2017 at 14:21, Andy Zhou  wrote:
>> Exporting Datapath runtime detected features can be useful for
>> both debugging and for writing system unit testing easier.
>>
>> Signed-off-by: Andy Zhou 
>
> 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 

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, _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(>backer->support, );
>> +unixctl_command_reply(conn, ds_cstr());
>> +}
>> +
>> +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
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto: Add appctl command to show Datapath features

[Joe Stringer]

On 13 March 2017 at 14:21, Andy Zhou  wrote:
> Exporting Datapath runtime detected features can be useful for
> both debugging and for writing system unit testing easier.
>
> Signed-off-by: Andy Zhou 

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 

> ---
>  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, _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(>backer->support, );
> +unixctl_command_reply(conn, ds_cstr());
> +}
> +
> +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
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ofproto: Add appctl command to show Datapath features

[Andy Zhou]

Exporting Datapath runtime detected features can be useful for
both debugging and for writing system unit testing easier.

Signed-off-by: Andy Zhou 
---
 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);
+}
+
+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, _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(>backer->support, );
+unixctl_command_reply(conn, ds_cstr());
+}
+
+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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev