With small nits below:
Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Apr 14, 2017, at 12:46 PM, Andy Zhou <az...@ovn.org> wrote:
>
> Allow action upcall meters, i.e. slowpath and controller meters,
> to be added and displayed.
>
> Keep track of datapath meter ID of those action upcall meters in
> ofproto to aid action translation. Later patches will make use of them.
>
> Signed-off-by: Andy Zhou <az...@ovn.org>
> ---
> lib/ofp-print.c | 33 ++++++++++++++++++++++++++---
> ofproto/ofproto-provider.h | 4 ++++
> ofproto/ofproto.c | 52 ++++++++++++++++++++++++++++++++++++++++++----
> 3 files changed, 82 insertions(+), 7 deletions(-)
>
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index a8cdfcbf20b1..140af05950b7 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1333,11 +1333,36 @@ ofp_print_meter_band(struct ds *s, uint16_t flags,
> }
>
> static void
> +ofp_print_meter_id(struct ds *s, uint32_t meter_id, char seperator)
> +{
> + if (meter_id <= OFPM13_MAX) {
> + ds_put_format(s, "meter%c%"PRIu32, seperator, meter_id);
> + } else {
> + const char *name;
> + switch (meter_id) {
> + case OFPM13_SLOWPATH:
> + name = "slowpath";
> + break;
> + case OFPM13_CONTROLLER:
> + name = "controller";
> + break;
> + case OFPM13_ALL:
> + name = "ALL”;
We require lower case “all” when parsing, so better print that way also.
> + break;
> + default:
> + name = "unknown";
> + }
> + ds_put_format(s, "meter%c%s", seperator, name);
> + }
> +}
> +
> +static void
> ofp_print_meter_stats(struct ds *s, const struct ofputil_meter_stats *ms)
> {
> uint16_t i;
>
> - ds_put_format(s, "meter:%"PRIu32" ", ms->meter_id);
> + ofp_print_meter_id(s, ms->meter_id, ':');
> + ds_put_char(s, ' ');
> ds_put_format(s, "flow_count:%"PRIu32" ", ms->flow_count);
> ds_put_format(s, "packet_in_count:%"PRIu64" ", ms->packet_in_count);
> ds_put_format(s, "byte_in_count:%"PRIu64" ", ms->byte_in_count);
> @@ -1358,7 +1383,8 @@ ofp_print_meter_config(struct ds *s, const struct
> ofputil_meter_config *mc)
> {
> uint16_t i;
>
> - ds_put_format(s, "meter=%"PRIu32" ", mc->meter_id);
> + ofp_print_meter_id(s, mc->meter_id, '=');
> + ds_put_char(s, ' ');
>
> ofp_print_meter_flags(s, mc->flags);
>
> @@ -1412,8 +1438,9 @@ ofp_print_meter_stats_request(struct ds *s, const
> struct ofp_header *oh)
> uint32_t meter_id;
>
> ofputil_decode_meter_request(oh, &meter_id);
> + ds_put_char(s, ' ');
>
> - ds_put_format(s, " meter=%"PRIu32, meter_id);
> + ofp_print_meter_id(s, meter_id, '=');
> }
>
> static const char *
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 000326d7f79d..688a9e5d32eb 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -112,6 +112,10 @@ struct ofproto {
> /* Meter table. */
> struct ofputil_meter_features meter_features;
> struct hmap meters; /* uint32_t indexed 'struct meter *'. */
> + uint32_t slowpath_meter_id; /* Datapath slowpath meter. UINT32_MAX
> + if not defined. */
> + uint32_t controller_meter_id; /* Datapath controller meter. UINT32_MAX
> + if not defined. */
>
> /* OpenFlow connections. */
> struct connmgr *connmgr;
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 8c4c7e67f213..abbb849a384b 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -568,6 +568,8 @@ ofproto_create(const char *datapath_name, const char
> *datapath_type,
> memset(&ofproto->meter_features, 0, sizeof ofproto->meter_features);
> }
> hmap_init(&ofproto->meters);
> + ofproto->slowpath_meter_id = UINT32_MAX;
> + ofproto->controller_meter_id = UINT32_MAX;
>
> /* Set the initial tables version. */
> ofproto_bump_tables_version(ofproto);
> @@ -6232,9 +6234,33 @@ ofproto_get_meter(const struct ofproto *ofproto,
> uint32_t meter_id)
> return NULL;
> }
>
> +static uint32_t *
> +ofproto_upcall_meter_ptr(struct ofproto *ofproto, uint32_t meter_id)
> +{
> + switch(meter_id) {
> + case OFPM13_SLOWPATH:
> + return &ofproto->slowpath_meter_id;
> + break;
> + case OFPM13_CONTROLLER:
> + return &ofproto->controller_meter_id;
> + break;
> + case OFPM13_ALL:
> + OVS_NOT_REACHED();
> + default:
> + return NULL;
> + }
> +}
> +
> static void
> ofproto_add_meter(struct ofproto *ofproto, struct meter *meter)
> {
> + uint32_t *upcall_meter_ptr = ofproto_upcall_meter_ptr(ofproto,
> meter->id);
> +
> + /* Cache datapath meter IDs of special meters. */
> + if (upcall_meter_ptr) {
> + *upcall_meter_ptr = meter->provider_meter_id.uint32;
> + }
> +
> hmap_insert(&ofproto->meters, &meter->node, hash_int(meter->id, 0));
> }
>
> @@ -6248,7 +6274,7 @@ static bool
> ofproto_fix_meter_action(const struct ofproto *ofproto,
> struct ofpact_meter *ma)
> {
> - if (ma->meter_id && ma->meter_id <= ofproto->meter_features.max_meters) {
> + if (ma->meter_id) {
This change would better be placed with patch 1/5?
> const struct meter *meter = ofproto_get_meter(ofproto, ma->meter_id);
>
> if (meter && meter->provider_meter_id.uint32 != UINT32_MAX) {
> @@ -6306,6 +6332,12 @@ static void
> meter_destroy(struct ofproto *ofproto, struct meter *meter)
> OVS_REQUIRES(ofproto_mutex)
> {
> + uint32_t *upcall_meter_ptr;
> + upcall_meter_ptr = ofproto_upcall_meter_ptr(ofproto, meter->id);
> + if (upcall_meter_ptr) {
> + *upcall_meter_ptr = UINT32_MAX;
> + }
> +
> if (!ovs_list_is_empty(&meter->rules)) {
> struct rule_collection rules;
> struct rule *rule;
> @@ -6438,12 +6470,24 @@ handle_meter_mod(struct ofconn *ofconn, const struct
> ofp_header *oh)
>
> if (mm.command != OFPMC13_DELETE) {
> /* Fails also when meters are not implemented by the provider. */
> - if (meter_id == 0 || meter_id > OFPM13_MAX) {
> + if (ofproto->meter_features.max_meters == 0) {
> error = OFPERR_OFPMMFC_INVALID_METER;
> goto exit_free_bands;
> - } else if (meter_id > ofproto->meter_features.max_meters) {
> - error = OFPERR_OFPMMFC_OUT_OF_METERS;
> + }
> +
> + if (meter_id == 0) {
> + error = OFPERR_OFPMMFC_INVALID_METER;
> goto exit_free_bands;
> + } else if (meter_id > OFPM13_MAX) {
> + switch(meter_id) {
> + case OFPM13_SLOWPATH:
> + case OFPM13_CONTROLLER:
> + break;
> + case OFPM13_ALL:
> + default:
> + error = OFPERR_OFPMMFC_INVALID_METER;
> + goto exit_free_bands;
> + }
> }
> if (mm.meter.n_bands > ofproto->meter_features.max_bands) {
> error = OFPERR_OFPMMFC_OUT_OF_BANDS;
> --
> 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
