From: Tomek Mrugalski Date: Mon, 6 Oct 2014 09:14:09 +0000 (+0200) Subject: [3546] Changes after review: X-Git-Tag: trac3489_base~2^2~7 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2b6d7f5e2ab85fd4447be3ffe8f3d09a2ee52764;p=thirdparty%2Fkea.git [3546] Changes after review: - Pkt4, Pkt6 classes now have descriptions - Many descriptions updated in Pkt class - Pkt::addOption() moved back to Pkt (from Pkt6) - DEFAULT_ADDRESS{,6} are now in private namespace. - copyright updated --- diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc index b0c936fb41..de63a1f5a9 100644 --- a/src/lib/dhcp/pkt.cc +++ b/src/lib/dhcp/pkt.cc @@ -48,6 +48,11 @@ Pkt::Pkt(const uint8_t* buf, uint32_t len, const isc::asiolink::IOAddress& local memcpy(&data_[0], buf, len); } +void +Pkt::addOption(const OptionPtr& opt) { + options_.insert(std::pair(opt->getType(), opt)); +} + OptionPtr Pkt::getOption(uint16_t type) const { OptionCollection::const_iterator x = options_.find(type); @@ -59,12 +64,14 @@ Pkt::getOption(uint16_t type) const { bool Pkt::delOption(uint16_t type) { + isc::dhcp::OptionCollection::iterator x = options_.find(type); if (x!=options_.end()) { options_.erase(x); return (true); // delete successful + } else { + return (false); // can't find option to be deleted } - return (false); // can't find option to be deleted } bool @@ -92,17 +99,17 @@ void Pkt::repack() { void Pkt::setRemoteHWAddr(const uint8_t htype, const uint8_t hlen, - const std::vector& mac_addr) { - setHWAddrMember(htype, hlen, mac_addr, remote_hwaddr_); + const std::vector& hw_addr) { + setHWAddrMember(htype, hlen, hw_addr, remote_hwaddr_); } void -Pkt::setRemoteHWAddr(const HWAddrPtr& addr) { - if (!addr) { +Pkt::setRemoteHWAddr(const HWAddrPtr& hw_addr) { + if (!hw_addr) { isc_throw(BadValue, "Setting remote HW address to NULL is" << " forbidden."); } - remote_hwaddr_ = addr; + remote_hwaddr_ = hw_addr; } void diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h index 0e219800d8..92d88e384e 100644 --- a/src/lib/dhcp/pkt.h +++ b/src/lib/dhcp/pkt.h @@ -163,30 +163,31 @@ public: /// @brief Returns reference to output buffer. /// /// Returned buffer will contain reasonable data only for - /// output (TX) packet and after pack() was called. This buffer - /// is only valid till Pkt{4,6} object is valid. + /// output (TX) packet and after pack() was called. /// /// RX packet or TX packet before pack() will return buffer with /// zero length. This buffer is returned as non-const, so hooks /// framework (and user's callouts) can modify them if needed /// + /// @note This buffer is only valid till object that returned it exists. + /// /// @return reference to output buffer isc::util::OutputBuffer& getBuffer() { return (buffer_out_); }; /// @brief Adds an option to this packet. /// - /// @note This is a pure virtual method and must be implemented in - /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective - /// implementations of this method. - /// - /// We can't have a unified method, as DHCPv6 allows option duplicates, - /// while DHCPv4 doesn't. + /// Derived classes may provide more specialized implementations. + /// In particular @c Pkt4 provides one that checks if option is + /// unique. /// /// @param opt option to be added. - virtual void addOption(const OptionPtr& opt) = 0; + virtual void addOption(const OptionPtr& opt); /// @brief Attempts to delete first suboption of requested type. /// + /// If there are several options of the same type present, only + /// the first option will be deleted. + /// /// @param type Type of option to be deleted. /// /// @return true if option was deleted, false if no such option existed @@ -217,16 +218,18 @@ public: /// @brief Returns message type (e.g. 1 = SOLICIT). /// - /// This is a pure virtual method. DHCPv4 stores type in option 53 and - /// DHCPv6 stores it in the header. + /// @note This is a pure virtual method and must be implemented in + /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective + /// implementations of this method. /// /// @return message type virtual uint8_t getType() const = 0; /// @brief Sets message type (e.g. 1 = SOLICIT). /// - /// This is a pure virtual method. DHCPv4 stores type in option 53 and - /// DHCPv6 stores it in the header. + /// @note This is a pure virtual method and must be implemented in + /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective + /// implementations of this method. /// /// @param type message type to be set virtual void setType(uint8_t type) = 0; @@ -241,13 +244,13 @@ public: /// @return transaction-id uint32_t getTransid() const { return (transid_); }; - /// @brief Checks whether a client belongs to a given class + /// @brief Checks whether a client belongs to a given class. /// /// @param client_class name of the class /// @return true if belongs bool inClass(const isc::dhcp::ClientClass& client_class); - /// @brief Adds packet to a specified class + /// @brief Adds packet to a specified class. /// /// A packet can be added to the same class repeatedly. Any additional /// attempts to add to a class the packet already belongs to, will be @@ -264,7 +267,7 @@ public: /// @param client_class name of the class to be added void addClass(const isc::dhcp::ClientClass& client_class); - /// unparsed data (in received packets) + /// @brief Unparsed data (in received packets). /// /// @warning This public member is accessed by derived /// classes directly. One of such derived classes is @@ -278,7 +281,9 @@ public: /// /// Returns the first option of specified type. Note that in DHCPv6 several /// instances of the same option are allowed (and frequently used). - /// Also see \ref getOptions(). + /// Also see \ref Pkt6::getOptions(). + /// + /// The options will be only returned after unpack() is called. /// /// @param type option type we are looking for /// @@ -326,7 +331,7 @@ public: remote_addr_ = remote; } - /// @brief Returns remote IP address + /// @brief Returns remote IP address. /// /// @return remote address const isc::asiolink::IOAddress& getRemoteAddr() const { @@ -347,21 +352,30 @@ public: return (local_addr_); } - /// @brief Sets local port. + /// @brief Sets local UDP (and soon TCP) port. + /// + /// This sets a local port, i.e. destination port for recently received + /// packet or a source port for to be transmitted packet. /// /// @param local specifies local port void setLocalPort(uint16_t local) { local_port_ = local; } - /// @brief Returns local port. + /// @brief Returns local UDP (and soon TCP) port. + /// + /// This sets a local port, i.e. destination port for recently received + /// packet or a source port for to be transmitted packet. /// /// @return local port uint16_t getLocalPort() const { return (local_port_); } - /// @brief Sets remote port. + /// @brief Sets remote UDP (and soon TCP) port. + /// + /// This sets a remote port, i.e. source port for recently received + /// packet or a destination port for to be transmitted packet. /// /// @param remote specifies remote port void setRemotePort(uint16_t remote) { @@ -395,7 +409,7 @@ public: /// going to be transmitted. /// /// @return interface name - std::string getIface() const { return iface_; }; + std::string getIface() const { return (iface_); }; /// @brief Sets interface name. /// @@ -405,22 +419,22 @@ public: /// @return interface name void setIface(const std::string& iface ) { iface_ = iface; }; - /// @brief Sets remote HW address. + /// @brief Sets remote hardware address. /// - /// Sets hardware address from an existing HWAddr structure. + /// Sets hardware address (MAC) from an existing HWAddr structure. /// The remote address is a destination address for outgoing /// packet and source address for incoming packet. When this /// is an outgoing packet, this address will be used to /// construct the link layer header. /// - /// @param addr structure representing HW address. + /// @param hw_addr structure representing HW address. /// /// @throw BadValue if addr is null - void setRemoteHWAddr(const HWAddrPtr& addr); + void setRemoteHWAddr(const HWAddrPtr& hw_addr); - /// @brief Sets remote HW address. + /// @brief Sets remote hardware address. /// - /// Sets the destination HW address for the outgoing packet + /// Sets the destination hardware (MAC) address for the outgoing packet /// or source HW address for the incoming packet. When this /// is an outgoing packet this address will be used to construct /// the link layer header. @@ -435,9 +449,9 @@ public: /// /// @param htype hardware type (will be sent in htype field) /// @param hlen hardware length (will be sent in hlen field) - /// @param mac_addr pointer to hardware address + /// @param hw_addr pointer to hardware address void setRemoteHWAddr(const uint8_t htype, const uint8_t hlen, - const std::vector& mac_addr); + const std::vector& hw_addr); /// @brief Returns the remote HW address obtained from raw sockets. /// @@ -493,23 +507,29 @@ public: protected: - /// transaction-id (32 bits for v4, 24 bits for v6) + /// Transaction-id (32 bits for v4, 24 bits for v6) uint32_t transid_; - /// name of the network interface the packet was received/to be sent over + /// Name of the network interface the packet was received/to be sent over. std::string iface_; - /// @brief interface index + + /// @brief Interface index. /// - /// interface index (each network interface has assigned unique ifindex - /// it is functional equivalent of name, but sometimes more useful, e.g. - /// when using crazy systems that allow spaces in interface names - /// e.g. windows + /// Each network interface has assigned an unique ifindex. + /// It is a functional equivalent of a name, but sometimes more useful, e.g. + /// when using odd systems that allow spaces in interface names. int ifindex_; - /// local address (dst if receiving packet, src if sending packet) + /// @brief Local IP (v4 or v6) address. + /// + /// Specifies local IPv4 or IPv6 address. It is a destination address for + /// received packet, and a source address if it is packet being transmitted. isc::asiolink::IOAddress local_addr_; - /// remote address (src if receiving packet, dst if sending packet) + /// @brief Remote IP address. + /// + /// Specifies local IPv4 or IPv6 address. It is source address for received + /// packet and a destination address for packet being transmitted. isc::asiolink::IOAddress remote_addr_; /// local TDP or UDP port diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc index 1130eb256a..41c8aa7355 100644 --- a/src/lib/dhcp/pkt4.cc +++ b/src/lib/dhcp/pkt4.cc @@ -27,10 +27,14 @@ using namespace std; using namespace isc::dhcp; using namespace isc::asiolink; -namespace isc { -namespace dhcp { +namespace { +/// @brief Default address used in Pkt4 constructor const IOAddress DEFAULT_ADDRESS("0.0.0.0"); +} + +namespace isc { +namespace dhcp { Pkt4::Pkt4(uint8_t msg_type, uint32_t transid) :Pkt(transid, DEFAULT_ADDRESS, DEFAULT_ADDRESS, DHCP4_SERVER_PORT, @@ -410,7 +414,8 @@ Pkt4::addOption(const OptionPtr& opt) { isc_throw(BadValue, "Option " << opt->getType() << " already present in this message."); } - options_.insert(pair >(opt->getType(), opt)); + + Pkt::addOption(opt); } bool diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h index 17eddd98c5..90fbc34f3c 100644 --- a/src/lib/dhcp/pkt4.h +++ b/src/lib/dhcp/pkt4.h @@ -34,6 +34,13 @@ namespace isc { namespace dhcp { +/// @brief Represents DHCPv4 packet +/// +/// This class represents a single DHCPv4 packet. It handles both incoming +/// and transmitted packets, parsing incoming options, options handling +/// (add, get, remove), on-wire assembly, sanity checks and other operations. +/// This specific class has several DHCPv4-specific methods, but it uses a lot +/// of common operations from its base @c Pkt class that is shared with Pkt6. class Pkt4 : public Pkt { public: @@ -155,7 +162,6 @@ public: /// @return flags field uint16_t getFlags() const { return (flags_); }; - /// @brief Returns ciaddr field. /// /// @return ciaddr field @@ -181,7 +187,6 @@ public: void setSiaddr(const isc::asiolink::IOAddress& siaddr) { siaddr_ = siaddr; }; - /// @brief Returns yiaddr field. /// /// @return yiaddr field @@ -194,7 +199,6 @@ public: void setYiaddr(const isc::asiolink::IOAddress& yiaddr) { yiaddr_ = yiaddr; }; - /// @brief Returns giaddr field. /// /// @return giaddr field @@ -435,6 +439,7 @@ protected: // end of real DHCPv4 fields }; // Pkt4 class +/// @brief A pointer to Pkt4 object. typedef boost::shared_ptr Pkt4Ptr; } // isc::dhcp namespace diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc index ca073ce003..a47bd67259 100644 --- a/src/lib/dhcp/pkt6.cc +++ b/src/lib/dhcp/pkt6.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -24,11 +24,12 @@ using namespace std; using namespace isc::asiolink; +/// @brief Default address used in Pkt6 constructor +const IOAddress DEFAULT_ADDRESS6("::"); + namespace isc { namespace dhcp { -const IOAddress DEFAULT_ADDRESS6("::"); - Pkt6::RelayInfo::RelayInfo() :msg_type_(0), hop_count_(0), linkaddr_("::"), peeraddr_("::"), relay_msg_len_(0) { @@ -113,11 +114,6 @@ OptionPtr Pkt6::getAnyRelayOption(uint16_t opt_type, RelaySearchOrder order) { return (OptionPtr()); } -void -Pkt6::addOption(const OptionPtr& opt) { - options_.insert(std::pair(opt->getType(), opt)); -} - OptionPtr Pkt6::getRelayOption(uint16_t opt_type, uint8_t relay_level) { if (relay_level >= relay_info_.size()) { isc_throw(OutOfRange, "This message was relayed " << relay_info_.size() << " time(s)." diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h index fc439a879e..9020c5830a 100644 --- a/src/lib/dhcp/pkt6.h +++ b/src/lib/dhcp/pkt6.h @@ -33,8 +33,21 @@ namespace isc { namespace dhcp { class Pkt6; + +/// @brief A pointer to Pkt6 packet typedef boost::shared_ptr Pkt6Ptr; +/// @brief Represents a DHCPv6 packet +/// +/// This class represents a single DHCPv6 packet. It handles both incoming +/// and transmitted packets, parsing incoming options, options handling +/// (add, get, remove), on-wire assembly, sanity checks and other operations. +/// This specific class has several DHCPv6-specific methods, but it uses a lot +/// of common operations from its base @c Pkt class that is shared with Pkt4. +/// +/// This class also handles relayed packets. For example, a RELAY-FORW message +/// with a SOLICIT inside will be represented as SOLICIT and the RELAY-FORW +/// layers will be stored in relay_info_ vector. class Pkt6 : public Pkt { public: /// specifies non-relayed DHCPv6 packet header length (over UDP) @@ -132,12 +145,6 @@ public: /// @return true if parsing was successful virtual bool unpack(); - /// @brief Adds an option. - /// - /// @param opt option to be added - virtual void - addOption(const OptionPtr& opt); - /// @brief Returns protocol of this packet (UDP or TCP). /// /// @return protocol type diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc index 0c09b412c0..0de4ceb329 100644 --- a/src/lib/dhcp/tests/pkt4_unittest.cc +++ b/src/lib/dhcp/tests/pkt4_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index bf121f08ea..fb91bb242d 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above