]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#3245] addressed review
authorRazvan Becheriu <razvan@isc.org>
Thu, 22 Feb 2024 19:23:58 +0000 (21:23 +0200)
committerThomas Markwalder <tmark@isc.org>
Fri, 23 Feb 2024 12:08:04 +0000 (12:08 +0000)
src/hooks/dhcp/perfmon/alarm.h
src/hooks/dhcp/perfmon/monitored_duration.cc
src/hooks/dhcp/perfmon/monitored_duration.h
src/hooks/dhcp/perfmon/tests/alarm_unittests.cc
src/hooks/dhcp/perfmon/tests/monitored_duration_unittests.cc
src/lib/dhcp/pkt_filter_lpf.cc

index f0080ae2adfb2466a68cf07d6a70523e51226e84..5797a84e2131daa8247f3bfe5aa60e725a087da6 100644 (file)
@@ -30,21 +30,21 @@ public:
     ///
     /// @param family protocol family AF_INET or AF_INET6
     /// @param query_type message type of the query packet
-    /// @param response_type_ message type of the response packet
+    /// @param response_type message type of the response packet
     /// @param start_event_label label of the start event
     /// @param end_event_label label of the end event
     /// @param SubnetID subnet_id id of the selected subnet
     /// @param low_water threshold below which the average duration must fall to clear the alarm
     /// @brief high_water threshold above which the average duration must rise to trigger the alarm.
     /// @brief enabled true sets state to CLEAR, otherwise DISABLED, defaults to true.
