]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#2834] Addressed review comments
authorPiotrek Zadroga <piotrek@isc.org>
Fri, 26 May 2023 12:21:23 +0000 (14:21 +0200)
committerPiotrek Zadroga <piotrek@isc.org>
Fri, 26 May 2023 13:22:53 +0000 (15:22 +0200)
ChangeLog
doc/sphinx/man/perfdhcp.8.rst
src/bin/perfdhcp/command_options.cc
src/bin/perfdhcp/command_options.h
src/bin/perfdhcp/tests/command_options_unittest.cc
src/bin/perfdhcp/tests/test_control_unittest.cc

index 6bf3810b56f1a2e5a909f703a012554b7c3878c7..5b64244ba3f72b8f5fdc7d3e174a90608bddfaec 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,7 +5,6 @@
     encapsulation is supported.
     (Gitlab #2834)
 
-
 2141.  [bug]           razvan
        The High Availability hook library is sending the 'origin'
        parameter set to 'ha-partner' when sending lease update
index d3127cde1342d0e148883dfb98b4b78971511f69..96cfdbd6ec4f7db7ff60af567cb43313b916b90e 100644 (file)
@@ -15,7 +15,7 @@
 Synopsis
 ~~~~~~~~
 
-:program:`perfdhcp` [**-1**] [**-4** | **-6**] [**-A** encapsulation-level] [**-b** base] [**-B**] [**-c**] [**-C** separator] [**-d** drop-time] [**-D** max-drop] [-e lease-type] [**-E** time-offset] [**-f** renew-rate] [**-F** release-rate] [**-g** thread-mode] [**-h**] [**-i**] [**-I** ip-offset] [**-J** remote-address-list-file] [**-l** local-address|interface] [**-L** local-port] [**-M** mac-list-file] [**-n** num-request] [**-N** remote-port] [**-O** random-offset] [**-o** code,hexstring] [**--o1r** code,hexstring] [**-p** test-period] [**-P** preload] [**-r** rate] [**-R** num-clients] [**-s** seed] [**-S** srvid-offset] [**--scenario** name] [**-t** report] [**-T** template-file] [**-u**] [**-v**] [**-W** exit-wait-time] [**-w** script_name] [**-x** diagnostic-selector] [**-X** xid-offset] [server]
+:program:`perfdhcp` [**-1**] [**-4** | **-6**] [**-A** encapsulation-level] [**-b** base] [**-B**] [**-c**] [**-C** separator] [**-d** drop-time] [**-D** max-drop] [-e lease-type] [**-E** time-offset] [**-f** renew-rate] [**-F** release-rate] [**-g** thread-mode] [**-h**] [**-i**] [**-I** ip-offset] [**-J** remote-address-list-file] [**-l** local-address|interface] [**-L** local-port] [**-M** mac-list-file] [**-n** num-request] [**-N** remote-port] [**-O** random-offset] [**-o** code,hexstring] [**--or** encapsulation-level:code,hexstring] [**-p** test-period] [**-P** preload] [**-r** rate] [**-R** num-clients] [**-s** seed] [**-S** srvid-offset] [**--scenario** name] [**-t** report] [**-T** template-file] [**-u**] [**-v**] [**-W** exit-wait-time] [**-w** script_name] [**-x** diagnostic-selector] [**-X** xid-offset] [server]
 
 Description
 ~~~~~~~~~~~
@@ -357,18 +357,21 @@ The following options only apply for DHCPv6 (i.e. when ``-6`` is given).
    1, which means that the generated traffic is equivalent to the amount of
    traffic passing through a single relay agent.
 
-``--o1r code,hexstring``
+``--or encapsulation-level:code,hexstring``
    This option is very similar to ``-o``, only that it forces ``perfdhcp``
    to insert the specified extra option (or options if used several times)
-   into relayed DHCPv6 message at first level of encapsulation. The code
+   into relayed DHCPv6 message at given level of encapsulation (currently
+   the only supported encapsulation-level value is 1). The code
    specifies the option code and the hexstring is a hexadecimal string that
    defines the content of the option. Care should be taken as ``perfdhcp``
    does not offer any kind of logic behind those options; they are simply
    inserted into packets and sent as is. Be careful not to duplicate
-   options that are already inserted. For example, to insert client
-   class identifier (option code 60) with a string "docsis", use
-   "--o1r 60,646f63736973". The ``--o1r`` may be used multiple times. It must
-   be used together with ``-A``.
+   options that are already inserted. Please notice that ``encapsulation-level:``
+   is optional and if omitted, default encapsulation-level value 1 is used.
+   For example, to insert client class identifier (option code 60) with a
+   string "docsis" at first level of encapsulation, use "--or 60,646f63736973"
+   or "--or 1:60,646f63736973". The ``--or`` may be used multiple times.
+   It must be used together with ``-A``.
 
 Template-Related Options
 ~~~~~~~~~~~~~~~~~~~~~~~~
index a01f967be308b460d027caf738f7ddfdac8349fc..3cf1117501ba5ffd0396de6579c20ff159157479 100644 (file)
@@ -216,7 +216,7 @@ CommandOptions::parse(int argc, char** const argv, bool print_cmd_line) {
 }
 
 const int LONG_OPT_SCENARIO = 300;
-const int LONG_OPT_RELAY_1_OPTION = 400;
+const int LONG_OPT_RELAY_OPTION = 400;
 
 bool
 CommandOptions::initialize(int argc, char** argv, bool print_cmd_line) {
@@ -238,7 +238,7 @@ CommandOptions::initialize(int argc, char** argv, bool print_cmd_line) {
 
     struct option long_options[] = {
         {"scenario", required_argument, 0, LONG_OPT_SCENARIO},
-        {"o1r", required_argument, 0, LONG_OPT_RELAY_1_OPTION},
+        {"or",       required_argument, 0, LONG_OPT_RELAY_OPTION},
         {0,          0,                 0, 0}
     };
 
@@ -603,30 +603,54 @@ CommandOptions::initialize(int argc, char** argv, bool print_cmd_line) {
             break;
         }
 
-        case LONG_OPT_RELAY_1_OPTION: {
+        case LONG_OPT_RELAY_OPTION: {
             // for now this is only available for v6
             // and must be used together with -A option.
             check((ipversion_ != 6),
-                  "-6 must be explicitly specified before --o1r is used.");
+                  "-6 must be explicitly specified before --or is used.");
             check(v6_relay_encapsulation_level_ <= 0,
-                  "-A must be explicitly specified before --o1r is used.");
+                  "-A must be explicitly specified before --or is used.");
 
-            // custom option (expected format: code,hexstring)
+            // custom option (expected format: encapsulation-level:code,hexstring)
             std::string opt_text = std::string(optarg);
+            size_t colon_loc = opt_text.find(':');
             size_t coma_loc = opt_text.find(',');
+
+            // if encapsulation level is skipped by user, let's assume it is 1
+            uint8_t option_encapsulation_level = 1;
+            if (colon_loc == std::string::npos) {
+                // if colon was not found, default encapsulation level will be used
+                // and let's reset colon_loc to -1 so option code could be parsed later
+                colon_loc = -1;
+            } else {
+                // Try to parse the encapsulation level
+                try {
+                    option_encapsulation_level =
+                        boost::lexical_cast<int>(opt_text.substr(0, colon_loc));
+                    check(option_encapsulation_level != 1, "Relayed option encapsulation level "
+                                                           "supports only value 1 at the moment.");
+                } catch (const boost::bad_lexical_cast&) {
+                    isc_throw(InvalidParameter,
+                              "Invalid relayed option encapsulation level specified for "
+                              "--or option, expected format: --or <integer>:<integer>,<hexstring>");
+                }
+            }
+
             check(coma_loc == std::string::npos,
-                  "--o1r option must provide option code, a coma and hexstring for"
-                  " the option content, e.g. --o1r 60,646f63736973 for sending option"
-                  " 60 (class-id) with the value 'docsis'");
+                  "--or option must provide encapsulation level, a colon, option code, a coma and "
+                  "hexstring for the option content, e.g. --or 1:60,646f63736973 for sending option"
+                  " 60 (class-id) with the value 'docsis' at first level of encapsulation");
             int code = 0;
 
             // Try to parse the option code
             try {
-                code = boost::lexical_cast<int>(opt_text.substr(0,coma_loc));
-                check(code <= 0, "Option code can't be negative");
+                code = boost::lexical_cast<int>(
+                    opt_text.substr(colon_loc + 1, coma_loc - colon_loc - 1));
+                check(code <= 0, "Option code can't be negative or zero");
             } catch (const boost::bad_lexical_cast&) {
-                isc_throw(InvalidParameter, "Invalid option code specified for "
-                                            "--o1r option, expected format: --o1r <integer>,<hexstring>");
+                isc_throw(InvalidParameter,
+                          "Invalid option code specified for "
+                          "--or option, expected format: --or <integer>:<integer>,<hexstring>");
             }
 
             // Now try to interpret the hexstring
@@ -635,17 +659,13 @@ CommandOptions::initialize(int argc, char** argv, bool print_cmd_line) {
             try {
                 isc::util::encode::decodeHex(opt_text, bin);
             } catch (const BadValue& e) {
-                isc_throw(InvalidParameter, "Error during decoding option --o1r:"
-                                                << e.what());
+                isc_throw(InvalidParameter, "Error during decoding option --or:" << e.what());
             }
 
             // Create and remember the option.
             OptionPtr option(new Option(Option::V6, code, bin));
-            // For now, only 1 level of encapsulation is allowed for relay options,
-            // thus 1 key is hardcoded below. But in the future, if needed, level of
-            // encapsulation of relay option could be taken from command option.
-            auto relay_1_opts = relay_opts_.find(1);
-            relay_1_opts->second.insert(make_pair(code, option));
+            auto relay_opts = relay_opts_.find(option_encapsulation_level);
+            relay_opts->second.insert(make_pair(code, option));
             break;
         }
         default:
@@ -878,6 +898,16 @@ CommandOptions::generateDuidTemplate() {
     memcpy(&duid_template_[8], &mac_template_[0], 6);
 }
 
+const isc::dhcp::OptionCollection&
+CommandOptions::getRelayOpts(uint8_t encapsulation_level) const {
+    if (encapsulation_level > RELAY_OPTIONS_MAX_ENCAPSULATION) {
+        isc_throw(isc::OutOfRange,
+                  "Trying to access relay options at encapsulation level that doesn't exist");
+    }
+
+    return relay_opts_.find(encapsulation_level)->second;
+}
+
 uint8_t
 CommandOptions::convertHexString(const std::string& text) const {
     unsigned int ui = 0;
index 15d9a6aaa99b2df6a5c8604d2a5e10045edda6e4..3ab8a1e04840685bcbfd026e361d95f6586b2cae 100644 (file)
@@ -384,10 +384,11 @@ public:
     ///
     /// @param encapsulation_level level of encapsulation, by default 1
     ///
+    /// @throws isc::OutOfRange When trying to access relay options at encapsulation
+    /// level that doesn't exist.
+    ///
     /// @return container with options.
-    const isc::dhcp::OptionCollection& getRelayOpts(uint8_t encapsulation_level = 1) const {
-        return relay_opts_.find(encapsulation_level)->second;
-    }
+    const isc::dhcp::OptionCollection& getRelayOpts(uint8_t encapsulation_level = 1) const;
 
     /// \brief Check if single-threaded mode is enabled.
     ///
index e2ac820c2597065c514695854f862e9dfb9bc374..c37a775d89fbcb81c50475929ad83f128d08531f 100644 (file)
@@ -901,33 +901,33 @@ TEST_F(CommandOptionsTest, ElapsedTime) {
 
 TEST_F(CommandOptionsTest, UseRelayV6OptionsWithoutV6) {
     CommandOptions opt;
-    EXPECT_NO_THROW(process(opt, "perfdhcp -6 -A1 --o1r 32,00000E10 -l ethx all"));
+    EXPECT_NO_THROW(process(opt, "perfdhcp -6 -A1 --or 32,00000E10 -l ethx all"));
     EXPECT_TRUE(opt.isUseRelayedV6());
     EXPECT_EQ(1, opt.getRelayOpts().size());
 
-    // --o1r must be used together with -6
-    EXPECT_THROW(process(opt, "perfdhcp -A1 --o1r 32,00000E10 -l ethx all"), isc::InvalidParameter);
+    // --or must be used together with -6
+    EXPECT_THROW(process(opt, "perfdhcp -A1 --or 32,00000E10 -l ethx all"), isc::InvalidParameter);
 }
 
 TEST_F(CommandOptionsTest, UseRelayV6OptionsWithoutRelayEncapsulation) {
     CommandOptions opt;
-    EXPECT_NO_THROW(process(opt, "perfdhcp -6 -A1 --o1r 32,00000E10 -l ethx all"));
+    EXPECT_NO_THROW(process(opt, "perfdhcp -6 -A1 --or 32,00000E10 -l ethx all"));
     EXPECT_TRUE(opt.isUseRelayedV6());
     EXPECT_EQ(1, opt.getRelayOpts().size());
 
-    // --o1r must be used together with -A
-    EXPECT_THROW(process(opt, "perfdhcp -6 --o1r 32,00000E10 -l ethx all"), isc::InvalidParameter);
+    // --or must be used together with -A
+    EXPECT_THROW(process(opt, "perfdhcp -6 --or 32,00000E10 -l ethx all"), isc::InvalidParameter);
 }
 
 TEST_F(CommandOptionsTest, UseMultipleRelayV6Options) {
     CommandOptions opt;
-    EXPECT_NO_THROW(process(opt, "perfdhcp -6 -A1 --o1r 32,00000E10 --o1r "
+    EXPECT_NO_THROW(process(opt, "perfdhcp -6 -A1 --or 32,00000E10 --or "
                                  "23,20010DB800010000000000000000CAFE -l ethx all"));
     EXPECT_TRUE(opt.isUseRelayedV6());
     // 2 options expected at 1st level of encapsulation
     EXPECT_EQ(2, opt.getRelayOpts().size());
     // no options expected at 2nd level of encapsulation
-    EXPECT_EQ(0, opt.getRelayOpts(2).size());
+    EXPECT_THROW(opt.getRelayOpts(2), isc::OutOfRange);
 }
 
 TEST_F(CommandOptionsTest, UseRelayV6OptionsWithMultiSubnets) {
@@ -935,7 +935,7 @@ TEST_F(CommandOptionsTest, UseRelayV6OptionsWithMultiSubnets) {
     std::string relay_addr_list_full_path = getFullPath("relay6-list.txt");
     std::ostringstream cmd;
     cmd << "perfdhcp -6 -J " << relay_addr_list_full_path
-        << " -A1 --o1r 32,00000E10 --o1r 23,20010DB800010000000000000000CAFE -l ethx all";
+        << " -A1 --or 32,00000E10 --or 23,20010DB800010000000000000000CAFE -l ethx all";
     EXPECT_NO_THROW(process(opt, cmd.str()));
     EXPECT_EQ(relay_addr_list_full_path, opt.getRelayAddrListFile());
     EXPECT_TRUE(opt.checkMultiSubnet());
@@ -944,26 +944,64 @@ TEST_F(CommandOptionsTest, UseRelayV6OptionsWithMultiSubnets) {
     // 2 options expected at 1st level of encapsulation
     EXPECT_EQ(2, opt.getRelayOpts().size());
     // no options expected at 2nd level of encapsulation
-    EXPECT_EQ(0, opt.getRelayOpts(2).size());
+    EXPECT_THROW(opt.getRelayOpts(2), isc::OutOfRange);
 }
 
 TEST_F(CommandOptionsTest, UseRelayV6OptionsNoComma) {
     CommandOptions opt;
 
-    // --o1r must be followed by option code, a coma and hexstring
-    EXPECT_THROW(process(opt, "perfdhcp -6 --o1r 3200000E10 -l ethx all"), isc::InvalidParameter);
+    // --or must be followed by encapsulation-level, colon, option code, a coma and hexstring
+    // in case encapsulation-level and colon are skipped, encapsulation-level is by default 1
+    EXPECT_THROW(process(opt, "perfdhcp -6 --or 3200000E10 -l ethx all"), isc::InvalidParameter);
 }
 
 TEST_F(CommandOptionsTest, UseRelayV6OptionsNegativeOptionCode) {
     CommandOptions opt;
 
-    // --o1r must be followed by positive option code, a coma and hexstring
-    EXPECT_THROW(process(opt, "perfdhcp -6 --o1r -32,00000E10 -l ethx all"), isc::InvalidParameter);
+    // --or must be followed by encapsulation-level, colon, positive option code, a coma and hexstring
+    // in case encapsulation-level and colon are skipped, encapsulation-level is by default 1
+    EXPECT_THROW(process(opt, "perfdhcp -6 --or -32,00000E10 -l ethx all"), isc::InvalidParameter);
 }
 
 TEST_F(CommandOptionsTest, UseRelayV6OptionsWrongHexstring) {
     CommandOptions opt;
 
-    // --o1r hexstring containing char Z which is not correct
-    EXPECT_THROW(process(opt, "perfdhcp -6 --o1r 32,Z0000E10 -l ethx all"), isc::InvalidParameter);
+    // --or hexstring containing char Z which is not correct
+    EXPECT_THROW(process(opt, "perfdhcp -6 --or 32,Z0000E10 -l ethx all"), isc::InvalidParameter);
 }
+
+TEST_F(CommandOptionsTest, UseRelayV6OptionsWithEncapsulationLevel) {
+    CommandOptions opt;
+    EXPECT_NO_THROW(process(opt, "perfdhcp -6 -A1 --or 1:32,00000E10 -l ethx all"));
+    EXPECT_TRUE(opt.isUseRelayedV6());
+    EXPECT_EQ(1, opt.getRelayOpts().size());
+}
+
+TEST_F(CommandOptionsTest, UseRelayV6OptionsWithNegativeEncapsulationLevel) {
+    CommandOptions opt;
+
+    // provided Relayed option encapsulation level must be a positive integer.
+    EXPECT_THROW(process(opt, "perfdhcp -6 -A1 --or -1:32,00000E10 -l ethx all"), isc::InvalidParameter);
+}
+
+TEST_F(CommandOptionsTest, UseRelayV6OptionsWithZeroEncapsulationLevel) {
+    CommandOptions opt;
+
+    // provided Relayed option encapsulation level must be a positive integer.
+    EXPECT_THROW(process(opt, "perfdhcp -6 -A1 --or 0:32,00000E10 -l ethx all"), isc::InvalidParameter);
+}
+
+TEST_F(CommandOptionsTest, UseRelayV6OptionsWithWrongEncapsulationLevel) {
+    CommandOptions opt;
+
+    // provided Relayed option encapsulation level must be a positive integer.
+    EXPECT_THROW(process(opt, "perfdhcp -6 -A1 --or x:32,00000E10 -l ethx all"), isc::InvalidParameter);
+}
+
+TEST_F(CommandOptionsTest, UseRelayV6OptionsWithEncapsulationLevelValueTwo) {
+    CommandOptions opt;
+
+    // Relayed option encapsulation level supports only value 1 at the moment.
+    EXPECT_THROW(process(opt, "perfdhcp -6 -A1 --or 2:32,00000E10 -l ethx all"), isc::InvalidParameter);
+}
+
index 7e54cd748fe27499690df04cc73acb6878425c7d..b8133651da70c3a240d3aa3b184a877d81c165bb 100644 (file)
@@ -1500,7 +1500,7 @@ TEST_F(TestControlTest, Packet6Relayed) {
 
 TEST_F(TestControlTest, Packet6RelayedWithRelayOpts) {
     CommandOptions opt;
-    processCmdLine(opt, "perfdhcp -6 -l fake -A1 --o1r 32,00000E10 -L 10547 servers");
+    processCmdLine(opt, "perfdhcp -6 -l fake -A1 --or 1:32,00000E10 -L 10547 servers");
     NakedTestControl tc(opt);
     uint32_t transid = 123;
     boost::shared_ptr<Pkt6> pkt6(new Pkt6(DHCPV6_SOLICIT, transid));