This is an automated email from the ASF dual-hosted git repository.
jieyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new d458311 Fixed rtnl_act leak in routing::filter::internal::attach().
d458311 is described below
commit d4583113dd921515323c71e296046fc61e6ffe14
Author: Ilya Pronin <[email protected]>
AuthorDate: Wed Sep 19 12:05:00 2018 -0700
Fixed rtnl_act leak in routing::filter::internal::attach().
We should free the action once we've attached it to a filter, because
rtnl_basic_add_action() and rtnl_u32_add_action() increase the
refcounter of rtnl_act object upon adding it to the filter's list.
Reference grabbing was added to libnl along with a corresponding
capability NL_CAPABILITY_ROUTE_LINK_CLS_ADD_ACT_OWN_REFERENCE. Its
support is already checked by Mesos in routing::check(). It seems that
we simply forgot to adapt our code.
Review: https://reviews.apache.org/r/68769/
---
src/linux/routing/filter/internal.hpp | 53 +++++++++++++++--------------------
1 file changed, 23 insertions(+), 30 deletions(-)
diff --git a/src/linux/routing/filter/internal.hpp
b/src/linux/routing/filter/internal.hpp
index 4be4797..e62d50b 100644
--- a/src/linux/routing/filter/internal.hpp
+++ b/src/linux/routing/filter/internal.hpp
@@ -66,6 +66,13 @@ inline void cleanup(struct rtnl_cls* cls)
rtnl_cls_put(cls);
}
+
+template <>
+inline void cleanup(struct rtnl_act* act)
+{
+ rtnl_act_put(act);
+}
+
namespace filter {
namespace internal {
@@ -103,39 +110,33 @@ inline Try<Nothing> attach(
return Error("Link '" + redirect.link + "' is not found");
}
- // TODO(jieyu): Note that currently, we don't use Netlink for 'act'
- // because libnl has a refcount issue for rtnl_act. Clean this up
- // once the bug is fixed in libnl.
- struct rtnl_act* act = rtnl_act_alloc();
- if (act == nullptr) {
+ Netlink<struct rtnl_act> act(rtnl_act_alloc());
+ if (act.get() == nullptr) {
return Error("Failed to allocate a libnl action (rtnl_act)");
}
// Set the kind of the action to 'mirred'. The kind 'mirred' stands
// for mirror or redirect actions.
- int error = rtnl_tc_set_kind(TC_CAST(act), "mirred");
+ int error = rtnl_tc_set_kind(TC_CAST(act.get()), "mirred");
if (error != 0) {
- rtnl_act_put(act);
return Error(
"Failed to set the kind of the action: " +
std::string(nl_geterror(error)));
}
- rtnl_mirred_set_ifindex(act, rtnl_link_get_ifindex(link->get()));
- rtnl_mirred_set_action(act, TCA_EGRESS_REDIR);
- rtnl_mirred_set_policy(act, TC_ACT_STOLEN);
+ rtnl_mirred_set_ifindex(act.get(), rtnl_link_get_ifindex(link->get()));
+ rtnl_mirred_set_action(act.get(), TCA_EGRESS_REDIR);
+ rtnl_mirred_set_policy(act.get(), TC_ACT_STOLEN);
const std::string kind = rtnl_tc_get_kind(TC_CAST(cls.get()));
if (kind == "basic") {
- error = rtnl_basic_add_action(cls.get(), act);
+ error = rtnl_basic_add_action(cls.get(), act.get());
if (error != 0) {
- rtnl_act_put(act);
return Error(std::string(nl_geterror(error)));
}
} else if (kind == "u32") {
- error = rtnl_u32_add_action(cls.get(), act);
+ error = rtnl_u32_add_action(cls.get(), act.get());
if (error != 0) {
- rtnl_act_put(act);
return Error(std::string(nl_geterror(error)));
}
@@ -148,7 +149,6 @@ inline Try<Nothing> attach(
std::string(nl_geterror(error)));
}
} else {
- rtnl_act_put(act);
return Error("Unsupported classifier kind: " + kind);
}
@@ -171,40 +171,33 @@ inline Try<Nothing> attach(
return Error("Link '" + _link + "' is not found");
}
- // TODO(jieyu): Note that currently, we don't use Netlink for
- // 'act' because libnl has a refcount issue for rtnl_act. Clean
- // this up once libnl fixes the bug.
- struct rtnl_act* act = rtnl_act_alloc();
- if (act == nullptr) {
+ Netlink<struct rtnl_act> act(rtnl_act_alloc());
+ if (act.get() == nullptr) {
return Error("Failed to allocate a libnl action (rtnl_act)");
}
- int error = rtnl_tc_set_kind(TC_CAST(act), "mirred");
+ int error = rtnl_tc_set_kind(TC_CAST(act.get()), "mirred");
if (error != 0) {
- rtnl_act_put(act);
return Error(
"Failed to set the kind of the action: " +
std::string(nl_geterror(error)));
}
- rtnl_mirred_set_ifindex(act, rtnl_link_get_ifindex(link->get()));
- rtnl_mirred_set_action(act, TCA_EGRESS_MIRROR);
- rtnl_mirred_set_policy(act, TC_ACT_PIPE);
+ rtnl_mirred_set_ifindex(act.get(), rtnl_link_get_ifindex(link->get()));
+ rtnl_mirred_set_action(act.get(), TCA_EGRESS_MIRROR);
+ rtnl_mirred_set_policy(act.get(), TC_ACT_PIPE);
if (kind == "basic") {
- error = rtnl_basic_add_action(cls.get(), act);
+ error = rtnl_basic_add_action(cls.get(), act.get());
if (error != 0) {
- rtnl_act_put(act);
return Error(std::string(nl_geterror(error)));
}
} else if (kind == "u32") {
- error = rtnl_u32_add_action(cls.get(), act);
+ error = rtnl_u32_add_action(cls.get(), act.get());
if (error != 0) {
- rtnl_act_put(act);
return Error(std::string(nl_geterror(error)));
}
} else {
- rtnl_act_put(act);
return Error("Unsupported classifier kind: " + kind);
}
}
