You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2015/06/10 20:33:32 UTC

[1/3] mesos git commit: Removed unnecessary accessors in filter code.

Repository: mesos
Updated Branches:
  refs/heads/master 80a94ceea -> 554a1a50d


Removed unnecessary accessors in filter code.

Review: https://reviews.apache.org/r/35273


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/554a1a50
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/554a1a50
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/554a1a50

Branch: refs/heads/master
Commit: 554a1a50d7305d3a07022f907ce7dc8582462605
Parents: 3ff61b5
Author: Jie Yu <yu...@gmail.com>
Authored: Tue Jun 9 15:31:37 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Jun 10 11:33:16 2015 -0700

----------------------------------------------------------------------
 src/linux/routing/filter/action.hpp             | 22 +++-----
 src/linux/routing/filter/basic.cpp              |  2 +-
 src/linux/routing/filter/basic.hpp              | 15 ++----
 src/linux/routing/filter/filter.hpp             | 49 +++++++-----------
 src/linux/routing/filter/icmp.cpp               |  4 +-
 src/linux/routing/filter/icmp.hpp               | 12 ++---
 src/linux/routing/filter/internal.hpp           | 54 ++++++++++----------
 src/linux/routing/filter/ip.cpp                 | 20 ++++----
 src/linux/routing/filter/ip.hpp                 | 37 +++++---------
 .../isolators/network/port_mapping.cpp          | 12 ++---
 src/tests/routing_tests.cpp                     | 38 +++++++-------
 11 files changed, 111 insertions(+), 154 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/action.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/action.hpp b/src/linux/routing/filter/action.hpp
index 410c15a..7b65638 100644
--- a/src/linux/routing/filter/action.hpp
+++ b/src/linux/routing/filter/action.hpp
@@ -40,40 +40,32 @@ protected:
 // Represents an action that redirects a packet to a given link.
 // Currently, kernel only supports redirecting to the egress of a
 // given link.
-class Redirect : public Action
+struct Redirect : public Action
 {
-public:
   explicit Redirect(const std::string& _link)
-    : link_(_link) {}
-
-  const std::string& link() const { return link_; }
+    : link(_link) {}
 
-private:
   // The link to which the packet will be redirected.
-  std::string link_;
+  std::string link;
 };
 
 
 // Represents an action that mirrors a packet to a set of links.
 // Currently, kernel only supports mirroring to the egress of each
 // link.
-class Mirror : public Action
+struct Mirror : public Action
 {
-public:
   explicit Mirror(const std::set<std::string>& _links)
-    : links_(_links) {}
-
-  const std::set<std::string>& links() const { return links_; }
+    : links(_links) {}
 
-private:
   // The set of links to which the packet will be mirrored.
-  std::set<std::string> links_;
+  std::set<std::string> links;
 };
 
 
 // Represents an action that stops the packet from being sent to the
 // next filter.
