linux: cache netmask for accurate subnet prefix calculation to fix implementation which always returned 0 (#1155)

Co-authored-by: Syed Amer Gilani <syed.gilani@trilux.com>

ports/linux/bip-init.c

@@ -45,6 +45,8 @@ static uint16_t BIP_Port;
 static struct in_addr BIP_Address;
 /* IP broadcast address - stored here in network byte order */
 static struct in_addr BIP_Broadcast_Addr;
+/* IP netmask - stored here in network byte order */
+static struct in_addr BIP_Netmask;
 /* broadcast binding mechanism */
 static bool BIP_Broadcast_Binding_Address_Override;
 static struct in_addr BIP_Broadcast_Binding_Address;
@@ -233,9 +235,24 @@ bool bip_get_broadcast_addr(BACNET_IP_ADDRESS *addr)
  */
 bool bip_set_subnet_prefix(uint8_t prefix)
 {
-    /* not something we do within this driver */
+    uint32_t mask = 0;
-    (void)prefix;
+
-    return false;
+    if ((prefix == 0) || (prefix > 32)) {
+        return false;
+    }
+
+    mask = (prefix == 32) ? UINT32_MAX : (UINT32_MAX << (32 - prefix));
+    BIP_Netmask.s_addr = htonl(mask);
+
+#if !defined(BACNET_IP_BROADCAST_USE_CLASSADDR)
+    if ((BIP_Address.s_addr != 0) && !BIP_Broadcast_Binding_Address_Override) {
+        uint32_t address = ntohl(BIP_Address.s_addr);
+        uint32_t broadcast = (address & mask) | (~mask);
+        BIP_Broadcast_Addr.s_addr = htonl(broadcast);
+    }
+#endif
+
+    return true;
 }
 
 /**
@@ -244,21 +261,24 @@ bool bip_set_subnet_prefix(uint8_t prefix)
  */
 uint8_t bip_get_subnet_prefix(void)
 {
-    uint32_t address = 0;
+    uint32_t mask = 0;
-    uint32_t broadcast = 0;
-    uint32_t mask = 0xFFFFFFFE;
     uint8_t prefix = 0;
 
-    address = BIP_Address.s_addr;
+    mask = ntohl(BIP_Netmask.s_addr);
-    broadcast = BIP_Broadcast_Addr.s_addr;
+    if (mask == 0) {
-    /* calculate the subnet prefix from the broadcast address */
+        return 0;
-    for (prefix = 1; prefix <= 32; prefix++) {
+    }
-        if ((address | mask) == broadcast) {
+
-            break;
+    while ((mask & 0x80000000) != 0) {
-        }
+        prefix++;
         mask = mask << 1;
     }
 
+    if (mask != 0) {
+        /* non-contiguous netmask */
+        return 0;
+    }
+
     return prefix;
 }
 
@@ -774,6 +794,13 @@ void bip_set_interface(const char *ifname)
         fprintf(stderr, "BIP: Address: %s\n", inet_ntoa(local_address));
         fflush(stderr);
     }
+    /* cache interface netmask */
+    rv = bip_get_local_address_ioctl(ifname, &netmask, SIOCGIFNETMASK);
+    if (rv < 0) {
+        BIP_Netmask.s_addr = 0;
+    } else {
+        BIP_Netmask.s_addr = netmask.s_addr;
+    }
     /* setup local broadcast address */
 #ifdef BACNET_IP_BROADCAST_USE_CLASSADDR
     long broadcast_address;
@@ -798,7 +825,6 @@ void bip_set_interface(const char *ifname)
     }
     BIP_Broadcast_Addr.s_addr = htonl(broadcast_address);
 #else
-    rv = bip_get_local_address_ioctl(ifname, &netmask, SIOCGIFNETMASK);
     if (rv < 0) {
         BIP_Broadcast_Addr.s_addr = ~0;
     } else {
@@ -980,6 +1006,7 @@ void bip_cleanup(void)
     /* these were set non-zero during interface configuration */
     BIP_Address.s_addr = 0;
     BIP_Broadcast_Addr.s_addr = 0;
+    BIP_Netmask.s_addr = 0;
 
     return;
 }

test/CMakeLists.txt

