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.

Sign up for GitHub

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

Jump to bottom

add bsn_counter request reply #149

Merged
merged 8 commits into from
Dec 9, 2013
Merged

add bsn_counter request reply #149

merged 8 commits into from
Dec 9, 2013

Conversation

xinwu
Copy link
Contributor

@xinwu xinwu commented Dec 4, 2013

  1. add prefix to counter names
  2. Move counter types into enum.
  3. Pass make check-all
  4. Split input file "bsn_counter" into two files "bsn_port_counter" and "bsn_vlan_counter"

Reviewer: @rlane @kenchiang

@rlane
Copy link
Contributor

rlane commented Dec 4, 2013

High level questions:

  1. Does this pass make check-all?
  2. Is the controller expected to query for subsets of ports/VLANs? Is there a significant performance hit for always querying for a single port/VLAN or every port/VLAN?
  3. Do you see a need for extensibility in the set of counters?

I'd prefer the input file be split into "bsn_port_counters" and "bsn_vlan_counters" since the messages are disjoint.

@rlane
Copy link
Contributor

rlane commented Dec 5, 2013

Also, all the counters need to be uint64_t. A 32-bit counter could overflow in less than a second.

@xinwu
Copy link
Contributor Author

xinwu commented Dec 5, 2013

They are already 64bit :)

Xin

On Wed, Dec 4, 2013 at 5:30 PM, Rich Lane notifications@github.com wrote:

Also, all the counters need to be uint64_t. A 32-bit counter could
overflow in less than a second.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/149#issuecomment-29864048
.

@rlane
Copy link
Contributor

rlane commented Dec 5, 2013

Ok, I was only looking at the first commit.

rlane
rlane reviewed Dec 6, 2013
View reviewed changes
openflow_input/bsn_port_counter
OFP_TX_PACKETS_BROADCAST = 8,
OFP_TX_PACKETS_MULTICAST = 9,
OFP_TX_DROPPED = 10,
OFP_TX_ERRORS = 11,
Copy link
Contributor

Choose a reason for hiding this comment

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

These names should be prefixed to avoid name collisions - how about OFP_BSN_COUNTER_* ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it'll have to be OFP_BSN_PORT_COUNTER_* to avoid collisions with the VLAN counter enum.

@rlane
Copy link
Contributor

rlane commented Dec 8, 2013

ACK

@bsn-abat
Copy link

bsn-abat commented Dec 9, 2013

ABAT: START_MERGE: Log file at http://sbs2/abat/2013.12.09.1836-m.loxigen.master/abat.log

bsn-abat pushed a commit to floodlight/loxigen-artifacts that referenced this pull request Dec 9, 2013
Artifacts from floodlight/loxigen@d0c6860...06b5b31
9f2cfa9 
Loxigen Head commit floodlight/loxigen@06b5b31
commit 06b5b3194220f8ed337b9b5a0cc94a9086cf6b75
Merge: d0c6860 9b8989c
Author: abat <abat@bigswitch.com>
Date:   Mon Dec 9 10:36:43 2013 -0800

    Merge into master from pull request #149:
    add bsn_counter request reply (floodlight/loxigen#149)

commit 9b8989cb74907fb14f230f5be7a914256c1f4bd7
Author: xinwu <xin.wu@bigswitch.com>
Date:   Fri Dec 6 16:01:32 2013 -0800

    replace tabs with spaces

commit 4f6987f6509a3ebd1fedacee7ad3d0c869febcb4
Author: xinwu <xin.wu@bigswitch.com>
Date:   Fri Dec 6 13:39:57 2013 -0800

    add prefix to avoid name collision

commit 7e2f382eb4cf4a00c524bc7af9561ad9626b4017
Author: xinwu <xin.wu@bigswitch.com>
Date:   Thu Dec 5 18:48:22 2013 -0800

    add test data bsn_port_counter_stats_reply.data

commit 37aaed5f6acd776f52d731787dbd43ddf60417cb
Author: xinwu <xin.wu@bigswitch.com>
Date:   Thu Dec 5 18:37:35 2013 -0800

    simplify variable names

commit f08ef687300b03ced9336bd8f63e2f0c8e8a064e
Author: xinwu <xin.wu@bigswitch.com>
Date:   Thu Dec 5 18:29:20 2013 -0800

    add bsn_port/vlan_counter_stats_request/reply

commit 54ceacaff4bb954369d0d91ae5579f190b208a58
Author: xinwu <xin.wu@bigswitch.com>
Date:   Wed Dec 4 17:02:27 2013 -0800

    fix typos

commit 358e6162420f94770872bf5c4c4949df35ccd8fd
Author: xinwu <xin.wu@bigswitch.com>
Date:   Wed Dec 4 16:49:48 2013 -0800

    separate bsn_port_counter from bsn_vlan_counter

commit 122ea6c633cc4c91ebb0813b8c5155ba648c6aa6
Author: xinwu <xin.wu@bigswitch.com>
Date:   Wed Dec 4 14:15:46 2013 -0800

    add bsn_counter request reply
bsn-abat pushed a commit that referenced this pull request Dec 9, 2013
Merge into master from pull request #149:
06b5b31 
add bsn_counter request reply (#149)
@bsn-abat bsn-abat merged commit 9b8989c into floodlight:master Dec 9, 2013
@bsn-abat
Copy link

bsn-abat commented Dec 9, 2013

ABAT: ACCEPT: Successfully merged

In case you want to see the build log, check out :
Log file at http://sbs2/abat/2013.12.09.1836-m.loxigen.master/abat.log
/cc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xinwu @rlane @bsn-abat