From: Marcin Siodelski Date: Tue, 15 Jul 2014 17:51:12 +0000 (+0200) Subject: [2893] Addressed most of the review comments, except for doc update. X-Git-Tag: trac3482_base~81^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e5c085f0e007e86cec31324fcf1811092f4edee1;p=thirdparty%2Fkea.git [2893] Addressed most of the review comments, except for doc update. --- diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index 28f680bca0..d3218f36bd 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -353,7 +353,7 @@ public: /// /// @return Pointer to the first element of the read buffer or /// NULL if the buffer is empty. - uint8_t* getReadBufferPtr() const { + uint8_t* getReadBuffer() const { return (read_buffer_); } diff --git a/src/lib/dhcp/pkt_filter_bpf.cc b/src/lib/dhcp/pkt_filter_bpf.cc index ba8dcf6e07..864cc37a3e 100644 --- a/src/lib/dhcp/pkt_filter_bpf.cc +++ b/src/lib/dhcp/pkt_filter_bpf.cc @@ -177,7 +177,7 @@ PktFilterBPF::openSocket(Iface& iface, // the BPF device below. int fallback = openFallbackSocket(addr, port); - // Fallback has opened so, let's open the BPF device that we will be + // Fallback has opened, so let's open the BPF device that we will be // using for receiving and sending packets. The BPF device is opened // by opening a file /dev/bpf%d where %d is a number. There may be // devices already open so we will try them one by one and open the @@ -190,7 +190,7 @@ PktFilterBPF::openSocket(Iface& iface, s << "/dev/bpf" << bpf_dev; sock = open(s.str().c_str(), O_RDWR, 0); if (sock < 0) { - // If device busy, try another one. + // If device is busy, try another one. if (errno == EBUSY) { continue; } @@ -231,7 +231,11 @@ PktFilterBPF::openSocket(Iface& iface, (ver.bv_minor < BPF_MINOR_VERSION)) { close(fallback); close(sock); - isc_throw(SocketConfigError, "Invalid BPF version"); + isc_throw(SocketConfigError, "Invalid BPF version: " + << ver.bv_major << "." << ver.bv_minor + << " Expected at least version:" + << BPF_MAJOR_VERSION << "." + << BPF_MINOR_VERSION);; } // Get the size of the read buffer for this device. We will need to @@ -323,14 +327,11 @@ PktFilterBPF::receive(const Iface& iface, const SocketInfo& socket_info) { // when the DHCP server is idle. int datalen; do { - datalen = recv(socket_info.fallbackfd_, iface.getReadBufferPtr(), + datalen = recv(socket_info.fallbackfd_, iface.getReadBuffer(), iface.getReadBufferSize(), 0); } while (datalen > 0); - // Now that we finished getting data from the fallback socket, we - // have to get the data from the raw socket too. - memset(iface.getReadBufferPtr(), 0, iface.getReadBufferSize()); - datalen = read(socket_info.sockfd_, iface.getReadBufferPtr(), + datalen = read(socket_info.sockfd_, iface.getReadBuffer(), iface.getReadBufferSize()); // If negative value is returned by read(), it indicates that an // error occured. If returned value is 0, no data was read from the @@ -363,11 +364,11 @@ PktFilterBPF::receive(const Iface& iface, const SocketInfo& socket_info) { // Copy the BPF header. memcpy(static_cast(&bpfh), - static_cast(iface.getReadBufferPtr()), + static_cast(iface.getReadBuffer()), sizeof(bpfh)); // Check if the captured data fit into the reminder of the buffer. - // Again, something is really wrong here. + // Again, something is really wrong here if it doesn't fit. if (offset + bpfh.bh_hdrlen + bpfh.bh_caplen > datalen) { isc_throw(SocketReadError, "packet received from the BPF device" << " attached to interface " << iface.getName() @@ -392,7 +393,7 @@ PktFilterBPF::receive(const Iface& iface, const SocketInfo& socket_info) { } // Skip the BPF header and create the buffer holding a frame. - InputBuffer buf(iface.getReadBufferPtr() + offset + bpfh.bh_hdrlen, + InputBuffer buf(iface.getReadBuffer() + offset + bpfh.bh_hdrlen, datalen - bpfh.bh_hdrlen - offset); @@ -468,7 +469,7 @@ PktFilterBPF::send(const Iface& iface, uint16_t sockfd, const Pkt4Ptr& pkt) { // Some interfaces may have no HW address - e.g. loopback interface. // For these interfaces the HW address length is 0. If this is the case, // then we will rely on the functions which construct the IP/UDP headers - // to provide a default HW addres. Otherwise, create the HW address + // to provide a default HW address. Otherwise, create the HW address // object using the HW address of the interface. if (iface.getMacLen() > 0) { HWAddrPtr hwaddr(new HWAddr(iface.getMac(), iface.getMacLen(), @@ -476,7 +477,7 @@ PktFilterBPF::send(const Iface& iface, uint16_t sockfd, const Pkt4Ptr& pkt) { pkt->setLocalHWAddr(hwaddr); } - /// Local loopback interface requires special treatment. It doesn't + /// Loopback interface requires special treatment. It doesn't /// use the ethernet header but rather a 4-bytes long pseudo header /// holding an address family type (see bpf.c in OS sources). if (iface.flag_loopback_) { @@ -497,9 +498,8 @@ PktFilterBPF::send(const Iface& iface, uint16_t sockfd, const Pkt4Ptr& pkt) { int result = write(sockfd, buf.getData(), buf.getLength()); if (result < 0) { - std::cout << strerror(errno) << std::endl; - isc_throw(SocketWriteError, "failed to send DHCPv4 packet, errno=" - << errno << " (check errno.h)"); + isc_throw(SocketWriteError, "failed to send DHCPv4 packet: " + << strerror(errno)); } return (0); diff --git a/src/lib/dhcp/pkt_filter_bpf.h b/src/lib/dhcp/pkt_filter_bpf.h index 04435b3770..bc9d80719c 100644 --- a/src/lib/dhcp/pkt_filter_bpf.h +++ b/src/lib/dhcp/pkt_filter_bpf.h @@ -25,7 +25,7 @@ namespace dhcp { /// @brief Packet handling class using Berkeley Packet Filtering (BPF) /// /// The BPF is supported on the BSD-like operating systems. It allows for access -/// to low level layers of the inbound and outbound packets. This is specifially +/// to low level layers of the inbound and outbound packets. This is specifically /// useful when the DHCP server is allocating new address to the client. /// /// The response being sent to the client must include the HW address in the diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc index d43e462b32..641b875862 100644 --- a/src/lib/dhcp/tests/iface_mgr_unittest.cc +++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc @@ -65,14 +65,14 @@ TEST(IfaceTest, readBuffer) { // The size of read buffer should initially be 0 and the returned // pointer should be NULL. ASSERT_EQ(0, iface.getReadBufferSize()); - EXPECT_EQ(NULL, iface.getReadBufferPtr()); + EXPECT_EQ(NULL, iface.getReadBuffer()); // Let's resize the buffer. iface.resizeReadBuffer(256); // Check that the buffer has expected size. ASSERT_EQ(256, iface.getReadBufferSize()); // The returned pointer should now be non-NULL. - uint8_t* buf_ptr = iface.getReadBufferPtr(); + uint8_t* buf_ptr = iface.getReadBuffer(); ASSERT_FALSE(buf_ptr == NULL); // Use the pointer to set some data. @@ -81,7 +81,7 @@ TEST(IfaceTest, readBuffer) { } // Get the pointer again and validate the data. - buf_ptr = iface.getReadBufferPtr(); + buf_ptr = iface.getReadBuffer(); ASSERT_EQ(256, iface.getReadBufferSize()); for (int i = 0; i < iface.getReadBufferSize(); ++i) { // Use assert so as it fails on the first failure, no need @@ -1324,7 +1324,7 @@ TEST_F(IfaceMgrTest, setMatchingPacketFilter) { // The fallback socket should fail to open when there is another IP/UDP // socket bound to the same address and port. Failing to open the fallback // socket should preclude the raw socket from being open. -TEST_F(IfaceMgrTest, checkPacketFilterLPFSocket) { +TEST_F(IfaceMgrTest, checkPacketFilterRawSocket) { IOAddress loAddr("127.0.0.1"); int socket1 = -1, socket2 = -1; // Create two instances of IfaceMgr. @@ -1368,15 +1368,16 @@ TEST_F(IfaceMgrTest, checkPacketFilterLPFSocket) { #else -// This non-Linux specific test checks whether it is possible to use -// IfaceMgr to figure out which Pakcket Filter object should be -// used when direct responses to hosts, having no address assigned -// are desired or not desired. Since direct responses aren't supported -// on systems other than Linux the function under test should always -// set object of PktFilterInet type as current Packet Filter. This -// object does not support direct responses. Once implementation is -// added on systems other than BSD and Linux the OS specific version -// of the test will be removed. +// Note: This test will only run on non-Linux and non-BSD systems. +// This test checks whether it is possible to use IfaceMgr to figure +// out which Pakcket Filter object should be used when direct responses +// to hosts, having no address assigned are desired or not desired. +// Since direct responses aren't supported on systems other than Linux +// and BSD the function under test should always set object of +// PktFilterInet type as current Packet Filter. This object does not +//support direct responses. Once implementation is added on systems +// other than BSD and Linux the OS specific version of the test will +// be removed. TEST_F(IfaceMgrTest, setMatchingPacketFilter) { // Create an instance of IfaceMgr.