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

network: log traffic and add a simple traffic analysis script #6170

Open
wants to merge 1 commit into
base: master
from

Conversation

@moneromooo-monero
Copy link
Contributor

moneromooo-monero commented Nov 22, 2019

No description provided.

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:traffic branch from 21f310f to 29ee31a Dec 2, 2019
@moneromooo-monero

This comment has been minimized.

Copy link
Contributor Author

moneromooo-monero commented Dec 2, 2019

Tests build fixed.

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:traffic branch 2 times, most recently from cd58dc2 to 233cfa9 Dec 3, 2019
Copy link
Contributor

vtnerd left a comment

Is this abusing our log a bit? With lots of connection plenty of stuff is being dumped to the log.

return true;
});

for (const boost::uuids::uuid& connection : connections)
zone_->p2p->send(message_.clone(), connection);
std::string category = "command-" + std::to_string(get_command_from_message(message_));

This comment has been minimized.

Copy link
@vtnerd

vtnerd Jan 2, 2020

Contributor

The result of get_command_from_message can be given directly to on_levin_traffic now.

template<typename context_t>
void on_levin_traffic(const context_t &context, bool initiator, bool sent, bool error, size_t bytes, int command)
{
return on_levin_traffic(context, initiator, sent, error, bytes, ("command-" + std::to_string(command)).c_str());

This comment has been minimized.

Copy link
@vtnerd

vtnerd Jan 2, 2020

Contributor

I think its preferable to not allocate a std::string here for this - keep cycles to a minimum with optional debugging stuff.

The levin protocol uses a std::uint32_t value command. So a negative command value is illegal. And noise uses "0", which is currently a unique value (I chose it for that reason). So negative values for invalid commands and zero for noise could be used to drop the const char* overload entirely.

template<typename context_t>
void on_levin_traffic(const context_t &context, bool initiator, bool sent, bool error, size_t bytes, const char *category)
{
MCINFO("net.p2p.traffic", context << bytes << " bytes " << (sent ? "sent" : "received") << (error ? "/corrupt" : "")

This comment has been minimized.

Copy link
@vtnerd

vtnerd Jan 2, 2020

Contributor

I meant to comment on this too - this level seems too low. We're writing to the log constantly just for bandwidth tracking stuff. If its that important, having the information tracked internally and then dumped periodically (either to log or elsewhere) should be a lot faster.

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jan 3, 2020

Author Contributor

It is not on by default.

std::string category = "command-" + std::to_string(get_command_from_message(message_));
for (const detail::p2p_context &context : connections)
{
on_levin_traffic(context, true, true, false, message_.size(), category.c_str());

This comment has been minimized.

Copy link
@vtnerd

vtnerd Jan 3, 2020

Contributor

Also, this should be moved into the foreach_connection above. Copying the entire context is going to be expensive, especially after the Dandelion++ fluff changes. There isn't a way to implement that without having a per-connection tx buffer.

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:traffic branch from 233cfa9 to 0e2d6cf Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.