-class Terminal : public Action {};
+struct Terminal : public Action {};
 
 } // namespace action {
 } // namespace routing {

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/basic.cpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/basic.cpp b/src/linux/routing/filter/basic.cpp
index 937e8fc..a9d604a 100644
--- a/src/linux/routing/filter/basic.cpp
+++ b/src/linux/routing/filter/basic.cpp
@@ -52,7 +52,7 @@ Try<Nothing> encode<basic::Classifier>(
     const Netlink<struct rtnl_cls>& cls,
     const basic::Classifier& classifier)
 {
-  rtnl_cls_set_protocol(cls.get(), classifier.protocol());
+  rtnl_cls_set_protocol(cls.get(), classifier.protocol);
 
   int error = rtnl_tc_set_kind(TC_CAST(cls.get()), "basic");
   if (error != 0) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/basic.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/basic.hpp b/src/linux/routing/filter/basic.hpp
index f1999bf..fea8976 100644
--- a/src/linux/routing/filter/basic.hpp
+++ b/src/linux/routing/filter/basic.hpp
@@ -46,24 +46,17 @@ namespace filter {
 namespace basic {
 
 // The classifier for the basic filter only contains a protocol.
-class Classifier
+struct Classifier
 {
-public:
   explicit Classifier(uint16_t _protocol)
-    : protocol_(_protocol) {}
+    : protocol(_protocol) {}
 
   bool operator == (const Classifier& that) const
   {
-    return protocol_ == that.protocol_;
+    return protocol == that.protocol;
   }
 
-  uint16_t protocol() const
-  {
-    return protocol_;
-  }
-
-private:
-  uint16_t protocol_;
+  uint16_t protocol;
 };
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/filter.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/filter.hpp b/src/linux/routing/filter/filter.hpp
index 7a737e0..bd44268 100644
--- a/src/linux/routing/filter/filter.hpp
+++ b/src/linux/routing/filter/filter.hpp
@@ -45,7 +45,7 @@ namespace filter {
 // associated with a filter. In other words, the list of actions
 // obtained from the filter might not be the complete list.
 template <typename Classifier>
-class Filter
+struct Filter
 {
 public:
   // Creates a filter with no action.
@@ -54,11 +54,11 @@ public:
          const Option<Priority>& _priority,
          const Option<Handle>& _handle,
          const Option<Handle>& _classid)
-    : parent_(_parent),
-      classifier_(_classifier),
-      priority_(_priority),
-      handle_(_handle),
-      classid_(_classid) {}
+    : parent(_parent),
+      classifier(_classifier),
+      priority(_priority),
+      handle(_handle),
+      classid(_classid) {}
 
   // TODO(jieyu): Support arbitrary number of actions.
   template <typename Action>
@@ -68,11 +68,11 @@ public:
          const Option<Handle>& _handle,
          const Option<Handle>& _classid,
          const Action& action)
-    : parent_(_parent),
-      classifier_(_classifier),
-      priority_(_priority),
-      handle_(_handle),
-      classid_(_classid)
+    : parent(_parent),
+      classifier(_classifier),
+      priority(_priority),
+      handle(_handle),
+      classid(_classid)
   {
     attach(action);
   }
@@ -81,34 +81,21 @@ public:
   template <typename A>
   void attach(const A& action)
   {
-    actions_.push_back(process::Shared<action::Action>(new A(action)));
+    actions.push_back(process::Shared<action::Action>(new A(action)));
   }
 
-  const Handle& parent() const { return parent_; }
-  const Classifier& classifier() const { return classifier_; }
-  const Option<Priority>& priority() const { return priority_; }
-  const Option<Handle>& handle() const { return handle_; }
-  const Option<Handle>& classid() const { return classid_; }
-
-  // Returns all the actions attached to this filter.
-  const std::vector<process::Shared<action::Action>>& actions() const
-  {
-    return actions_;
-  }
-
-private:
   // Each filter is attached to a queueing object (either a queueing
   // discipline or a queueing class).
-  Handle parent_;
+  Handle parent;
 
   // The filter specific classifier.
-  Classifier classifier_;
+  Classifier classifier;
 
   // The priority of this filter.
-  Option<Priority> priority_;
+  Option<Priority> priority;
 
   // The handle of this filter.
-  Option<Handle> handle_;
+  Option<Handle> handle;
 
   // The classid of this filter.
   //
@@ -123,11 +110,11 @@ private:
   //
   // Kernel uses classid and flowid interchangeably. However, in our
   // code base, we use classid consistently.
-  Option<Handle> classid_;
+  Option<Handle> classid;
 
   // The set of actions attached to this filer. Note that we use
   // Shared here to make Filter copyable.
-  std::vector<process::Shared<action::Action>> actions_;
+  std::vector<process::Shared<action::Action>> actions;
 };
 
 } // namespace filter {

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/icmp.cpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/icmp.cpp b/src/linux/routing/filter/icmp.cpp
index 12797d7..577577b 100644
--- a/src/linux/routing/filter/icmp.cpp
+++ b/src/linux/routing/filter/icmp.cpp
@@ -100,8 +100,8 @@ Try<Nothing> encode<icmp::Classifier>(
         string(nl_geterror(error)));
   }
 
