From fa18fe1e9236f44fe40c3048685646ab1997ccf1 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 10 Jun 2019 23:08:23 +0200 Subject: [PATCH 01/35] Obtain network information from ip command instead of through procfs Signed-off-by: DL6ER --- networktable.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/networktable.c b/networktable.c index 2a35e1ca1..3fc7903db 100644 --- a/networktable.c +++ b/networktable.c @@ -11,7 +11,6 @@ #include "FTL.h" #include "shmem.h" #include "sqlite3.h" -#define ARPCACHE "/proc/net/arp" // Private prototypes static char* getMACVendor(const char* hwaddr); @@ -39,14 +38,14 @@ bool create_network_table(void) return true; } -// Read kernel's ARP cache using procfs +// Parse kernel's ARP cache void parse_arp_cache(void) { FILE* arpfp = NULL; - // Try to access the kernel's ARP cache - if((arpfp = fopen(ARPCACHE, "r")) == NULL) + // Try to access the kernel's ARP/NDP cache + if((arpfp = popen("ip neigh show", "r")) == NULL) { - logg("WARN: Opening of %s failed!", ARPCACHE); + logg("WARN: Command \"ip neigh show\" failed!"); logg(" Message: %s", strerror(errno)); return; } @@ -54,8 +53,8 @@ void parse_arp_cache(void) // Open database file if(!dbopen()) { - logg("read_arp_cache() - Failed to open DB"); - fclose(arpfp); + logg("parse_arp_cache() - Failed to open DB"); + pclose(arpfp); return; } @@ -65,8 +64,8 @@ void parse_arp_cache(void) // Prepare buffers char * linebuffer = NULL; size_t linebuffersize = 0; - char ip[100], mask[100], hwaddr[100], iface[100]; - unsigned int type, flags, entries = 0; + char ip[100], status[100], hwaddr[100], iface[100]; + unsigned int entries = 0; time_t now = time(NULL); // Start collecting database commands @@ -76,15 +75,16 @@ void parse_arp_cache(void) // Read ARP cache line by line while(getline(&linebuffer, &linebuffersize, arpfp) != -1) { - int num = sscanf(linebuffer, "%99s 0x%x 0x%x %99s %99s %99s\n", - ip, &type, &flags, hwaddr, mask, iface); + int num = sscanf(linebuffer, "%99s dev %99s lladdr %99s %99s", + ip, iface, hwaddr, status); - // Skip header and empty lines - if (num < 4) + // Skip addresses without hardware address information + if (num != 4) continue; - // Skip incomplete entires, i.e., entries without C (complete) flag - if(!(flags & 0x02)) + // Only process active entries + if(strcasecmp(status, "reachable") != 0 && + strcasecmp(status, "stale") != 0) continue; // Get ID of this device in our network database. If it cannot be @@ -102,7 +102,7 @@ void parse_arp_cache(void) int ret = asprintf(&querystr, "SELECT id FROM network WHERE hwaddr = \'%s\';", hwaddr); if(querystr == NULL || ret < 0) { - logg("Memory allocation failed in parse_arp_cache (%i)", ret); + logg("Memory allocation failed in parse_arp_cache(): %i", ret); break; } @@ -202,7 +202,7 @@ void parse_arp_cache(void) logg("ARP table processing (%i entries) took %.1f ms", entries, timer_elapsed_msec(ARP_TIMER)); // Close file handle - fclose(arpfp); + pclose(arpfp); // Close database connection dbclose(); From 264e3dbf57170d328382e4e543d33c08a417b9b3 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 10 Jun 2019 23:19:54 +0200 Subject: [PATCH 02/35] Create network-addresses table. This updates the databsae version to 5 Signed-off-by: DL6ER --- database.c | 19 +++++++++++++++++-- networktable.c | 20 ++++++++++++++++++-- routines.h | 3 ++- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/database.c b/database.c index 43ba8395b..86ddf5be7 100644 --- a/database.c +++ b/database.c @@ -279,6 +279,21 @@ void db_init(void) dbversion = db_get_FTL_property(DB_VERSION); } + // Update to version 5 if lower + if(dbversion < 5) + { + // Update to version 5: Create network-addresses table + logg("Updating long-term database to version 5"); + if(!create_network_addresses_table()) + { + logg("Network-addresses table not initialized, database not available"); + database = false; + return; + } + // Get updated version + dbversion = db_get_FTL_property(DB_VERSION); + } + // Close database to prevent having it opened all time // we already closed the database when we returned earlier sqlite3_close(db); @@ -674,9 +689,9 @@ void *DB_thread(void *val) DBdeleteoldqueries = false; } - // Parse ARP cache (fill network table) if enabled + // Parse neighbor cache (fill network table) if enabled if (config.parse_arp_cache) - parse_arp_cache(); + parse_neigh_cache(); } sleepms(100); } diff --git a/networktable.c b/networktable.c index 3fc7903db..bf5bd1b7f 100644 --- a/networktable.c +++ b/networktable.c @@ -38,8 +38,24 @@ bool create_network_table(void) return true; } -// Parse kernel's ARP cache -void parse_arp_cache(void) +bool create_network_addresses_table(void) +{ + bool ret; + // Create network-addresses table in the database + ret = dbquery("CREATE TABLE network-addresses ( id INTEGER NOT NULL, " \ + "lastQuery INTEGER NOT NULL, " \ + "ip TEXT NOT NULL);"); + if(!ret){ dbclose(); return false; } + + // Update database version to 5 + ret = db_set_FTL_property(DB_VERSION, 5); + if(!ret){ dbclose(); return false; } + + return true; +} + +// Parse kernel's neighbor cache +void parse_neigh_cache(void) { FILE* arpfp = NULL; // Try to access the kernel's ARP/NDP cache diff --git a/routines.h b/routines.h index 5bee30594..295d73ab3 100644 --- a/routines.h +++ b/routines.h @@ -156,7 +156,8 @@ bool check_capabilities(void); // networktable.c bool create_network_table(void); -void parse_arp_cache(void); +bool create_network_addresses_table(void); +void parse_neigh_cache(void); void updateMACVendorRecords(void); // gravity.c From 836c35965e6115f1dcaaf77d9b79c1797df5c951 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 10 Jun 2019 23:46:27 +0200 Subject: [PATCH 03/35] Store addresses in new table network_addresses Signed-off-by: DL6ER --- networktable.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/networktable.c b/networktable.c index bf5bd1b7f..310074a6f 100644 --- a/networktable.c +++ b/networktable.c @@ -41,10 +41,11 @@ bool create_network_table(void) bool create_network_addresses_table(void) { bool ret; - // Create network-addresses table in the database - ret = dbquery("CREATE TABLE network-addresses ( id INTEGER NOT NULL, " \ + // Create network_addresses table in the database + ret = dbquery("CREATE TABLE network_addresses ( id INTEGER NOT NULL, " \ + "ip TEXT NOT NULL, "\ "lastQuery INTEGER NOT NULL, " \ - "ip TEXT NOT NULL);"); + "UNIQUE(id,ip));"); if(!ret){ dbclose(); return false; } // Update database version to 5 @@ -58,7 +59,7 @@ bool create_network_addresses_table(void) void parse_neigh_cache(void) { FILE* arpfp = NULL; - // Try to access the kernel's ARP/NDP cache + // Try to access the kernel's neighbor cache if((arpfp = popen("ip neigh show", "r")) == NULL) { logg("WARN: Command \"ip neigh show\" failed!"); @@ -192,6 +193,15 @@ void parse_neigh_cache(void) "WHERE id = %i;",\ ip, dbID); + // Add unique pair of ID (corresponds to one particular hardware + // address) and IP address if it does not exist (INSERT). In case + // this pair already exists, the UNIQUE(id,ip) trigger becomes + // active and the line is instead REPLACEd, causing the lastQuery + // timestamp to be updated + dbquery("INSERT OR REPLACE INTO network_addresses "\ + "(id,ip,lastQuery) VALUES(%i,\'%s\',%i);",\ + dbID, ip, client->lastQuery); + // Store hostname if available if(strlen(hostname) > 0) { From a85b97394ffe42dd00540c2d98553e7e080ef50e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 12 Jun 2019 20:52:50 +0200 Subject: [PATCH 04/35] Rename network_addresses(id) to network_addresses(network_id) and reference to the network(id) field using a foreign key contraint. SQLite foreign key constraints are used to enforce "exists" relationships between the two tables. Attempting to insert a row into the network_addresses table that does not correspond to any row in the network table will fail, as will attempting to delete a row from the network table when there exist dependent rows in the network_addresses table. Signed-off-by: DL6ER --- networktable.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/networktable.c b/networktable.c index 310074a6f..f5c17a2a9 100644 --- a/networktable.c +++ b/networktable.c @@ -42,10 +42,11 @@ bool create_network_addresses_table(void) { bool ret; // Create network_addresses table in the database - ret = dbquery("CREATE TABLE network_addresses ( id INTEGER NOT NULL, " \ + ret = dbquery("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ "ip TEXT NOT NULL, "\ - "lastQuery INTEGER NOT NULL, " \ - "UNIQUE(id,ip));"); + "lastQuery INTEGER NOT NULL, "\ + "UNIQUE(network_id,ip)), "\ + "FOREIGN KEY(network_id) REFERENCES network(id));"); if(!ret){ dbclose(); return false; } // Update database version to 5 @@ -195,11 +196,11 @@ void parse_neigh_cache(void) // Add unique pair of ID (corresponds to one particular hardware // address) and IP address if it does not exist (INSERT). In case - // this pair already exists, the UNIQUE(id,ip) trigger becomes - // active and the line is instead REPLACEd, causing the lastQuery - // timestamp to be updated + // this pair already exists, the UNIQUE(network_id,ip) trigger + // becomes active and the line is instead REPLACEd, causing the + // lastQuery timestamp to be updated dbquery("INSERT OR REPLACE INTO network_addresses "\ - "(id,ip,lastQuery) VALUES(%i,\'%s\',%i);",\ + "(network_id,ip,lastQuery) VALUES(%i,\'%s\',%i);",\ dbID, ip, client->lastQuery); // Store hostname if available From 717cca035bd2f76f2eb3a55853957ed31f315375 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 12 Jun 2019 21:07:07 +0200 Subject: [PATCH 05/35] Rename function to parse_neighbor_cache() Signed-off-by: DL6ER --- database.c | 2 +- networktable.c | 4 ++-- routines.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/database.c b/database.c index 86ddf5be7..84a78b061 100644 --- a/database.c +++ b/database.c @@ -691,7 +691,7 @@ void *DB_thread(void *val) // Parse neighbor cache (fill network table) if enabled if (config.parse_arp_cache) - parse_neigh_cache(); + parse_neighbor_cache(); } sleepms(100); } diff --git a/networktable.c b/networktable.c index f5c17a2a9..d736af5f6 100644 --- a/networktable.c +++ b/networktable.c @@ -45,7 +45,7 @@ bool create_network_addresses_table(void) ret = dbquery("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ "ip TEXT NOT NULL, "\ "lastQuery INTEGER NOT NULL, "\ - "UNIQUE(network_id,ip)), "\ + "UNIQUE(network_id,ip), "\ "FOREIGN KEY(network_id) REFERENCES network(id));"); if(!ret){ dbclose(); return false; } @@ -57,7 +57,7 @@ bool create_network_addresses_table(void) } // Parse kernel's neighbor cache -void parse_neigh_cache(void) +void parse_neighbor_cache(void) { FILE* arpfp = NULL; // Try to access the kernel's neighbor cache diff --git a/routines.h b/routines.h index 295d73ab3..1f5053f21 100644 --- a/routines.h +++ b/routines.h @@ -157,7 +157,7 @@ bool check_capabilities(void); // networktable.c bool create_network_table(void); bool create_network_addresses_table(void); -void parse_neigh_cache(void); +void parse_neighbor_cache(void); void updateMACVendorRecords(void); // gravity.c From 2f1fbda5aeac7fe4fefd0da6d6646e0abf8ce472 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 14 Jun 2019 17:13:23 +0200 Subject: [PATCH 06/35] Use PRIMARY KEY for pair (network_id,ip). This implies UNIQUE. Signed-off-by: DL6ER --- networktable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/networktable.c b/networktable.c index d736af5f6..29a28c28f 100644 --- a/networktable.c +++ b/networktable.c @@ -45,7 +45,7 @@ bool create_network_addresses_table(void) ret = dbquery("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ "ip TEXT NOT NULL, "\ "lastQuery INTEGER NOT NULL, "\ - "UNIQUE(network_id,ip), "\ + "PRIMARY KEY(network_id,ip), "\ "FOREIGN KEY(network_id) REFERENCES network(id));"); if(!ret){ dbclose(); return false; } From 2a1361d08ff057fb28adbc51229843b297f72c3d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 14 Jun 2019 17:50:20 +0200 Subject: [PATCH 07/35] Always add IP addresses to network_addresses, even if the client isn't make queries to FTL at the moment. Signed-off-by: DL6ER --- networktable.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/networktable.c b/networktable.c index 29a28c28f..a6b8a101e 100644 --- a/networktable.c +++ b/networktable.c @@ -194,15 +194,6 @@ void parse_neighbor_cache(void) "WHERE id = %i;",\ ip, dbID); - // Add unique pair of ID (corresponds to one particular hardware - // address) and IP address if it does not exist (INSERT). In case - // this pair already exists, the UNIQUE(network_id,ip) trigger - // becomes active and the line is instead REPLACEd, causing the - // lastQuery timestamp to be updated - dbquery("INSERT OR REPLACE INTO network_addresses "\ - "(network_id,ip,lastQuery) VALUES(%i,\'%s\',%i);",\ - dbID, ip, client->lastQuery); - // Store hostname if available if(strlen(hostname) > 0) { @@ -216,6 +207,15 @@ void parse_neighbor_cache(void) // else: // Device in database but not known to Pi-hole: No action required + // Add unique pair of ID (corresponds to one particular hardware + // address) and IP address if it does not exist (INSERT). In case + // this pair already exists, the UNIQUE(network_id,ip) trigger + // becomes active and the line is instead REPLACEd, causing the + // lastQuery timestamp to be updated + dbquery("INSERT OR REPLACE INTO network_addresses "\ + "(network_id,ip,lastQuery) VALUES(%i,\'%s\',%i);",\ + dbID, ip, client->lastQuery); + // Count number of processed ARP cache entries entries++; } From 7a4c4f3c9a1a42f187b4ec3e2d9f6deab15a383c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 14 Jun 2019 17:56:03 +0200 Subject: [PATCH 08/35] Also analyze loops with router related information Signed-off-by: DL6ER --- networktable.c | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/networktable.c b/networktable.c index 29a28c28f..1caf3a443 100644 --- a/networktable.c +++ b/networktable.c @@ -82,7 +82,7 @@ void parse_neighbor_cache(void) // Prepare buffers char * linebuffer = NULL; size_t linebuffersize = 0; - char ip[100], status[100], hwaddr[100], iface[100]; + char ip[100], status1[100], status2[100], hwaddr[100], iface[100]; unsigned int entries = 0; time_t now = time(NULL); @@ -93,17 +93,35 @@ void parse_neighbor_cache(void) // Read ARP cache line by line while(getline(&linebuffer, &linebuffersize, arpfp) != -1) { - int num = sscanf(linebuffer, "%99s dev %99s lladdr %99s %99s", - ip, iface, hwaddr, status); + int num = sscanf(linebuffer, "%99s dev %99s lladdr %99s %99s %99s", + ip, iface, hwaddr, status1, status2); - // Skip addresses without hardware address information - if (num != 4) - continue; - - // Only process active entries - if(strcasecmp(status, "reachable") != 0 && - strcasecmp(status, "stale") != 0) - continue; + // Check if we want to process the line we just read + switch(num) + { + // Example: 2003:17:7:dc:e1b1:1e01:1a5a:ff dev eth0 lladdr a0:20:29:24:98:99 REACHABLE + case 4: + if(strcasecmp(status1, "reachable") != 0 && + strcasecmp(status1, "stale") != 0) + { + continue; + } + break; + + // Example: dead:beef:dead:beef:dead:beef dev wlp3s0 lladdr de:ad:be:ef:de:ad router REACHABLE + case 5: + if(strcasecmp(status1, "router") != 0 && + (strcasecmp(status2, "reachable") != 0 && + strcasecmp(status2, "stale") != 0 )) + { + continue; + } + break; + + // Skip lines without hardware information + default: + continue; + } // Get ID of this device in our network database. If it cannot be // found, then this is a new device. We only use the hardware address @@ -215,6 +233,8 @@ void parse_neighbor_cache(void) } // else: // Device in database but not known to Pi-hole: No action required + else if(config.debug & DEBUG_ARP) + logg("Skipping %s %s",ip,hwaddr); // Count number of processed ARP cache entries entries++; From 4a2b38be73faa110b59fff85557022a00da6a080 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 14 Jun 2019 18:10:36 +0200 Subject: [PATCH 09/35] Change lastQuery column to lastSeen as this is a better description (last seen in the ARP/NDP cache) Signed-off-by: DL6ER --- networktable.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/networktable.c b/networktable.c index 0b0cbee85..a443aa4f1 100644 --- a/networktable.c +++ b/networktable.c @@ -44,7 +44,7 @@ bool create_network_addresses_table(void) // Create network_addresses table in the database ret = dbquery("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ "ip TEXT NOT NULL, "\ - "lastQuery INTEGER NOT NULL, "\ + "lastSeen INTEGER NOT NULL, "\ "PRIMARY KEY(network_id,ip), "\ "FOREIGN KEY(network_id) REFERENCES network(id));"); if(!ret){ dbclose(); return false; } @@ -233,8 +233,8 @@ void parse_neighbor_cache(void) // becomes active and the line is instead REPLACEd, causing the // lastQuery timestamp to be updated dbquery("INSERT OR REPLACE INTO network_addresses "\ - "(network_id,ip,lastQuery) VALUES(%i,\'%s\',%i);",\ - dbID, ip, client->lastQuery); + "(network_id,ip,lastSeen) VALUES(%i,\'%s\',%d);",\ + dbID, ip, time(NULL)); // Count number of processed ARP cache entries entries++; From a78807edbc0ae2a142e2edab1d39f9245bbefe77 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 14 Jun 2019 19:50:45 +0200 Subject: [PATCH 10/35] Remove obsolete debugging output Signed-off-by: DL6ER --- networktable.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/networktable.c b/networktable.c index a443aa4f1..90b38a11a 100644 --- a/networktable.c +++ b/networktable.c @@ -224,8 +224,6 @@ void parse_neighbor_cache(void) } // else: // Device in database but not known to Pi-hole: No action required - else if(config.debug & DEBUG_ARP) - logg("Skipping %s %s",ip,hwaddr); // Add unique pair of ID (corresponds to one particular hardware // address) and IP address if it does not exist (INSERT). In case From 945734173a1dc200aedfd3ed2079a9f9e7ec5fbf Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 14 Jun 2019 19:56:24 +0200 Subject: [PATCH 11/35] Add test for successful creation of network_addresses table. Signed-off-by: DL6ER --- test/test_suite.bats | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_suite.bats b/test/test_suite.bats index 16c5a02b7..006a5ba2d 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -244,6 +244,7 @@ [[ "${lines[@]}" == *"CREATE TABLE ftl ( id INTEGER PRIMARY KEY NOT NULL, value BLOB NOT NULL );"* ]] [[ "${lines[@]}" == *"CREATE TABLE counters ( id INTEGER PRIMARY KEY NOT NULL, value INTEGER NOT NULL );"* ]] [[ "${lines[@]}" == *"CREATE TABLE network ( id INTEGER PRIMARY KEY NOT NULL, ip TEXT NOT NULL, hwaddr TEXT NOT NULL, interface TEXT NOT NULL, name TEXT, firstSeen INTEGER NOT NULL, lastQuery INTEGER NOT NULL, numQueries INTEGER NOT NULL,macVendor TEXT);"* ]] + [[ "${lines[@]}" == *"CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, ip TEXT NOT NULL, lastSeen INTEGER NOT NULL, PRIMARY KEY(network_id,ip), FOREIGN KEY(network_id) REFERENCES network(id));"* ]] [[ "${lines[@]}" == *"CREATE INDEX idx_queries_timestamps ON queries (timestamp);"* ]] [[ "${lines[@]}" == *"CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr);"* ]] } From 5f075e1515fe7913e66d2d941454e525cd3e3ded Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 15 Jun 2019 07:56:34 +0200 Subject: [PATCH 12/35] Remove extra parentheses Signed-off-by: DL6ER --- networktable.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/networktable.c b/networktable.c index 90b38a11a..7ac70b8b4 100644 --- a/networktable.c +++ b/networktable.c @@ -110,9 +110,9 @@ void parse_neighbor_cache(void) // Example: dead:beef:dead:beef:dead:beef dev wlp3s0 lladdr de:ad:be:ef:de:ad router REACHABLE case 5: - if(strcasecmp(status1, "router") != 0 && - (strcasecmp(status2, "reachable") != 0 && - strcasecmp(status2, "stale") != 0 )) + if(strcasecmp(status1, "router") != 0 && + strcasecmp(status2, "reachable") != 0 && + strcasecmp(status2, "stale") != 0) { continue; } From 01b2205a1eece4ac2b2027efd7babb958b0b6d34 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 15 Jun 2019 09:47:37 +0200 Subject: [PATCH 13/35] Enable the enforcement of foreign key constraints. PRAGMA FOREIGN_KEYS must be set to ON for each database connection. Signed-off-by: DL6ER --- database.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/database.c b/database.c index 84a78b061..ae4b91d18 100644 --- a/database.c +++ b/database.c @@ -82,6 +82,21 @@ bool dbopen(void) return false; } + // Enable the enforcement of foreign key constraints. As of SQLite + // version 3.6.19 (2009-10-14), the default setting for foreign key + // enforcement is false. + // Foreign key constraints are disabled by default (for backwards + // compatibility), so must be enabled separately for each database + // connection (see also https://www.sqlite.org/foreignkeys.html). + rc = dbquery("PRAGMA FOREIGN_KEYS=ON;"); + if( !rc ) + { + logg("dbopen() - FOREIGN_KEYS error (%i): %s", rc, sqlite3_errmsg(db)); + dbclose(); + check_database(rc); + return false; + } + return true; } From 8fd02cd7452435bd912c9783da60fef560f097c3 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 15 Jun 2019 10:03:27 +0200 Subject: [PATCH 14/35] Use UNIQUE for the pair (network_id,ip) as PRIMARY KEY does not enforce uniqueness on a pair of columns. Signed-off-by: DL6ER --- networktable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/networktable.c b/networktable.c index 7ac70b8b4..3ea323bb1 100644 --- a/networktable.c +++ b/networktable.c @@ -45,7 +45,7 @@ bool create_network_addresses_table(void) ret = dbquery("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ "ip TEXT NOT NULL, "\ "lastSeen INTEGER NOT NULL, "\ - "PRIMARY KEY(network_id,ip), "\ + "UNIQUE(network_id,ip), "\ "FOREIGN KEY(network_id) REFERENCES network(id));"); if(!ret){ dbclose(); return false; } From 634f97a6fb05c9492a376e79d72ac7a91d66e776 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 15 Jun 2019 10:06:19 +0200 Subject: [PATCH 15/35] Update tests after having changed table schema Signed-off-by: DL6ER --- test/test_suite.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_suite.bats b/test/test_suite.bats index 006a5ba2d..e64d47da6 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -244,7 +244,7 @@ [[ "${lines[@]}" == *"CREATE TABLE ftl ( id INTEGER PRIMARY KEY NOT NULL, value BLOB NOT NULL );"* ]] [[ "${lines[@]}" == *"CREATE TABLE counters ( id INTEGER PRIMARY KEY NOT NULL, value INTEGER NOT NULL );"* ]] [[ "${lines[@]}" == *"CREATE TABLE network ( id INTEGER PRIMARY KEY NOT NULL, ip TEXT NOT NULL, hwaddr TEXT NOT NULL, interface TEXT NOT NULL, name TEXT, firstSeen INTEGER NOT NULL, lastQuery INTEGER NOT NULL, numQueries INTEGER NOT NULL,macVendor TEXT);"* ]] - [[ "${lines[@]}" == *"CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, ip TEXT NOT NULL, lastSeen INTEGER NOT NULL, PRIMARY KEY(network_id,ip), FOREIGN KEY(network_id) REFERENCES network(id));"* ]] + [[ "${lines[@]}" == *"CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, ip TEXT NOT NULL, lastSeen INTEGER NOT NULL, UNIQUE(network_id,ip), FOREIGN KEY(network_id) REFERENCES network(id));"* ]] [[ "${lines[@]}" == *"CREATE INDEX idx_queries_timestamps ON queries (timestamp);"* ]] [[ "${lines[@]}" == *"CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr);"* ]] } From e9834d489b5c98b93f6a8a1dca0e9463c4e6842a Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 15 Jun 2019 10:46:21 +0200 Subject: [PATCH 16/35] Update dbID when INSERTing a new row into the network table to have this index when adding the entry to the network_addresses table. Signed-off-by: DL6ER --- database.c | 9 +++++++++ networktable.c | 5 ++++- routines.h | 1 + 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/database.c b/database.c index ae4b91d18..2be5c3018 100644 --- a/database.c +++ b/database.c @@ -931,3 +931,12 @@ void read_data_from_DB(void) dbclose(); free(rstr); } + +// Returns ID of the most recent successful INSERT. +long db_lastID(void) +{ + long id = sqlite3_last_insert_rowid(db); + if(config.debug & DEBUG_DATABASE) + logg("db_lastID(): %ld", id); + return id; +} diff --git a/networktable.c b/networktable.c index 3ea323bb1..625fc436d 100644 --- a/networktable.c +++ b/networktable.c @@ -143,7 +143,7 @@ void parse_neighbor_cache(void) } // Perform SQL query - const int dbID = db_query_int(querystr); + int dbID = db_query_int(querystr); free(querystr); if(dbID == DB_FAILED) @@ -184,6 +184,9 @@ void parse_neighbor_cache(void) hostname, macVendor); free(macVendor); + + // Obtain ID which was given to this new entry + dbID = db_lastID(); } // Device in database AND client known to Pi-hole else if(client != NULL) diff --git a/routines.h b/routines.h index 1f5053f21..35873bb77 100644 --- a/routines.h +++ b/routines.h @@ -93,6 +93,7 @@ bool dbopen(void); void dbclose(void); int db_query_int(const char*); void SQLite3LogCallback(void *pArg, int iErrCode, const char *zMsg); +long db_lastID(void); // memory.c void memory_check(const int which); From e8e64c0e0f8229fedfbd2152dfca4cfbb3de2859 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 11:00:14 +0200 Subject: [PATCH 17/35] Add an initial entry in network_addresses for each device in the network table + remove ip column from network table. Signed-off-by: DL6ER --- networktable.c | 91 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 79 insertions(+), 12 deletions(-) diff --git a/networktable.c b/networktable.c index 625fc436d..11616a6df 100644 --- a/networktable.c +++ b/networktable.c @@ -44,10 +44,84 @@ bool create_network_addresses_table(void) // Create network_addresses table in the database ret = dbquery("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ "ip TEXT NOT NULL, "\ - "lastSeen INTEGER NOT NULL, "\ + "lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%%s', 'now') as int)), "\ "UNIQUE(network_id,ip), "\ "FOREIGN KEY(network_id) REFERENCES network(id));"); - if(!ret){ dbclose(); return false; } + if(!ret) + { + logg("create_network_addresses_table(): CREATE TABLE network_addresses failed!"); + dbclose(); + return false; + } + + // Create a network_addresses row for each entry in the network table + ret = dbquery("INSERT INTO network_addresses (network_id,ip) SELECT id,ip FROM network;"); + if(!ret) + { + logg("create_network_addresses_table(): INSERT INTO network_addresses failed!"); + dbclose(); + return false; + } + + // Remove IP column from network table. + // As ALTER TABLE is severely limit, we have to do the column deletion manually. + // Step 1: We create a new table without the ip column + // We add the UNIQUE constraint on (ip,hwaddr) already here + ret = dbquery("CREATE TABLE network_bck ( id INTEGER PRIMARY KEY NOT NULL, " \ + "hwaddr TEXT NOT NULL, " \ + "interface TEXT NOT NULL, " \ + "name TEXT, " \ + "firstSeen INTEGER NOT NULL, " \ + "lastQuery INTEGER NOT NULL, " \ + "numQueries INTEGER NOT NULL, " \ + "macVendor TEXT, " \ + "UNIQUE(id,hwaddr));"); + if(!ret) + { + logg("create_network_addresses_table(): CREATE TABLE network_bck failed!"); + dbclose(); + return false; + } + + // Step 2: Copy data (except ip column) from network into network_back + ret = dbquery("INSERT INTO network_bck "\ + "SELECT id, hwaddr, interface, name, firstSeen, "\ + "lastQuery, numQueries, macVendor "\ + "FROM network;"); + if(!ret) + { + logg("create_network_addresses_table(): INSERT INTO network_bck failed!"); + dbclose(); + return false; + } + + // Step 3: Drop unique index in hwaddr before dropping the network table + ret = dbquery("DROP INDEX network_hwaddr_idx;"); + if(!ret) + { + logg("create_network_addresses_table(): DROP INDEX network_hwaddr_idx failed!"); + dbclose(); + return false; + } + + // Step 4: Drop the network table + ret = dbquery("DROP TABLE network;"); + if(!ret) + { + logg("create_network_addresses_table(): DROP TABLE network failed!"); + dbclose(); + return false; + } + + + // Step 5: Rename network_bck table to network table as last step + ret = dbquery("ALTER TABLE network_bck RENAME TO network;"); + if(!ret) + { + logg("create_network_addresses_table(): ALTER TABLE network_bck failed!"); + dbclose(); + return false; + } // Update database version to 5 ret = db_set_FTL_property(DB_VERSION, 5); @@ -176,9 +250,9 @@ void parse_neighbor_cache(void) { char* macVendor = getMACVendor(hwaddr); dbquery("INSERT INTO network "\ - "(ip,hwaddr,interface,firstSeen,lastQuery,numQueries,name,macVendor) "\ - "VALUES (\'%s\',\'%s\',\'%s\',%lu, %ld, %u, \'%s\', \'%s\');",\ - ip, hwaddr, iface, now, + "(hwaddr,interface,firstSeen,lastQuery,numQueries,name,macVendor) "\ + "VALUES (\'%s\',\'%s\',%lu, %ld, %u, \'%s\', \'%s\');",\ + hwaddr, iface, now, client != NULL ? client->lastQuery : 0L, client != NULL ? client->numQueriesARP : 0u, hostname, @@ -208,13 +282,6 @@ void parse_neighbor_cache(void) client->numQueriesARP, dbID); client->numQueriesARP = 0; - // Update IP address in case it changed. This might happen with - // sequential DHCP servers as found in many commercial routers - dbquery("UPDATE network "\ - "SET ip = \'%s\' "\ - "WHERE id = %i;",\ - ip, dbID); - // Store hostname if available if(strlen(hostname) > 0) { From 46ed609203e5ee90ff53870e6ea07140140aec79 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 11:06:12 +0200 Subject: [PATCH 18/35] Tests: ALTER TABLE ... RENAME TO ... created a table using CREATE TABLE IS NOT EXISTS. Signed-off-by: DL6ER --- test/test_suite.bats | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_suite.bats b/test/test_suite.bats index cd4051ba7..a66de442f 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -243,10 +243,9 @@ [[ "${lines[@]}" == *"CREATE TABLE queries ( id INTEGER PRIMARY KEY AUTOINCREMENT, timestamp INTEGER NOT NULL, type INTEGER NOT NULL, status INTEGER NOT NULL, domain TEXT NOT NULL, client TEXT NOT NULL, forward TEXT );"* ]] [[ "${lines[@]}" == *"CREATE TABLE ftl ( id INTEGER PRIMARY KEY NOT NULL, value BLOB NOT NULL );"* ]] [[ "${lines[@]}" == *"CREATE TABLE counters ( id INTEGER PRIMARY KEY NOT NULL, value INTEGER NOT NULL );"* ]] - [[ "${lines[@]}" == *"CREATE TABLE network ( id INTEGER PRIMARY KEY NOT NULL, ip TEXT NOT NULL, hwaddr TEXT NOT NULL, interface TEXT NOT NULL, name TEXT, firstSeen INTEGER NOT NULL, lastQuery INTEGER NOT NULL, numQueries INTEGER NOT NULL,macVendor TEXT);"* ]] + [[ "${lines[@]}" == *"CREATE TABLE IF NOT EXISTS \"network\" ( id INTEGER PRIMARY KEY NOT NULL, hwaddr TEXT NOT NULL, interface TEXT NOT NULL, name TEXT, firstSeen INTEGER NOT NULL, lastQuery INTEGER NOT NULL, numQueries INTEGER NOT NULL, macVendor TEXT, UNIQUE(id,hwaddr));"* ]] [[ "${lines[@]}" == *"CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, ip TEXT NOT NULL, lastSeen INTEGER NOT NULL, UNIQUE(network_id,ip), FOREIGN KEY(network_id) REFERENCES network(id));"* ]] [[ "${lines[@]}" == *"CREATE INDEX idx_queries_timestamps ON queries (timestamp);"* ]] - [[ "${lines[@]}" == *"CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr);"* ]] } @test "Fail on invalid argument" { From 5f44b5b7bb64062961fb7cc26b1a4f738dca718e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 12:21:23 +0200 Subject: [PATCH 19/35] Tests: Added default value for lastSeen column in network_addresses table. Signed-off-by: DL6ER --- networktable.c | 3 +-- test/test_suite.bats | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/networktable.c b/networktable.c index 11616a6df..75aec3a5a 100644 --- a/networktable.c +++ b/networktable.c @@ -301,8 +301,7 @@ void parse_neighbor_cache(void) // becomes active and the line is instead REPLACEd, causing the // lastQuery timestamp to be updated dbquery("INSERT OR REPLACE INTO network_addresses "\ - "(network_id,ip,lastSeen) VALUES(%i,\'%s\',%d);",\ - dbID, ip, time(NULL)); + "(network_id,ip) VALUES(%i,\'%s\');", dbID, ip); // Count number of processed ARP cache entries entries++; diff --git a/test/test_suite.bats b/test/test_suite.bats index a66de442f..0e288e447 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -244,7 +244,7 @@ [[ "${lines[@]}" == *"CREATE TABLE ftl ( id INTEGER PRIMARY KEY NOT NULL, value BLOB NOT NULL );"* ]] [[ "${lines[@]}" == *"CREATE TABLE counters ( id INTEGER PRIMARY KEY NOT NULL, value INTEGER NOT NULL );"* ]] [[ "${lines[@]}" == *"CREATE TABLE IF NOT EXISTS \"network\" ( id INTEGER PRIMARY KEY NOT NULL, hwaddr TEXT NOT NULL, interface TEXT NOT NULL, name TEXT, firstSeen INTEGER NOT NULL, lastQuery INTEGER NOT NULL, numQueries INTEGER NOT NULL, macVendor TEXT, UNIQUE(id,hwaddr));"* ]] - [[ "${lines[@]}" == *"CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, ip TEXT NOT NULL, lastSeen INTEGER NOT NULL, UNIQUE(network_id,ip), FOREIGN KEY(network_id) REFERENCES network(id));"* ]] + [[ "${lines[@]}" == *"CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, ip TEXT NOT NULL, lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%s', 'now') as int)), UNIQUE(network_id,ip), FOREIGN KEY(network_id) REFERENCES network(id));"* ]] [[ "${lines[@]}" == *"CREATE INDEX idx_queries_timestamps ON queries (timestamp);"* ]] } From 7f4bb012ae7ce87eaad9b0c3b3a40d4e9e4ccd0c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 12:23:43 +0200 Subject: [PATCH 20/35] Wrap upgrading database into SQL transaction. Signed-off-by: DL6ER --- networktable.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/networktable.c b/networktable.c index 75aec3a5a..1f519b33f 100644 --- a/networktable.c +++ b/networktable.c @@ -41,6 +41,9 @@ bool create_network_table(void) bool create_network_addresses_table(void) { bool ret; + + dbquery("BEGIN TRANSACTION"); + // Create network_addresses table in the database ret = dbquery("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ "ip TEXT NOT NULL, "\ @@ -125,7 +128,14 @@ bool create_network_addresses_table(void) // Update database version to 5 ret = db_set_FTL_property(DB_VERSION, 5); - if(!ret){ dbclose(); return false; } + if(!ret) + { + logg("create_network_addresses_table(): Failed to update database version!"); + dbclose(); + return false; + } + + dbquery("COMMIT"); return true; } From ecffbd6ab33e2d6ab5fdae0cfb5f1ae19cba3553 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 21:06:26 +0200 Subject: [PATCH 21/35] Put UNIQUE constraint directly on definition of the hwaddr column instead of using the pair (id,hwaddr) Signed-off-by: DL6ER --- networktable.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/networktable.c b/networktable.c index 1f519b33f..1916aec23 100644 --- a/networktable.c +++ b/networktable.c @@ -71,14 +71,13 @@ bool create_network_addresses_table(void) // Step 1: We create a new table without the ip column // We add the UNIQUE constraint on (ip,hwaddr) already here ret = dbquery("CREATE TABLE network_bck ( id INTEGER PRIMARY KEY NOT NULL, " \ - "hwaddr TEXT NOT NULL, " \ + "hwaddr TEXT UNIQUE NOT NULL, " \ "interface TEXT NOT NULL, " \ "name TEXT, " \ "firstSeen INTEGER NOT NULL, " \ "lastQuery INTEGER NOT NULL, " \ "numQueries INTEGER NOT NULL, " \ - "macVendor TEXT, " \ - "UNIQUE(id,hwaddr));"); + "macVendor TEXT);"); if(!ret) { logg("create_network_addresses_table(): CREATE TABLE network_bck failed!"); From 46b97d563f4ddf2c3a7ce131df7226b50bcdcb04 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 21:14:41 +0200 Subject: [PATCH 22/35] DROP TABLE also DROPs the index so we do not need to do this separately. Signed-off-by: DL6ER --- networktable.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/networktable.c b/networktable.c index 1916aec23..8e7d26f31 100644 --- a/networktable.c +++ b/networktable.c @@ -97,16 +97,7 @@ bool create_network_addresses_table(void) return false; } - // Step 3: Drop unique index in hwaddr before dropping the network table - ret = dbquery("DROP INDEX network_hwaddr_idx;"); - if(!ret) - { - logg("create_network_addresses_table(): DROP INDEX network_hwaddr_idx failed!"); - dbclose(); - return false; - } - - // Step 4: Drop the network table + // Step 3: Drop the network table, the unique index will be automatically dropped ret = dbquery("DROP TABLE network;"); if(!ret) { @@ -116,7 +107,7 @@ bool create_network_addresses_table(void) } - // Step 5: Rename network_bck table to network table as last step + // Step 4: Rename network_bck table to network table as last step ret = dbquery("ALTER TABLE network_bck RENAME TO network;"); if(!ret) { From c922835b72dd236f64862c8a9eb30f5f4d3b978c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 21:18:23 +0200 Subject: [PATCH 23/35] Tests: Change schema of network table Signed-off-by: DL6ER --- test/test_suite.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_suite.bats b/test/test_suite.bats index 0e288e447..3908bd23f 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -243,7 +243,7 @@ [[ "${lines[@]}" == *"CREATE TABLE queries ( id INTEGER PRIMARY KEY AUTOINCREMENT, timestamp INTEGER NOT NULL, type INTEGER NOT NULL, status INTEGER NOT NULL, domain TEXT NOT NULL, client TEXT NOT NULL, forward TEXT );"* ]] [[ "${lines[@]}" == *"CREATE TABLE ftl ( id INTEGER PRIMARY KEY NOT NULL, value BLOB NOT NULL );"* ]] [[ "${lines[@]}" == *"CREATE TABLE counters ( id INTEGER PRIMARY KEY NOT NULL, value INTEGER NOT NULL );"* ]] - [[ "${lines[@]}" == *"CREATE TABLE IF NOT EXISTS \"network\" ( id INTEGER PRIMARY KEY NOT NULL, hwaddr TEXT NOT NULL, interface TEXT NOT NULL, name TEXT, firstSeen INTEGER NOT NULL, lastQuery INTEGER NOT NULL, numQueries INTEGER NOT NULL, macVendor TEXT, UNIQUE(id,hwaddr));"* ]] + [[ "${lines[@]}" == *"CREATE TABLE IF NOT EXISTS \"network\" ( id INTEGER PRIMARY KEY NOT NULL, hwaddr TEXT UNIQUE NOT NULL, interface TEXT NOT NULL, name TEXT, firstSeen INTEGER NOT NULL, lastQuery INTEGER NOT NULL, numQueries INTEGER NOT NULL, macVendor TEXT);"* ]] [[ "${lines[@]}" == *"CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, ip TEXT NOT NULL, lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%s', 'now') as int)), UNIQUE(network_id,ip), FOREIGN KEY(network_id) REFERENCES network(id));"* ]] [[ "${lines[@]}" == *"CREATE INDEX idx_queries_timestamps ON queries (timestamp);"* ]] } From 48093661fe5bb2000f7c36d5b568f9293a8de396 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 23:39:16 +0200 Subject: [PATCH 24/35] Use ip-neighbor's Neighbour Unreachability Detection (NUD) option to query only entries which are interesting for us. This simplifies the post-processing of the read data. Signed-off-by: DL6ER --- networktable.c | 38 ++++++++------------------------------ 1 file changed, 8 insertions(+), 30 deletions(-) diff --git a/networktable.c b/networktable.c index 8e7d26f31..1f1faee0a 100644 --- a/networktable.c +++ b/networktable.c @@ -135,9 +135,10 @@ void parse_neighbor_cache(void) { FILE* arpfp = NULL; // Try to access the kernel's neighbor cache - if((arpfp = popen("ip neigh show", "r")) == NULL) + // We are only interested in entries which are in either STALE or REACHABLE state + if((arpfp = popen("ip neigh show nud stale nud reachable", "r")) == NULL) { - logg("WARN: Command \"ip neigh show\" failed!"); + logg("WARN: Command \"ip neigh show nud stale nud reachable\" failed!"); logg(" Message: %s", strerror(errno)); return; } @@ -156,7 +157,7 @@ void parse_neighbor_cache(void) // Prepare buffers char * linebuffer = NULL; size_t linebuffersize = 0; - char ip[100], status1[100], status2[100], hwaddr[100], iface[100]; + char ip[100], hwaddr[100], iface[100]; unsigned int entries = 0; time_t now = time(NULL); @@ -167,35 +168,12 @@ void parse_neighbor_cache(void) // Read ARP cache line by line while(getline(&linebuffer, &linebuffersize, arpfp) != -1) { - int num = sscanf(linebuffer, "%99s dev %99s lladdr %99s %99s %99s", - ip, iface, hwaddr, status1, status2); + int num = sscanf(linebuffer, "%99s dev %99s lladdr %99s", + ip, iface, hwaddr); // Check if we want to process the line we just read - switch(num) - { - // Example: 2003:17:7:dc:e1b1:1e01:1a5a:ff dev eth0 lladdr a0:20:29:24:98:99 REACHABLE - case 4: - if(strcasecmp(status1, "reachable") != 0 && - strcasecmp(status1, "stale") != 0) - { - continue; - } - break; - - // Example: dead:beef:dead:beef:dead:beef dev wlp3s0 lladdr de:ad:be:ef:de:ad router REACHABLE - case 5: - if(strcasecmp(status1, "router") != 0 && - strcasecmp(status2, "reachable") != 0 && - strcasecmp(status2, "stale") != 0) - { - continue; - } - break; - - // Skip lines without hardware information - default: - continue; - } + if(num != 3) + continue; // Get ID of this device in our network database. If it cannot be // found, then this is a new device. We only use the hardware address From 95b4ac592c9fb3d113d79b2c8f6148729866117c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 23:44:40 +0200 Subject: [PATCH 25/35] Update comment Signed-off-by: DL6ER --- networktable.c | 1 - 1 file changed, 1 deletion(-) diff --git a/networktable.c b/networktable.c index 1f1faee0a..618c08ac4 100644 --- a/networktable.c +++ b/networktable.c @@ -69,7 +69,6 @@ bool create_network_addresses_table(void) // Remove IP column from network table. // As ALTER TABLE is severely limit, we have to do the column deletion manually. // Step 1: We create a new table without the ip column - // We add the UNIQUE constraint on (ip,hwaddr) already here ret = dbquery("CREATE TABLE network_bck ( id INTEGER PRIMARY KEY NOT NULL, " \ "hwaddr TEXT UNIQUE NOT NULL, " \ "interface TEXT NOT NULL, " \ From 26c52dcf45b33e943567d92f4c25b21f0c8da59a Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 16 Jun 2019 23:53:26 +0200 Subject: [PATCH 26/35] Use macro to reduce code duplication in create_network_addresses_table(). Signed-off-by: DL6ER --- networktable.c | 64 ++++++++++++++------------------------------------ 1 file changed, 17 insertions(+), 47 deletions(-) diff --git a/networktable.c b/networktable.c index 618c08ac4..ff4af0556 100644 --- a/networktable.c +++ b/networktable.c @@ -38,38 +38,33 @@ bool create_network_table(void) return true; } +// Private macro +#define SQL(sql) {\ + if(!dbquery(sql)) {\ + logg("create_network_addresses_table(): \"%s\" failed!", sql);\ + dbclose();\ + return false;\ + }\ +} + bool create_network_addresses_table(void) { - bool ret; - - dbquery("BEGIN TRANSACTION"); + SQL("BEGIN TRANSACTION"); // Create network_addresses table in the database - ret = dbquery("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ + SQL("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ "ip TEXT NOT NULL, "\ "lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%%s', 'now') as int)), "\ "UNIQUE(network_id,ip), "\ "FOREIGN KEY(network_id) REFERENCES network(id));"); - if(!ret) - { - logg("create_network_addresses_table(): CREATE TABLE network_addresses failed!"); - dbclose(); - return false; - } // Create a network_addresses row for each entry in the network table - ret = dbquery("INSERT INTO network_addresses (network_id,ip) SELECT id,ip FROM network;"); - if(!ret) - { - logg("create_network_addresses_table(): INSERT INTO network_addresses failed!"); - dbclose(); - return false; - } + SQL("INSERT INTO network_addresses (network_id,ip) SELECT id,ip FROM network;"); // Remove IP column from network table. // As ALTER TABLE is severely limit, we have to do the column deletion manually. // Step 1: We create a new table without the ip column - ret = dbquery("CREATE TABLE network_bck ( id INTEGER PRIMARY KEY NOT NULL, " \ + SQL("CREATE TABLE network_bck ( id INTEGER PRIMARY KEY NOT NULL, " \ "hwaddr TEXT UNIQUE NOT NULL, " \ "interface TEXT NOT NULL, " \ "name TEXT, " \ @@ -77,47 +72,22 @@ bool create_network_addresses_table(void) "lastQuery INTEGER NOT NULL, " \ "numQueries INTEGER NOT NULL, " \ "macVendor TEXT);"); - if(!ret) - { - logg("create_network_addresses_table(): CREATE TABLE network_bck failed!"); - dbclose(); - return false; - } // Step 2: Copy data (except ip column) from network into network_back - ret = dbquery("INSERT INTO network_bck "\ + SQL("INSERT INTO network_bck "\ "SELECT id, hwaddr, interface, name, firstSeen, "\ "lastQuery, numQueries, macVendor "\ "FROM network;"); - if(!ret) - { - logg("create_network_addresses_table(): INSERT INTO network_bck failed!"); - dbclose(); - return false; - } // Step 3: Drop the network table, the unique index will be automatically dropped - ret = dbquery("DROP TABLE network;"); - if(!ret) - { - logg("create_network_addresses_table(): DROP TABLE network failed!"); - dbclose(); - return false; - } + SQL("DROP TABLE network;"); // Step 4: Rename network_bck table to network table as last step - ret = dbquery("ALTER TABLE network_bck RENAME TO network;"); - if(!ret) - { - logg("create_network_addresses_table(): ALTER TABLE network_bck failed!"); - dbclose(); - return false; - } + SQL("ALTER TABLE network_bck RENAME TO network;"); // Update database version to 5 - ret = db_set_FTL_property(DB_VERSION, 5); - if(!ret) + if(!db_set_FTL_property(DB_VERSION, 5)) { logg("create_network_addresses_table(): Failed to update database version!"); dbclose(); From 26978299d349aa163f59a79ae00ccbbd9b213b58 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 17 Jun 2019 00:06:29 +0200 Subject: [PATCH 27/35] Use new macros where possible to reduce code duplication. Signed-off-by: DL6ER --- networktable.c | 92 ++++++++++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 41 deletions(-) diff --git a/networktable.c b/networktable.c index ff4af0556..a8d617100 100644 --- a/networktable.c +++ b/networktable.c @@ -16,47 +16,57 @@ static char* getMACVendor(const char* hwaddr); bool unify_hwaddr(sqlite3 *db); +// Private macro +#define SQL(sql) {\ + if(!dbquery(sql)) {\ + logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ + dbclose();\ + return false;\ + }\ +} +#define SQL_void(sql) {\ + if(!dbquery(sql)) {\ + logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ + dbclose();\ + return;\ + }\ +} + bool create_network_table(void) { - bool ret; // Create network table in the database - ret = dbquery("CREATE TABLE network ( id INTEGER PRIMARY KEY NOT NULL, " \ - "ip TEXT NOT NULL, " \ - "hwaddr TEXT NOT NULL, " \ - "interface TEXT NOT NULL, " \ - "name TEXT, " \ - "firstSeen INTEGER NOT NULL, " \ - "lastQuery INTEGER NOT NULL, " \ - "numQueries INTEGER NOT NULL," \ - "macVendor TEXT);"); - if(!ret){ dbclose(); return false; } + SQL("CREATE TABLE network ( id INTEGER PRIMARY KEY NOT NULL, " \ + "ip TEXT NOT NULL, " \ + "hwaddr TEXT NOT NULL, " \ + "interface TEXT NOT NULL, " \ + "name TEXT, " \ + "firstSeen INTEGER NOT NULL, " \ + "lastQuery INTEGER NOT NULL, " \ + "numQueries INTEGER NOT NULL," \ + "macVendor TEXT);"); // Update database version to 3 - ret = db_set_FTL_property(DB_VERSION, 3); - if(!ret){ dbclose(); return false; } + if(!db_set_FTL_property(DB_VERSION, 3)) + { + logg("create_network_table(): Failed to update database version!"); + dbclose(); + return false; + } return true; } -// Private macro -#define SQL(sql) {\ - if(!dbquery(sql)) {\ - logg("create_network_addresses_table(): \"%s\" failed!", sql);\ - dbclose();\ - return false;\ - }\ -} - bool create_network_addresses_table(void) { + // Begin new transaction SQL("BEGIN TRANSACTION"); // Create network_addresses table in the database SQL("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ - "ip TEXT NOT NULL, "\ - "lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%%s', 'now') as int)), "\ - "UNIQUE(network_id,ip), "\ - "FOREIGN KEY(network_id) REFERENCES network(id));"); + "ip TEXT NOT NULL, "\ + "lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%%s', 'now') as int)), "\ + "UNIQUE(network_id,ip), "\ + "FOREIGN KEY(network_id) REFERENCES network(id));"); // Create a network_addresses row for each entry in the network table SQL("INSERT INTO network_addresses (network_id,ip) SELECT id,ip FROM network;"); @@ -65,24 +75,23 @@ bool create_network_addresses_table(void) // As ALTER TABLE is severely limit, we have to do the column deletion manually. // Step 1: We create a new table without the ip column SQL("CREATE TABLE network_bck ( id INTEGER PRIMARY KEY NOT NULL, " \ - "hwaddr TEXT UNIQUE NOT NULL, " \ - "interface TEXT NOT NULL, " \ - "name TEXT, " \ - "firstSeen INTEGER NOT NULL, " \ - "lastQuery INTEGER NOT NULL, " \ - "numQueries INTEGER NOT NULL, " \ - "macVendor TEXT);"); + "hwaddr TEXT UNIQUE NOT NULL, " \ + "interface TEXT NOT NULL, " \ + "name TEXT, " \ + "firstSeen INTEGER NOT NULL, " \ + "lastQuery INTEGER NOT NULL, " \ + "numQueries INTEGER NOT NULL, " \ + "macVendor TEXT);"); // Step 2: Copy data (except ip column) from network into network_back SQL("INSERT INTO network_bck "\ - "SELECT id, hwaddr, interface, name, firstSeen, "\ - "lastQuery, numQueries, macVendor "\ - "FROM network;"); + "SELECT id, hwaddr, interface, name, firstSeen, "\ + "lastQuery, numQueries, macVendor "\ + "FROM network;"); // Step 3: Drop the network table, the unique index will be automatically dropped SQL("DROP TABLE network;"); - // Step 4: Rename network_bck table to network table as last step SQL("ALTER TABLE network_bck RENAME TO network;"); @@ -94,7 +103,8 @@ bool create_network_addresses_table(void) return false; } - dbquery("COMMIT"); + // Finish transaction + SQL("COMMIT"); return true; } @@ -132,7 +142,7 @@ void parse_neighbor_cache(void) // Start collecting database commands lock_shm(); - dbquery("BEGIN TRANSACTION"); + SQL_void("BEGIN TRANSACTION"); // Read ARP cache line by line while(getline(&linebuffer, &linebuffersize, arpfp) != -1) @@ -255,7 +265,7 @@ void parse_neighbor_cache(void) } // Actually update the database - dbquery("COMMIT"); + SQL_void("COMMIT"); unlock_shm(); // Debug logging @@ -344,7 +354,7 @@ bool unify_hwaddr(sqlite3 *db) // See https://www.sqlite.org/lang_createtable.html#constraints: // >>> In most cases, UNIQUE and PRIMARY KEY constraints are // >>> implemented by creating a unique index in the database. - dbquery("CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr)"); + SQL("CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr)"); // Update database version to 4 if(!db_set_FTL_property(DB_VERSION, 4)) From 07acaf91873da52fe1ce63d72bd872d9876a8394 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 17 Jun 2019 00:09:12 +0200 Subject: [PATCH 28/35] Rename MACRO SQL to SQL_bool to highlight that it will return a boolean value on error. Signed-off-by: DL6ER --- networktable.c | 66 +++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/networktable.c b/networktable.c index a8d617100..f29c7ae8c 100644 --- a/networktable.c +++ b/networktable.c @@ -17,7 +17,7 @@ static char* getMACVendor(const char* hwaddr); bool unify_hwaddr(sqlite3 *db); // Private macro -#define SQL(sql) {\ +#define SQL_bool(sql) {\ if(!dbquery(sql)) {\ logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ dbclose();\ @@ -35,15 +35,15 @@ bool unify_hwaddr(sqlite3 *db); bool create_network_table(void) { // Create network table in the database - SQL("CREATE TABLE network ( id INTEGER PRIMARY KEY NOT NULL, " \ - "ip TEXT NOT NULL, " \ - "hwaddr TEXT NOT NULL, " \ - "interface TEXT NOT NULL, " \ - "name TEXT, " \ - "firstSeen INTEGER NOT NULL, " \ - "lastQuery INTEGER NOT NULL, " \ - "numQueries INTEGER NOT NULL," \ - "macVendor TEXT);"); + SQL_bool("CREATE TABLE network ( id INTEGER PRIMARY KEY NOT NULL, " \ + "ip TEXT NOT NULL, " \ + "hwaddr TEXT NOT NULL, " \ + "interface TEXT NOT NULL, " \ + "name TEXT, " \ + "firstSeen INTEGER NOT NULL, " \ + "lastQuery INTEGER NOT NULL, " \ + "numQueries INTEGER NOT NULL," \ + "macVendor TEXT);"); // Update database version to 3 if(!db_set_FTL_property(DB_VERSION, 3)) @@ -59,41 +59,41 @@ bool create_network_table(void) bool create_network_addresses_table(void) { // Begin new transaction - SQL("BEGIN TRANSACTION"); + SQL_bool("BEGIN TRANSACTION"); // Create network_addresses table in the database - SQL("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ - "ip TEXT NOT NULL, "\ - "lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%%s', 'now') as int)), "\ - "UNIQUE(network_id,ip), "\ - "FOREIGN KEY(network_id) REFERENCES network(id));"); + SQL_bool("CREATE TABLE network_addresses ( network_id INTEGER NOT NULL, "\ + "ip TEXT NOT NULL, "\ + "lastSeen INTEGER NOT NULL DEFAULT (cast(strftime('%%s', 'now') as int)), "\ + "UNIQUE(network_id,ip), "\ + "FOREIGN KEY(network_id) REFERENCES network(id));"); // Create a network_addresses row for each entry in the network table - SQL("INSERT INTO network_addresses (network_id,ip) SELECT id,ip FROM network;"); + SQL_bool("INSERT INTO network_addresses (network_id,ip) SELECT id,ip FROM network;"); // Remove IP column from network table. // As ALTER TABLE is severely limit, we have to do the column deletion manually. // Step 1: We create a new table without the ip column - SQL("CREATE TABLE network_bck ( id INTEGER PRIMARY KEY NOT NULL, " \ - "hwaddr TEXT UNIQUE NOT NULL, " \ - "interface TEXT NOT NULL, " \ - "name TEXT, " \ - "firstSeen INTEGER NOT NULL, " \ - "lastQuery INTEGER NOT NULL, " \ - "numQueries INTEGER NOT NULL, " \ - "macVendor TEXT);"); + SQL_bool("CREATE TABLE network_bck ( id INTEGER PRIMARY KEY NOT NULL, " \ + "hwaddr TEXT UNIQUE NOT NULL, " \ + "interface TEXT NOT NULL, " \ + "name TEXT, " \ + "firstSeen INTEGER NOT NULL, " \ + "lastQuery INTEGER NOT NULL, " \ + "numQueries INTEGER NOT NULL, " \ + "macVendor TEXT);"); // Step 2: Copy data (except ip column) from network into network_back - SQL("INSERT INTO network_bck "\ - "SELECT id, hwaddr, interface, name, firstSeen, "\ - "lastQuery, numQueries, macVendor "\ - "FROM network;"); + SQL_bool("INSERT INTO network_bck "\ + "SELECT id, hwaddr, interface, name, firstSeen, "\ + "lastQuery, numQueries, macVendor "\ + "FROM network;"); // Step 3: Drop the network table, the unique index will be automatically dropped - SQL("DROP TABLE network;"); + SQL_bool("DROP TABLE network;"); // Step 4: Rename network_bck table to network table as last step - SQL("ALTER TABLE network_bck RENAME TO network;"); + SQL_bool("ALTER TABLE network_bck RENAME TO network;"); // Update database version to 5 if(!db_set_FTL_property(DB_VERSION, 5)) @@ -104,7 +104,7 @@ bool create_network_addresses_table(void) } // Finish transaction - SQL("COMMIT"); + SQL_bool("COMMIT"); return true; } @@ -354,7 +354,7 @@ bool unify_hwaddr(sqlite3 *db) // See https://www.sqlite.org/lang_createtable.html#constraints: // >>> In most cases, UNIQUE and PRIMARY KEY constraints are // >>> implemented by creating a unique index in the database. - SQL("CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr)"); + SQL_bool("CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr)"); // Update database version to 4 if(!db_set_FTL_property(DB_VERSION, 4)) From faee4710e2add276ac68d6e3ac9ecf1e40958d2d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 19 Jun 2019 15:53:09 +0200 Subject: [PATCH 29/35] Move database macros into routines.h Signed-off-by: DL6ER --- networktable.c | 16 ---------------- routines.h | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/networktable.c b/networktable.c index f29c7ae8c..0455a640a 100644 --- a/networktable.c +++ b/networktable.c @@ -16,22 +16,6 @@ static char* getMACVendor(const char* hwaddr); bool unify_hwaddr(sqlite3 *db); -// Private macro -#define SQL_bool(sql) {\ - if(!dbquery(sql)) {\ - logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ - dbclose();\ - return false;\ - }\ -} -#define SQL_void(sql) {\ - if(!dbquery(sql)) {\ - logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ - dbclose();\ - return;\ - }\ -} - bool create_network_table(void) { // Create network table in the database diff --git a/routines.h b/routines.h index 0e3d973d8..ee915cc4a 100644 --- a/routines.h +++ b/routines.h @@ -169,4 +169,23 @@ bool gravityDB_getTable(unsigned char list); const char* gravityDB_getDomain(void); void gravityDB_finalizeTable(void); int gravityDB_count(unsigned char list); + + +// Database macros +#define SQL_bool(sql) {\ + if(!dbquery(sql)) {\ + logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ + dbclose();\ + return false;\ + }\ +} + +#define SQL_void(sql) {\ + if(!dbquery(sql)) {\ + logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ + dbclose();\ + return;\ + }\ +} + #endif // ROUTINES_H From 63b07246c6d28a6dac428e9ec6d6776e88906a48 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 19 Jun 2019 15:59:40 +0200 Subject: [PATCH 30/35] Use SQL_* macros where possible in database.c Signed-off-by: DL6ER --- database.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/database.c b/database.c index 36cee33be..709a057d0 100644 --- a/database.c +++ b/database.c @@ -88,14 +88,7 @@ bool dbopen(void) // Foreign key constraints are disabled by default (for backwards // compatibility), so must be enabled separately for each database // connection (see also https://www.sqlite.org/foreignkeys.html). - rc = dbquery("PRAGMA FOREIGN_KEYS=ON;"); - if( !rc ) - { - logg("dbopen() - FOREIGN_KEYS error (%i): %s", rc, sqlite3_errmsg(db)); - dbclose(); - check_database(rc); - return false; - } + SQL_bool("PRAGMA FOREIGN_KEYS=ON;"); return true; } @@ -134,26 +127,20 @@ bool dbquery(const char *format, ...) static bool create_counter_table(void) { - bool ret; // Create FTL table in the database (holds properties like database version, etc.) - ret = dbquery("CREATE TABLE counters ( id INTEGER PRIMARY KEY NOT NULL, value INTEGER NOT NULL );"); - if(!ret){ dbclose(); return false; } + SQL_bool("CREATE TABLE counters ( id INTEGER PRIMARY KEY NOT NULL, value INTEGER NOT NULL );"); // ID 0 = total queries - ret = db_set_counter(DB_TOTALQUERIES, 0); - if(!ret){ dbclose(); return false; } + if(!db_set_counter(DB_TOTALQUERIES, 0)){ dbclose(); return false; } // ID 1 = total blocked queries - ret = db_set_counter(DB_BLOCKEDQUERIES, 0); - if(!ret){ dbclose(); return false; } + if(!db_set_counter(DB_BLOCKEDQUERIES, 0)){ dbclose(); return false; } // Time stamp of creation of the counters database - ret = db_set_FTL_property(DB_FIRSTCOUNTERTIMESTAMP, time(NULL)); - if(!ret){ dbclose(); return false; } + if(!db_set_FTL_property(DB_FIRSTCOUNTERTIMESTAMP, time(NULL))){ dbclose(); return false; } // Update database version to 2 - ret = db_set_FTL_property(DB_VERSION, 2); - if(!ret){ dbclose(); return false; } + if(!db_set_FTL_property(DB_VERSION, 2)){ dbclose(); return false; } return true; } @@ -169,14 +156,13 @@ static bool db_create(void) return false; } // Create Queries table in the database - ret = dbquery("CREATE TABLE queries ( id INTEGER PRIMARY KEY AUTOINCREMENT, timestamp INTEGER NOT NULL, type INTEGER NOT NULL, status INTEGER NOT NULL, domain TEXT NOT NULL, client TEXT NOT NULL, forward TEXT );"); - if(!ret){ dbclose(); return false; } + SQL_bool("CREATE TABLE queries ( id INTEGER PRIMARY KEY AUTOINCREMENT, timestamp INTEGER NOT NULL, type INTEGER NOT NULL, status INTEGER NOT NULL, domain TEXT NOT NULL, client TEXT NOT NULL, forward TEXT );"); + // Add an index on the timestamps (not a unique index!) - ret = dbquery("CREATE INDEX idx_queries_timestamps ON queries (timestamp);"); - if(!ret){ dbclose(); return false; } + SQL_bool("CREATE INDEX idx_queries_timestamps ON queries (timestamp);"); + // Create FTL table in the database (holds properties like database version, etc.) - ret = dbquery("CREATE TABLE ftl ( id INTEGER PRIMARY KEY NOT NULL, value BLOB NOT NULL );"); - if(!ret){ dbclose(); return false; } + SQL_bool("CREATE TABLE ftl ( id INTEGER PRIMARY KEY NOT NULL, value BLOB NOT NULL );"); // Set DB version 1 ret = dbquery("INSERT INTO ftl (ID,VALUE) VALUES(%i,1);", DB_VERSION); @@ -615,9 +601,9 @@ void save_to_DB(void) } // Finish prepared statement - ret = dbquery("END TRANSACTION"); + SQL_void("END TRANSACTION"); int ret2 = sqlite3_finalize(stmt); - if(!ret || ret2 != SQLITE_OK){ dbclose(); return; } + if(ret2 != SQLITE_OK){ dbclose(); return; } // Store index for next loop interation round and update last time stamp // in the database only if all queries have been saved successfully From 9d93763d438f7df4194391ba1a4a0bca67af17d4 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 24 Jun 2019 17:35:18 +0200 Subject: [PATCH 31/35] Enable the enforcement of foreign key constraints for all database connections through a compile time flag. Signed-off-by: DL6ER --- Makefile | 3 ++- database.c | 17 ++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 12b8cb438..1bb69098b 100644 --- a/Makefile +++ b/Makefile @@ -49,7 +49,8 @@ DEBUG_FLAGS=-rdynamic -fno-omit-frame-pointer # -DSQLITE_DEFAULT_MEMSTATUS=0: This setting causes the sqlite3_status() interfaces that track memory usage to be disabled. This helps the sqlite3_malloc() routines run much faster, and since SQLite uses sqlite3_malloc() internally, this helps to make the entire library faster. # -DSQLITE_OMIT_DEPRECATED: Omitting deprecated interfaces and features will not help SQLite to run any faster. It will reduce the library footprint, however. And it is the right thing to do. # -DSQLITE_OMIT_PROGRESS_CALLBACK: The progress handler callback counter must be checked in the inner loop of the bytecode engine. By omitting this interface, a single conditional is removed from the inner loop of the bytecode engine, helping SQL statements to run slightly faster. -SQLITEFLAGS=-DSQLITE_OMIT_LOAD_EXTENSION -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_OMIT_DEPRECATED -DSQLITE_OMIT_PROGRESS_CALLBACK -DSQLITE_OMIT_MEMORYDB +# -DSQLITE_DEFAULT_FOREIGN_KEYS=1: This macro determines whether enforcement of foreign key constraints is enabled or disabled by default for new database connections. +SQLITEFLAGS=-DSQLITE_OMIT_LOAD_EXTENSION -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_OMIT_DEPRECATED -DSQLITE_OMIT_PROGRESS_CALLBACK -DSQLITE_OMIT_MEMORYDB -DSQLITE_DEFAULT_FOREIGN_KEYS=1 # -Wall: This enables all the warnings about constructions that some users consider questionable, and that are easy to avoid (or modify to prevent the warning), even in conjunction with macros. This also enables some language-specific warnings described in C++ Dialect Options and Objective-C and Objective-C++ Dialect Options. # -Wextra: This enables some extra warning flags that are not enabled by -Wall. diff --git a/database.c b/database.c index 709a057d0..6a056ed24 100644 --- a/database.c +++ b/database.c @@ -47,8 +47,10 @@ void dbclose(void) { int rc = sqlite3_close(db); // Report any error - if( rc ) + if( rc != SQLITE_OK ) + { logg("dbclose() - SQL error (%i): %s", rc, sqlite3_errmsg(db)); + } // Unlock mutex on the database pthread_mutex_unlock(&dblock); @@ -75,21 +77,13 @@ bool dbopen(void) { pthread_mutex_lock(&dblock); int rc = sqlite3_open_v2(FTLfiles.db, &db, SQLITE_OPEN_READWRITE, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("dbopen() - SQL error (%i): %s", rc, sqlite3_errmsg(db)); dbclose(); check_database(rc); return false; } - // Enable the enforcement of foreign key constraints. As of SQLite - // version 3.6.19 (2009-10-14), the default setting for foreign key - // enforcement is false. - // Foreign key constraints are disabled by default (for backwards - // compatibility), so must be enabled separately for each database - // connection (see also https://www.sqlite.org/foreignkeys.html). - SQL_bool("PRAGMA FOREIGN_KEYS=ON;"); - return true; } @@ -108,7 +102,8 @@ bool dbquery(const char *format, ...) return false; } - if(config.debug & DEBUG_DATABASE) logg("dbquery: %s", query); + if(config.debug & DEBUG_DATABASE) + logg("dbquery: \"%s\"", query); int rc = sqlite3_exec(db, query, NULL, NULL, &zErrMsg); From adbdf3ff056a711984fa9567b49638ebbd01d644 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 24 Jun 2019 22:58:28 +0200 Subject: [PATCH 32/35] Running dbclose() within check_database() further reduces code duplication Signed-off-by: DL6ER --- database.c | 45 ++++++++++++++------------------------------- networktable.c | 34 ++++++++++++++-------------------- routines.h | 3 +-- 3 files changed, 29 insertions(+), 53 deletions(-) diff --git a/database.c b/database.c index 6a056ed24..2d5de83ad 100644 --- a/database.c +++ b/database.c @@ -25,7 +25,7 @@ static int db_get_FTL_property(const unsigned int ID); // defined in networktable.c extern bool unify_hwaddr(sqlite3 *db); -static bool check_database(int rc) +bool check_database(int rc) { // We will retry if the database is busy at the moment // However, we won't retry if any other error happened @@ -37,6 +37,7 @@ static bool check_database(int rc) rc != SQLITE_BUSY) { logg("check_database(%i): Disabling database connection due to error", rc); + dbclose(); database = false; } @@ -142,11 +143,9 @@ static bool create_counter_table(void) static bool db_create(void) { - bool ret; int rc = sqlite3_open_v2(FTLfiles.db, &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("db_create() - SQL error (%i): %s", rc, sqlite3_errmsg(db)); - dbclose(); check_database(rc); return false; } @@ -160,12 +159,12 @@ static bool db_create(void) SQL_bool("CREATE TABLE ftl ( id INTEGER PRIMARY KEY NOT NULL, value BLOB NOT NULL );"); // Set DB version 1 - ret = dbquery("INSERT INTO ftl (ID,VALUE) VALUES(%i,1);", DB_VERSION); - if(!ret){ dbclose(); return false; } + if(!dbquery("INSERT INTO ftl (ID,VALUE) VALUES(%i,1);", DB_VERSION)) + return false; // Most recent timestamp initialized to 00:00 1 Jan 1970 - ret = dbquery("INSERT INTO ftl (ID,VALUE) VALUES(%i,0);", DB_LASTTIMESTAMP); - if(!ret){ dbclose(); return false; } + if(!dbquery("INSERT INTO ftl (ID,VALUE) VALUES(%i,0);", DB_LASTTIMESTAMP)) + return false; // Create counter table // Will update DB version to 2 @@ -355,9 +354,8 @@ int db_query_int(const char* querystr) { sqlite3_stmt* stmt; int rc = sqlite3_prepare_v2(db, querystr, -1, &stmt, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("db_query_int(%s) - SQL error prepare (%i): %s", querystr, rc, sqlite3_errmsg(db)); - dbclose(); check_database(rc); return DB_FAILED; } @@ -377,7 +375,6 @@ int db_query_int(const char* querystr) else { logg("db_query_int(%s) - SQL error step (%i): %s", querystr, rc, sqlite3_errmsg(db)); - dbclose(); check_database(rc); return DB_FAILED; } @@ -393,9 +390,8 @@ static int number_of_queries_in_DB(void) // Count number of rows using the index timestamp is faster than select(*) int rc = sqlite3_prepare_v2(db, "SELECT COUNT(timestamp) FROM queries", -1, &stmt, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("number_of_queries_in_DB() - SQL error prepare (%i): %s", rc, sqlite3_errmsg(db)); - dbclose(); check_database(rc); return DB_FAILED; } @@ -403,7 +399,6 @@ static int number_of_queries_in_DB(void) rc = sqlite3_step(stmt); if( rc != SQLITE_ROW ){ logg("number_of_queries_in_DB() - SQL error step (%i): %s", rc, sqlite3_errmsg(db)); - dbclose(); check_database(rc); return DB_FAILED; } @@ -420,9 +415,8 @@ static sqlite3_int64 last_ID_in_DB(void) sqlite3_stmt* stmt; int rc = sqlite3_prepare_v2(db, "SELECT MAX(ID) FROM queries", -1, &stmt, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("last_ID_in_DB() - SQL error prepare (%i): %s", rc, sqlite3_errmsg(db)); - dbclose(); check_database(rc); return DB_FAILED; } @@ -430,7 +424,6 @@ static sqlite3_int64 last_ID_in_DB(void) rc = sqlite3_step(stmt); if( rc != SQLITE_ROW ){ logg("last_ID_in_DB() - SQL error step (%i): %s", rc, sqlite3_errmsg(db)); - dbclose(); check_database(rc); return DB_FAILED; } @@ -482,19 +475,12 @@ void save_to_DB(void) // Get last ID stored in the database sqlite3_int64 lastID = last_ID_in_DB(); - bool ret = dbquery("BEGIN TRANSACTION"); - if(!ret) - { - logg("save_to_DB() - unable to begin transaction (%i): %s", ret, sqlite3_errmsg(db)); - dbclose(); - return; - } + SQL_void("BEGIN TRANSACTION"); int rc = sqlite3_prepare_v2(db, "INSERT INTO queries VALUES (NULL,?,?,?,?,?,?)", -1, &stmt, NULL); - if( rc ) + if( rc != SQLITE_OK ) { - logg("save_to_DB() - error in preparing SQL statement (%i): %s", ret, sqlite3_errmsg(db)); - dbclose(); + logg("save_to_DB() - error in preparing SQL statement (%i): %s", rc, sqlite3_errmsg(db)); check_database(rc); return; } @@ -639,9 +625,7 @@ static void delete_old_queries_in_DB(void) if(!dbquery("DELETE FROM queries WHERE timestamp <= %i", timestamp)) { - dbclose(); logg("delete_old_queries_in_DB(): Deleting queries due to age of entries failed!"); - database = true; return; } @@ -735,9 +719,8 @@ void read_data_from_DB(void) // Prepare SQLite3 statement sqlite3_stmt* stmt = NULL; rc = sqlite3_prepare_v2(db, rstr, -1, &stmt, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("read_data_from_DB() - SQL error prepare (%i): %s", rc, sqlite3_errmsg(db)); - dbclose(); check_database(rc); return; } diff --git a/networktable.c b/networktable.c index 0455a640a..dfc43bed7 100644 --- a/networktable.c +++ b/networktable.c @@ -33,7 +33,6 @@ bool create_network_table(void) if(!db_set_FTL_property(DB_VERSION, 3)) { logg("create_network_table(): Failed to update database version!"); - dbclose(); return false; } @@ -83,7 +82,6 @@ bool create_network_addresses_table(void) if(!db_set_FTL_property(DB_VERSION, 5)) { logg("create_network_addresses_table(): Failed to update database version!"); - dbclose(); return false; } @@ -96,9 +94,16 @@ bool create_network_addresses_table(void) // Parse kernel's neighbor cache void parse_neighbor_cache(void) { - FILE* arpfp = NULL; + // Open database file + if(!dbopen()) + { + logg("parse_arp_cache() - Failed to open DB"); + return; + } + // Try to access the kernel's neighbor cache // We are only interested in entries which are in either STALE or REACHABLE state + FILE* arpfp = NULL; if((arpfp = popen("ip neigh show nud stale nud reachable", "r")) == NULL) { logg("WARN: Command \"ip neigh show nud stale nud reachable\" failed!"); @@ -106,14 +111,6 @@ void parse_neighbor_cache(void) return; } - // Open database file - if(!dbopen()) - { - logg("parse_arp_cache() - Failed to open DB"); - pclose(arpfp); - return; - } - // Start ARP timer if(config.debug & DEBUG_ARP) timer_start(ARP_TIMER); @@ -286,9 +283,9 @@ bool unify_hwaddr(sqlite3 *db) // Perform SQL query sqlite3_stmt* stmt; ret = sqlite3_prepare_v2(db, querystr, -1, &stmt, NULL); - if( ret ){ + if( ret != SQLITE_OK){ logg("unify_hwaddr(%s) - SQL error prepare (%i): %s", querystr, ret, sqlite3_errmsg(db)); - dbclose(); + check_database(ret); return false; } @@ -342,10 +339,7 @@ bool unify_hwaddr(sqlite3 *db) // Update database version to 4 if(!db_set_FTL_property(DB_VERSION, 4)) - { - dbclose(); return false; - } return true; } @@ -370,7 +364,7 @@ static char* getMACVendor(const char* hwaddr) sqlite3 *macdb; int rc = sqlite3_open_v2(FTLfiles.macvendordb, &macdb, SQLITE_OPEN_READONLY, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("getMACVendor(%s) - SQL error (%i): %s", hwaddr, rc, sqlite3_errmsg(macdb)); sqlite3_close(macdb); return strdup(""); @@ -391,7 +385,7 @@ static char* getMACVendor(const char* hwaddr) sqlite3_stmt* stmt; rc = sqlite3_prepare_v2(macdb, querystr, -1, &stmt, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("getMACVendor(%s) - SQL error prepare (%s, %i): %s", hwaddr, querystr, rc, sqlite3_errmsg(macdb)); sqlite3_close(macdb); return strdup(""); @@ -435,7 +429,7 @@ void updateMACVendorRecords() sqlite3 *db; int rc = sqlite3_open_v2(FTLfiles.db, &db, SQLITE_OPEN_READWRITE, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("updateMACVendorRecords() - SQL error (%i): %s", rc, sqlite3_errmsg(db)); sqlite3_close(db); return; @@ -444,7 +438,7 @@ void updateMACVendorRecords() sqlite3_stmt* stmt; const char* selectstr = "SELECT id,hwaddr FROM network;"; rc = sqlite3_prepare_v2(db, selectstr, -1, &stmt, NULL); - if( rc ){ + if( rc != SQLITE_OK ){ logg("updateMACVendorRecords() - SQL error prepare (%s, %i): %s", selectstr, rc, sqlite3_errmsg(db)); sqlite3_close(db); return; diff --git a/routines.h b/routines.h index ee915cc4a..a1ce1f34f 100644 --- a/routines.h +++ b/routines.h @@ -85,6 +85,7 @@ void *GC_thread(void *val); // database.c void db_init(void); +bool check_database(int rc); void *DB_thread(void *val); int get_number_of_queries_in_DB(void); void save_to_DB(void); @@ -175,7 +176,6 @@ int gravityDB_count(unsigned char list); #define SQL_bool(sql) {\ if(!dbquery(sql)) {\ logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ - dbclose();\ return false;\ }\ } @@ -183,7 +183,6 @@ int gravityDB_count(unsigned char list); #define SQL_void(sql) {\ if(!dbquery(sql)) {\ logg("%s(): \"%s\" failed!", __FUNCTION__, sql);\ - dbclose();\ return;\ }\ } From b98535c7c508f2ed6e1dc6b8986cd0452413a2cd Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 26 Jun 2019 17:26:19 +0200 Subject: [PATCH 33/35] Close the database connection in case the ip neigh command fails. Signed-off-by: DL6ER --- networktable.c | 1 + 1 file changed, 1 insertion(+) diff --git a/networktable.c b/networktable.c index dfc43bed7..e1ad7e2d0 100644 --- a/networktable.c +++ b/networktable.c @@ -108,6 +108,7 @@ void parse_neighbor_cache(void) { logg("WARN: Command \"ip neigh show nud stale nud reachable\" failed!"); logg(" Message: %s", strerror(errno)); + dbclose(); return; } From f7cbe107163fb58100d3c330f6b9ec9f55cb8ac5 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 1 Jul 2019 10:39:47 +0200 Subject: [PATCH 34/35] Temporarily disable foreign key enforcement during update. This becomes necessary as we enabled foreign key enforcement by default for all database connections. Signed-off-by: DL6ER --- networktable.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/networktable.c b/networktable.c index e1ad7e2d0..ebde40a0c 100644 --- a/networktable.c +++ b/networktable.c @@ -41,6 +41,10 @@ bool create_network_table(void) bool create_network_addresses_table(void) { + // Disable foreign key enforcement for this transaction + // Otherwise, dropping the network table would not be allowed + SQL_bool("PRAGMA foreign_keys=OFF"); + // Begin new transaction SQL_bool("BEGIN TRANSACTION"); @@ -88,6 +92,9 @@ bool create_network_addresses_table(void) // Finish transaction SQL_bool("COMMIT"); + // Re-enable foreign key enforcement + SQL_bool("PRAGMA foreign_keys=OFF"); + return true; } From 26de08684086896841a0f26d7a4782f80b57b53d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 3 Jul 2019 11:46:14 +0200 Subject: [PATCH 35/35] Re-enable foreign key enforcement at the end of the upgrade transaction. Signed-off-by: DL6ER --- networktable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/networktable.c b/networktable.c index ebde40a0c..31c1711f1 100644 --- a/networktable.c +++ b/networktable.c @@ -93,7 +93,7 @@ bool create_network_addresses_table(void) SQL_bool("COMMIT"); // Re-enable foreign key enforcement - SQL_bool("PRAGMA foreign_keys=OFF"); + SQL_bool("PRAGMA foreign_keys=ON"); return true; }