@@ -231,6 +231,7 @@ elseif(${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
 
   list(APPEND testdirs
   ports/linux/bsc_event
+  ports/linux/bip_subnet
   )
 
 elseif(WIN32)

test/ports/linux/bip_subnet/CMakeLists.txt

@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: MIT
+
+cmake_minimum_required(VERSION 3.10 FATAL_ERROR)
+get_filename_component(basename ${CMAKE_CURRENT_SOURCE_DIR} NAME)
+
+project(test_${basename}
+    VERSION 1.0.0
+    LANGUAGES C)
+
+find_package(Threads)
+
+set(CMAKE_C_FLAGS -pthread)
+
+string(REGEX REPLACE
+    "/test/ports/[a-zA-Z_/-]*$"
+    "/src"
+    SRC_DIR
+    ${CMAKE_CURRENT_SOURCE_DIR})
+string(REGEX REPLACE
+    "/test/ports/[a-zA-Z_/-]*$"
+    "/ports"
+    PORTS_DIR
+    ${CMAKE_CURRENT_SOURCE_DIR})
+string(REGEX REPLACE
+    "/test/ports/[a-zA-Z_/-]*$"
+    "/test"
+    TST_DIR
+    ${CMAKE_CURRENT_SOURCE_DIR})
+set(ZTST_DIR "${TST_DIR}/ztest/src")
+
+add_compile_definitions(
+    BIG_ENDIAN=0
+    CONFIG_ZTEST=1
+    )
+
+include_directories(
+    ${SRC_DIR}
+    ${TST_DIR}/ztest/include
+    )
+
+add_executable(${PROJECT_NAME}
+  ${PORTS_DIR}/linux/bip-init.c
+  ./src/bvlc_stubs.c
+  ./src/main.c
+  ${ZTST_DIR}/ztest_mock.c
+  ${ZTST_DIR}/ztest.c
+  )
+
+target_link_libraries(${PROJECT_NAME} Threads::Threads)

test/ports/linux/bip_subnet/src/bvlc_stubs.c

@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: MIT */
+#include "bacnet/basic/bbmd/h_bbmd.h"
+
+int bvlc_handler(
+    BACNET_IP_ADDRESS *addr,
+    BACNET_ADDRESS *src,
+    uint8_t *npdu,
+    uint16_t npdu_len)
+{
+    (void)addr;
+    (void)src;
+    (void)npdu;
+    (void)npdu_len;
+
+    return 0;
+}
+
+int bvlc_broadcast_handler(
+    BACNET_IP_ADDRESS *addr,
+    BACNET_ADDRESS *src,
+    uint8_t *npdu,
+    uint16_t npdu_len)
+{
+    (void)addr;
+    (void)src;
+    (void)npdu;
+    (void)npdu_len;
+
+    return 0;
+}
+
+int bvlc_send_pdu(
+    const BACNET_ADDRESS *dest,
+    const BACNET_NPDU_DATA *npdu_data,
+    const uint8_t *pdu,
+    unsigned pdu_len)
+{
+    (void)dest;
+    (void)npdu_data;
+    (void)pdu;
+    (void)pdu_len;
+
+    return 0;
+}
+
+void bvlc_init(void)
+{
+}

test/ports/linux/bip_subnet/src/main.c

@@ -0,0 +1,49 @@
+/**
+ * @file
+ * @brief Tests for BACnet/IP subnet prefix caching on Linux
+ * @copyright SPDX-License-Identifier: MIT
+ */
+#include <zephyr/ztest.h>
+#include <bacnet/datalink/bip.h>
+
+static void test_prefix_defaults_to_zero(void)
+{
+    bip_cleanup();
+    zassert_equal(
+        bip_get_subnet_prefix(), 0, "Prefix should be zero by default");
+}
+
+static void test_prefix_roundtrip(void)
+{
+    bip_cleanup();
+    zassert_true(bip_set_subnet_prefix(24), NULL);
+    zassert_equal(bip_get_subnet_prefix(), 24, NULL);
+
+    zassert_true(bip_set_subnet_prefix(16), NULL);
+    zassert_equal(bip_get_subnet_prefix(), 16, NULL);
+}
+
+static void test_prefix_invalid_values(void)
+{
+    bip_cleanup();
+    zassert_false(bip_set_subnet_prefix(0), NULL);
+    zassert_false(bip_set_subnet_prefix(33), NULL);
+    zassert_equal(bip_get_subnet_prefix(), 0, NULL);
+}
+
+static void test_prefix_all_bits_set(void)
+{
+    bip_cleanup();
+    zassert_true(bip_set_subnet_prefix(32), NULL);
+    zassert_equal(bip_get_subnet_prefix(), 32, NULL);
+}
+
+void test_main(void)
+{
+    ztest_test_suite(
+        bip_subnet_test, ztest_unit_test(test_prefix_defaults_to_zero),
+        ztest_unit_test(test_prefix_roundtrip),
+        ztest_unit_test(test_prefix_invalid_values),
+        ztest_unit_test(test_prefix_all_bits_set));
+    ztest_run_test_suite(bip_subnet_test);
+}