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

Support multiple gatehooks of the same type installed per gate #821

Merged
merged 9 commits into from
Aug 1, 2018
Merged

Support multiple gatehooks of the same type installed per gate #821

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
7d6ea04
Gatehooks has its own name than classname
shinae-woo Jul 18, 2018
7396d30
Unifiying class naming: Factory -> Builder
shinae-woo Jul 19, 2018
602a45a
Change Interface to use multiple gate hooks per gate
shinae-woo Jul 20, 2018
22d987a
Remove temporary gatehooks initiated by bessctl
shinae-woo Jul 20, 2018
6fa1d75
Add interface for finding gate hooks by class name
shinae-woo Jul 23, 2018
34bee63
Make enable gatehook for all gates in a module as all or nothing
shinae-woo Jul 25, 2018
a9b91b1
Minor fixes per comments
shinae-woo Jul 25, 2018
ca7c145
Fix gate tests
shinae-woo Jul 25, 2018
7945b52
Apply clang-format-5.0
shinae-woo Jul 30, 2018
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
17 changes: 11 additions & 6 deletions bessctl/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,8 @@ def _show_module(cli, module_name):
(gate.igate, track_str,
', '.join('%s:%d ->' % (g.name, g.ogate)
for g in gate.ogates),
', '.join(gate.hook_name)))
', '.join('%s::%s' % (h.class_name, h.hook_name)
for h in gate.gatehooks)))

if len(info.ogates) > 0:
cli.fout.write(' Output gates:\n')
Expand All @@ -1485,7 +1486,8 @@ def _show_module(cli, module_name):
cli.fout.write(
' %3d: %s -> %d:%s\t%s\n' %
(gate.ogate, track_str, gate.igate, gate.name,
', '.join(gate.hook_name)))
', '.join("%s::%s" % (h.class_name, h.hook_name)
for h in gate.gatehooks)))

if hasattr(info, 'dump'):
dump_str = pprint.pformat(info.dump, width=74)
Expand Down Expand Up @@ -1545,14 +1547,17 @@ def show_gatehook_all(cli):

if not gatehooks:
raise cli.CommandError('There is no active gatehook to show.')

for gatehook in gatehooks:
if gatehook.HasField('igate'):
cli.fout.write('%-16s %d:%s\n' % (gatehook.hook_name, gatehook.igate,
cli.fout.write('%-16s %d:%s\n' % ('%s::%s' % (gatehook.class_name,
gatehook.hook_name),
gatehook.igate,
gatehook.module_name))
else:
cli.fout.write('%-16s %s:%d\n' % (gatehook.hook_name,
gatehook.module_name, gatehook.ogate))
cli.fout.write('%-16s %s:%d\n' % ('%s::%s' % (gatehook.class_name,
gatehook.hook_name),
gatehook.module_name,
gatehook.ogate))


def _show_gatehook_class(cli, cls_name, detail):
Expand Down
26 changes: 17 additions & 9 deletions core/bessctl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ static CommandResponse enable_hook_for_module(
return CommandFailure(EINVAL, "'%s': %cgate '%hu' does not exist",
m->name().c_str(), is_igate ? 'i' : 'o', gate_idx);
}
return gate->NewGateHook(&factory, gate, is_igate, arg);
return gate->NewGateHook(&factory, gate, is_igate, "noname", arg);
}

if (is_igate) {
for (auto& gate : m->igates()) {
if (!gate) {
continue;
}
CommandResponse ret = gate->NewGateHook(&factory, gate, is_igate, arg);
CommandResponse ret = gate->NewGateHook(&factory, gate, is_igate, "noname", arg);
if (ret.error().code() != 0) {
return ret;
}
Expand All @@ -131,7 +131,7 @@ static CommandResponse enable_hook_for_module(
if (!gate) {
continue;
}
CommandResponse ret = gate->NewGateHook(&factory, gate, is_igate, arg);
CommandResponse ret = gate->NewGateHook(&factory, gate, is_igate, "noname", arg);
if (ret.error().code() != 0) {
return ret;
}
Expand Down Expand Up @@ -197,8 +197,11 @@ static int collect_igates(Module* m, GetModuleInfoResponse* response) {
}

for (const auto& hook : g->hooks()) {
igate->add_hook_name(hook->name());
GetModuleInfoResponse_GateHook *hook_info = igate->add_gatehooks();
hook_info->set_class_name(hook->class_name());
hook_info->set_hook_name(hook->name());
}

}

return 0;
Expand All @@ -224,8 +227,11 @@ static int collect_ogates(Module* m, GetModuleInfoResponse* response) {
ogate->set_igate(g->igate()->gate_idx());

for (const auto& hook : g->hooks()) {
ogate->add_hook_name(hook->name());
GetModuleInfoResponse_GateHook *hook_info = ogate->add_gatehooks();
hook_info->set_class_name(hook->class_name());
hook_info->set_hook_name(hook->name());
}

}

return 0;
Expand Down Expand Up @@ -1381,7 +1387,7 @@ class BESSControlImpl final : public BESSControl::Service {

for (const auto& pair : bess::GateHookFactory::all_gate_hook_factories()) {
const auto& factory = pair.second;
response->add_names(factory.hook_name());
response->add_names(factory.class_name());
}
return Status::OK;
}
Expand All @@ -1406,7 +1412,7 @@ class BESSControlImpl final : public BESSControl::Service {
}
const bess::GateHookFactory* cls = &it->second;

response->set_name(cls->hook_name());
response->set_name(cls->class_name());
response->set_help(cls->help_text());
for (const auto& cmd : cls->cmds()) {
response->add_cmds(cmd.first);
Expand All @@ -1427,6 +1433,7 @@ class BESSControlImpl final : public BESSControl::Service {
}
for (auto& hook : gate->hooks()) {
GateHookInfo* info = response->add_hooks();
info->set_class_name(hook->class_name());
info->set_hook_name(hook->name());
info->set_module_name(m->name());
info->set_igate(gate->gate_idx());
Expand All @@ -1439,6 +1446,7 @@ class BESSControlImpl final : public BESSControl::Service {
}
for (auto& hook : gate->hooks()) {
GateHookInfo* info = response->add_hooks();
info->set_class_name(hook->class_name());
info->set_hook_name(hook->name());
info->set_module_name(m->name());
info->set_ogate(gate->gate_idx());
Expand Down Expand Up @@ -1469,10 +1477,10 @@ class BESSControlImpl final : public BESSControl::Service {
}

const auto factory = bess::GateHookFactory::all_gate_hook_factories().find(
request->hook().hook_name());
request->hook().class_name());
if (factory == bess::GateHookFactory::all_gate_hook_factories().end()) {
return return_with_error(response, ENOENT, "No such gate hook: %s",
request->hook().hook_name().c_str());
request->hook().class_name().c_str());
}

if (request->hook().module_name().length() == 0) {
Expand Down
27 changes: 15 additions & 12 deletions core/gate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@ namespace bess {
const GateHookCommands GateHook::cmds;

bool GateHookFactory::RegisterGateHook(GateHook::constructor_t constructor,
const std::string &class_name,
const std::string &name_template,
const std::string &help_text,
const GateHookCommands &cmds,
GateHook::init_func_t init_func,
const std::string &hook_name,
const std::string &help_text) {
GateHook::init_func_t init_func) {
return all_gate_hook_factories_holder()
.emplace(std::piecewise_construct, std::forward_as_tuple(hook_name),
std::forward_as_tuple(constructor, cmds, init_func, hook_name,
help_text))
.emplace(std::piecewise_construct, std::forward_as_tuple(class_name),
std::forward_as_tuple(constructor, class_name, name_template,
help_text, cmds, init_func))
.second;
}

Expand All @@ -77,22 +78,24 @@ const std::map<std::string, GateHookFactory>
// the gatehook instance is destroyed and we return a failed
// CommandResponse, otherwise we return a successful one.
CommandResponse Gate::NewGateHook(const GateHookFactory *factory, Gate *gate,
bool is_igate,
bool is_igate, const std::string &name,
const google::protobuf::Any &arg) {
bess::GateHook *hook = factory->hook_constructor_();
CommandResponse init_ret = factory->hook_init_func_(hook, gate, arg);
if (init_ret.error().code() != 0) {
delete hook;
return init_ret;
}
hook->set_class_name(factory->class_name());
hook->set_name(name);
hook->set_factory(factory);
hook->set_arg(arg);
hook->set_gate(gate);
int ret = gate->AddHook(hook);
if (ret != 0) {
delete hook;
return CommandFailure(ret, "Unable to add hook '%s' to '%s' %cgate '%hu'",
factory->hook_name_.c_str(),
return CommandFailure(ret, "Unable to add hook '%s::%s' to '%s' %cgate '%hu'",
factory->class_name_.c_str(), name.c_str(),
gate->module()->name().c_str(), is_igate ? 'i' : 'o',
gate->gate_idx());
}
Expand Down Expand Up @@ -156,7 +159,7 @@ CommandResponse GateHookFactory::RunCommand(
}

return CommandFailure(ENOTSUP, "'%s' does not support command '%s'",
hook_name_.c_str(), user_cmd.c_str());
class_name_.c_str(), user_cmd.c_str());
}

void Gate::ClearHooks() {
Expand Down Expand Up @@ -193,7 +196,7 @@ void OGate::AddTrackHook() {
// Would like to use CHECK_NE here, but cannot because
// operator<< is not defined on the arguments.
if (it == bess::GateHookFactory::all_gate_hook_factories().end()) {
CHECK(0) << "track gate hook factory is missing";
CHECK(0) << "Track gate hook factory is missing";
}
track_factory = &it->second;

Expand All @@ -202,7 +205,7 @@ void OGate::AddTrackHook() {
track_arg.set_bits(false);
arg.PackFrom(track_arg);
}
this->NewGateHook(track_factory, this, false, arg);
this->NewGateHook(track_factory, this, false, "track", arg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: should this be Track::kName rather than "track"? (If so perhaps we should use that in the CHECK(0) above as well...)

}

void OGate::SetIgate(IGate *ig) {
Expand Down
53 changes: 34 additions & 19 deletions core/gate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,24 @@ class GateHook {
using init_func_t = std::function<CommandResponse(
GateHook *, const Gate *, const google::protobuf::Any &)>;

explicit GateHook(const std::string &name, uint16_t priority = 0,
Gate *gate = nullptr)
: gate_(gate), name_(name), priority_(priority), factory_() {}
explicit GateHook(const std::string &class_name, const std::string &name,
uint16_t priority = 0, Gate *gate = nullptr)
: gate_(gate), class_name_(class_name), name_(name), priority_(priority), factory_() {}

virtual ~GateHook() {}

const std::string &class_name() const { return class_name_; }

const std::string &name() const { return name_; }

const Gate *gate() const { return gate_; }

const google::protobuf::Any &arg() const { return arg_; }

void set_class_name(const std::string& name) { class_name_ = name; }

void set_name(const std::string &name) { name_ = name; }

void set_gate(Gate *gate) { gate_ = gate; }

void set_arg(const google::protobuf::Any &arg) { arg_ = arg; }
Expand All @@ -106,7 +112,8 @@ class GateHook {
Gate *gate_;

private:
const std::string &name_;
std::string class_name_;
std::string name_;
const uint16_t priority_;
const GateHookFactory *factory_;
google::protobuf::Any arg_;
Expand All @@ -118,27 +125,33 @@ class GateHook {
class GateHookFactory {
public:
GateHookFactory(GateHook::constructor_t constructor,
const GateHookCommands &cmds, GateHook::init_func_t init_func,
const std::string &hook_name, const std::string &help_text)
const std::string &class_name,
const std::string &name_template,
const std::string &help_text,
const GateHookCommands &cmds,
GateHook::init_func_t init_func)
: hook_constructor_(constructor),
class_name_(class_name),
name_template_(name_template),
help_text_(help_text),
cmds_(cmds),
hook_init_func_(init_func),
hook_name_(hook_name),
help_text_(help_text){}
hook_init_func_(init_func) {}

static bool RegisterGateHook(GateHook::constructor_t constructor,
const std::string &class_name,
const std::string &name_template,
const std::string &help_text,
const GateHookCommands &cmds,
GateHook::init_func_t init_func,
const std::string &hook_name,
const std::string &help_text);
GateHook::init_func_t init_func);

static std::map<std::string, GateHookFactory> &all_gate_hook_factories_holder(
bool reset = false);

static const std::map<std::string, GateHookFactory>
&all_gate_hook_factories();

const std::string &hook_name() const { return hook_name_; }
const std::string &class_name() const { return class_name_; }
const std::string &name_template() const { return name_template_; }
const std::string &help_text() const { return help_text_; }

const std::vector<std::pair<std::string, std::string>> cmds() const {
Expand All @@ -155,10 +168,11 @@ class GateHookFactory {
friend class Gate;

GateHook::constructor_t hook_constructor_;
std::string class_name_;
std::string name_template_;
std::string help_text_;
const GateHookCommands cmds_;
GateHook::init_func_t hook_init_func_;
std::string hook_name_;
std::string help_text_;
};

inline CommandResponse GateHook::RunCommand(const std::string &cmd,
Expand Down Expand Up @@ -187,7 +201,8 @@ class Gate {

// Creates, initializes, and then inserts gate hook in priority order.
CommandResponse NewGateHook(const GateHookFactory *factory, Gate *gate,
bool is_igate, const google::protobuf::Any &arg);
bool is_gate, const std::string &name,
const google::protobuf::Any &arg);

GateHook *FindHook(const std::string &name);

Expand Down Expand Up @@ -290,10 +305,10 @@ static inline bess::GateHook::init_func_t InitGateHookWithGenericArg(
};
}

#define ADD_GATE_HOOK(_HOOK, _HELP) \
#define ADD_GATE_HOOK(_HOOK, _NAME_TEMPLATE, _HELP) \
bool __gate_hook__##_HOOK = bess::GateHookFactory::RegisterGateHook( \
std::function<bess::GateHook *()>([]() { return new _HOOK(); }), \
_HOOK::cmds, InitGateHookWithGenericArg(&_HOOK::Init), _HOOK::kName, \
_HELP);
_HOOK::kName, _NAME_TEMPLATE, _HELP, _HOOK::cmds, \
InitGateHookWithGenericArg(&_HOOK::Init));

#endif // BESS_GATE_H_
4 changes: 2 additions & 2 deletions core/gate_hooks/pcapng.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ void BytesToHexDump(const void *src, size_t len, char *dst) {
const std::string Pcapng::kName = "PcapNg";

Pcapng::Pcapng()
: bess::GateHook(Pcapng::kName, Pcapng::kPriority),
: bess::GateHook(Pcapng::kName, "pcapng", Pcapng::kPriority),
opener_(),
attrs_(),
attr_template_() {}
Expand Down Expand Up @@ -247,4 +247,4 @@ void Pcapng::ProcessBatch(const bess::PacketBatch *batch) {
}
}

ADD_GATE_HOOK(Pcapng, "metadata-dump-able packet dump")
ADD_GATE_HOOK(Pcapng, "pcapng", "metadata-dump-able packet dump")
2 changes: 1 addition & 1 deletion core/gate_hooks/tcpdump.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,4 @@ void Tcpdump::ProcessBatch(const bess::PacketBatch *batch) {
}
}

ADD_GATE_HOOK(Tcpdump, "dump traffic on a network")
ADD_GATE_HOOK(Tcpdump, "tcpdump", "dump traffic on a network")
3 changes: 2 additions & 1 deletion core/gate_hooks/tcpdump.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ class TcpdumpOpener final : public bess::utils::FifoOpener {
// Tcpdump dumps copies of the packets seen by a gate. Useful for debugging.
class Tcpdump final : public bess::GateHook {
public:
Tcpdump() : bess::GateHook(Tcpdump::kName, Tcpdump::kPriority), opener_() {}
Tcpdump() : bess::GateHook(Tcpdump::kName, "tcpdump",
Tcpdump::kPriority), opener_() {}

virtual ~Tcpdump() {}

Expand Down
4 changes: 2 additions & 2 deletions core/gate_hooks/track.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const GateHookCommands Track::cmds = {{"reset", "EmptyArg",
GateHookCommand::THREAD_UNSAFE}};

Track::Track()
: bess::GateHook(Track::kName, Track::kPriority),
: bess::GateHook(Track::kName, "track", Track::kPriority),
track_bytes_(),
cnt_(),
pkts_(),
Expand Down Expand Up @@ -74,4 +74,4 @@ void Track::ProcessBatch(const bess::PacketBatch *batch) {
}
}

ADD_GATE_HOOK(Track, "count the packets and batches")
ADD_GATE_HOOK(Track, "track", "count the packets and batches")
Loading