-    Alarm(uint16_t family, uint8_t query_type_, uint8_t response_type_,
+    Alarm(uint16_t family, uint8_t query_type, uint8_t response_type,
           const std::string& start_event_label, const std::string& end_event_label,
-          dhcp::SubnetID subnet_id_,
+          dhcp::SubnetID subnet_id,
           const Duration& low_water, const Duration& high_water, bool enabled = true);
 
     /// @brief Constructor
     ///
-    /// param key composite key that identifies the alarm
+    /// @param key composite key that identifies the alarm
     /// @param low_water threshold below which the average duration must fall to clear the alarm
     /// @brief high_water threshold above which the average duration must rise to trigger the alarm.
     /// @brief enabled true sets state to CLEAR, otherwise DISABLED, defaults to true.
index abc434387236da9b947f0f6118308607f2529cb2..d6ddd172fb5a37e47bfec6a2903226e12b724fe9 100644 (file)
@@ -21,9 +21,9 @@ namespace perfmon {
 // DurationDataInterval methods
 
 DurationDataInterval::DurationDataInterval(const Timestamp& start_time /* = PktEvent::now()*/)
- : start_time_(start_time), occurrences_(0),
-   min_duration_(pos_infin), max_duration_(neg_infin),
-   total_duration_(microseconds(0)) {
   : start_time_(start_time), occurrences_(0),
+      min_duration_(pos_infin), max_duration_(neg_infin),
+      total_duration_(microseconds(0)) {
 }
 
 void
index 9709c864b58c1f3442546a1db27879e29099c5f1..535a7882fedfcc3b88f301ad5c3d1481d4e11655 100644 (file)
@@ -63,7 +63,7 @@ public:
     /// @brief Get the number of occurrences that have contributed to the
     /// interval.
     ///
-    /// @return uint64_t containing the number of occurrences.
+    /// @return the number of occurrences.
     uint64_t getOccurrences() const {
         return (occurrences_);
     };
@@ -119,20 +119,20 @@ typedef boost::shared_ptr<DurationDataInterval> DurationDataIntervalPtr;
 /// -# Response Packet Type
 /// -# Start Event
 /// -# End Event
-/// -# Subnet ID  can be GLOBAL_SUBNET_ID for aggregate durations
+/// -# Subnet ID can be GLOBAL_SUBNET_ID for aggregate durations
 class DurationKey {
 public:
     /// @brief Constructor
     ///
     /// @param family protocol family AF_INET or AF_INET6
     /// @param query_type message type of the query packet
-    /// @param response_type_ message type of the response packet
+    /// @param response_type message type of the response packet
     /// @param start_event_label label of the start event
     /// @param end_event_label label of the end event
-    /// @param SubnetID subnet_id id of the selected subnet
-    DurationKey(uint16_t family, uint8_t query_type_, uint8_t response_type_,
+    /// @param subnet_id id of the selected subnet
+    DurationKey(uint16_t family, uint8_t query_type, uint8_t response_type,
                 const std::string& start_event_label, const std::string& end_event_label,
-                dhcp::SubnetID subnet_id_);
+                dhcp::SubnetID subnet_id);
 
     /// @brief Destructor
     virtual ~DurationKey() = default;
@@ -141,33 +141,33 @@ public:
     ///
     /// @return uint16_t containing the family (AF_INET or AF_INET6)
     uint16_t getFamily() {
-        return(family_);
+        return (family_);
     }
 
     /// @brief Get the query packet type.
     ///
-    /// @return uint8_t containing the query packet type.
+    /// @return the query packet type.
     uint8_t getQueryType() const {
         return (query_type_);
     }
 
     /// @brief Get the response packet type.
     ///
-    /// @return uint8_t containing the response packet type.
+    /// @return the response packet type.
     uint8_t getResponseType() const {
         return (response_type_);
     };
 
     /// @brief Get the start event label.
     ///
-    /// @return String containing the start event label.
+    /// @return the start event label.
     std::string getStartEventLabel() const {
         return (start_event_label_);
     }
 
     /// @brief Get the end event label.
     ///
-    /// @return String containing the end event label.
+    /// @return the end event label.
     std::string getEndEventLabel() const {
         return (end_event_label_);
     }
@@ -197,7 +197,7 @@ public:
     ///
     /// @endcode
     ///
-    /// @return String containing the composite label.
+    /// @return the composite label.
     std::string getLabel() const;
 
     /// @brief Validates that a query and response message type pair is sane.
@@ -205,19 +205,19 @@ public:
     /// @param family Protocol family of the key (AF_INET or AF_INET6)
     /// The format of the string:
     /// @param query_type message type of the query packet
-    /// @param response_type_ message type of the response packet
+    /// @param response_type message type of the response packet
     ///
     /// @throw BadValue is the pairing does not make sense.
     static void validateMessagePair(uint16_t family, uint8_t query_type, uint8_t response_type);
 
 protected:
-    /// @brief Protocol family AF_INET or AF_INET6
+    /// @brief Protocol family AF_INET or AF_INET6.
     uint16_t family_;
 
-    /// @brief Query message type (e.g. DHCPDISCOVER, DHCP6_SOLICIT)
+    /// @brief Query message type (e.g. DHCPDISCOVER, DHCP6_SOLICIT).
     uint8_t query_type_;
 
-    /// @brief Response message type. (e.g. DHCPOFFER, DHCP6_ADVERTISE)
+    /// @brief Response message type (e.g. DHCPOFFER, DHCP6_ADVERTISE).
     uint8_t response_type_;
 
     /// @brief Label of the start event which begins the duration.
@@ -239,39 +239,39 @@ public:
     ///
     /// @param family protocol family AF_INET or AF_INET6
     /// @param query_type message type of the query packet
-    /// @param response_type_ message type of the response packet
+    /// @param response_type message type of the response packet
     /// @param start_event_label label of the start event
     /// @param end_event_label label of the end event
-    /// @param SubnetID subnet_id id of the selected subnet
-    /// @param Duration interval_duration_;
-    MonitoredDuration(uint16_t family, uint8_t query_type_, uint8_t response_type_,
+    /// @param subnet_id id of the selected subnet
+    /// @param interval_duration the interval duration
+    MonitoredDuration(uint16_t family, uint8_t query_type, uint8_t response_type,
                       const std::string& start_event_label, const std::string& end_event_label,
-                      dhcp::SubnetID subnet_id_, const Duration& interval_duration_);
+                      dhcp::SubnetID subnet_id, const Duration& interval_duration);
 
     /// @brief Constructor
     ///
-    /// param key composite key that identifies the alarm
-    /// @param Duration interval_duration_;
-    MonitoredDuration(const DurationKey& key, const Duration& interval_duration_);
+    /// @param key composite key that identifies the alarm
+    /// @param interval_duration the interval duration
+    MonitoredDuration(const DurationKey& key, const Duration& interval_duration);
 
     /// @brief Destructor
     virtual ~MonitoredDuration() = default;
 
-    /// @brief Get the interval duration
+    /// @brief Get the interval duration.
     ///
     /// @return Duration containing the interval duration.
     Duration getIntervalDuration() const {
-        return(interval_duration_);
+        return (interval_duration_);
     }
 
-    /// @brief Get the previous interval
+    /// @brief Get the previous interval.
     ///
     /// @return Pointer to the previous interval if it exists or an empty pointer.
     DurationDataIntervalPtr getPreviousInterval() const {
         return (previous_interval_);
     }
 
-    /// @brief Get the current interval
+    /// @brief Get the current interval.
     ///
     /// @return Pointer to the current interval if it exists or an empty pointer.
     DurationDataIntervalPtr getCurrentInterval() const {
index 759b3bdc7330e4c28e6e23533dc5f9cae347073f..ffae6d12612461923f5c512a8dd6bb9b83db43e9 100644 (file)
@@ -30,9 +30,9 @@ TEST(Alarm, validConstructors) {
     Duration low_water(milliseconds(50));
     Duration high_water(milliseconds(250));
     ASSERT_NO_THROW_LOG(alarm.reset(new Alarm(AF_INET, DHCPDISCOVER, DHCPOFFER,
-                              "process_started", "process_completed",
-                              SUBNET_ID_GLOBAL,
-                              low_water, high_water)));
+                                              "process_started", "process_completed",
+                                              SUBNET_ID_GLOBAL,
+                                              low_water, high_water)));
     ASSERT_TRUE(alarm);
     EXPECT_EQ(alarm->getFamily(), AF_INET);
     EXPECT_EQ(alarm->getQueryType(), DHCPDISCOVER);
@@ -52,7 +52,7 @@ TEST(Alarm, validConstructors) {
     // Create valid v6 key and use that to create an alarm. Verify contents and label.
     DurationKeyPtr key;
     ASSERT_NO_THROW_LOG(key.reset(new DurationKey(AF_INET6, DHCPV6_SOLICIT, DHCPV6_ADVERTISE,
-                                  "mt_queued", "process_started", 77)));
+                                                  "mt_queued", "process_started", 77)));
 
     ASSERT_NO_THROW_LOG(alarm.reset(new Alarm(*key, low_water, high_water, false)));
     ASSERT_TRUE(alarm);
@@ -77,15 +77,15 @@ TEST(Alarm, invalidConstructors) {
     Duration low_water(milliseconds(50));
     Duration high_water(milliseconds(250));
     ASSERT_THROW_MSG(alarm.reset(new Alarm(AF_INET, DHCPDISCOVER, DHCPDISCOVER,
-                                 "process_started", "process_completed",
-                                 SUBNET_ID_GLOBAL, low_water, high_water)),
+                                           "process_started", "process_completed",
+                                           SUBNET_ID_GLOBAL, low_water, high_water)),
                       BadValue,
                       "Response type: DHCPDISCOVER not valid for query type: DHCPDISCOVER");
 
     // Low water too high, should throw.
     ASSERT_THROW_MSG(alarm.reset(new Alarm(AF_INET, DHCPDISCOVER, DHCPOFFER,
-                                "process_started", "process_completed",
-                                 SUBNET_ID_GLOBAL, high_water, low_water)),
+                                           "process_started", "process_completed",
+                                           SUBNET_ID_GLOBAL, high_water, low_water)),
                       BadValue,
                       "low water: 00:00:00.250000, must be less than high water:"
                       " 00:00:00.050000");
