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

Add per-queue stats to bessctl #1007

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
12 changes: 0 additions & 12 deletions core/bessctl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1133,22 +1133,10 @@ class BESSControlImpl final : public BESSControl::Service {
response->mutable_inc()->set_packets(stats.inc.packets);
response->mutable_inc()->set_dropped(stats.inc.dropped);
response->mutable_inc()->set_bytes(stats.inc.bytes);
*response->mutable_inc()->mutable_requested_hist() = {
stats.inc.requested_hist.begin(), stats.inc.requested_hist.end()};
*response->mutable_inc()->mutable_actual_hist() = {
stats.inc.actual_hist.begin(), stats.inc.actual_hist.end()};
*response->mutable_inc()->mutable_diff_hist() = {
stats.inc.diff_hist.begin(), stats.inc.diff_hist.end()};

response->mutable_out()->set_packets(stats.out.packets);
response->mutable_out()->set_dropped(stats.out.dropped);
response->mutable_out()->set_bytes(stats.out.bytes);
*response->mutable_out()->mutable_requested_hist() = {
stats.out.requested_hist.begin(), stats.out.requested_hist.end()};
*response->mutable_out()->mutable_actual_hist() = {
stats.out.actual_hist.begin(), stats.out.actual_hist.end()};
*response->mutable_out()->mutable_diff_hist() = {
stats.out.diff_hist.begin(), stats.out.diff_hist.end()};

response->set_timestamp(get_epoch_time());

Expand Down
7 changes: 1 addition & 6 deletions core/drivers/pmd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -479,12 +479,7 @@ int PMDPort::RecvPackets(queue_t qid, bess::Packet **pkts, int cnt) {
int PMDPort::SendPackets(queue_t qid, bess::Packet **pkts, int cnt) {
int sent = rte_eth_tx_burst(dpdk_port_id_, qid,
reinterpret_cast<rte_mbuf **>(pkts), cnt);
auto &stats = queue_stats_[PACKET_DIR_OUT][qid];
int dropped = cnt - sent;
stats.dropped += dropped;
stats.requested_hist[cnt]++;
stats.actual_hist[sent]++;
stats.diff_hist[dropped]++;
queue_stats_[PACKET_DIR_OUT][qid].dropped += cnt - sent;
return sent;
}

Expand Down
7 changes: 2 additions & 5 deletions core/modules/port_inc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,12 @@ struct task_result PortInc::RunTask(Context *ctx, bess::PacketBatch *batch,
const int pkt_overhead = 24;

uint32_t cnt = port_->RecvPackets(qid, batch->pkts(), burst);
batch->set_cnt(cnt);
qstats.requested_hist[burst]++;
qstats.actual_hist[cnt]++;
qstats.diff_hist[burst - cnt]++;

if (cnt == 0) {
return {.block = true, .packets = 0, .bits = 0};
}

batch->set_cnt(cnt);
Copy link
Member

Choose a reason for hiding this comment

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

nit: Just to prevent future mistake, how about setting this before returning whenif (cnt==0) {... }


// NOTE: we cannot skip this step since it might be used by scheduler.
if (prefetch_) {
for (uint32_t i = 0; i < cnt; i++) {
Expand Down
7 changes: 2 additions & 5 deletions core/modules/queue_inc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,12 @@ struct task_result QueueInc::RunTask(Context *ctx, bess::PacketBatch *batch,
const int pkt_overhead = 24;

uint32_t cnt = port_->RecvPackets(qid, batch->pkts(), burst);
batch->set_cnt(cnt);
qstats.requested_hist[burst]++;
qstats.actual_hist[cnt]++;
qstats.diff_hist[burst - cnt]++;

if (cnt == 0) {
return {.block = true, .packets = 0, .bits = 0};
}

batch->set_cnt(cnt);
Copy link
Member

Choose a reason for hiding this comment

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

nit: Just to prevent future mistake, how about setting this before returning whenif (cnt==0) {... }


// NOTE: we cannot skip this step since it might be used by scheduler.
uint64_t received_bytes = 0;
if (prefetch_) {
Expand Down
6 changes: 0 additions & 6 deletions core/port.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,19 +178,13 @@ Port::PortStats Port::GetPortStats() {
ret.inc.packets += inc.packets;
ret.inc.dropped += inc.dropped;
ret.inc.bytes += inc.bytes;
ret.inc.requested_hist += inc.requested_hist;
ret.inc.actual_hist += inc.actual_hist;
ret.inc.diff_hist += inc.diff_hist;
}

for (queue_t qid = 0; qid < num_queues[PACKET_DIR_OUT]; qid++) {
const QueueStats &out = queue_stats_[PACKET_DIR_OUT][qid];
ret.out.packets += out.packets;
ret.out.dropped += out.dropped;
ret.out.bytes += out.bytes;
ret.out.requested_hist += out.requested_hist;
ret.out.actual_hist += out.actual_hist;
ret.out.diff_hist += out.diff_hist;
}

return ret;
Expand Down
13 changes: 0 additions & 13 deletions core/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,23 +177,10 @@ class PortBuilder {
// InitPortClass()?
};

struct BatchHistogram
: public std::array<uint64_t, bess::PacketBatch::kMaxBurst + 1> {
BatchHistogram &operator+=(const BatchHistogram &rhs) {
for (size_t i = 0; i < size(); i++) {
(*this)[i] += rhs[i];
}
return *this;
}
};

struct QueueStats {
uint64_t packets;
uint64_t dropped; // Not all drivers support this for INC direction
uint64_t bytes; // It doesn't include Ethernet overhead
BatchHistogram requested_hist;
BatchHistogram actual_hist;
BatchHistogram diff_hist;
};

class Port {
Expand Down
6 changes: 3 additions & 3 deletions protobuf/bess_msg.proto
Original file line number Diff line number Diff line change
Expand Up @@ -321,15 +321,15 @@ message GetPortStatsResponse {

// Histogram of how many times a given number of packets in a batch was
// requested.
repeated uint64 requested_hist = 4;
reserved 4; // repeated uint64 requested_hist = 4;

// Histogram of how many times a given number of packets in a batch were
// actually processed.
repeated uint64 actual_hist = 5;
reserved 5; // repeated uint64 actual_hist = 5;

// Histogram of the difference between the requested batch size and the
// actual number of packets processed in that batch.
repeated uint64 diff_hist = 6;
reserved 6; //repeated uint64 diff_hist = 6;
}
Error error = 1;
Stat inc = 2; /// Port stats for incoming (Ext -> BESS) direction.
Expand Down