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/05/04 23:01:34 UTC

mesos git commit: Added classid to the Filter abstraction in routing library.

Repository: mesos
Updated Branches:
  refs/heads/master 16a2e7317 -> ed8253f40


Added classid to the Filter abstraction in routing library.

Currently we don't support to read actions, so intead of using an
action, we add the classid to the generic Filter class.

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


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

Branch: refs/heads/master
Commit: ed8253f406ae74d5f0339ef60ad3c62dbf1b4608
Parents: 16a2e73
Author: Cong Wang <cw...@twopensource.com>
Authored: Wed Mar 11 18:31:18 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon May 4 12:26:46 2015 -0700

----------------------------------------------------------------------
 src/linux/routing/filter/arp.cpp      | 17 ++++++++
 src/linux/routing/filter/arp.hpp      | 12 ++++++
 src/linux/routing/filter/filter.hpp   | 28 +++++++++++++
 src/linux/routing/filter/icmp.cpp     | 18 +++++++++
 src/linux/routing/filter/icmp.hpp     | 14 +++++++
 src/linux/routing/filter/internal.hpp | 25 +++++++++++-
 src/linux/routing/filter/ip.cpp       | 16 ++++++++
 src/linux/routing/filter/ip.hpp       | 12 ++++++
 src/linux/routing/queueing/handle.hpp | 10 +++++
 src/tests/routing_tests.cpp           | 65 ++++++++++++++++++++++++++++++
 10 files changed, 216 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/filter/arp.cpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/arp.cpp b/src/linux/routing/filter/arp.cpp
index bf19264..8c49766 100644
--- a/src/linux/routing/filter/arp.cpp
+++ b/src/linux/routing/filter/arp.cpp
@@ -146,6 +146,23 @@ Try<bool> create(
 }
 
 
+Try<bool> create(
+    const string& link,
+    const queueing::Handle& parent,
+    const Option<Priority>& priority,
+    const Option<queueing::Handle>& classid)
+{
+  return internal::create(
+      link,
+      Filter<Classifier>(
+          parent,
+          Classifier(),
+          priority,
+          None(),
+          classid));
+}
+
+
 Try<bool> remove(const string& link, const queueing::Handle& parent)
 {
   return internal::remove(link, parent, Classifier());

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/filter/arp.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/arp.hpp b/src/linux/routing/filter/arp.hpp
index fa0ea6f..819e814 100644
--- a/src/linux/routing/filter/arp.hpp
+++ b/src/linux/routing/filter/arp.hpp
@@ -63,6 +63,18 @@ Try<bool> create(
     const action::Mirror& mirror);
 
 
+// Creates an ARP packet filter attached to the given parent on the
+// link which will set the classid for packets. Returns false if an
+// ARP packet filter attached to the given parent already exists on
+// the link. The user can choose to specify an optional priority for
+// the filter.
+Try<bool> create(
+    const std::string& link,
+    const queueing::Handle& parent,
+    const Option<Priority>& priority,
+    const Option<queueing::Handle>& classid);
+
+
 // Removes the ARP packet filter attached to the parent from the link.
 // Returns false if no ARP packet filter attached to the given parent
 // is found on the link.

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/filter/filter.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/filter.hpp b/src/linux/routing/filter/filter.hpp
index d4ea099..024582c 100644
--- a/src/linux/routing/filter/filter.hpp
+++ b/src/linux/routing/filter/filter.hpp
@@ -59,6 +59,18 @@ public:
       priority_(_priority),
       handle_(_handle) {}
 
+  // Creates a filter with specified classid.
+  Filter(const queueing::Handle& _parent,
+         const Classifier& _classifier,
+         const Option<Priority>& _priority,
+         const Option<Handle>& _handle,
+         const Option<queueing::Handle>& _classid)
+    : parent_(_parent),
+      classifier_(_classifier),
+      priority_(_priority),
+      handle_(_handle),
+      classid_(_classid) {}
+
   // TODO(jieyu): Support arbitrary number of actions.
   template <typename Action>
   Filter(const queueing::Handle& _parent,
@@ -85,6 +97,7 @@ public:
   const Classifier& classifier() const { return classifier_; }
   const Option<Priority>& priority() const { return priority_; }
   const Option<Handle>& handle() const { return handle_; }
+  const Option<queueing::Handle>& classid() const { return classid_; }
 
   // Returns all the actions attached to this filter.
   const std::vector<process::Shared<action::Action>>& actions() const
@@ -106,6 +119,21 @@ private:
   // The handle of this filter.
   Option<Handle> handle_;
 
+  // The classid of this filter.
+  //
+  // Note: the classid can be used for two purposes:
+  //  1) For a classful queueing discipline, set the class id which
+  //     refers to a TC class;
+  //  2) For a classless queueing discipline, set the flow id which
+  //     refers to a flow defined by its parent qdisc.
+  // In both cases, the primary portion of a classid refers to its
+  // parent queueing discipline, the secondary portion refers to
+  // either its child queueing discipline or a flow.
+  //
+  // Kernel uses classid and flowid interchangeably. However, in our
+  // code base, we use classid consistently.
+  Option<queueing::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_;

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/filter/icmp.cpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/icmp.cpp b/src/linux/routing/filter/icmp.cpp
index 706b5d1..60703e7 100644
--- a/src/linux/routing/filter/icmp.cpp
+++ b/src/linux/routing/filter/icmp.cpp
@@ -248,6 +248,24 @@ Try<bool> create(
 }
 
 
+Try<bool> create(
+    const string& link,
+    const queueing::Handle& parent,
+    const Classifier& classifier,
+    const Option<Priority>& priority,
+    const Option<queueing::Handle>& classid)
+{
+  return internal::create(
+      link,
+      Filter<Classifier>(
+          parent,
+          classifier,
+          priority,
+          None(),
+          classid));
+}
+
+
 Try<bool> remove(
     const string& link,
     const queueing::Handle& parent,

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/filter/icmp.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/icmp.hpp b/src/linux/routing/filter/icmp.hpp
index 431bc19..d55eeee 100644
--- a/src/linux/routing/filter/icmp.hpp
+++ b/src/linux/routing/filter/icmp.hpp
@@ -94,6 +94,20 @@ Try<bool> create(
     const action::Mirror& mirror);
 
 
+// Creates an ICMP packet filter attached to the given parent on the
+// link which will set the classid for packets that satisfy the
+// conditions specified by the classifier. Returns false if an ICMP
+// packet filter attached to the given parent with the same classifier
+// already exists. The user can choose to specify an optional priority
+// for the filter.
+Try<bool> create(
+    const std::string& link,
+    const queueing::Handle& parent,
+    const Classifier& classifier,
+    const Option<Priority>& priority,
+    const Option<queueing::Handle>& classid);
+
+
 // Removes the ICMP packet filter attached to the given parent that
 // matches the specified classifier from the link. Returns false if
 // such a filter is not found.

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/filter/internal.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/internal.hpp b/src/linux/routing/filter/internal.hpp
index 8a6c0c0..c74098d 100644
--- a/src/linux/routing/filter/internal.hpp
+++ b/src/linux/routing/filter/internal.hpp
@@ -414,6 +414,15 @@ Try<Netlink<struct rtnl_cls>> encodeFilter(
     }
   }
 
+  // Set the classid if needed.
+  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());
+    } else if (rtnl_tc_get_kind(TC_CAST(cls.get())) == std::string("basic")) {
+      rtnl_basic_set_target(cls.get(), filter.classid().get().get());
+    }
+  }
+
   return cls;
 }
 
