Change the nwfilter driver loading mechanism to read from nwfilter.conf.
By default, it will use the existing ebiptables driver, which can be
replaced in the future to remove the {eb,ip}tables dependency.

Added nftables to *filter_tech_drivers as an available driver option
for users to choose from.

Signed-off-by: Dion Bosschieter <dionbosschieter@gmail.com>
---
 src/nwfilter/nwfilter_driver.c         | 49 +++++++++++++++++++--
 src/nwfilter/nwfilter_gentech_driver.c | 60 +++++++++++---------------
 src/nwfilter/nwfilter_gentech_driver.h |  4 +-
 3 files changed, 73 insertions(+), 40 deletions(-)

diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 522cfda022..18d322574d 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -26,9 +26,7 @@
 
 #include "virgdbus.h"
 #include "virlog.h"
-
 #include "internal.h"
-
 #include "virerror.h"
 #include "datatypes.h"
 #include "nwfilter_driver.h"
@@ -36,7 +34,6 @@
 #include "configmake.h"
 #include "virpidfile.h"
 #include "viraccessapicheck.h"
-
 #include "nwfilter_ipaddrmap.h"
 #include "nwfilter_dhcpsnoop.h"
 #include "nwfilter_learnipaddr.h"
@@ -203,6 +200,41 @@ nwfilterStateCleanup(void)
 }
 
 
+/**
+ * virNWFilterLoadGentechDriverFromConfig:
+ *
+ * Loading driver name from nwfilter.conf config file
+ */
+static char *
+virNWFilterLoadGentechDriverFromConfig(const char *configfile)
+{
+    g_autoptr(virConf) conf = NULL;
+    g_autofree char *drivername = NULL;
+
+    if (access(configfile, R_OK) == 0) {
+
+        conf = virConfReadFile(configfile, 0);
+        if (!conf)
+            return NULL;
+
+        if (virConfGetValueString(conf, "driver", &drivername) < 0)
+            return NULL;
+
+        if (drivername) {
+            VIR_DEBUG("nwfilter driver setting requested from config file %s: '%s'",
+                      configfile, drivername);
+        }
+    }
+
+    if (!drivername) {
+        drivername = g_strdup(NWFILTER_DEFAULT_DRIVER);
+    }
+
+
+    return g_steal_pointer(&drivername);
+}
+
+
 /**
  * nwfilterStateInitialize:
  *
@@ -217,6 +249,8 @@ nwfilterStateInitialize(bool privileged,
 {
     VIR_LOCK_GUARD lock = virLockGuardLock(&driverMutex);
     GDBusConnection *sysbus = NULL;
+    g_autofree char *configfile = NULL;
+    char *gentechdrivername = NULL;
 
     if (root != NULL) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -266,7 +300,14 @@ nwfilterStateInitialize(bool privileged,
     if (virNWFilterDHCPSnoopInit() < 0)
         goto error;
 
-    if (virNWFilterTechDriversInit(privileged) < 0)
+    configfile = g_strdup(SYSCONFDIR "/libvirt/nwfilter.conf");
+
+    /* get chosen driver from config file */
+    gentechdrivername = virNWFilterLoadGentechDriverFromConfig(configfile);
+    if (gentechdrivername == NULL)
+        goto error;
+
+    if (virNWFilterTechDriversInit(privileged, gentechdrivername) < 0)
         goto error;
 
     if (virNWFilterConfLayerInit(virNWFilterTriggerRebuildImpl, driver) < 0)
diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 1465734a54..adb96acca6 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -32,6 +32,7 @@
 #include "nwfilter_dhcpsnoop.h"
 #include "nwfilter_ipaddrmap.h"
 #include "nwfilter_learnipaddr.h"
+#include "nwfilter_nftables_driver.h"
 #include "virnetdev.h"
 
 #define VIR_FROM_THIS VIR_FROM_NWFILTER