@@ -93,7 +93,7 @@ TEST(Alarm, invalidConstructors) {
     // Create valid v6 key.
     DurationKeyPtr key;
     ASSERT_NO_THROW_LOG(key.reset(new DurationKey(AF_INET6, DHCPV6_SOLICIT, DHCPV6_ADVERTISE,
-                                  "mt_queued", "process_started", 77)));
+                                                  "mt_queued", "process_started", 77)));
 
     // Low water too high, should throw.
     ASSERT_THROW_MSG(alarm.reset(new Alarm(*key, high_water, low_water)),
@@ -108,9 +108,9 @@ TEST(Alarm, lowWaterHighWaterSetters) {
     Duration high_water(milliseconds(250));
     AlarmPtr alarm;
     ASSERT_NO_THROW_LOG(alarm.reset(new Alarm(AF_INET, DHCPDISCOVER, DHCPOFFER,
-                              "process_started", "process_completed",
-                              SUBNET_ID_GLOBAL,
-                              low_water, high_water)));
+                                              "process_started", "process_completed",
+                                              SUBNET_ID_GLOBAL,
+                                              low_water, high_water)));
 
     // Should be able to set thresholds to new, valid values.
     low_water += milliseconds(50);
@@ -135,8 +135,8 @@ TEST(Alarm, clearAndDisable) {
     auto start_time = PktEvent::now();
     AlarmPtr alarm;
     ASSERT_NO_THROW_LOG(alarm.reset(new Alarm(AF_INET, DHCPDISCOVER, DHCPOFFER,
-                              "process_started", "process_completed",
-                              SUBNET_ID_GLOBAL, milliseconds(100), milliseconds(200))));
+                                              "process_started", "process_completed",
+                                              SUBNET_ID_GLOBAL, milliseconds(100), milliseconds(200))));
 
     // Initial state should be CLEAR, stos_time_ should be close to now, no report time.
     EXPECT_EQ(alarm->getState(), Alarm::CLEAR);