@@ -452,10 +461,24 @@ Result<Filter<Classifier>> decodeFilter(const Netlink<struct rtnl_cls>& cls)
     return None();
   }
 
+  Option<queueing::Handle> classid;
+  if (rtnl_tc_get_kind(TC_CAST(cls.get())) == std::string("u32")) {
+    uint32_t _classid;
+    if (rtnl_u32_get_classid(cls.get(), &_classid) == 0) {
+      classid = _classid;
+    }
+  } else if (rtnl_tc_get_kind(TC_CAST(cls.get())) == std::string("basic")) {
+    classid = rtnl_basic_get_target(cls.get());
+  }
+
   // TODO(jieyu): Decode all the actions attached to the filter.
   // Currently, libnl does not support that (but will support that in
   // the future).
-  return Filter<Classifier>(parent, classifier.get(), priority, handle);
+  return Filter<Classifier>(parent,
+                            classifier.get(),
+                            priority,
+                            handle,
+                            classid);
 }
 
 /////////////////////////////////////////////////

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/filter/ip.cpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/ip.cpp b/src/linux/routing/filter/ip.cpp
index de64071..0d25e2d 100644
--- a/src/linux/routing/filter/ip.cpp
+++ b/src/linux/routing/filter/ip.cpp
@@ -518,6 +518,22 @@ Try<bool> create(
           terminal));
 }
 
+Try<bool> create(
+    const string& link,
+    const queueing::Handle& parent,
+    const Classifier& classifier,
+    const Option<Priority>& priority,
+    const Option<queueing::Handle>& classid)
+{
+  return internal::create(
+      link,
+      Filter<Classifier>(
+          parent,
+          classifier,
+          priority,
+          None(),
+          classid));
+}
 
 Try<bool> remove(
     const string& link,

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/filter/ip.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/filter/ip.hpp b/src/linux/routing/filter/ip.hpp
index b540602..62bb5f8 100644
--- a/src/linux/routing/filter/ip.hpp
+++ b/src/linux/routing/filter/ip.hpp
@@ -188,6 +188,18 @@ Try<bool> create(
     const action::Terminal& terminal);
 
 
+// Creates an IP packet filter attached to the given parent on the
+// link which will set the classid for packets. Returns false if an IP
+// packet filter attached to the given parent with the same classifier
+// already exists.
+Try<bool> create(
+    const std::string& link,
+    const queueing::Handle& parent,
+    const Classifier& classifier,
+    const Option<Priority>& priority,
+    const Option<queueing::Handle>& classid);
+
+
 // Removes the IP packet filter attached to the given parent that
 // matches the specified classifier from the link. Returns false if
 // such a filter is not found.

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/linux/routing/queueing/handle.hpp
----------------------------------------------------------------------
diff --git a/src/linux/routing/queueing/handle.hpp b/src/linux/routing/queueing/handle.hpp
index 2725d07..5f0cb77 100644
--- a/src/linux/routing/queueing/handle.hpp
+++ b/src/linux/routing/queueing/handle.hpp
@@ -38,6 +38,16 @@ public:
     handle = (((uint32_t) primary) << 16) + secondary;
   }
 
+  // NOTE: This is used to construct a classid. The higher 16 bits of
+  // the given 'parent' will be the primary and the lower 16 bits is
+  // specified by the given 'id'.
+  Handle(Handle parent, uint16_t id)
+  {
+    handle = (((uint32_t) parent.primary()) << 16) + id;
+  }
+
+  uint16_t primary() const { return handle >> 16; }
+  uint16_t secondary() const { return handle & 0x0000ffff; }
   uint32_t get() const { return handle; }
 
 private:

http://git-wip-us.apache.org/repos/asf/mesos/blob/ed8253f4/src/tests/routing_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/routing_tests.cpp b/src/tests/routing_tests.cpp
index 7cc3b57..ce583b5 100644
--- a/src/tests/routing_tests.cpp
+++ b/src/tests/routing_tests.cpp
@@ -426,6 +426,71 @@ TEST_F(RoutingVethTest, ROOT_FqCodelCreate)
 }
 
 
+TEST_F(RoutingVethTest, ROOT_FqCodelClassifier)
+{
+  ASSERT_SOME(link::create(TEST_VETH_LINK, TEST_PEER_LINK, None()));
+
+  EXPECT_SOME_TRUE(link::exists(TEST_VETH_LINK));
+  EXPECT_SOME_TRUE(link::exists(TEST_PEER_LINK));
+
+  ASSERT_SOME_TRUE(fq_codel::create(TEST_VETH_LINK));
+
+  EXPECT_SOME_TRUE(arp::create(
+      TEST_VETH_LINK,
+      fq_codel::HANDLE,
+      None(),
+      queueing::Handle(fq_codel::HANDLE, 0)));
+
+  // There is a kernel bug which could cause this test fail. Please
+  // make sure your kernel, if newer than 3.14, has commit:
+  // b057df24a7536cce6c372efe9d0e3d1558afedf4
+  // (https://git.kernel.org/cgit/linux/kernel/git/davem/net.git).
+  // Please fix your kernel if you see this failure.
+  EXPECT_SOME_TRUE(arp::exists(TEST_VETH_LINK, fq_codel::HANDLE));
+
+  EXPECT_SOME_TRUE(icmp::create(
+      TEST_VETH_LINK,
+      fq_codel::HANDLE,
+      icmp::Classifier(None()),
+      None(),
+      queueing::Handle(fq_codel::HANDLE, 0)));
+
+  EXPECT_SOME_TRUE(icmp::exists(
+      TEST_VETH_LINK,
+      fq_codel::HANDLE,
+      icmp::Classifier(None())));
+
+  Result<net::MAC> mac = net::mac(TEST_VETH_LINK);
+  ASSERT_SOME(mac);
+
+  net::IP ip = net::IP(0x01020304); // 1.2.3.4
+
+  Try<ip::PortRange> sourcePorts =
+    ip::PortRange::fromBeginEnd(1024, 1027);
+  ASSERT_SOME(sourcePorts);
+
+  Try<ip::PortRange> destinationPorts =
+    ip::PortRange::fromBeginEnd(2000, 2000);
+  ASSERT_SOME(destinationPorts);
+
+  ip::Classifier classifier =
+    ip::Classifier(
+        mac.get(),
+        ip,
+        sourcePorts.get(),
+        destinationPorts.get());
+
+  EXPECT_SOME_TRUE(ip::create(
+      TEST_VETH_LINK,
+      fq_codel::HANDLE,
+      classifier,
+      None(),
+      queueing::Handle(fq_codel::HANDLE, 1)));
+
+  EXPECT_SOME_TRUE(ip::exists(TEST_VETH_LINK, fq_codel::HANDLE, classifier));
+}
+
+
 TEST_F(RoutingVethTest, ROOT_ARPFilterCreate)
 {
   ASSERT_SOME(link::create(TEST_VETH_LINK, TEST_PEER_LINK, None()));