From c1ea0a3e9ce658a252258453f756d3af8a302b66 Mon Sep 17 00:00:00 2001 From: Piotrek Zadroga Date: Fri, 26 May 2023 14:21:23 +0200 Subject: [PATCH] [#2834] Addressed review comments --- ChangeLog | 1 - doc/sphinx/man/perfdhcp.8.rst | 17 +++-- src/bin/perfdhcp/command_options.cc | 70 +++++++++++++------ src/bin/perfdhcp/command_options.h | 7 +- .../tests/command_options_unittest.cc | 70 ++++++++++++++----- .../perfdhcp/tests/test_control_unittest.cc | 2 +- 6 files changed, 119 insertions(+), 48 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6bf3810b56..5b64244ba3 100644 --- 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 diff --git a/doc/sphinx/man/perfdhcp.8.rst b/doc/sphinx/man/perfdhcp.8.rst index d3127cde13..96cfdbd6ec 100644 --- a/doc/sphinx/man/perfdhcp.8.rst +++ b/doc/sphinx/man/perfdhcp.8.rst @@ -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 ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/bin/perfdhcp/command_options.cc b/src/bin/perfdhcp/command_options.cc index a01f967be3..3cf1117501 100644 --- a/src/bin/perfdhcp/command_options.cc +++ b/src/bin/perfdhcp/command_options.cc @@ -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(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 :,"); + } + } + 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(opt_text.substr(0,coma_loc)); - check(code <= 0, "Option code can't be negative"); + code = boost::lexical_cast( + 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 ,"); + isc_throw(InvalidParameter, + "Invalid option code specified for " + "--or option, expected format: --or :,"); } // 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; diff --git a/src/bin/perfdhcp/command_options.h b/src/bin/perfdhcp/command_options.h index 15d9a6aaa9..3ab8a1e048 100644 --- a/src/bin/perfdhcp/command_options.h +++ b/src/bin/perfdhcp/command_options.h @@ -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. /// diff --git a/src/bin/perfdhcp/tests/command_options_unittest.cc b/src/bin/perfdhcp/tests/command_options_unittest.cc index e2ac820c25..c37a775d89 100644 --- a/src/bin/perfdhcp/tests/command_options_unittest.cc +++ b/src/bin/perfdhcp/tests/command_options_unittest.cc @@ -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); +} + diff --git a/src/bin/perfdhcp/tests/test_control_unittest.cc b/src/bin/perfdhcp/tests/test_control_unittest.cc index 7e54cd748f..b8133651da 100644 --- a/src/bin/perfdhcp/tests/test_control_unittest.cc +++ b/src/bin/perfdhcp/tests/test_control_unittest.cc @@ -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(new Pkt6(DHCPV6_SOLICIT, transid)); -- 2.47.3