From 396e4e0698d9fb542f2eb8b32790a069e1c0df61 Mon Sep 17 00:00:00 2001

From: Laine Stump <laine@laine.org>

Date: Wed, 7 Oct 2015 13:49:45 -0400

Subject: [PATCH] optimize aug_match() query for all ifcfg files related to an

 interface

This resolves:

 https://bugzilla.redhat.com/show_bug.cgi?id=1271341 (Fedora)

 https://bugzilla.redhat.com/show_bug.cgi?id=1269613 (RHEL7)

The original augeas search term used by netcf to find, for example, all the

ifcfg files associated with device "br1" was:

     "/files/etc/sysconfig/network-scripts/*[ "

     "DEVICE = 'br1' or BRIDGE = 'br1' or MASTER = 'br1' or MASTER = "

     "../*[BRIDGE = 'br1']/DEVICE ]/DEVICE"

This is *extremely* inefficient - on a test host with 514 host

bridges, each with an attached vlan interface, a dumpxml of all

toplevel interfaces took 6m40s (*after* installing an augeas that

included augeas upstream commits a659f09a, 41e989ca, and 23d5e480

which were all pushed after the augeas-1.4.0 release).

In these two messages:

 https://www.redhat.com/archives/augeas-devel/2015-October/msg00003.html

 https://www.redhat.com/archives/augeas-devel/2015-October/msg00004.html

David Lutterkort suggested changing the search term to:

  "(/files/etc/sysconfig/network-scripts/*[(DEVICE|BRIDGE|MASTER) = 'br1']"

  "|/files/etc/sysconfig/network-scripts/*[MASTER]"

  "[MASTER = ../*[BRIDGE = 'br1']/DEVICE ])/DEVICE

That's what this patch does. Testing shows that it is functionally

equivalent, and reduces the dumpxml time in the previously described

test from 6m40s down to 17 seconds.

---

 src/drv_redhat.c | 44 ++++++++++++++++++++++++++++++++++----------

 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/src/drv_redhat.c b/src/drv_redhat.c

index 4935f98..092ef5c 100644

--- a/src/drv_redhat.c

+++ b/src/drv_redhat.c

@@ -88,6 +88,38 @@ static const struct augeas_xfm_table augeas_xfm_common =

     { .size = ARRAY_CARDINALITY(augeas_xfm_common_pv),

       .pv = augeas_xfm_common_pv };

+/* aug_all_related_ifcfgs() - return the count of (and optionally a list

+ * of, if matches != NULL) the paths for all ifcfg files that are

+ * related to the interface "name".

+ */

+static

+int aug_all_related_ifcfgs(struct netcf *ncf, char ***matches, const char *name) {

+    int nmatches;

+

+    /* this includes the ifcfg files for:

+     *

+     * 1) the named interface itself (DEVICE=$name)

+     *

+     * 2) any interface naming $name as a bridge it is attached to

+     *    (BRIDGE=$name)

+     *

+     * 3) any interface naming $name as the master of a bond it is

+     *    enslaved to (MASTER=$name)

+     *

+     * 4) any interface with a MASTER, where the device named as

+     *    MASTER contains a BRIDGE=$name *and* DEVICE=$itself (thus

+     *    catching ethernet devices that are enslaved to a bond that

+     *    is attached to a bridge).

+     */

+    nmatches = aug_fmt_match(ncf, matches,

+                             "(%s[(DEVICE|BRIDGE|MASTER) = '%s']"

+                             "|%s[MASTER][MASTER = ../*[BRIDGE = '%s']/DEVICE "

+                             "])/DEVICE",

+                             ifcfg_path, name, ifcfg_path, name);

+    return nmatches;

+

+}

+

 /* Entries in a ifcfg file that tell us that the interface

  * is not a toplevel interface

  */

@@ -108,12 +140,7 @@ static int is_slave(struct netcf *ncf, const char *intf) {

 static bool has_ifcfg_file(struct netcf *ncf, const char *name) {

     int nmatches;

-    nmatches = aug_fmt_match(ncf, NULL,

-                             "%s[ DEVICE = '%s'"

-                             "    or BRIDGE = '%s'"

-                             "    or MASTER = '%s'"

-                             "    or MASTER = ../*[BRIDGE = '%s']/DEVICE ]/DEVICE",

-                             ifcfg_path, name, name, name, name);

+    nmatches = aug_all_related_ifcfgs(ncf, NULL, name);

     return nmatches > 0;

 }

@@ -588,10 +615,7 @@ static xmlDocPtr aug_get_xml_for_nif(struct netcf_if *nif) {

     int ndevs = 0, nint = 0;

     ncf = nif->ncf;

-    ndevs = aug_fmt_match(ncf, &devs,

-              "%s[ DEVICE = '%s' or BRIDGE = '%s' or MASTER = '%s'"

-              "    or MASTER = ../*[BRIDGE = '%s']/DEVICE ]/DEVICE",

-              ifcfg_path, nif->name, nif->name, nif->name, nif->name);

+    ndevs = aug_all_related_ifcfgs(ncf, &devs, nif->name);

     ERR_BAIL(ncf);

     nint = uniq_ifcfg_paths(ncf, ndevs, devs, &intf;;

-- 

2.4.3