Incorrect Return Value of IfaceMgr::send (#417) · Issues · ISC Open Source Projects / Kea

Incorrect Return Value of IfaceMgr::send

Describe the bug

Precondition: PktFilterLPF or PktFilterBPF is used

If IfaceMgr::send(const Pkt4Ptr& pkt) is called, it returns false if successfull.

The Documentation and the header state: /// @return true if sending was successful bool send(const Pkt4Ptr& pkt);

The return statement of this method looks like this: return (packet_filter_->send(*iface, getSocket(*pkt).sockfd_, pkt));

PktFilter::send shall return 0 on success, which converts to false. pkt_filter.h: class PktFilter { /// @return result of sending the packet. It is 0 if successful. virtual int send(const Iface& iface, uint16_t sockfd, const Pkt4Ptr& pkt) = 0;

This behavior is not reproducible if PktFilterInet is used. The reason is that it returns the RV of sendmsg. sendmsg returns the number of sent characters.

pkt_filter_inet.cc: int result = sendmsg(sockfd, &m, 0); return (result);

This means PktFilterInet::send will most likely return values larger than 0 if successfull.

PktFilterBPF performs correct, just as PktFilterLPF does.

To Reproduce Steps to reproduce the behavior:

Run Kea dhcpv4 with the default PktFilter (PktFilterLpf) on a Linux Machine
Register a Plugin and send a packet by calling IfaceMgr::send(const Pkt4Ptr& pkt)

Expected behavior The Documentation states that the method shall return true on success. In fact it returns false.

Environment:

Kea version: 1.3.0 (git 62af6072a51d2fa319268e9ca615e244506fc5ef) But the bug is still present on the current master branch
OS: Linux OS based on Kernel 4.14
Which features were compiled in
```
Kea source configure results:
```
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Package: Name: kea Version: 1.3.0 Extended version:1.3.0 (git 62af6072a51d2fa319268e9ca615e244506fc5ef) OS Family: Linux Using GNU sed: yes Premium package: no

C++ Compiler: CXX: g++ CXX_VERSION: g++ (Debian 6.3.0-18+deb9u1) 6.3.0 20170516 CXX_STANDARD: 201402 DEFS: -DHAVE_CONFIG_H CPPFLAGS: -DOS_LINUX -I$(top_srcdir)/ext/coroutine -DBOOST_ASIO_HEADER_ONLY -DBOOST_ASIO_DISABLE_THREADS=1 CXXFLAGS: -g -O2 LDFLAGS: -Wl,-R/build/3.5.0.x/botan/_/usr/lib -lpthread KEA_CXXFLAGS: -Wall -Wextra -Wnon-virtual-dtor -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare -pthread -Wno-missing-field-initializers -fPIC

Python: PYTHON_VERSION: not needed (because kea-shell is disabled)

Boost: BOOST_VERSION: 1.67 BOOST_INCLUDES: BOOST_LIBS: -lboost_system

Botan: CRYPTO_VERSION: 2.4.0 CRYPTO_CFLAGS: CRYPTO_INCLUDES: -I/build/3.5.0.x/botan//usr/include/botan-2 CRYPTO_LDFLAGS: -L/build/3.5.0.x/botan//usr/lib CRYPTO_LIBS: -L/build/3.5.0.x/botan/_/usr/lib -lbotan-2 -lbotan-2

OpenSSL: no

Log4cplus: LOG4CPLUS_VERSION: 1.2.0 LOG4CPLUS_INCLUDES: LOG4CPLUS_LIBS: -llog4cplus

Flex/bison: FLEX: flex BISON: bison -y

MySQL: no

PostgreSQL: no

Cassandra CQL: no

Developer: Enable Debugging: yes Google Tests: no Valgrind: found C++ Code Coverage: no Logger checks: no Generate Documentation: no Parser Generation: no Kea-shell: no

Hooks loaded: lease4_renew, lease4_select, lease6_rebind, lease6_renew, lease6_select, pkt4_receive, pkt4_send, pkt6_receive, pkt6_send

Describe the solution you'd like Fix of IfaceMgr::send(const Pkt4Ptr& pkt) and PktFilterInet::send(const Iface& iface, uint16_t sockfd, const Pkt4Ptr& pkt) to make them perform according to the Documentation.

Contacting you Email me at matthias.stoeckl@secunet.com

**Describe the bug**

Precondition: PktFilterLPF or PktFilterBPF is used

If  IfaceMgr::send(const Pkt4Ptr& pkt) is called, it returns false if successfull.

The Documentation and the header state:
/// @return true if sending was successful
bool send(const Pkt4Ptr& pkt);

The return statement of this method looks like this:
return (packet_filter_->send(*iface, getSocket(*pkt).sockfd_, pkt));

PktFilter::send shall return 0 on success, which converts to false.
pkt_filter.h:
class PktFilter {
/// @return result of sending the packet. It is 0 if successful.
virtual int send(const Iface& iface, uint16_t sockfd,
                     const Pkt4Ptr& pkt) = 0;

This behavior is not reproducible if PktFilterInet is used.
The reason is that it returns the RV of sendmsg.
sendmsg returns the number of sent characters.

pkt_filter_inet.cc:
int result = sendmsg(sockfd, &m, 0);
return (result);

This means PktFilterInet::send will most likely return values larger than 0 if successfull.

PktFilterBPF performs correct, just as PktFilterLPF does.

**To Reproduce**
Steps to reproduce the behavior:
1. Run Kea dhcpv4 with the default PktFilter (PktFilterLpf) on a Linux Machine
2. Register a Plugin and send a packet by calling IfaceMgr::send(const Pkt4Ptr& pkt)

**Expected behavior**
The Documentation states that the method shall return true on success. In fact it returns false.

**Environment:**
 - Kea version: 1.3.0 (git 62af6072a51d2fa319268e9ca615e244506fc5ef)
But the bug is still present on the current master branch
 - OS: Linux OS based on Kernel 4.14
 - Which features were compiled in

Kea source configure results:
    -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Package:
  Name:            kea
  Version:         1.3.0
  Extended version:1.3.0 (git 62af6072a51d2fa319268e9ca615e244506fc5ef)
  OS Family:       Linux
  Using GNU sed:   yes
  Premium package: no

C++ Compiler:
  CXX:             g++
  CXX_VERSION:     g++ (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
  CXX_STANDARD:    201402
  DEFS:            -DHAVE_CONFIG_H
  CPPFLAGS:         -DOS_LINUX  -I$(top_srcdir)/ext/coroutine -DBOOST_ASIO_HEADER_ONLY -DBOOST_ASIO_DISABLE_THREADS=1
  CXXFLAGS:        -g -O2
  LDFLAGS:          -Wl,-R/build/3.5.0.x/botan/_/usr/lib    -lpthread
  KEA_CXXFLAGS:     -Wall -Wextra -Wnon-virtual-dtor -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare -pthread -Wno-missing-field-initializers -fPIC

Python:
  PYTHON_VERSION:  not needed (because kea-shell is disabled)

Boost:
  BOOST_VERSION:   1.67
  BOOST_INCLUDES:
  BOOST_LIBS:       -lboost_system

Botan:
  CRYPTO_VERSION:  2.4.0
  CRYPTO_CFLAGS:
  CRYPTO_INCLUDES: -I/build/3.5.0.x/botan/_/usr/include/botan-2
  CRYPTO_LDFLAGS:   -L/build/3.5.0.x/botan/_/usr/lib
  CRYPTO_LIBS:     -L/build/3.5.0.x/botan/_/usr/lib -lbotan-2  -lbotan-2

OpenSSL: no

Log4cplus:
  LOG4CPLUS_VERSION: 1.2.0
  LOG4CPLUS_INCLUDES:
  LOG4CPLUS_LIBS:   -llog4cplus

Flex/bison:
  FLEX:  flex
  BISON: bison -y

MySQL:
  no

PostgreSQL:
  no

Cassandra CQL:
  no

Developer:
  Enable Debugging: yes
  Google Tests: no
  Valgrind: found
  C++ Code Coverage: no
  Logger checks: no
  Generate Documentation: no
  Parser Generation: no
  Kea-shell: no

- Hooks loaded: lease4_renew, lease4_select, lease6_rebind, lease6_renew, lease6_select, pkt4_receive, pkt4_send, pkt6_receive, pkt6_send

**Describe the solution you'd like**
Fix of  IfaceMgr::send(const Pkt4Ptr& pkt) and
PktFilterInet::send(const Iface& iface, uint16_t sockfd,
                     const Pkt4Ptr& pkt)
to make them perform according to the Documentation.

**Contacting you**
Email me at matthias.stoeckl@secunet.com

Edited Jan 18, 2019 by matstoeckl

To upload designs, you'll need to enable LFS. More information