@@ -48,18 +49,20 @@ static int _virNWFilterTeardownFilter(const char *ifname);
 
 static virNWFilterTechDriver *filter_tech_drivers[] = {
     &ebiptables_driver,
-    NULL
+    &nftables_driver,
 };
 
-int virNWFilterTechDriversInit(bool privileged)
+int virNWFilterTechDriversInit(bool privileged, const char *drivername)
 {
     size_t i = 0;
-    VIR_DEBUG("Initializing NWFilter technology drivers");
-    while (filter_tech_drivers[i]) {
-        if (!(filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED))
+    VIR_DEBUG("Initializing NWFilter technology drivers, chosen %s", drivername);
+
+    for (i = 0; i < G_N_ELEMENTS(filter_tech_drivers); i++) {
+        if (!(filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED)
+            && STREQ(filter_tech_drivers[i]->name, drivername))
             filter_tech_drivers[i]->init(privileged);
-        i++;
     }
+
     return 0;
 }
 
@@ -67,25 +70,20 @@ int virNWFilterTechDriversInit(bool privileged)
 void virNWFilterTechDriversShutdown(void)
 {
     size_t i = 0;
-    while (filter_tech_drivers[i]) {
+    for (i = 0; i < G_N_ELEMENTS(filter_tech_drivers); i++) {
         if ((filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED))
             filter_tech_drivers[i]->shutdown();
-        i++;
     }
 }
 
 
 static virNWFilterTechDriver *
-virNWFilterTechDriverForName(const char *name)
+virNWFilterInitializedTechDriver(void)
 {
     size_t i = 0;
-    while (filter_tech_drivers[i]) {
-        if (STREQ(filter_tech_drivers[i]->name, name)) {
-            if ((filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED) == 0)
-                break;
+    for (i = 0; i < G_N_ELEMENTS(filter_tech_drivers); i++) {
+        if ((filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED))
             return filter_tech_drivers[i];
-        }
-        i++;
     }
     return NULL;
 }
@@ -617,7 +615,6 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverState *driver,
                                    bool *foundNewFilter)
 {
     int rc = -1;
-    const char *drvname = EBIPTABLES_DRIVER_ID;
     virNWFilterTechDriver *techdriver;
     virNWFilterObj *obj;
     virNWFilterDef *filter;
@@ -625,12 +622,11 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverState *driver,
     char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0};
     virNWFilterVarValue *ipaddr;
 
-    techdriver = virNWFilterTechDriverForName(drvname);
+    techdriver = virNWFilterInitializedTechDriver();
 
     if (!techdriver) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Could not get access to ACL tech driver '%1$s'"),
-                       drvname);
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not get access to ACL tech driver"));
         return -1;
     }
 
@@ -768,15 +764,13 @@ virNWFilterUpdateInstantiateFilter(virNWFilterDriverState *driver,
 static int
 virNWFilterRollbackUpdateFilter(virNWFilterBindingDef *binding)
 {
-    const char *drvname = EBIPTABLES_DRIVER_ID;
     int ifindex;
     virNWFilterTechDriver *techdriver;
 
-    techdriver = virNWFilterTechDriverForName(drvname);
+    techdriver = virNWFilterInitializedTechDriver();
     if (!techdriver) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Could not get access to ACL tech driver '%1$s'"),
-                       drvname);
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not get access to ACL tech driver"));
         return -1;
     }
 
@@ -793,15 +787,13 @@ virNWFilterRollbackUpdateFilter(virNWFilterBindingDef *binding)
 static int
 virNWFilterTearOldFilter(virNWFilterBindingDef *binding)
 {
-    const char *drvname = EBIPTABLES_DRIVER_ID;
     int ifindex;
     virNWFilterTechDriver *techdriver;
 
-    techdriver = virNWFilterTechDriverForName(drvname);
+    techdriver = virNWFilterInitializedTechDriver();
     if (!techdriver) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Could not get access to ACL tech driver '%1$s'"),
-                       drvname);
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not get access to ACL tech driver"));
         return -1;
     }
 
