Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ovs_stats: Implement OVS statistics plugin. #2137

Merged
merged 3 commits into from Feb 28, 2017

Conversation

maryamtahhan
Copy link
Contributor

OVS stats plugin uses OVS utils to get available
bridge/interface statistics from OVSDB.

Change-Id: Iabe71d64fef7fdbfd5a272c3599a92bce2adf055
Signed-off-by: Mytnyk, VolodymyrX volodymyrx.mytnyk@intel.com

OVS stats plugin uses OVS utils to get available
bridge/interface statistics from OVSDB.

Change-Id: Iabe71d64fef7fdbfd5a272c3599a92bce2adf055
Signed-off-by: Mytnyk, VolodymyrX <volodymyrx.mytnyk@intel.com>
@@ -5524,6 +5524,54 @@ use B<Interval> option of the OVS B<LoadPlugin> block settings. For milliseconds
simple divide the time by 1000 for example if the desired interval is 50ms, set
interval to 0.05.

=head2 Plugin C<ovs_stats>

The I<ovs_stats> plugin collects a statistics of OVS connected interfaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

statistics is plurar, so please remove the 'a'

src/ovs_stats.c Outdated
};

/* Entry into the list of network bridges */
static bridge_list_t *g_bridge_list_head = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to zero-initialize statics.

src/ovs_stats.c Outdated
};

static const iface_counter ovs_stats_counter_name_to_type(const char *counter) {
iface_counter index = not_supportred;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not_supported

src/ovs_stats.c Outdated
static const char plugin_name[] = "ovs_stats";

typedef enum iface_counter {
not_supportred = -1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not_supported

src/ovs_stats.c Outdated
static int ovs_stats_is_monitored_bridge(const char *br_name) {
int rc = 0;
/* if no bridges are configured, return true */
if (!(rc = (g_monitored_bridge_list_head == NULL)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit hard to follow this code. I believe if you can get rid of rc which you are treating as a bool this can be simplified a bit.

src/ovs_stats.c Outdated

/* Delete all ports from port list */
static void ovs_stats_free_port_list(port_list_t *head) {
port_list_t *i, *del;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move these into the for loop?

src/ovs_stats.c Outdated

/* Delete all bridges from bridge list */
static void ovs_stats_free_bridge_list(bridge_list_t *head) {
bridge_list_t *i, *del;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

src/ovs_stats.c Outdated

/* Handle OVSDB lost connection callback */
static void ovs_stats_conn_terminate() {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove blank linke

}

/* Handle OVSDB lost connection callback */
static void ovs_stats_conn_terminate() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(void)

src/ovs_stats.c Outdated
plugin_name, ovs_stats_cfg.ovs_db_node, ovs_stats_cfg.ovs_db_serv,
ovs_stats_cfg.ovs_db_unix);
/* connect to OvS DB */
if ((ovs_db = ovs_db_init (ovs_stats_cfg.ovs_db_node,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ovs_db is static. You're using the g_ prefix for all other statics, I believe you should do the same here.

@rubenk
Copy link
Contributor

rubenk commented Feb 19, 2017

@maryamtahhan ping?

@taraschornyi
Copy link
Contributor

@rubenk sorry for long reply will update this PR today or tomorrow

Fixed issue with missing counter

Signed-off-by: Taras Chornyi <tarasx.chornyi@intel.com>
Signed-off-by: Taras Chornyi <tarasx.chornyi@intel.com>
@rubenk rubenk merged commit 79a9420 into collectd:master Feb 28, 2017
@rubenk
Copy link
Contributor

rubenk commented Feb 28, 2017

Thank you @maryamtahhan and @taraschornyi.

jaroug pushed a commit to jaroug/collectd that referenced this pull request Mar 1, 2017
ovs_stats: Implement OVS statistics plugin.
@maryamtahhan maryamtahhan deleted the feat_ovs_stats branch March 7, 2017 16:31
@octo octo added this to the 5.8 milestone Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants