[#3860] Checkpoint vendor in def

src/hooks/dhcp/radius/client_dictionary.cc

@@ -68,25 +68,25 @@ AttrDefs::instance() {
 }
 
 AttrDefPtr
-AttrDefs::getByType(const uint8_t type) const {
+AttrDefs::getByType(const uint8_t type, const uint32_t vendor) const {
     auto const& idx = container_.get<0>();
-    auto it = idx.find(type);
+    auto it = idx.find(boost::make_tuple(vendor, type));
     if (it != idx.end()) {
         return (*it);
     }
     return (AttrDefPtr());
 }
 
 AttrDefPtr
-AttrDefs::getByName(const string& name) const {
+AttrDefs::getByName(const string& name, const uint32_t vendor) const {
     auto const& idx = container_.get<1>();
-    auto it = idx.find(name);
+    auto it = idx.find(boost::make_tuple(vendor, name));
     if (it != idx.end()) {
         return (*it);
     }
-    auto alias = aliases_.find(name);
+    auto alias = aliases_.find(boost::make_tuple(vendor, name));
     if (alias != aliases_.end()) {
-        auto ita = idx.find(alias->second);
+        auto ita = idx.find(boost::make_tuple(vendor, alias->name_));
         if (ita != idx.end()) {
             return (*ita);
         }
@@ -100,42 +100,51 @@ AttrDefs::add(AttrDefPtr def) {
         return;
     }
     auto& idx1 = container_.get<1>();
-    auto it1 = idx1.find(def->name_);
+    auto it1 = idx1.find(boost::make_tuple(def->vendor_, def->name_));
     if (it1 != idx1.end()) {
         if ((def->type_ == (*it1)->type_) &&
             (def->value_type_ == (*it1)->value_type_)) {
             // Duplicate: ignore.
             return;
         }
-        isc_throw(BadValue, "Illegal attribute redefinition of '" << def->name_
-                  << "' type " << static_cast<unsigned>((*it1)->type_)
-                  << " value type " << attrValueTypeToText((*it1)->value_type_)
-                  << " by " << static_cast<unsigned>(def->type_)
-                  << " " << attrValueTypeToText(def->value_type_));
+        ostringstream msg;
+        msg << "Illegal attribute redefinition of '" << def->name_ << "'";
+        if (def->vendor_ != 0) {
+            msg << " vendor " << def->vendor_;
+        }
+        msg << " type " << static_cast<unsigned>((*it1)->type_)
+            << " value type " << attrValueTypeToText((*it1)->value_type_)
+            << " by " << static_cast<unsigned>(def->type_)
+            << " " << attrValueTypeToText(def->value_type_);
+        isc_throw(BadValue, msg.str());
     }
     auto& idx0 = container_.get<0>();
-    auto it0 = idx0.find(def->type_);
+    auto it0 = idx0.find(boost::make_tuple(def->vendor_, def->type_));
     if (it0 != idx0.end()) {
         if (def->value_type_ == (*it0)->value_type_) {
             // Alias.
-            auto p = pair<string, string>(def->name_, (*it0)->name_);
-            static_cast<void>(aliases_.insert(p));
+            AttrDefAlias alias(def->name_, (*it0)->name_, def->vendor_);
+            static_cast<void>(aliases_.insert(alias));
             return;
         }
-        isc_throw(BadValue, "Illegal attribute redefinition of '"
-                  << (*it0)->name_ << "' type "
-                  << static_cast<unsigned>((*it0)->type_) << " value type "
-                  << attrValueTypeToText((*it0)->value_type_)
-                  << " by '" << def->name_ << "' "
-                  << static_cast<unsigned>(def->type_) << " "
-                  << attrValueTypeToText(def->value_type_));
+        ostringstream msg;
+        msg << "Illegal attribute redefinition of '" << (*it0)->name_ << "'";
+        if (def->vendor_ != 0) {
+            msg << " vendor " << def->vendor_;
+        }
+        msg << " type " << static_cast<unsigned>((*it0)->type_)
+            << " value type " << attrValueTypeToText((*it0)->value_type_)
+            << " by '" << def->name_ << "' "
+            << static_cast<unsigned>(def->type_) << " "
+            << attrValueTypeToText(def->value_type_);
+        isc_throw(BadValue, msg.str());
     }
     static_cast<void>(container_.insert(def));
 }
 
 string
-AttrDefs::getName(const uint8_t type) const {
-    AttrDefPtr def = getByType(type);
+AttrDefs::getName(const uint8_t type, const uint32_t vendor) const {
+    AttrDefPtr def = getByType(type, vendor);
     if (def) {
         return (def->name_);
     }

src/hooks/dhcp/radius/client_dictionary.h

@@ -58,9 +58,10 @@ class AttrDef {
     /// @param type attribute type.
     /// @param name attribute name.
     /// @param value_type attribute value type.
+    /// @param vendor vendor id (default 0).
     AttrDef(const uint8_t type, const std::string& name,
-                 const AttrValueType value_type)
-        : type_(type), name_(name), value_type_(value_type) {
+            const AttrValueType value_type, const uint32_t vendor = 0)
+    : type_(type), name_(name), value_type_(value_type), vendor_(vendor) {
     }
 
     /// @brief type.
@@ -71,6 +72,9 @@ class AttrDef {
 
     /// @brief value_type.
     const AttrValueType value_type_;
+
+    /// @brief vendor id (default 0).
+    const uint32_t vendor_;
 };
 
 /// @brief Shared pointers to Attribute definition.
@@ -79,6 +83,30 @@ typedef boost::shared_ptr<AttrDef> AttrDefPtr;
 /// @brief List of Attribute definitions.
 typedef std::list<AttrDef> AttrDefList;
 
+/// @brief RADIUS attribute aliases.
+class AttrDefAlias {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// @param alias attribute alias name.
+    /// @param name attribute name.
+    /// @param vendor vendor id (default 0).
+    AttrDefAlias(const std::string& alias, const std::string& name,
+                 const uint32_t vendor = 0)
+    : alias_(alias), name_(name), vendor_(vendor) {
+    }
+
+    /// @brief alias.
+    const std::string alias_;
+
+    /// @brief name.
+    const std::string name_;
+
+    /// @brief vendor id (default 0).
+    const uint32_t vendor_;
+};
+
 /// @brief RADIUS integer constant definitions.
 ///
 /// Include vendor ids with Vendor-Specific attribute.
@@ -118,23 +146,53 @@ class AttrDefs : public boost::noncopyable {
         AttrDefPtr,
         // Start specification of indexes here.
         boost::multi_index::indexed_by<
-            // Hash index for by type.
+            // Hash index for by vendor and type.
             boost::multi_index::hashed_unique<
-                boost::multi_index::member<
-                    AttrDef, const uint8_t, &AttrDef::type_
+                boost::multi_index::composite_key<
+                    AttrDef,
+                    boost::multi_index::member<
+                        AttrDef, const uint32_t, &AttrDef::vendor_
+                    >,
+                    boost::multi_index::member<
+                        AttrDef, const uint8_t, &AttrDef::type_
+                    >
                 >
             >,
-            // Hash index for by name.
+            // Hash index for by vendor and name.
             boost::multi_index::hashed_unique<
-                boost::multi_index::member<
-                    AttrDef, const std::string, &AttrDef::name_
+                boost::multi_index::composite_key<
+                    AttrDef,
+                    boost::multi_index::member<
+                        AttrDef, const uint32_t, &AttrDef::vendor_
+                    >,
+                    boost::multi_index::member<
+                        AttrDef, const std::string, &AttrDef::name_
+                    >
                 >
             >
         >
     > AttrDefContainer;
 
     /// @brief Type of the alias table (alias -> standard name map).
-    typedef std::unordered_map<std::string, std::string> AttrDefAliases;
+    typedef boost::multi_index_container<
+        // This container stores aliases.
+        AttrDefAlias,
+        // Start specification of indexes here.
+        boost::multi_index::indexed_by<
+            // Hash index for by vendor and alias.
+            boost::multi_index::hashed_unique<
+                boost::multi_index::composite_key<
+                    AttrDefAlias,
+                    boost::multi_index::member<
+                        AttrDefAlias, const uint32_t, &AttrDefAlias::vendor_
+                    >,
+                    boost::multi_index::member<
+                        AttrDefAlias, const std::string, &AttrDefAlias::alias_
+                    >
+                >
+            >
+        >
+    > AttrDefAliases;
 
     /// @brief Type of the integer constant definition container.
     typedef boost::multi_index_container<
@@ -176,17 +234,20 @@ class AttrDefs : public boost::noncopyable {
     /// @return the single instance.
     static AttrDefs& instance();
 
-    /// @brief Get attribute definition by type.
+    /// @brief Get attribute definition by type and vendor.
     ///
     /// @param type type to look for.
+    /// @param vendor vendor id to look for (default 0).
     /// @return pointer to the attribute definition or null.
-    AttrDefPtr getByType(const uint8_t type) const;
+    AttrDefPtr getByType(const uint8_t type, const uint32_t vendor = 0) const;
 
-    /// @brief Get attribute definition by name.
+    /// @brief Get attribute definition by name and vendor.
     ///
     /// @param name name to look for.
+    /// @param vendor vendor id to look for (default 0).
     /// @return pointer to the attribute definition or null.
-    AttrDefPtr getByName(const std::string& name) const;
+    AttrDefPtr getByName(const std::string& name,
+                         const uint32_t vendor = 0) const;
 
     /// @brief Add (or replace) an attribute definition.
     ///
@@ -203,7 +264,8 @@ class AttrDefs : public boost::noncopyable {
     /// @brief Get attribute name.
     ///
     /// @param type type to look for.
-    std::string getName(const uint8_t type) const;
+    /// @param vendor vendor id to look for (default 0).
+    std::string getName(const uint8_t type, const uint32_t vendor = 0) const;
 
     /// @brief Get integer constant definition by attribute type and name.
     ///

src/hooks/dhcp/radius/tests/config_unittests.cc

@@ -807,7 +807,7 @@ TEST_F(ConfigTest, attribute) {
     attr->set("data", Element::create("foobar"));
     attr->set("type", Element::create(26));
     expected = "can't create Vendor-Specific attribute from [foobar]: ";
-    expected += "Can't decode vsa from text");
+    expected += "Can't decode vsa from text";
     EXPECT_THROW_MSG(parser.parse(srv, attr), ConfigError, expected);
 
     // One of expr, data, raw