@@ -818,14 +810,12 @@ virNWFilterTearOldFilter(virNWFilterBindingDef *binding)
 static int
 _virNWFilterTeardownFilter(const char *ifname)
 {
-    const char *drvname = EBIPTABLES_DRIVER_ID;
     virNWFilterTechDriver *techdriver;
-    techdriver = virNWFilterTechDriverForName(drvname);
+    techdriver = virNWFilterInitializedTechDriver();
 
     if (!techdriver) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Could not get access to ACL tech driver '%1$s'"),
-                       drvname);
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not get access to ACL tech driver"));
         return -1;
     }
 
diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h
index 946d5d3d56..8f6f4d164a 100644
--- a/src/nwfilter/nwfilter_gentech_driver.h
+++ b/src/nwfilter/nwfilter_gentech_driver.h
@@ -25,7 +25,9 @@
 #include "virnwfilterobj.h"
 #include "virnwfilterbindingdef.h"
 
-int virNWFilterTechDriversInit(bool privileged);
+#define NWFILTER_DEFAULT_DRIVER "ebiptables"
+
+int virNWFilterTechDriversInit(bool privileged, const char *drivername);
 void virNWFilterTechDriversShutdown(void);
 
 enum instCase {
-- 
2.43.0

On Fri, Oct 31, 2025 at 01:05:44PM +0100, Dion Bosschieter wrote:
> Change the nwfilter driver loading mechanism to read from nwfilter.conf.
> By default, it will use the existing ebiptables driver, which can be
> replaced in the future to remove the {eb,ip}tables dependency.
> 
> Added nftables to *filter_tech_drivers as an available driver option
> for users to choose from.
> 
> Signed-off-by: Dion Bosschieter <dionbosschieter@gmail.com>
> ---
>  src/nwfilter/nwfilter_driver.c         | 49 +++++++++++++++++++--
>  src/nwfilter/nwfilter_gentech_driver.c | 60 +++++++++++---------------
>  src/nwfilter/nwfilter_gentech_driver.h |  4 +-
>  3 files changed, 73 insertions(+), 40 deletions(-)

Adding a config file makes sense, but we need todo a bit more.

The default file should be in git as src/nwfilter/nwfilter.conf.in
so we show (& thus document) the available config options, and
installed by meson.

Also there needs to be augeas files for parsing the config file.
For a trivial example, follow what's done for the virtual network
driver with src/network/libvirtd_network.aug  and
src/network/test_libvirtd_network.aug.in. Also see the meson
file for how to invoke the test file as part of unit tests.


> 
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index 522cfda022..18d322574d 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -26,9 +26,7 @@
>  
>  #include "virgdbus.h"
>  #include "virlog.h"
> -
>  #include "internal.h"
> -
>  #include "virerror.h"
>  #include "datatypes.h"
>  #include "nwfilter_driver.h"
> @@ -36,7 +34,6 @@
>  #include "configmake.h"
>  #include "virpidfile.h"
>  #include "viraccessapicheck.h"
> -
>  #include "nwfilter_ipaddrmap.h"
>  #include "nwfilter_dhcpsnoop.h"
>  #include "nwfilter_learnipaddr.h"
> @@ -203,6 +200,41 @@ nwfilterStateCleanup(void)
>  }
>  
>  
> +/**
> + * virNWFilterLoadGentechDriverFromConfig:
> + *
> + * Loading driver name from nwfilter.conf config file
> + */
> +static char *
> +virNWFilterLoadGentechDriverFromConfig(const char *configfile)
> +{
> +    g_autoptr(virConf) conf = NULL;
> +    g_autofree char *drivername = NULL;
> +
> +    if (access(configfile, R_OK) == 0) {
> +
> +        conf = virConfReadFile(configfile, 0);
> +        if (!conf)
> +            return NULL;
> +
> +        if (virConfGetValueString(conf, "driver", &drivername) < 0)
> +            return NULL;
> +
> +        if (drivername) {
> +            VIR_DEBUG("nwfilter driver setting requested from config file %s: '%s'",
> +                      configfile, drivername);
> +        }
> +    }
> +
> +    if (!drivername) {
> +        drivername = g_strdup(NWFILTER_DEFAULT_DRIVER);
> +    }

Rather than harcoding this, we should try to auto-detect the right
option, and we should honour the existing 'firewall_backend_priority'
meson option that is used for the network driver.

Take a look at src/network/bridge_driver_conf.c which can be used
as a template for the nwfilter driver - worth putting it into
separate nwfilter_driver_conf.{h,c} files to match the common
pattern we use in other drivers.

> +
> +
> +    return g_steal_pointer(&drivername);
> +}
> +
> +
>  /**
>   * nwfilterStateInitialize:
>   *
> @@ -217,6 +249,8 @@ nwfilterStateInitialize(bool privileged,
>  {
>      VIR_LOCK_GUARD lock = virLockGuardLock(&driverMutex);
>      GDBusConnection *sysbus = NULL;
> +    g_autofree char *configfile = NULL;
> +    char *gentechdrivername = NULL;
>  
>      if (root != NULL) {
>          virReportError(VIR_ERR_INVALID_ARG, "%s",
> @@ -266,7 +300,14 @@ nwfilterStateInitialize(bool privileged,
>      if (virNWFilterDHCPSnoopInit() < 0)
>          goto error;
>  
> -    if (virNWFilterTechDriversInit(privileged) < 0)
> +    configfile = g_strdup(SYSCONFDIR "/libvirt/nwfilter.conf");
> +
> +    /* get chosen driver from config file */
> +    gentechdrivername = virNWFilterLoadGentechDriverFromConfig(configfile);
> +    if (gentechdrivername == NULL)
> +        goto error;
> +
> +    if (virNWFilterTechDriversInit(privileged, gentechdrivername) < 0)
>          goto error;
>  
>      if (virNWFilterConfLayerInit(virNWFilterTriggerRebuildImpl, driver) < 0)
> diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
> index 1465734a54..adb96acca6 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.c
> +++ b/src/nwfilter/nwfilter_gentech_driver.c
> @@ -32,6 +32,7 @@
>  #include "nwfilter_dhcpsnoop.h"
>  #include "nwfilter_ipaddrmap.h"
>  #include "nwfilter_learnipaddr.h"
> +#include "nwfilter_nftables_driver.h"
>  #include "virnetdev.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_NWFILTER
> @@ -48,18 +49,20 @@ static int _virNWFilterTeardownFilter(const char *ifname);
>  
>  static virNWFilterTechDriver *filter_tech_drivers[] = {
>      &ebiptables_driver,
> -    NULL
> +    &nftables_driver,
>  };
>  
> -int virNWFilterTechDriversInit(bool privileged)
> +int virNWFilterTechDriversInit(bool privileged, const char *drivername)
>  {
>      size_t i = 0;
> -    VIR_DEBUG("Initializing NWFilter technology drivers");
> -    while (filter_tech_drivers[i]) {
> -        if (!(filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED))
> +    VIR_DEBUG("Initializing NWFilter technology drivers, chosen %s", drivername);
> +
> +    for (i = 0; i < G_N_ELEMENTS(filter_tech_drivers); i++) {
> +        if (!(filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED)
> +            && STREQ(filter_tech_drivers[i]->name, drivername))
>              filter_tech_drivers[i]->init(privileged);
> -        i++;
>      }
> +
>      return 0;
>  }
>  
> @@ -67,25 +70,20 @@ int virNWFilterTechDriversInit(bool privileged)
>  void virNWFilterTechDriversShutdown(void)
>  {
>      size_t i = 0;
> -    while (filter_tech_drivers[i]) {
> +    for (i = 0; i < G_N_ELEMENTS(filter_tech_drivers); i++) {
>          if ((filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED))
>              filter_tech_drivers[i]->shutdown();
> -        i++;
>      }
>  }
>  
>  
>  static virNWFilterTechDriver *
> -virNWFilterTechDriverForName(const char *name)
> +virNWFilterInitializedTechDriver(void)
>  {
>      size_t i = 0;
> -    while (filter_tech_drivers[i]) {
> -        if (STREQ(filter_tech_drivers[i]->name, name)) {
> -            if ((filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED) == 0)
> -                break;
> +    for (i = 0; i < G_N_ELEMENTS(filter_tech_drivers); i++) {
> +        if ((filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED))
>              return filter_tech_drivers[i];
> -        }
> -        i++;
>      }
>      return NULL;
>  }
> @@ -617,7 +615,6 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverState *driver,
>                                     bool *foundNewFilter)
>  {
>      int rc = -1;
> -    const char *drvname = EBIPTABLES_DRIVER_ID;
>      virNWFilterTechDriver *techdriver;
>      virNWFilterObj *obj;
>      virNWFilterDef *filter;
> @@ -625,12 +622,11 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverState *driver,
>      char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0};
>      virNWFilterVarValue *ipaddr;
>  
> -    techdriver = virNWFilterTechDriverForName(drvname);
> +    techdriver = virNWFilterInitializedTechDriver();
>  
>      if (!techdriver) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Could not get access to ACL tech driver '%1$s'"),
> -                       drvname);
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not get access to ACL tech driver"));
>          return -1;
>      }
>  
> @@ -768,15 +764,13 @@ virNWFilterUpdateInstantiateFilter(virNWFilterDriverState *driver,
>  static int
>  virNWFilterRollbackUpdateFilter(virNWFilterBindingDef *binding)
>  {
> -    const char *drvname = EBIPTABLES_DRIVER_ID;
>      int ifindex;
>      virNWFilterTechDriver *techdriver;
>  
> -    techdriver = virNWFilterTechDriverForName(drvname);
> +    techdriver = virNWFilterInitializedTechDriver();
>      if (!techdriver) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Could not get access to ACL tech driver '%1$s'"),
> -                       drvname);
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not get access to ACL tech driver"));
>          return -1;
>      }
>  
> @@ -793,15 +787,13 @@ virNWFilterRollbackUpdateFilter(virNWFilterBindingDef *binding)
>  static int
>  virNWFilterTearOldFilter(virNWFilterBindingDef *binding)
>  {
> -    const char *drvname = EBIPTABLES_DRIVER_ID;
>      int ifindex;
>      virNWFilterTechDriver *techdriver;
>  
> -    techdriver = virNWFilterTechDriverForName(drvname);
> +    techdriver = virNWFilterInitializedTechDriver();
>      if (!techdriver) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Could not get access to ACL tech driver '%1$s'"),
> -                       drvname);
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not get access to ACL tech driver"));
>          return -1;
>      }
>  
> @@ -818,14 +810,12 @@ virNWFilterTearOldFilter(virNWFilterBindingDef *binding)
>  static int
>  _virNWFilterTeardownFilter(const char *ifname)
>  {
> -    const char *drvname = EBIPTABLES_DRIVER_ID;
>      virNWFilterTechDriver *techdriver;
> -    techdriver = virNWFilterTechDriverForName(drvname);
> +    techdriver = virNWFilterInitializedTechDriver();
>  
>      if (!techdriver) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Could not get access to ACL tech driver '%1$s'"),
> -                       drvname);
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not get access to ACL tech driver"));
>          return -1;
>      }
>  
> diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h
> index 946d5d3d56..8f6f4d164a 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.h
> +++ b/src/nwfilter/nwfilter_gentech_driver.h
> @@ -25,7 +25,9 @@
>  #include "virnwfilterobj.h"
>  #include "virnwfilterbindingdef.h"
>  
> -int virNWFilterTechDriversInit(bool privileged);
> +#define NWFILTER_DEFAULT_DRIVER "ebiptables"
> +
> +int virNWFilterTechDriversInit(bool privileged, const char *drivername);
>  void virNWFilterTechDriversShutdown(void);
>  
>  enum instCase {
> -- 
> 2.43.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|