-  if (classifier.destinationIP().isSome()) {
-    Try<struct in_addr> in = classifier.destinationIP().get().in();
+  if (classifier.destinationIP.isSome()) {
+    Try<struct in_addr> in = classifier.destinationIP.get().in();
     if (in.isError()) {
       return Error("Destination IP is not an IPv4 address");
     }

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/icmp.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/icmp.hpp b/src/linux/routing/filter/icmp.hpp
index 9e167b0..b732193 100644
--- a/src/linux/routing/filter/icmp.hpp
+++ b/src/linux/routing/filter/icmp.hpp
@@ -37,23 +37,19 @@ namespace routing {
 namespace filter {
 namespace icmp {
 
-class Classifier
+struct Classifier
 {
-public:
   explicit Classifier(const Option<net::IP>& _destinationIP)
-    : destinationIP_(_destinationIP) {}
+    : destinationIP(_destinationIP) {}
 
   bool operator == (const Classifier& that) const
   {
-    return destinationIP_ == that.destinationIP_;
+    return destinationIP == that.destinationIP;
   }
 
-  const Option<net::IP>& destinationIP() const { return destinationIP_; }
-
-private:
   // TODO(evelinad): Replace net::IP with net::IPNetwork when we will
   // support classifiers for the entire subnet.
-  Option<net::IP> destinationIP_;
+  Option<net::IP> destinationIP;
 };
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/internal.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/internal.hpp b/src/linux/routing/filter/internal.hpp
index 8a48102..dcc5366 100644
--- a/src/linux/routing/filter/internal.hpp
+++ b/src/linux/routing/filter/internal.hpp
@@ -90,12 +90,12 @@ inline Try<Nothing> attach(
     const action::Redirect& redirect)
 {
   Result<Netlink<struct rtnl_link>> link =
-    link::internal::get(redirect.link());
+    link::internal::get(redirect.link);
 
   if (link.isError()) {
     return Error(link.error());
   } else if (link.isNone()) {
-    return Error("Link '" + redirect.link() + "' is not found");
+    return Error("Link '" + redirect.link + "' is not found");
   }
 
   // TODO(jieyu): Note that currently, we don't use Netlink for 'act'
@@ -158,7 +158,7 @@ inline Try<Nothing> attach(
 {
   const std::string kind = rtnl_tc_get_kind(TC_CAST(cls.get()));
 
-  foreach (const std::string& _link, mirror.links()) {
+  foreach (const std::string& _link, mirror.links) {
     Result<Netlink<struct rtnl_link>> link = link::internal::get(_link);
     if (link.isError()) {
       return Error(link.error());
@@ -283,7 +283,7 @@ Result<U32Handle> generateU32Handle(
   // If the user does not specify a priority, we have no choice but
   // let the kernel choose the handle because we do not know the
   // 'htid' that is associated with that priority.
-  if (filter.priority().isNone()) {
+  if (filter.priority.isNone()) {
     return None();
   }
 
@@ -299,7 +299,7 @@ Result<U32Handle> generateU32Handle(
   int error = rtnl_cls_alloc_cache(
       socket.get().get(),
       rtnl_link_get_ifindex(link.get()),
-      filter.parent().get(),
+      filter.parent.get(),
       &c);
 
   if (error != 0) {
@@ -333,7 +333,7 @@ Result<U32Handle> generateU32Handle(
   // If this filter has a new priority, we need to let the kernel
   // decide the handle because we don't know which 'htid' this
   // priority will be associated with.
-  if (!htids.contains(filter.priority().get().get())) {
+  if (!htids.contains(filter.priority.get().get())) {
     return None();
   }
 
@@ -341,7 +341,7 @@ Result<U32Handle> generateU32Handle(
   // means all filters will be in hash bucket 0. Also, kernel assigns
   // node id starting from 0x800 by default. Here, we keep the same
   // semantics as kernel.
-  uint32_t htid = htids[filter.priority().get().get()];
+  uint32_t htid = htids[filter.priority.get().get()];
   for (uint32_t node = 0x800; node <= 0xfff; node++) {
     if (!nodes[htid].contains(node)) {
       return U32Handle(htid, 0x0, node);
@@ -368,21 +368,21 @@ Try<Netlink<struct rtnl_cls>> encodeFilter(
   Netlink<struct rtnl_cls> cls(c);
 
   rtnl_tc_set_link(TC_CAST(cls.get()), link.get());
-  rtnl_tc_set_parent(TC_CAST(cls.get()), filter.parent().get());
+  rtnl_tc_set_parent(TC_CAST(cls.get()), filter.parent.get());
 
   // Encode the priority.
-  if (filter.priority().isSome()) {
-    rtnl_cls_set_prio(cls.get(), filter.priority().get().get());
+  if (filter.priority.isSome()) {
+    rtnl_cls_set_prio(cls.get(), filter.priority.get().get());
   }
 
   // Encode the classifier using the classifier specific function.
-  Try<Nothing> encoding = encode(cls, filter.classifier());
+  Try<Nothing> encoding = encode(cls, filter.classifier);
   if (encoding.isError()) {
     return Error("Failed to encode the classifier " + encoding.error());
   }
 
   // Attach actions to the libnl filter.
-  foreach (const process::Shared<action::Action>& action, filter.actions()) {
+  foreach (const process::Shared<action::Action>& action, filter.actions) {
     Try<Nothing> attaching = attach(cls, action);
     if (attaching.isError()) {
       return Error("Failed to attach an action " + attaching.error());
@@ -390,8 +390,8 @@ Try<Netlink<struct rtnl_cls>> encodeFilter(
   }
 
   // Encode the handle.
-  if (filter.handle().isSome()) {
-    rtnl_tc_set_handle(TC_CAST(cls.get()), filter.handle().get().get());
+  if (filter.handle.isSome()) {
+    rtnl_tc_set_handle(TC_CAST(cls.get()), filter.handle.get().get());
   } else {
     // NOTE: This is a workaround for MESOS-1617. Normally, if the
     // user does not specify the handle for a filter, the kernel will
@@ -415,11 +415,11 @@ Try<Netlink<struct rtnl_cls>> encodeFilter(
   }
 
   // Set the classid if needed.
-  if (filter.classid().isSome()) {
+  if (filter.classid.isSome()) {
     if (rtnl_tc_get_kind(TC_CAST(cls.get())) == std::string("u32")) {
-      rtnl_u32_set_classid(cls.get(), filter.classid().get().get());
+      rtnl_u32_set_classid(cls.get(), filter.classid.get().get());
     } else if (rtnl_tc_get_kind(TC_CAST(cls.get())) == std::string("basic")) {
-      rtnl_basic_set_target(cls.get(), filter.classid().get().get());
+      rtnl_basic_set_target(cls.get(), filter.classid.get().get());
     }
   }
 
@@ -551,7 +551,7 @@ Result<Netlink<struct rtnl_cls>> getCls(
     Result<Filter<Classifier>> filter = decodeFilter<Classifier>(cls);
     if (filter.isError()) {
       return Error("Failed to decode: " + filter.error());
-    } else if (filter.isSome() && filter.get().classifier() == classifier) {
+    } else if (filter.isSome() && filter.get().classifier == classifier) {
       return cls;
     }
   }
@@ -600,7 +600,7 @@ Try<bool> create(const std::string& _link, const Filter<Classifier>& filter)
   // between the existence check and the following add operation. So
   // if two threads try to create the same filter, both of them may
   // succeed and end up with two filters in the kernel.
-  Try<bool> _exists = exists(_link, filter.parent(), filter.classifier());
+  Try<bool> _exists = exists(_link, filter.parent, filter.classifier);
   if (_exists.isError()) {
     return Error("Check filter existence failed: " + _exists.error());
   } else if (_exists.get()) {
@@ -700,7 +700,7 @@ Try<bool> update(const std::string& _link, const Filter<Classifier>& filter)
 
   // Get the old libnl classifier (to-be-updated) from kernel.
   Result<Netlink<struct rtnl_cls>> oldCls =
-    getCls(link.get(), filter.parent(), filter.classifier());
+    getCls(link.get(), filter.parent, filter.classifier);
 
   if (oldCls.isError()) {
     return Error(oldCls.error());
@@ -710,25 +710,25 @@ Try<bool> update(const std::string& _link, const Filter<Classifier>& filter)
 
   // The kernel does not allow us to update the priority. So if the
   // user specifies a priority, we will check to make sure they match.
-  if (filter.priority().isSome() &&
-      filter.priority().get().get() != rtnl_cls_get_prio(oldCls.get().get())) {
+  if (filter.priority.isSome() &&
+      filter.priority.get().get() != rtnl_cls_get_prio(oldCls.get().get())) {
     return Error(
         "The priorities do not match. The old priority is " +
         stringify(rtnl_cls_get_prio(oldCls.get().get())) +
         " and the new priority is " +
-        stringify(filter.priority().get().get()));
+        stringify(filter.priority.get().get()));
   }
 
   // The kernel does not allow us to update the handle. So if the user
   // specifies a handle, we will check to make sure they match.
-  if (filter.handle().isSome() &&
-      filter.handle().get().get() !=
+  if (filter.handle.isSome() &&
+      filter.handle.get().get() !=
         rtnl_tc_get_handle(TC_CAST(oldCls.get().get()))) {
     return Error(
         "The handles do not match. The old handle is " +
         stringify(rtnl_tc_get_handle(TC_CAST(oldCls.get().get()))) +
         " and the new handle is " +
-        stringify(filter.handle().get().get()));
+        stringify(filter.handle.get().get()));
   }
 
   Try<Netlink<struct rtnl_cls>> newCls = encodeFilter(link.get(), filter);
@@ -824,7 +824,7 @@ Result<std::vector<Classifier>> classifiers(
   std::vector<Classifier> results;
 
   foreach (const Filter<Classifier>& filter, _filters.get()) {
-    results.push_back(filter.classifier());
+    results.push_back(filter.classifier);
   }
 
   return results;

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/ip.cpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/ip.cpp b/src/linux/routing/filter/ip.cpp
index ddce93f..03d8df6 100644
--- a/src/linux/routing/filter/ip.cpp
+++ b/src/linux/routing/filter/ip.cpp
@@ -94,13 +94,13 @@ Try<Nothing> encode<ip::Classifier>(
         string(nl_geterror(error)));
   }
 
-  if (classifier.destinationMAC().isSome()) {
+  if (classifier.destinationMAC.isSome()) {
     // Since we set the protocol of this classifier to be ETH_P_IP
     // above, all IP packets that contain 802.1Q tag (i.e., VLAN tag)
     // will not match this classifier (those packets have protocol
     // ETH_P_8021Q). Therefore, the offset of the start of the MAC
     // destination address is at -14 (0xfffffff2).
-    net::MAC mac = classifier.destinationMAC().get();
+    net::MAC mac = classifier.destinationMAC.get();
 
     // To avoid confusion, we only use u32 selectors which are used to
     // match arbitrary 32-bit content in a packet.
@@ -154,8 +154,8 @@ Try<Nothing> encode<ip::Classifier>(
     }
   }
 
-  if (classifier.destinationIP().isSome()) {
-    Try<struct in_addr> in = classifier.destinationIP().get().in();
+  if (classifier.destinationIP.isSome()) {
+    Try<struct in_addr> in = classifier.destinationIP.get().in();
     if (in.isError()) {
       return Error(in.error());
     }
@@ -180,14 +180,14 @@ Try<Nothing> encode<ip::Classifier>(
   // source port and the destination ports to be 20 and 22
   // respectively. Users can choose to add a high priority filter to
   // filter all the IP packets that have IP options.
-  if (classifier.sourcePorts().isSome()) {
+  if (classifier.sourcePorts.isSome()) {
     // Format of an IP packet at offset 20:
     //        +--------+--------+--------+--------+
     //        |   Source Port   |   X    |   X    |
     //        +--------+--------+--------+--------+
     // Offset:    20       21       22       23
-    uint32_t value = ((uint32_t) classifier.sourcePorts().get().begin()) << 16;
-    uint32_t mask = ((uint32_t) classifier.sourcePorts().get().mask()) << 16;
+    uint32_t value = ((uint32_t) classifier.sourcePorts.get().begin()) << 16;
+    uint32_t mask = ((uint32_t) classifier.sourcePorts.get().mask()) << 16;
 
     // To match IP packets that have the given source ports.
     error = rtnl_u32_add_key(
@@ -204,14 +204,14 @@ Try<Nothing> encode<ip::Classifier>(
     }
   }
 
-  if (classifier.destinationPorts().isSome()) {
+  if (classifier.destinationPorts.isSome()) {
     // Format of an IP packet at offset 20:
     //        +--------+--------+--------+--------+
     //        |   X    |   X    |    Dest. Port   |
     //        +--------+--------+--------+--------+
     // Offset:    20       21       22       23
-    uint32_t value = (uint32_t) classifier.destinationPorts().get().begin();
-    uint32_t mask = (uint32_t) classifier.destinationPorts().get().mask();
+    uint32_t value = (uint32_t) classifier.destinationPorts.get().begin();
+    uint32_t mask = (uint32_t) classifier.destinationPorts.get().mask();
 
     // To match IP packets that have the given destination ports.
     error = rtnl_u32_add_key(

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/linux/routing/filter/ip.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/ip.hpp b/src/linux/routing/filter/ip.hpp
index b8f5204..ec6f643 100644
--- a/src/linux/routing/filter/ip.hpp
+++ b/src/linux/routing/filter/ip.hpp
@@ -101,45 +101,34 @@ inline size_t hash_value(const PortRange& range)
 }
 
 
-class Classifier
+struct Classifier
 {
-public:
   Classifier(
     const Option<net::MAC>& _destinationMAC,
     const Option<net::IP>& _destinationIP,
     const Option<PortRange>& _sourcePorts,
     const Option<PortRange>& _destinationPorts)
-    : destinationMAC_(_destinationMAC),
-      destinationIP_(_destinationIP),
-      sourcePorts_(_sourcePorts),
-      destinationPorts_(_destinationPorts) {}
+    : destinationMAC(_destinationMAC),
+      destinationIP(_destinationIP),
+      sourcePorts(_sourcePorts),
+      destinationPorts(_destinationPorts) {}
 
   bool operator == (const Classifier& that) const
   {
-    return (destinationMAC_ == that.destinationMAC_ &&
-        destinationIP_ == that.destinationIP_ &&
-        destinationPorts_ == that.destinationPorts_ &&
-        sourcePorts_ == that.sourcePorts_);
-  }
-
-  const Option<net::MAC>& destinationMAC() const { return destinationMAC_; }
-  const Option<net::IP>& destinationIP() const { return destinationIP_; }
-  const Option<PortRange>& sourcePorts() const { return sourcePorts_; }
-
-  const Option<PortRange>& destinationPorts() const
-  {
-    return destinationPorts_;
+    return (destinationMAC == that.destinationMAC &&
+        destinationIP == that.destinationIP &&
+        destinationPorts == that.destinationPorts &&
+        sourcePorts == that.sourcePorts);
   }
 
-private:
-  Option<net::MAC> destinationMAC_;
+  Option<net::MAC> destinationMAC;
 
   // TODO(evelinad): Replace net::IP with net::IPNetwork when we will
   // support classifiers for the entire subnet.
-  Option<net::IP> destinationIP_;
+  Option<net::IP> destinationIP;
 
-  Option<PortRange> sourcePorts_;
-  Option<PortRange> destinationPorts_;
+  Option<PortRange> sourcePorts;
+  Option<PortRange> destinationPorts;
 };
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/slave/containerizer/isolators/network/port_mapping.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/network/port_mapping.cpp b/src/slave/containerizer/isolators/network/port_mapping.cpp
index d2da1a4..432b05c 100644
--- a/src/slave/containerizer/isolators/network/port_mapping.cpp
+++ b/src/slave/containerizer/isolators/network/port_mapping.cpp
@@ -1864,8 +1864,8 @@ PortMappingIsolatorProcess::_recover(pid_t pid)
     // egress. This map will be used later.
     foreach (const filter::Filter<ip::Classifier>& filter,
              eth0EgressFilters.get()) {
-      const Option<PortRange> sourcePorts = filter.classifier().sourcePorts();
-      const Option<Handle> classid = filter.classid();
+      const Option<PortRange> sourcePorts = filter.classifier.sourcePorts;
+      const Option<Handle> classid = filter.classid;
 
       if (sourcePorts.isNone()) {
         return Error("Missing source ports for filters on egress of " + eth0);
@@ -1890,8 +1890,8 @@ PortMappingIsolatorProcess::_recover(pid_t pid)
   Option<uint16_t> flowId;
 
   foreach (const ip::Classifier& classifier, vethIngressClassifiers.get()) {
-    const Option<PortRange> sourcePorts = classifier.sourcePorts();
-    const Option<PortRange> destinationPorts = classifier.destinationPorts();
+    const Option<PortRange> sourcePorts = classifier.sourcePorts;
+    const Option<PortRange> destinationPorts = classifier.destinationPorts;
 
     // All the IP filters on veth used by us only have source ports.
     if (sourcePorts.isNone() || destinationPorts.isSome()) {
@@ -2507,8 +2507,8 @@ Future<Nothing> PortMappingIsolatorProcess::update(
   IntervalSet<uint16_t> remaining = info->nonEphemeralPorts;
 
   foreach (const ip::Classifier& classifier, classifiers.get()) {
-    Option<PortRange> sourcePorts = classifier.sourcePorts();
-    Option<PortRange> destinationPorts = classifier.destinationPorts();
+    Option<PortRange> sourcePorts = classifier.sourcePorts;
+    Option<PortRange> destinationPorts = classifier.destinationPorts;
 
     // All the IP filters on veth used by us only have source ports.
     if (sourcePorts.isNone() || destinationPorts.isSome()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/554a1a50/src/tests/routing_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/routing_tests.cpp b/src/tests/routing_tests.cpp
index 463c2e5..e4f1bcf 100644
--- a/src/tests/routing_tests.cpp
+++ b/src/tests/routing_tests.cpp
@@ -865,7 +865,7 @@ TEST_F(RoutingVethTest, ROOT_ICMPFilterCreate)
 
   ASSERT_SOME(classifiers);
   ASSERT_EQ(1u, classifiers.get().size());
-  EXPECT_SOME_EQ(ip, classifiers.get().front().destinationIP());
+  EXPECT_SOME_EQ(ip, classifiers.get().front().destinationIP);
 }
 
 
@@ -933,8 +933,8 @@ TEST_F(RoutingVethTest, ROOT_ICMPFilterCreateMultiple)
 
   ASSERT_SOME(classifiers);
   ASSERT_EQ(2u, classifiers.get().size());
-  EXPECT_SOME_EQ(ip1, classifiers.get().front().destinationIP());
-  EXPECT_SOME_EQ(ip2, classifiers.get().back().destinationIP());
+  EXPECT_SOME_EQ(ip1, classifiers.get().front().destinationIP);
+  EXPECT_SOME_EQ(ip2, classifiers.get().back().destinationIP);
 }
 
 
@@ -1073,16 +1073,16 @@ TEST_F(RoutingVethTest, ROOT_IPFilterCreate)
 
   ASSERT_SOME(classifiers);
   ASSERT_EQ(1u, classifiers.get().size());
-  EXPECT_SOME_EQ(mac.get(), classifiers.get().front().destinationMAC());
-  EXPECT_SOME_EQ(ip, classifiers.get().front().destinationIP());
+  EXPECT_SOME_EQ(mac.get(), classifiers.get().front().destinationMAC);
+  EXPECT_SOME_EQ(ip, classifiers.get().front().destinationIP);
 
   EXPECT_SOME_EQ(
       sourcePorts.get(),
-      classifiers.get().front().sourcePorts());
+      classifiers.get().front().sourcePorts);
 
   EXPECT_SOME_EQ(
       destinationPorts.get(),
-      classifiers.get().front().destinationPorts());
+      classifiers.get().front().destinationPorts);
 }
 
 
@@ -1114,10 +1114,10 @@ TEST_F(RoutingVethTest, ROOT_IPFilterCreate2)
 
   ASSERT_SOME(classifiers);
   ASSERT_EQ(1u, classifiers.get().size());
-  EXPECT_NONE(classifiers.get().front().destinationMAC());
-  EXPECT_SOME_EQ(ip, classifiers.get().front().destinationIP());
-  EXPECT_NONE(classifiers.get().front().sourcePorts());
-  EXPECT_NONE(classifiers.get().front().destinationPorts());
+  EXPECT_NONE(classifiers.get().front().destinationMAC);
+  EXPECT_SOME_EQ(ip, classifiers.get().front().destinationIP);
+  EXPECT_NONE(classifiers.get().front().sourcePorts);
+  EXPECT_NONE(classifiers.get().front().destinationPorts);
 }
 
 
@@ -1238,27 +1238,27 @@ TEST_F(RoutingVethTest, ROOT_IPFilterCreateMultiple)
   ASSERT_SOME(classifiers);
   ASSERT_EQ(2u, classifiers.get().size());
 
-  EXPECT_SOME_EQ(mac.get(), classifiers.get().front().destinationMAC());
-  EXPECT_SOME_EQ(ip, classifiers.get().front().destinationIP());
+  EXPECT_SOME_EQ(mac.get(), classifiers.get().front().destinationMAC);
+  EXPECT_SOME_EQ(ip, classifiers.get().front().destinationIP);
 
   EXPECT_SOME_EQ(
       sourcePorts1.get(),
-      classifiers.get().front().sourcePorts());
+      classifiers.get().front().sourcePorts);
 
   EXPECT_SOME_EQ(
       destinationPorts1.get(),
-      classifiers.get().front().destinationPorts());
+      classifiers.get().front().destinationPorts);
 
-  EXPECT_SOME_EQ(mac.get(), classifiers.get().back().destinationMAC());
-  EXPECT_SOME_EQ(ip, classifiers.get().back().destinationIP());
+  EXPECT_SOME_EQ(mac.get(), classifiers.get().back().destinationMAC);
+  EXPECT_SOME_EQ(ip, classifiers.get().back().destinationIP);
 
   EXPECT_SOME_EQ(
       sourcePorts2.get(),
-      classifiers.get().back().sourcePorts());
+      classifiers.get().back().sourcePorts);
 
   EXPECT_SOME_EQ(
       destinationPorts2.get(),
-      classifiers.get().back().destinationPorts());
+      classifiers.get().back().destinationPorts);
 }
 
 


[2/3] mesos git commit: Removed unnecessary accessors in route code.

Posted by ji...@apache.org.
Removed unnecessary accessors in route code.

Review: https://reviews.apache.org/r/35272


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3ff61b57
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3ff61b57
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3ff61b57

Branch: refs/heads/master
Commit: 3ff61b572b50db0b76b75367041eec77244d1202
Parents: c319aea
Author: Jie Yu <yu...@gmail.com>
Authored: Tue Jun 9 15:18:57 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Jun 10 11:33:16 2015 -0700

----------------------------------------------------------------------
 src/linux/routing/link/link.cpp | 10 +++++-----
 src/linux/routing/route.cpp     |  4 ++--
 src/linux/routing/route.hpp     | 20 +++++++-------------
 3 files changed, 14 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/3ff61b57/src/linux/routing/link/link.cpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/link/link.cpp b/src/linux/routing/link/link.cpp
index 6606e4a..552e2d7 100644
--- a/src/linux/routing/link/link.cpp
+++ b/src/linux/routing/link/link.cpp
@@ -72,18 +72,18 @@ Result<string> eth0()
   }
 
   foreach (const route::Rule& rule, mainRoutingTable.get()) {
-    if (rule.destination().isNone()) {
+    if (rule.destination.isNone()) {
       // Check if the public interface exists.
-      Try<bool> exists = link::exists(rule.link());
+      Try<bool> exists = link::exists(rule.link);
       if (exists.isError()) {
         return Error(
-            "Failed to check if " + rule.link() + " exists: " + exists.error());
+            "Failed to check if " + rule.link + " exists: " + exists.error());
       } else if (!exists.get()) {
         return Error(
-            rule.link() + " is in the routing table but not in the system");
+            rule.link + " is in the routing table but not in the system");
       }
 
-      return rule.link();
+      return rule.link;
     }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/3ff61b57/src/linux/routing/route.cpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/route.cpp b/src/linux/routing/route.cpp
index 87f2ed1..befba6c 100644
--- a/src/linux/routing/route.cpp
+++ b/src/linux/routing/route.cpp
@@ -128,8 +128,8 @@ Result<net::IP> defaultGateway()
   }
 
   foreach (const Rule& rule, rules.get()) {
-    if (rule.destination().isNone() && rule.gateway().isSome()) {
-      return rule.gateway().get();
+    if (rule.destination.isNone() && rule.gateway.isSome()) {
+      return rule.gateway.get();
     }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/3ff61b57/src/linux/routing/route.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/route.hpp b/src/linux/routing/route.hpp
index 933c22c..1cdb9a4 100644
--- a/src/linux/routing/route.hpp
+++ b/src/linux/routing/route.hpp
@@ -31,24 +31,18 @@ namespace routing {
 namespace route {
 
 // Represents a rule in the routing table (for IPv4).
-class Rule
+struct Rule
 {
-public:
   Rule(const Option<net::IPNetwork>& _destination,
        const Option<net::IP>& _gateway,
        const std::string& _link)
-    : destination_(_destination),
-      gateway_(_gateway),
-      link_(_link) {}
+    : destination(_destination),
+      gateway(_gateway),
+      link(_link) {}
 
-  const Option<net::IPNetwork>& destination() const { return destination_; }
-  const Option<net::IP>& gateway() const { return gateway_; }
-  const std::string& link() const { return link_; }
-
-private:
-  Option<net::IPNetwork> destination_;
-  Option<net::IP> gateway_;
-  std::string link_;
+  Option<net::IPNetwork> destination;
+  Option<net::IP> gateway;
+  std::string link;
 };
 
 


[3/3] mesos git commit: Removed unnecessary accessors in qdisc code.

Posted by ji...@apache.org.
Removed unnecessary accessors in qdisc code.

Review: https://reviews.apache.org/r/35271


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c319aea4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c319aea4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c319aea4

Branch: refs/heads/master
Commit: c319aea42d87c719d9d94fc8912e8087f966889e
Parents: 80a94ce
Author: Jie Yu <yu...@gmail.com>
Authored: Tue Jun 9 15:06:07 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Jun 10 11:33:16 2015 -0700

----------------------------------------------------------------------
 src/linux/routing/queueing/discipline.hpp | 25 +++++++++----------------
 src/linux/routing/queueing/internal.hpp   | 10 +++++-----
 2 files changed, 14 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/c319aea4/src/linux/routing/queueing/discipline.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/queueing/discipline.hpp b/src/linux/routing/queueing/discipline.hpp
index c154eb8..064fca5 100644
--- a/src/linux/routing/queueing/discipline.hpp
+++ b/src/linux/routing/queueing/discipline.hpp
@@ -29,31 +29,24 @@ namespace routing {
 namespace queueing {
 
 template <typename Config>
-class Discipline
+struct Discipline
 {
-public:
   Discipline(
       const std::string& _kind,
       const Handle& _parent,
       const Option<Handle>& _handle,
       const Config& _config)
-    : kind_(_kind),
-      parent_(_parent),
-      handle_(_handle),
-      config_(_config) {}
+    : kind(_kind),
+      parent(_parent),
+      handle(_handle),
+      config(_config) {}
 
-  const std::string& kind() const { return kind_; }
-  const Handle& parent() const { return parent_; }
-  const Option<Handle>& handle() const { return handle_; }
-  const Config& config() const { return config_; }
-
-private:
-  std::string kind_;
-  Handle parent_;
-  Option<Handle> handle_;
+  std::string kind;
+  Handle parent;
+  Option<Handle> handle;
 
   // TODO(jieyu): Consider making it optional.
-  Config config_;
+  Config config;
 };
 
 } // namespace queueing {

http://git-wip-us.apache.org/repos/asf/mesos/blob/c319aea4/src/linux/routing/queueing/internal.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/queueing/internal.hpp b/src/linux/routing/queueing/internal.hpp
index 49d8166..9143c2a 100644
--- a/src/linux/routing/queueing/internal.hpp
+++ b/src/linux/routing/queueing/internal.hpp
@@ -88,13 +88,13 @@ Try<Netlink<struct rtnl_qdisc>> encodeDiscipline(
   Netlink<struct rtnl_qdisc> qdisc(q);
 
   rtnl_tc_set_link(TC_CAST(qdisc.get()), link.get());
-  rtnl_tc_set_parent(TC_CAST(qdisc.get()), discipline.parent().get());
+  rtnl_tc_set_parent(TC_CAST(qdisc.get()), discipline.parent.get());
 
-  if (discipline.handle().isSome()) {
-    rtnl_tc_set_handle(TC_CAST(qdisc.get()), discipline.handle().get().get());
+  if (discipline.handle.isSome()) {
+    rtnl_tc_set_handle(TC_CAST(qdisc.get()), discipline.handle.get().get());
   }
 
-  int error = rtnl_tc_set_kind(TC_CAST(qdisc.get()), discipline.kind().c_str());
+  int error = rtnl_tc_set_kind(TC_CAST(qdisc.get()), discipline.kind.c_str());
   if (error != 0) {
     return Error(
         "Failed to set the kind of the queueing discipline: " +
@@ -102,7 +102,7 @@ Try<Netlink<struct rtnl_qdisc>> encodeDiscipline(
   }
 
   // Perform queue discipline specific encoding.
-  Try<Nothing> encoding = encode(qdisc, discipline.config());
+  Try<Nothing> encoding = encode(qdisc, discipline.config);
   if (encoding.isError()) {
     return Error(
         "Failed to encode the queueing discipline: " +