@@ -175,7 +175,7 @@ TEST(Alarm, clearAndDisable) {
 // ```
 //                     INPUT                           |          OUTPUT
 //    Test sample relationship       Input  Report Int.|
-//     to the thresholds              State   Elapsed   | Report  State  Stos   Last Report
+//    to the thresholds              State   Elapsed   | Report  State  Stos   Last Report
 //    -------------------------------------------------|----------------------------------
 //    sample < low_water                C      false   | false     C      -       -
 //    sample < low_water                C      true    | false     C      -       -
index cce92f068d107c2096e32f6fb6238a5b6457544b..c85bd3631a458c46a14b7a6d03432604eac4c8a9 100644 (file)
@@ -86,8 +86,8 @@ TEST(DurationKey, basics) {
 
     // Create valid v4 key, verify contents and label.
     ASSERT_NO_THROW_LOG(key.reset(new DurationKey(AF_INET, DHCPDISCOVER, DHCPOFFER,
-                              "process_started", "process_completed",
-                              SUBNET_ID_GLOBAL)));
+                                  "process_started", "process_completed",
+                                  SUBNET_ID_GLOBAL)));
     ASSERT_TRUE(key);
     EXPECT_EQ(key->getFamily(), AF_INET);
     EXPECT_EQ(key->getQueryType(), DHCPDISCOVER);
@@ -127,28 +127,28 @@ TEST(DurationKey, validateMessagePairs4) {
 
     // List of scenarios to test, one per v4 message type.
     std::list<Scenario> scenarios {
-        { DHCP_NOTYPE, {DHCP_NOTYPE, DHCPOFFER, DHCPACK, DHCPNAK}},
-        { DHCPDISCOVER, {DHCP_NOTYPE, DHCPOFFER, DHCPNAK}},
-        { DHCPOFFER, {}},
-        { DHCPREQUEST, {DHCP_NOTYPE, DHCPACK, DHCPNAK}},
-        { DHCPDECLINE, {}},
-        { DHCPACK, {}},
-        { DHCPNAK, {}},
-        { DHCPRELEASE, {}},
-        { DHCPINFORM, {DHCP_NOTYPE, DHCPACK}},
-//      { DHCPFORCERENEW, {}},        commented out in dhcp4.h
-        { DHCPLEASEQUERY, {}},
-        { DHCPLEASEUNASSIGNED, {}},
-        { DHCPLEASEUNKNOWN, {}},
-        { DHCPLEASEACTIVE, {}},
-        { DHCPBULKLEASEQUERY, {}},
-        { DHCPLEASEQUERYDONE, {}},
-//      {  DHCPACTIVELEASEQUERY, {}}, commented out in dhcp4.h
-        { DHCPLEASEQUERYSTATUS, {}},
-        { DHCPTLS, {}},
+        {DHCP_NOTYPE, {DHCP_NOTYPE, DHCPOFFER, DHCPACK, DHCPNAK}},
+        {DHCPDISCOVER, {DHCP_NOTYPE, DHCPOFFER, DHCPNAK}},
+        {DHCPOFFER, {}},
+        {DHCPREQUEST, {DHCP_NOTYPE, DHCPACK, DHCPNAK}},
+        {DHCPDECLINE, {}},
+        {DHCPACK, {}},
+        {DHCPNAK, {}},
+        {DHCPRELEASE, {}},
+        {DHCPINFORM, {DHCP_NOTYPE, DHCPACK}},
+//      {DHCPFORCERENEW, {}},        commented out in dhcp4.h
+        {DHCPLEASEQUERY, {}},
+        {DHCPLEASEUNASSIGNED, {}},
+        {DHCPLEASEUNKNOWN, {}},
+        {DHCPLEASEACTIVE, {}},
+        {DHCPBULKLEASEQUERY, {}},
+        {DHCPLEASEQUERYDONE, {}},
+//      {DHCPACTIVELEASEQUERY, {}}, commented out in dhcp4.h
+        {DHCPLEASEQUERYSTATUS, {}},
+        {DHCPTLS, {}},
     };
 
-    // Iterate over the scenarios.  Attempt to pair each scenario query type with every v6 message
+    // Iterate over the scenarios.  Attempt to pair each scenario query type with every v4 message
     // type as a response type.  If the response type is in the scenario's valid list, the pair
     // should validate, otherwise it should throw.
     for (auto const& scenario : scenarios) {
@@ -165,7 +165,7 @@ TEST(DurationKey, validateMessagePairs4) {
     }
 }
 
-// Verify v4 message pair validation works.
+// Verify v6 message pair validation works.
 TEST(DurationKey, validateMessagePairs6) {
     //  Defines a test scenario.
     struct Scenario {
@@ -279,24 +279,24 @@ TEST(MonitoredDuration, invalidConstructors) {
     // Make sure we catch an invalid message pairing.
     Duration interval_duration = seconds(60);
     ASSERT_THROW_MSG(mond.reset(new MonitoredDuration(AF_INET, DHCPDISCOVER, DHCPDISCOVER,
-                                "process_started", "process_completed",
-                                SUBNET_ID_GLOBAL, interval_duration)),
+                                                      "process_started", "process_completed",
+                                                      SUBNET_ID_GLOBAL, interval_duration)),
                      BadValue,
                      "Response type: DHCPDISCOVER not valid for query type: DHCPDISCOVER");
 
-    // Interval duration cannot be than zero.
+    // Interval duration cannot be zero.
     interval_duration = DurationDataInterval::ZERO_DURATION();
     ASSERT_THROW_MSG(mond.reset(new MonitoredDuration(AF_INET, DHCPDISCOVER, DHCPOFFER,
-                                "process_started", "process_completed",
-                                 SUBNET_ID_GLOBAL, interval_duration)),
+                                                      "process_started", "process_completed",
+                                                      SUBNET_ID_GLOBAL, interval_duration)),
                      BadValue,
                      "MonitoredDuration - interval_duration 00:00:00,"
                      " is invalid, it must be greater than 0");
 
     // Interval duration cannot be negative.
     ASSERT_THROW_MSG(mond.reset(new MonitoredDuration(AF_INET, DHCPDISCOVER, DHCPOFFER,
-                                "process_started", "process_completed",
-                                 SUBNET_ID_GLOBAL, seconds(-5))),
+                                                      "process_started", "process_completed",
+                                                      SUBNET_ID_GLOBAL, seconds(-5))),
                      BadValue,
                      "MonitoredDuration - interval_duration -00:00:05,"
                      " is invalid, it must be greater than 0");
