Hi Ben, 

Did you have a chance to look at this patch re-work? Thank you in advance!

Br, 
Michal. 

> -----Original Message-----
> From: Weglicki, MichalX
> Sent: Tuesday, December 5, 2017 3:55 PM
> To: [email protected]
> Cc: Weglicki, MichalX <[email protected]>
> Subject: [PATCH v2] netdev: Custom statistics.
> 
> - New get_custom_stats interface function is added to netdev. It
>   allows particular netdev implementation to expose custom
>   counters in dictionary format (counter name/counter value).
> - New statistics are retrieved using experimenter code and
>   are printed as a result to ofctl dump-ports.
> - New counters are available for OpenFlow 1.4+.
> - New statistics are printed to output via ofctl only if those
>   are present in reply message.
> - New statistics definition is added to include/openflow/intel-ext.h.
> - Custom statistics are implemented only for dpdk-physical
>   port type.
> - DPDK-physical implementation uses xstats to collect statistics.
>   Only dropped and error counters are exposed.
> 
> v1->v2:
> - Buffer overrun check in parse_intel_port_custom_property.
> - ofputil_append_ofp14_port_stats uses "postappend" instead
>   of "reserve" during message creation.
> - NEWS update.
> - DPDK documentation update.
> - Compilation and sparse warnings corrections.
> 
> Signed-off-by: Michal Weglicki <[email protected]>
> ---
>  Documentation/howto/dpdk.rst   |  15 ++--
>  NEWS                           |   6 ++
>  include/openflow/intel-ext.h   |  28 ++++++
>  include/openvswitch/netdev.h   |  17 ++++
>  include/openvswitch/ofp-util.h |   1 +
>  lib/netdev-dpdk.c              | 195 
> ++++++++++++++++++++++++++++++++++++++++-
>  lib/netdev-dummy.c             |   1 +
>  lib/netdev-linux.c             |   1 +
>  lib/netdev-provider.h          |  13 +++
>  lib/netdev-vport.c             |   1 +
>  lib/netdev.c                   |  27 ++++++
>  lib/netdev.h                   |   2 +
>  lib/ofp-print.c                |  18 ++++
>  lib/ofp-util.c                 | 119 +++++++++++++++++++++++--
>  lib/util.c                     |  13 +++
>  lib/util.h                     |   2 +
>  ofproto/ofproto-dpif.c         |  13 +++
>  ofproto/ofproto-provider.h     |   4 +
>  ofproto/ofproto.c              |  21 +++++
>  ofproto/ofproto.h              |   3 +
>  vswitchd/bridge.c              |  24 +++++
>  21 files changed, 512 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index d123819..c99ec29 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -311,12 +311,16 @@ performance of non-tunnel traffic, specifically for 
> smaller size packet.
> 
>  .. _extended-statistics:
> 
> -Extended Statistics
> --------------------
> +Extended & Custom Statistics
> +----------------------------
> 
>  DPDK Extended Statistics API allows PMD to expose unique set of statistics.
>  The Extended statistics are implemented and supported only for DPDK physical
> -and vHost ports.
> +and vHost ports. Custom statistics are dynamic set of counters which can
> +vary depenend on a driver. Those statistics are implemented
> +for DPDK physical ports and contain all "dropped" and "error"
> +counters from XSTATS. XSTATS counters list can be found here:
> +<https://wiki.opnfv.org/display/fastpath/Collectd+Metrics+and+Events>`__.
> 
>  To enable statistics, you have to enable OpenFlow 1.4 support for OVS.
>  Configure bridge br0 to support OpenFlow version 1.4::
> @@ -333,8 +337,9 @@ Query the port statistics by explicitly specifying -O 
> OpenFlow14 option::
> 
>      $ ovs-ofctl -O OpenFlow14 dump-ports br0
> 
> -Note: vHost ports supports only partial statistics. RX packet size based
> -counter are only supported and doesn't include TX packet size counters.
> +Note about "Extended Statistics": vHost ports supports only partial
> +statistics. RX packet size based counter are only supported and
> +doesn't include TX packet size counters.
> 
>  .. _port-hotplug:
> 
> diff --git a/NEWS b/NEWS
> index 427c8f8..727142c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -13,6 +13,12 @@ Post-v2.8.0
>       * ovn-ctl: New commands run_nb_ovsdb and run_sb_ovsdb.
>     - Linux kernel 4.13
>       * Add support for compiling OVS with the latest Linux 4.13 kernel
> +   - DPDK:
> +     * Custom statistics:
> +        - DPDK physical ports now return custom set of "dropped" and "error"
> +          statistics.
> +        - ovs-ofctl dump-ports command now prints new of set custom 
> statistics
> +          if available (for OpenFlow 1.4+).
> 
>  v2.8.0 - 31 Aug 2017
>  --------------------
> diff --git a/include/openflow/intel-ext.h b/include/openflow/intel-ext.h
> index 974e63e..3d73171 100644
> --- a/include/openflow/intel-ext.h
> +++ b/include/openflow/intel-ext.h
> @@ -27,9 +27,11 @@
> 
>  enum intel_port_stats_subtype {
>      INTEL_PORT_STATS_RFC2819 = 1,
> +    INTEL_PORT_STATS_CUSTOM
>  };
> 
>  #define INTEL_PORT_STATS_RFC2819_SIZE 184
> +#define INTEL_PORT_STATS_CUSTOM_SIZE 16
> 
>  /* Struct implements custom property type based on
>   * 'ofp_prop_experimenter'. */
> @@ -70,4 +72,30 @@ struct intel_port_stats_rfc2819 {
>  OFP_ASSERT(sizeof (struct intel_port_stats_rfc2819) ==
>             INTEL_PORT_STATS_RFC2819_SIZE);
> 
> +/* Structure implements custom property type based on
> + * 'ofp_prop_experimenter'. It contains custom
> + * statistics in dictionary format */
> +struct intel_port_custom_stats {
> +    ovs_be16 type;              /* OFPPSPT14_EXPERIMENTER. */
> +    ovs_be16 length;            /* Length in bytes of this property excluding
> +                                 * trailing padding. */
> +    ovs_be32 experimenter;      /* INTEL_VENDOR_ID. */
> +    ovs_be32 exp_type;          /* INTEL_PORT_STATS_*. */
> +
> +    uint8_t pad[2];
> +    ovs_be16 stats_array_size;  /* number of counters. */
> +
> +    /* Followed by:
> +     *   - Exactly 'stats_array_size' array elements of
> +     *     dynamic structure which contains:
> +     *     - "NAME SIZE" - counter name size (number of characters)
> +     *     - "COUNTER NAME" - Exact number of characters
> +     *       defined by "NAME SIZE".
> +     *     - "COUNTER VALUE" -  ovs_be64 counter value,
> +     *   - Zero or more bytes to fill out the
> +     *     overall length in header.length. */
> +};
> +OFP_ASSERT(sizeof(struct intel_port_custom_stats) ==
> +                                  INTEL_PORT_STATS_CUSTOM_SIZE);
> +
>  #endif /* openflow/intel-ext.h */
> diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
> index 50bafc3..e25c241 100644
> --- a/include/openvswitch/netdev.h
> +++ b/include/openvswitch/netdev.h
> @@ -27,6 +27,9 @@ extern "C" {
> 
>  struct netdev;
> 
> +/* Maximum name length for custom statistics counters */
> +#define NETDEV_CUSTOM_STATS_NAME_SIZE 64
> +
>  /* Network device statistics.
>   *
>   * Values of unsupported statistics are set to all-1-bits (UINT64_MAX). */
> @@ -85,6 +88,18 @@ struct netdev_stats {
>      uint64_t rx_jabber_errors;
>  };
> 
> +/* Structure representation of custom statistics counter */
> +struct netdev_custom_counter {
> +    uint64_t value;
> +    char name[NETDEV_CUSTOM_STATS_NAME_SIZE];
> +};
> +
> +/* Structure representation of custom statistics */
> +struct netdev_custom_stats {
> +    uint16_t size;
> +    struct netdev_custom_counter *counters;
> +};
> +
>  /* Features. */
>  enum netdev_features {
>      NETDEV_F_10MB_HD =    1 << 0,  /* 10 Mb half-duplex rate support. */
> @@ -115,6 +130,8 @@ uint64_t netdev_features_to_bps(enum netdev_features 
> features,
>  bool netdev_features_is_full_duplex(enum netdev_features features);
>  int netdev_set_advertisements(struct netdev *, enum netdev_features 
> advertise);
> 
> +void netdev_free_custom_stats_counters(struct netdev_custom_stats *);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
> index a9e57ed..296078a 100644
> --- a/include/openvswitch/ofp-util.h
> +++ b/include/openvswitch/ofp-util.h
> @@ -1174,6 +1174,7 @@ bool ofputil_parse_key_value(char **stringp, char 
> **keyp, char **valuep);
>  struct ofputil_port_stats {
>      ofp_port_t port_no;
>      struct netdev_stats stats;
> +    struct netdev_custom_stats custom_stats;
>      uint32_t duration_sec;      /* UINT32_MAX if unknown. */
>      uint32_t duration_nsec;
>  };
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index faff842..8acc5e0 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -415,6 +415,14 @@ struct netdev_dpdk {
>           * from the enum set 'dpdk_hw_ol_features' */
>          uint32_t hw_ol_features;
>      );
> +
> +    PADDED_MEMBERS(CACHE_LINE_SIZE,
> +        /* Names of all XSTATS counters */
> +        struct rte_eth_xstat_name *rte_xstats_names;
> +        int rte_xstats_names_size;
> +        int rte_xstats_ids_size;
> +        uint64_t *rte_xstats_ids;
> +    );
>  };
> 
>  struct netdev_rxq_dpdk {
> @@ -897,6 +905,12 @@ common_construct(struct netdev *netdev, dpdk_port_t 
> port_no,
> 
>      netdev_request_reconfigure(netdev);
> 
> +    dev->rte_xstats_names = NULL;
> +    dev->rte_xstats_names_size = 0;
> +
> +    dev->rte_xstats_ids = NULL;
> +    dev->rte_xstats_ids_size = 0;
> +
>      return 0;
>  }
> 
> @@ -1135,6 +1149,128 @@ netdev_dpdk_dealloc(struct netdev *netdev)
>      rte_free(dev);
>  }
> 
> +static void
> +netdev_dpdk_clear_xstats(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
> +{
> +    /* If statistics are already allocated, we have to
> +     * reconfigure, as port_id could have been changed. */
> +    if (dev->rte_xstats_names) {
> +        free(dev->rte_xstats_names);
> +        dev->rte_xstats_names = NULL;
> +        dev->rte_xstats_names_size = 0;
> +    }
> +    if (dev->rte_xstats_ids) {
> +        free(dev->rte_xstats_ids);
> +        dev->rte_xstats_ids = NULL;
> +        dev->rte_xstats_ids_size = 0;
> +    }
> +}
> +
> +static const char*
> +netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, uint64_t id)
> +{
> +    if (id >= dev->rte_xstats_names_size) {
> +        return "UNKNOWN";
> +    }
> +    return dev->rte_xstats_names[id].name;
> +}
> +
> +static bool
> +netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
> +    OVS_REQUIRES(dev->mutex)
> +{
> +    int rte_xstats_len;
> +    bool ret;
> +    struct rte_eth_xstat *rte_xstats;
> +    uint64_t id;
> +    int xstats_no;
> +    const char *name;
> +
> +
> +    /* Retrieving all XSTATS names. If something will go wrong
> +     * or amount of counters will be equal 0, rte_xstats_names
> +     * buffer will be marked as NULL, and any further xstats
> +     * query won't be performed (e.g. during netdev_dpdk_get_stats
> +     * execution). */
> +
> +    ret = false;
> +    rte_xstats = NULL;
> +
> +    if (dev->rte_xstats_names == NULL || dev->rte_xstats_ids == NULL) {
> +
> +        dev->rte_xstats_names_size =
> +                rte_eth_xstats_get_names(dev->port_id, NULL, 0);
> +
> +        if (dev->rte_xstats_names_size < 0) {
> +            VLOG_WARN("Cannot get XSTATS for port: %"PRIu8, dev->port_id);
> +            dev->rte_xstats_names_size = 0;
> +        } else {
> +            /* Reserve memory for xstats names and values */
> +            dev->rte_xstats_names = xcalloc(dev->rte_xstats_names_size,
> +                                            sizeof *dev->rte_xstats_names);
> +
> +            if (dev->rte_xstats_names) {
> +                /* Retreive xstats names */
> +                rte_xstats_len =
> +                        rte_eth_xstats_get_names(dev->port_id,
> +                                                 dev->rte_xstats_names,
> +                                                 dev->rte_xstats_names_size);
> +
> +                if (rte_xstats_len < 0) {
> +                    VLOG_WARN("Cannot get XSTATS names for port: %"PRIu8,
> +                               dev->port_id);
> +                    goto out;
> +                }
> +                else if (rte_xstats_len != dev->rte_xstats_names_size) {
> +                    VLOG_WARN("XSTATS size doesn't match for port: %"PRIu8,
> +                              dev->port_id);
> +                    goto out;
> +                }
> +
> +                dev->rte_xstats_ids = xcalloc(dev->rte_xstats_names_size,
> +                                              sizeof(uint64_t));
> +
> +                /* We have to calculate number of counters */
> +                rte_xstats = xcalloc(rte_xstats_len, sizeof *rte_xstats);
> +                memset(rte_xstats, 0xff, sizeof *rte_xstats * 
> rte_xstats_len);
> +
> +                /* Retreive xstats values */
> +                if (rte_eth_xstats_get(dev->port_id, rte_xstats,
> +                                       rte_xstats_len) > 0) {
> +                    dev->rte_xstats_ids_size = 0;
> +                    xstats_no = 0;
> +                    for (uint32_t i = 0 ; i < rte_xstats_len ; i++) {
> +                        id = rte_xstats[i].id;
> +                        name = netdev_dpdk_get_xstat_name(dev, id);
> +                        /* We need to filter out everything except
> +                         * dropped and error counters */
> +                        if (string_ends_with(name, "_errors") ||
> +                            string_ends_with(name, "_dropped")) {
> +
> +                            dev->rte_xstats_ids[xstats_no] = id;
> +                            xstats_no++;
> +                        }
> +                    }
> +                    dev->rte_xstats_ids_size = xstats_no;
> +                    ret = true;
> +                } else {
> +                    VLOG_WARN("Can't get XSTATS IDs for port: %"PRIu8,
> +                              dev->port_id);
> +                }
> +            }
> +        }
> +    } else {
> +        /* Already configured */
> +        ret = true;
> +    }
> +
> +out:
> +    if (!ret) {
> +        netdev_dpdk_clear_xstats(dev);
> +    }
> +    return ret;
> +}
> +
>  static int
>  netdev_dpdk_get_config(const struct netdev *netdev, struct smap *args)
>  {
> @@ -1307,6 +1443,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
> struct smap *args,
>                      dev->devargs = xstrdup(new_devargs);
>                      dev->port_id = new_port_id;
>                      netdev_request_reconfigure(&dev->up);
> +                    netdev_dpdk_clear_xstats(dev);
>                      err = 0;
>                  }
>              }
> @@ -2171,6 +2308,56 @@ out:
>  }
> 
>  static int
> +netdev_dpdk_get_custom_stats(const struct netdev *netdev,
> +                             struct netdev_custom_stats *custom_stats)
> +{
> +
> +    uint32_t i;
> +    struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +    int rte_xstats_ret;
> +
> +    ovs_mutex_lock(&dev->mutex);
> +
> +    if (netdev_dpdk_configure_xstats(dev)) {
> +
> +        uint64_t *values = xcalloc(dev->rte_xstats_ids_size,
> +                                   sizeof(uint64_t));
> +
> +        rte_xstats_ret =
> +                rte_eth_xstats_get_by_id(dev->port_id, dev->rte_xstats_ids,
> +                                         values, dev->rte_xstats_ids_size);
> +
> +        if (rte_xstats_ret > 0 &&
> +            rte_xstats_ret <= dev->rte_xstats_ids_size) {
> +
> +            custom_stats->size = rte_xstats_ret;
> +            custom_stats->counters =
> +                    (struct netdev_custom_counter *) xcalloc(rte_xstats_ret,
> +                            sizeof(struct netdev_custom_counter));
> +
> +            for (i = 0; i < rte_xstats_ret; i++) {
> +                strncpy(custom_stats->counters[i].name,
> +                      netdev_dpdk_get_xstat_name(dev, 
> dev->rte_xstats_ids[i]),
> +                      NETDEV_CUSTOM_STATS_NAME_SIZE);
> +                custom_stats->counters[i].value = values[i];
> +            }
> +        } else {
> +            VLOG_WARN("Cannot get XSTATS values for port: %"PRIu8,
> +                      dev->port_id);
> +            custom_stats->counters = NULL;
> +            custom_stats->size = 0;
> +            /* Let's clear statistics cache, so it will be
> +             * reconfigured */
> +            netdev_dpdk_clear_xstats(dev);
> +        }
> +    }
> +
> +    ovs_mutex_unlock(&dev->mutex);
> +
> +    return 0;
> +}
> +
> +static int
>  netdev_dpdk_get_features(const struct netdev *netdev,
>                           enum netdev_features *current,
>                           enum netdev_features *advertised,
> @@ -3313,7 +3500,8 @@ unlock:
> 
>  #define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT,    \
>                            SET_CONFIG, SET_TX_MULTIQ, SEND,    \
> -                          GET_CARRIER, GET_STATS,             \
> +                          GET_CARRIER, GET_STATS,                      \
> +                          GET_CUSTOM_STATS,                                  
>   \
>                            GET_FEATURES, GET_STATUS,           \
>                            RECONFIGURE, RXQ_RECV)              \
>  {                                                             \
> @@ -3348,6 +3536,7 @@ unlock:
>      netdev_dpdk_get_carrier_resets,                           \
>      netdev_dpdk_set_miimon,                                   \
>      GET_STATS,                                                \
> +    GET_CUSTOM_STATS,                                                        
>                           \
>      GET_FEATURES,                                             \
>      NULL,                       /* set_advertisements */      \
>      NULL,                       /* get_pt_mode */             \
> @@ -3397,6 +3586,7 @@ static const struct netdev_class dpdk_class =
>          netdev_dpdk_eth_send,
>          netdev_dpdk_get_carrier,
>          netdev_dpdk_get_stats,
> +        netdev_dpdk_get_custom_stats,
>          netdev_dpdk_get_features,
>          netdev_dpdk_get_status,
>          netdev_dpdk_reconfigure,
> @@ -3413,6 +3603,7 @@ static const struct netdev_class dpdk_ring_class =
>          netdev_dpdk_ring_send,
>          netdev_dpdk_get_carrier,
>          netdev_dpdk_get_stats,
> +        netdev_dpdk_get_custom_stats,
>          netdev_dpdk_get_features,
>          netdev_dpdk_get_status,
>          netdev_dpdk_reconfigure,
> @@ -3431,6 +3622,7 @@ static const struct netdev_class dpdk_vhost_class =
>          netdev_dpdk_vhost_get_stats,
>          NULL,
>          NULL,
> +        NULL,
>          netdev_dpdk_vhost_reconfigure,
>          netdev_dpdk_vhost_rxq_recv);
>  static const struct netdev_class dpdk_vhost_client_class =
> @@ -3446,6 +3638,7 @@ static const struct netdev_class 
> dpdk_vhost_client_class =
>          netdev_dpdk_vhost_get_stats,
>          NULL,
>          NULL,
> +        NULL,
>          netdev_dpdk_vhost_client_reconfigure,
>          netdev_dpdk_vhost_rxq_recv);
> 
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index 246cdf1..1731b77 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -1383,6 +1383,7 @@ netdev_dummy_update_flags(struct netdev *netdev_,
>      NULL,                       /* get_carrier_resets */        \
>      NULL,                       /* get_miimon */                \
>      netdev_dummy_get_stats,                                     \
> +    NULL,                                                       \
>                                                                  \
>      NULL,                       /* get_features */              \
>      NULL,                       /* set_advertisements */        \
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index e809b88..4e9be30 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2853,6 +2853,7 @@ netdev_linux_update_flags(struct netdev *netdev_, enum 
> netdev_flags off,
>      netdev_linux_get_carrier_resets,                            \
>      netdev_linux_set_miimon_interval,                           \
>      GET_STATS,                                                  \
> +    NULL,                                                                    
>                                         \
>                                                                  \
>      GET_FEATURES,                                               \
>      netdev_linux_set_advertisements,                            \
> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
> index 1720deb..d66fd5b 100644
> --- a/lib/netdev-provider.h
> +++ b/lib/netdev-provider.h
> @@ -459,6 +459,19 @@ struct netdev_class {
>       * (UINT64_MAX). */
>      int (*get_stats)(const struct netdev *netdev, struct netdev_stats *);
> 
> +    /* Retrieves current device custom stats for 'netdev' into 
> 'custom_stats'.
> +     *
> +     * A network device should return only available statistics (if any).
> +     * If there are not statistics available, empty array should be
> +     * returned.
> +     *
> +     * The caller initializes 'custom_stats' before calling this function.
> +     * The caller takes ownership over allocated array of counters inside
> +     * structure netdev_custom_stats.
> +     * */
> +    int (*get_custom_stats)(const struct netdev *netdev,
> +                            struct netdev_custom_stats *custom_stats);
> +
>      /* Stores the features supported by 'netdev' into each of '*current',
>       * '*advertised', '*supported', and '*peer'.  Each value is a bitmap of
>       * NETDEV_F_* bits.
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 518058a..1a3322b 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -906,6 +906,7 @@ netdev_vport_get_ifindex(const struct netdev *netdev_)
>      NULL,                       /* get_carrier_resets */    \
>      NULL,                       /* get_miimon */            \
>      get_stats,                                              \
> +    NULL,                                                   \
>                                                              \
>      NULL,                       /* get_features */          \
>      NULL,                       /* set_advertisements */    \
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 2d69fe5..cd11930 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -1425,6 +1425,21 @@ netdev_get_stats(const struct netdev *netdev, struct 
> netdev_stats *stats)
>      return error;
>  }
> 
> +/* Retrieves current device custom stats for 'netdev'. */
> +int
> +netdev_get_custom_stats(const struct netdev *netdev,
> +                        struct netdev_custom_stats *custom_stats)
> +{
> +    int error;
> +    memset(custom_stats, 0, sizeof *custom_stats);
> +    error = (netdev->netdev_class->get_custom_stats
> +             ? netdev->netdev_class->get_custom_stats(netdev, custom_stats)
> +             : EOPNOTSUPP);
> +
> +    return error;
> +}
> +
> +
>  /* Attempts to set input rate limiting (policing) policy, such that up to
>   * 'kbits_rate' kbps of traffic is accepted, with a maximum accumulative 
> burst
>   * size of 'kbits' kb. */
> @@ -2380,6 +2395,18 @@ netdev_ports_flow_get(const struct dpif_class 
> *dpif_class, struct match *match,
>      return ENOENT;
>  }
> 
> +void
> +netdev_free_custom_stats_counters(struct netdev_custom_stats *custom_stats)
> +{
> +    if (custom_stats) {
> +        if (custom_stats->counters) {
> +            free(custom_stats->counters);
> +            custom_stats->counters = NULL;
> +            custom_stats->size = 0;
> +        }
> +    }
> +}
> +
>  #ifdef __linux__
>  static void
>  netdev_ports_flow_init(void)
> diff --git a/lib/netdev.h b/lib/netdev.h
> index 3a545fe..34df7c9 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -296,6 +296,8 @@ struct netdev *netdev_find_dev_by_in4(const struct 
> in_addr *);
> 
>  /* Statistics. */
>  int netdev_get_stats(const struct netdev *, struct netdev_stats *);
> +int netdev_get_custom_stats(const struct netdev *,
> +                            struct netdev_custom_stats *);
> 
>  /* Quality of service. */
>  struct netdev_qos_capabilities {
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index 151d618..9a5768d 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1834,6 +1834,7 @@ ofp_print_ofpst_port_reply(struct ds *string, const 
> struct ofp_header *oh,
>                             const struct ofputil_port_map *port_map,
>                             int verbosity)
>  {
> +    uint32_t i;
>      ds_put_format(string, " %"PRIuSIZE" ports\n", 
> ofputil_count_port_stats(oh));
>      if (verbosity < 1) {
>          return;
> @@ -1946,6 +1947,23 @@ ofp_print_ofpst_port_reply(struct ds *string, const 
> struct ofp_header *oh,
>              ds_put_cstr(string, "\n");
>              ds_destroy(&string_ext_stats);
>          }
> +
> +        if (ps.custom_stats.size) {
> +            ds_put_cstr(string, "           CUSTOM Statistics ");
> +            for (i = 0 ; i < ps.custom_stats.size ; i++) {
> +                /* 3 counters in the row */
> +                if (strlen(ps.custom_stats.counters[i].name)) {
> +                    if ((i % 3) == 0) {
> +                        ds_put_cstr(string, "\n");
> +                        ds_put_cstr(string, "                      ");
> +                    }
> +                    ds_put_format(string, "%s=%"PRIu64", ",
> +                                  ps.custom_stats.counters[i].name,
> +                                  ps.custom_stats.counters[i].value);
> +                }
> +            }
> +            ds_put_cstr(string, "\n");
> +        }
>      }
>  }
> 
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 47f30c7..0652588 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -7999,15 +7999,18 @@ ofputil_append_ofp14_port_stats(const struct 
> ofputil_port_stats *ops,
>  {
>      struct ofp14_port_stats_prop_ethernet *eth;
>      struct intel_port_stats_rfc2819 *stats_rfc2819;
> +    struct intel_port_custom_stats *stats_custom;
>      struct ofp14_port_stats *ps14;
>      struct ofpbuf *reply;
> +    uint16_t i;
> +    uint64_t counter_value;
> +    size_t custom_stats_start, start_ofs;
> 
> -    reply = ofpmp_reserve(replies, sizeof *ps14 + sizeof *eth +
> -                          sizeof *stats_rfc2819);
> +    reply = ofpbuf_from_list(ovs_list_back(replies));
> +    start_ofs = reply->size;
> 
>      ps14 = ofpbuf_put_uninit(reply, sizeof *ps14);
> -    ps14->length = htons(sizeof *ps14 + sizeof *eth +
> -                         sizeof *stats_rfc2819);
> +
>      memset(ps14->pad, 0, sizeof ps14->pad);
>      ps14->port_no = ofputil_port_to_ofp11(ops->port_no);
>      ps14->duration_sec = htonl(ops->duration_sec);
> @@ -8027,10 +8030,10 @@ ofputil_append_ofp14_port_stats(const struct 
> ofputil_port_stats *ops,
>      eth->rx_crc_err = htonll(ops->stats.rx_crc_errors);
>      eth->collisions = htonll(ops->stats.collisions);
> 
> -    uint64_t prop_type = OFPPROP_EXP(INTEL_VENDOR_ID,
> +    uint64_t prop_type_stats = OFPPROP_EXP(INTEL_VENDOR_ID,
>                                       INTEL_PORT_STATS_RFC2819);
> 
> -    stats_rfc2819 = ofpprop_put_zeros(reply, prop_type,
> +    stats_rfc2819 = ofpprop_put_zeros(reply, prop_type_stats,
>                                        sizeof *stats_rfc2819);
> 
>      memset(stats_rfc2819->pad, 0, sizeof stats_rfc2819->pad);
> @@ -8076,6 +8079,44 @@ ofputil_append_ofp14_port_stats(const struct 
> ofputil_port_stats *ops,
>          htonll(ops->stats.rx_fragmented_errors);
>      stats_rfc2819->rx_jabber_errors =
>          htonll(ops->stats.rx_jabber_errors);
> +
> +    if (ops->custom_stats.counters && ops->custom_stats.size) {
> +
> +        custom_stats_start = reply->size;
> +
> +        uint64_t prop_type_custom = OFPPROP_EXP(INTEL_VENDOR_ID,
> +                                        INTEL_PORT_STATS_CUSTOM);
> +
> +        stats_custom = ofpprop_put_zeros(reply, prop_type_custom,
> +                                         sizeof *stats_custom);
> +
> +        stats_custom->stats_array_size = htons(ops->custom_stats.size);
> +        memset(stats_custom->pad, 0, sizeof stats_custom->pad);
> +
> +        for (i = 0 ; i < ops->custom_stats.size; i++) {
> +            uint8_t counter_size = 
> strnlen(ops->custom_stats.counters[i].name,
> +                                           NETDEV_CUSTOM_STATS_NAME_SIZE);
> +            /* Counter name size */
> +            ofpbuf_put(reply, &counter_size, sizeof(counter_size));
> +            /* Counter name */
> +            ofpbuf_put(reply, ops->custom_stats.counters[i].name,
> +                       counter_size);
> +            /* Counter value */
> +            counter_value = htonll(ops->custom_stats.counters[i].value);
> +            ofpbuf_put(reply, &counter_value,
> +                       sizeof(ops->custom_stats.counters[i].value));
> +        }
> +
> +        ofpbuf_padto(reply, ROUND_UP(reply->size, 8));
> +        stats_custom = ofpbuf_at_assert(reply, custom_stats_start,
> +                                        sizeof *stats_custom);
> +        stats_custom->length = htons(reply->size - custom_stats_start);
> +    }
> +
> +    ps14 = ofpbuf_at_assert(reply, start_ofs, sizeof *ps14);
> +    ps14->length = htons(reply->size - start_ofs);
> +
> +    ofpmp_postappend(replies, start_ofs);
>  }
> 
>  /* Encode a ports stat for 'ops' and append it to 'replies'. */
> @@ -8239,6 +8280,63 @@ parse_intel_port_stats_rfc2819_property(const struct 
> ofpbuf *payload,
>  }
> 
>  static enum ofperr
> +parse_intel_port_custom_property(const struct ofpbuf *payload,
> +                                 struct ofputil_port_stats *ops)
> +{
> +    uint16_t i;
> +
> +    const struct intel_port_custom_stats *custom_stats = payload->data;
> +
> +    ops->custom_stats.size = ntohs(custom_stats->stats_array_size);
> +
> +    ops->custom_stats.counters = (struct netdev_custom_counter *)
> +                                  xcalloc(ops->custom_stats.size,
> +                                  sizeof(struct netdev_custom_counter));
> +
> +    uint16_t msg_size = ntohs(custom_stats->length);
> +    uint16_t current_len = sizeof( *custom_stats);
> +    uint8_t *current = (uint8_t *)payload->data + current_len;
> +    uint8_t string_size = 0;
> +    uint8_t value_size = 0;
> +    uint64_t counter_value = 0;
> +
> +    for (i = 0 ; i < ops->custom_stats.size ; i++) {
> +
> +        current_len += string_size + value_size;
> +        current += string_size + value_size;
> +
> +        value_size = sizeof(uint64_t);
> +        /* Counter name size */
> +        string_size = *current;
> +
> +        /* Buffer overrun check */
> +        if (current_len + string_size + value_size > msg_size) {
> +            VLOG_ERR("Custom statistics buffer overrun! "
> +                     "Further message parsing is aborted.");
> +            break;
> +        }
> +
> +        current++;
> +        current_len++;
> +        /* Counter name */
> +        if (string_size > sizeof(ops->custom_stats.counters[i].name)) {
> +            VLOG_WARN("Counter name size too big! Only part "
> +                      "of the name will be copied.");
> +            memcpy(ops->custom_stats.counters[i].name, current,
> +                   sizeof(ops->custom_stats.counters[i].name));
> +        } else {
> +            memcpy(ops->custom_stats.counters[i].name, current, string_size);
> +        }
> +        /* Counter value */
> +        memcpy(&counter_value, current + string_size,
> +               value_size);
> +        ops->custom_stats.counters[i].value = ntohll(counter_value);
> +    }
> +
> +    return 0;
> +}
> +
> +static enum ofperr
>  parse_intel_port_stats_property(const struct ofpbuf *payload,
>                                  uint32_t exp_type,
>                                  struct ofputil_port_stats *ops)
> @@ -8249,6 +8347,9 @@ parse_intel_port_stats_property(const struct ofpbuf 
> *payload,
>      case INTEL_PORT_STATS_RFC2819:
>          error = parse_intel_port_stats_rfc2819_property(payload, ops);
>          break;
> +    case INTEL_PORT_STATS_CUSTOM:
> +        error = parse_intel_port_custom_property(payload, ops);
> +        break;
>      default:
>          error = OFPERR_OFPBPC_BAD_EXP_TYPE;
>          break;
> @@ -8308,6 +8409,11 @@ ofputil_pull_ofp14_port_stats(struct 
> ofputil_port_stats *ops,
>                                                      INTEL_PORT_STATS_RFC2819,
>                                                      ops);
>              break;
> +        case OFPPROP_EXP(INTEL_VENDOR_ID, INTEL_PORT_STATS_CUSTOM):
> +            error = parse_intel_port_stats_property(&payload,
> +                                                    INTEL_PORT_STATS_CUSTOM,
> +                                                    ops);
> +            break;
>          default:
>              error = OFPPROP_UNKNOWN(true, "port stats", type);
>              break;
> @@ -8354,6 +8460,7 @@ ofputil_decode_port_stats(struct ofputil_port_stats 
> *ps, struct ofpbuf *msg)
>      enum ofpraw raw;
> 
>      memset(&(ps->stats), 0xFF, sizeof (ps->stats));
> +    memset(&(ps->custom_stats), 0, sizeof (ps->custom_stats));
> 
>      error = (msg->header ? ofpraw_decode(&raw, msg->header)
>               : ofpraw_pull(&raw, msg));
> diff --git a/lib/util.c b/lib/util.c
> index 2965656..462c1fa 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -321,6 +321,19 @@ ovs_strzcpy(char *dst, const char *src, size_t size)
>      }
>  }
> 
> +/*
> + * Returns true if 'str' ends with given 'suffix'.
> + */
> +int
> +string_ends_with(const char * str, const char * suffix)
> +{
> +    int str_len = strlen(str);
> +    int suffix_len = strlen(suffix);
> +
> +    return (str_len >= suffix_len) &&
> +           (0 == strcmp(str + (str_len - suffix_len), suffix));
> +}
> +
>  /* Prints 'format' on stderr, formatting it like printf() does.  If 'err_no' 
> is
>   * nonzero, then it is formatted with ovs_retval_to_string() and appended to
>   * the message inside parentheses.  Then, terminates with abort().
> diff --git a/lib/util.h b/lib/util.h
> index b01f421..e75f6ce 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -157,6 +157,8 @@ void free_cacheline(void *);
>  void ovs_strlcpy(char *dst, const char *src, size_t size);
>  void ovs_strzcpy(char *dst, const char *src, size_t size);
> 
> +int string_ends_with(const char * str, const char * suffix);
> +
>  /* The C standards say that neither the 'dst' nor 'src' argument to
>   * memcpy() may be null, even if 'n' is zero.  This wrapper tolerates
>   * the null case. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index b99f04f..e53ee35 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3775,6 +3775,18 @@ port_get_stats(const struct ofport *ofport_, struct 
> netdev_stats *stats)
>  }
> 
>  static int
> +port_get_custom_stats(const struct ofport *ofport_,
> +                      struct netdev_custom_stats *custom_stats)
> +{
> +    struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
> +    int error;
> +
> +    error = netdev_get_custom_stats(ofport->up.netdev, custom_stats);
> +
> +    return error;
> +}
> +
> +static int
>  port_get_lacp_stats(const struct ofport *ofport_, struct lacp_slave_stats 
> *stats)
>  {
>      struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
> @@ -5785,6 +5797,7 @@ const struct ofproto_class ofproto_dpif_class = {
>      port_del,
>      port_set_config,
>      port_get_stats,
> +    port_get_custom_stats,
>      port_dump_start,
>      port_dump_next,
>      port_dump_done,
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 9dc73c4..55e772e 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1049,6 +1049,10 @@ struct ofproto_class {
>      int (*port_get_stats)(const struct ofport *port,
>                            struct netdev_stats *stats);
> 
> +    /* Get port custom stats */
> +    int (*port_get_custom_stats)(const struct ofport *port,
> +                                 struct netdev_custom_stats *custom_stats);
> +
>      /* Port iteration functions.
>       *
>       * The client might not be entirely in control of the ports within an
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 82c2bb2..c76a9a1 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2605,6 +2605,24 @@ ofproto_port_get_stats(const struct ofport *port, 
> struct netdev_stats *stats)
>      return error;
>  }
> 
> +int
> +ofproto_port_get_custom_stats(const struct ofport *port,
> +                              struct netdev_custom_stats *custom_stats)
> +{
> +    struct ofproto *ofproto = port->ofproto;
> +    int error;
> +
> +    if (ofproto->ofproto_class->port_get_custom_stats) {
> +        memset(custom_stats, 0, sizeof *custom_stats);
> +        error = ofproto->ofproto_class->port_get_custom_stats(port,
> +                                                              custom_stats);
> +    } else {
> +        error = EOPNOTSUPP;
> +    }
> +
> +    return error;
> +}
> +
>  static int
>  update_port(struct ofproto *ofproto, const char *name)
>  {
> @@ -3887,8 +3905,11 @@ append_port_stat(struct ofport *port, struct ovs_list 
> *replies)
>       * 'stats' to all-1s, which is correct for OpenFlow, and
>       * netdev_get_stats() will log errors. */
>      ofproto_port_get_stats(port, &ops.stats);
> +    ofproto_port_get_custom_stats(port, &ops.custom_stats);
> 
>      ofputil_append_port_stat(replies, &ops);
> +
> +    netdev_free_custom_stats_counters(&ops.custom_stats);
>  }
> 
>  static void
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 1e48e19..15e478d 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -48,6 +48,7 @@ struct shash;
>  struct simap;
>  struct smap;
>  struct netdev_stats;
> +struct netdev_custom_stats;
>  struct ovs_list;
>  struct lldp_status;
>  struct aa_settings;
> @@ -297,6 +298,8 @@ int ofproto_port_del(struct ofproto *, ofp_port_t 
> ofp_port);
>  void ofproto_port_set_config(struct ofproto *, ofp_port_t ofp_port,
>                               const struct smap *cfg);
>  int ofproto_port_get_stats(const struct ofport *, struct netdev_stats 
> *stats);
> +int ofproto_port_get_custom_stats(const struct ofport *port,
> +                                  struct netdev_custom_stats *custom_stats);
> 
>  int ofproto_port_query_by_name(const struct ofproto *, const char *devname,
>                                 struct ofproto_port *);
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 630c6fa..1856908 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -2347,6 +2347,11 @@ iface_refresh_cfm_stats(struct iface *iface)
>  static void
>  iface_refresh_stats(struct iface *iface)
>  {
> +    struct netdev_custom_stats custom_stats;
> +    uint32_t i;
> +    int64_t *custom_values = NULL;
> +    char **custom_keys = NULL;
> +
>  #define IFACE_STATS                             \
>      IFACE_STAT(rx_packets,              "rx_packets")               \
>      IFACE_STAT(tx_packets,              "tx_packets")               \
> @@ -2413,6 +2418,25 @@ iface_refresh_stats(struct iface *iface)
> 
>      ovsrec_interface_set_statistics(iface->cfg, keys, values, n);
>  #undef IFACE_STATS
> +
> +    netdev_get_custom_stats(iface->netdev, &custom_stats);
> +
> +    if (custom_stats.size && custom_stats.counters) {
> +        custom_values = xmalloc(custom_stats.size * sizeof *custom_values);
> +        custom_keys = xmalloc(custom_stats.size * sizeof(char *));
> +        if (custom_values && custom_keys) {
> +            for (i = 0 ; i < custom_stats.size ; i++) {
> +                custom_values[i] = custom_stats.counters[i].value;
> +                custom_keys[i] = custom_stats.counters[i].name;
> +            }
> +            ovsrec_interface_set_statistics(iface->cfg,
> +                                    (const char **)custom_keys,
> +                                    custom_values, custom_stats.size);
> +            free(custom_values);
> +            free(custom_keys);
> +        }
> +        netdev_free_custom_stats_counters(&custom_stats);
> +    }
>  }
> 
>  static void
> --
> 1.8.3.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to