Skip to content

Add per-queue stats to bessctl #1007

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

Open
sangjinhan wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add per-queue stats to bessctl #1007

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
038f3ea
adjust log level for CollectStats
sangjinhan Jul 19, 2020
1cc5c1c
s/queue_stats/queue_stats_/g
sangjinhan Jul 19, 2020
4f83896
s/DRIVER_FLAG_SELF_*/offload*Stats/
sangjinhan Jul 20, 2020
593fcdd
remove batch-size histogram feature
sangjinhan Jul 20, 2020
85b7cef
hide queue_stats_ as a private member
sangjinhan Jul 20, 2020
84ab7ab
fix a whitespace issue in vdev logs
sangjinhan Jul 20, 2020
dfafbec
show admin up/down status for ports
sangjinhan Jul 20, 2020
fe91b49
add heuristics for drivers with per-port only stat
sangjinhan Jul 20, 2020
22c9c09
add per-queue stats
sangjinhan Jul 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/drivers/pmd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ class PMDPort final : public Port {
*/
int SendPackets(queue_t qid, bess::Packet **pkts, int cnt) override;

uint64_t GetFlags() const override {
return DRIVER_FLAG_SELF_INC_STATS | DRIVER_FLAG_SELF_OUT_STATS;
DriverFeatures GetFeatures() const override {
return {.offloadIncStats = true, .offloadOutStats = true};
}

LinkStatus GetLinkStatus() override;
Expand Down
2 changes: 1 addition & 1 deletion core/modules/port_inc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ struct task_result PortInc::RunTask(Context *ctx, bess::PacketBatch *batch,
}
}

if (!(port_->GetFlags() & DRIVER_FLAG_SELF_INC_STATS)) {
if (!(port_->GetFeatures().offloadIncStats)) {
qstats.packets += cnt;
qstats.bytes += received_bytes;
}
Expand Down
2 changes: 1 addition & 1 deletion core/modules/port_out.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ static inline int SendBatch(bess::PacketBatch *batch, Port *p, queue_t qid) {

int sent_pkts = p->SendPackets(qid, batch->pkts(), batch->cnt());

if (!(p->GetFlags() & DRIVER_FLAG_SELF_OUT_STATS)) {
if (!(p->GetFeatures().offloadOutStats)) {
uint64_t sent_bytes = 0;
for (int i = 0; i < sent_pkts; i++) {
sent_bytes += batch->pkts()[i]->total_len();
Expand Down
2 changes: 1 addition & 1 deletion core/modules/queue_inc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ struct task_result QueueInc::RunTask(Context *ctx, bess::PacketBatch *batch,
}
}

if (!(port_->GetFlags() & DRIVER_FLAG_SELF_INC_STATS)) {
if (!(port_->GetFeatures().offloadIncStats)) {
qstats.packets += cnt;
qstats.bytes += received_bytes;
}
Expand Down
2 changes: 1 addition & 1 deletion core/modules/queue_out.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void QueueOut::ProcessBatch(Context *, bess::PacketBatch *batch) {
sent_pkts = port_->SendPackets(qid_, batch->pkts(), batch->cnt());
}

if (!(port_->GetFlags() & DRIVER_FLAG_SELF_OUT_STATS)) {
if (!(port_->GetFeatures().offloadOutStats)) {
uint64_t sent_bytes = 0;
for (int i = 0; i < sent_pkts; i++) {
sent_bytes += batch->pkts()[i]->total_len();
Expand Down
24 changes: 15 additions & 9 deletions core/port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,18 @@ using queue_t = uint8_t;

#define MAX_QUEUES_PER_DIR 128 /* [0, 127] (for each RX/TX) */

#define DRIVER_FLAG_SELF_INC_STATS 0x0001
#define DRIVER_FLAG_SELF_OUT_STATS 0x0002

#define MAX_QUEUE_SIZE 4096

#define ETH_ALEN 6

/* The term RX/TX could be very confusing for a virtual switch.
* Instead, we use the "incoming/outgoing" convention:
* - incoming: outside -> BESS
* - outgoing: BESS -> outside */
// The term RX/TX could be very confusing for a virtual switch.
// Instead, we use the "incoming/outgoing" convention:
// - incoming: outside -> BESS
// - outgoing: BESS -> outside
//
// NOTE: for new code, avoid using arrays like `Foo bar[PACKET_DIR]`.
// The use of array doesn't really make the code more readable.
// Instead, follow the `Foo bar_inc; Foo bar_out` style.
typedef enum {
PACKET_DIR_INC = 0,
PACKET_DIR_OUT = 1,
Expand Down Expand Up @@ -215,6 +216,11 @@ class Port {
QueueStats out;
};

struct DriverFeatures {
bool offloadIncStats;
bool offloadOutStats;
};

// overide this section to create a new driver -----------------------------
Port()
: queue_stats_(),
Expand Down Expand Up @@ -247,7 +253,7 @@ class Port {
virtual size_t DefaultIncQueueSize() const { return kDefaultIncQueueSize; }
virtual size_t DefaultOutQueueSize() const { return kDefaultOutQueueSize; }

virtual uint64_t GetFlags() const { return 0; }
virtual DriverFeatures GetFeatures() const { return {}; }
Copy link
Copy Markdown
Contributor

@melvinw melvinw Jul 31, 2020

Choose a reason for hiding this comment

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

huh. are there plans to make this real/settable?


/*!
* Get any placement constraints that need to be met when receiving from this
Expand Down Expand Up @@ -299,7 +305,7 @@ class Port {
protected:
friend class PortBuilder;

/* for stats that do NOT belong to any queues */
// for stats that do NOT belong to any queues
PortStats port_stats_;

// Current configuration
Expand Down