1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/3] network: Delay creating private chains until starting network
  4. View patch

[libvirt] [PATCH 1/3] network: pull global chain init into separate method

Daniel P. Berrangé posted 3 patches 6 years, 6 months ago
[libvirt] [PATCH 1/3] network: pull global chain init into separate method
Posted by Daniel P. Berrangé 6 years, 6 months ago 
Pull the logic for creating global iptables chains into a separate
method and protect its invokation with virOnce, to make it possible
to reuse it in non-startup paths.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/network/bridge_driver_linux.c | 39 +++++++++++++++++++------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index f2827543ca..0d849173b2 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -35,22 +35,18 @@ VIR_LOG_INIT("network.bridge_driver_linux");
 
 #define PROC_NET_ROUTE "/proc/net/route"
 
+static virOnceControl createdOnce;
+static bool createdChains;
 static virErrorPtr errInitV4;
 static virErrorPtr errInitV6;
 
-void networkPreReloadFirewallRules(bool startup)
+/* Only call via virOnce */
+static void networkSetupPrivateChains(void)
 {
-    bool created = false;
     int rc;
 
-    /* We create global rules upfront as we don't want
-     * the perf hit of conditionally figuring out whether
-     * to create them each time a network is started.
-     *
-     * Any errors here are saved to be reported at time
-     * of starting the network though as that makes them
-     * more likely to be seen by a human
-     */
+    createdChains = false;
+
     rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4);
     if (rc < 0) {
         errInitV4 = virSaveLastError();
@@ -58,9 +54,9 @@ void networkPreReloadFirewallRules(bool startup)
     } else {
         virFreeError(errInitV4);
         errInitV4 = NULL;
+        if (rc)
+            createdChains = true;
     }
-    if (rc)
-        created = true;
 
     rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6);
     if (rc < 0) {
@@ -69,9 +65,22 @@ void networkPreReloadFirewallRules(bool startup)
     } else {
         virFreeError(errInitV6);
         errInitV6 = NULL;
+        if (rc)
+            createdChains = true;
     }
-    if (rc)
-        created = true;
+}
+
+void networkPreReloadFirewallRules(bool startup)
+{
+    /* We create global rules upfront as we don't want
+     * the perf hit of conditionally figuring out whether
+     * to create them each time a network is started.
+     *
+     * Any errors here are saved to be reported at time
+     * of starting the network though as that makes them
+     * more likely to be seen by a human
+     */
+    ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
 
     /*
      * If this is initial startup, and we just created the
@@ -86,7 +95,7 @@ void networkPreReloadFirewallRules(bool startup)
      * rules will be present. Thus we can safely just tell it
      * to always delete from the builin chain
      */
-    if (startup && created)
+    if (startup && createdChains)
         iptablesSetDeletePrivate(false);
 }
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] network: pull global chain init into separate method
Posted by Eric Blake 6 years, 6 months ago 
On 5/22/19 7:29 AM, Daniel P. Berrangé wrote:
> Pull the logic for creating global iptables chains into a separate
> method and protect its invokation with virOnce, to make it possible

invocation

> to reuse it in non-startup paths.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/network/bridge_driver_linux.c | 39 +++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] network: pull global chain init into separate method
Posted by Jim Fehlig 6 years, 6 months ago 
On 5/22/19 6:29 AM, Daniel P. Berrangé wrote:
> Pull the logic for creating global iptables chains into a separate
> method and protect its invokation with virOnce, to make it possible
> to reuse it in non-startup paths.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   src/network/bridge_driver_linux.c | 39 +++++++++++++++++++------------
>   1 file changed, 24 insertions(+), 15 deletions(-)

Eric already mentioned the typo in the commit message. Otherwise

Reviewed-by: Jim Fehlig <jfehlig@suse.com>

Regards,
Jim

> 
> diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> index f2827543ca..0d849173b2 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -35,22 +35,18 @@ VIR_LOG_INIT("network.bridge_driver_linux");
>   
>   #define PROC_NET_ROUTE "/proc/net/route"
>   
> +static virOnceControl createdOnce;
> +static bool createdChains;
>   static virErrorPtr errInitV4;
>   static virErrorPtr errInitV6;
>   
> -void networkPreReloadFirewallRules(bool startup)
> +/* Only call via virOnce */
> +static void networkSetupPrivateChains(void)
>   {
> -    bool created = false;
>       int rc;
>   
> -    /* We create global rules upfront as we don't want
> -     * the perf hit of conditionally figuring out whether
> -     * to create them each time a network is started.
> -     *
> -     * Any errors here are saved to be reported at time
> -     * of starting the network though as that makes them
> -     * more likely to be seen by a human
> -     */
> +    createdChains = false;
> +
>       rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4);
>       if (rc < 0) {
>           errInitV4 = virSaveLastError();
> @@ -58,9 +54,9 @@ void networkPreReloadFirewallRules(bool startup)
>       } else {
>           virFreeError(errInitV4);
>           errInitV4 = NULL;
> +        if (rc)
> +            createdChains = true;
>       }
> -    if (rc)
> -        created = true;
>   
>       rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6);
>       if (rc < 0) {
> @@ -69,9 +65,22 @@ void networkPreReloadFirewallRules(bool startup)
>       } else {
>           virFreeError(errInitV6);
>           errInitV6 = NULL;
> +        if (rc)
> +            createdChains = true;
>       }
> -    if (rc)
> -        created = true;
> +}
> +
> +void networkPreReloadFirewallRules(bool startup)
> +{
> +    /* We create global rules upfront as we don't want
> +     * the perf hit of conditionally figuring out whether
> +     * to create them each time a network is started.
> +     *
> +     * Any errors here are saved to be reported at time
> +     * of starting the network though as that makes them
> +     * more likely to be seen by a human
> +     */
> +    ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
>   
>       /*
>        * If this is initial startup, and we just created the
> @@ -86,7 +95,7 @@ void networkPreReloadFirewallRules(bool startup)
>        * rules will be present. Thus we can safely just tell it
>        * to always delete from the builin chain
>        */
> -    if (startup && created)
> +    if (startup && createdChains)
>           iptablesSetDeletePrivate(false);
>   }
>   
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.