@@ -304,9 +304,9 @@ TEST(MonitoredDuration, invalidConstructors) {
     // Create valid v6 key.
     DurationKeyPtr key;
     ASSERT_NO_THROW_LOG(key.reset(new DurationKey(AF_INET6, DHCPV6_SOLICIT, DHCPV6_ADVERTISE,
-                                  "mt_queued", "process_started", 77)));
+                                                  "mt_queued", "process_started", 77)));
 
-    // Interval duration cannot be than zero.
+    // Interval duration cannot be zero.
     ASSERT_THROW_MSG(mond.reset(new MonitoredDuration(*key, interval_duration)),
                      BadValue,
                      "MonitoredDuration - interval_duration 00:00:00,"
@@ -326,8 +326,8 @@ TEST(MonitoredDuration, addSampleAndClear) {
 
     // Create valid v4 duration with interval duration of 50ms.
     ASSERT_NO_THROW_LOG(mond.reset(new MonitoredDuration(AF_INET, DHCPDISCOVER, DHCPOFFER,
-                                  "process_started", "process_completed",
-                                   SUBNET_ID_GLOBAL, interval_duration)));
+                                                         "process_started", "process_completed",
+                                                         SUBNET_ID_GLOBAL, interval_duration)));
     ASSERT_TRUE(mond);
 
     // Initially there are no intervals.
index f0f19e946ab6ccb6db2333ea5c174fe2f3104e1f..37ce7c2d14883bbfad43d302d9aa369aa816a751 100644 (file)
@@ -192,7 +192,7 @@ PktFilterLPF::openSocket(Iface& iface,
     int enable = 1;
     if (setsockopt(sock, SOL_SOCKET, SO_TIMESTAMP, &enable, sizeof(enable))) {
         const char* errmsg = strerror(errno);
-        isc_throw(SocketConfigError, "Can't enable SO_TIMESTAMP for " << addr.toText()
+        isc_throw(SocketConfigError, "Can not enable SO_TIMESTAMP for " << addr.toText()
                   << ", error: " << errmsg);
     }
 #endif