On Sun, Apr 30, 2023 at 11:19:21PM -0400, Laine Stump wrote: > and take advantage of this to replace all the ternary operators when > calling virFirewallAddRule() with virIptablesActionTypeToString(). > > (NB: the VIR_ENUM declaration uses "virIptablesAction" rather than > "virFirewallAction" because the string it produces is specific to the > iptables backend. A separate VIR_ENUM for "virNftablesAction", > producing slightly different strings, will be added later for the > nftables backend.) > > Signed-off-by: Laine Stump <laine@xxxxxxxxxx> > --- > src/util/virfirewall.h | 8 +++++ > src/util/viriptables.c | 69 ++++++++++++++++++++++++----------------- > src/util/viriptables.h | 21 +++++++------ > src/util/virnetfilter.c | 49 +++++++++++++++-------------- > src/util/virnetfilter.h | 5 --- > 5 files changed, 84 insertions(+), 68 deletions(-) > > diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h > index 0f40dae859..ed0bc8b6f7 100644 > --- a/src/util/virfirewall.h > +++ b/src/util/virfirewall.h > @@ -34,6 +34,14 @@ typedef enum { > VIR_FIREWALL_LAYER_LAST, > } virFirewallLayer; > > +typedef enum { > + VIR_FIREWALL_ACTION_INSERT, > + VIR_FIREWALL_ACTION_APPEND, > + VIR_FIREWALL_ACTION_DELETE, > + > + VIR_FIREWALL_ACTION_LAST > +} virFirewallAction; This enum isn't used by anything in virfirewall.c, so I don't think it should be in this file / namespace. Why not VIR_NETFILTER_ACTION / virNetfilterAction, since its scope is limited to that file and the related iptables.c/nftables.c files ? > void virFirewallFree(virFirewall *firewall); > diff --git a/src/util/viriptables.c b/src/util/viriptables.c > index a85f3ea603..dc2a4335bf 100644 > --- a/src/util/viriptables.c > +++ b/src/util/viriptables.c > @@ -33,11 +33,22 @@ > #include "virerror.h" > #include "virlog.h" > #include "virhash.h" > +#include "virenum.h" > > VIR_LOG_INIT("util.iptables"); > > #define VIR_FROM_THIS VIR_FROM_NONE > > + > +VIR_ENUM_DECL(virIptablesAction); > +VIR_ENUM_IMPL(virIptablesAction, > + VIR_FIREWALL_ACTION_LAST, > + "--insert", > + "--append", > + "--delete", > +); > + > + > typedef struct { > const char *parent; > const char *child; > @@ -156,14 +167,14 @@ iptablesInput(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > int port, > - int action, > + virFirewallAction action, > int tcp) > { > g_autofree char *portstr = g_strdup_printf("%d", port); > > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_INP", > "--in-interface", iface, > "--protocol", tcp ? "tcp" : "udp", > @@ -177,14 +188,14 @@ iptablesOutput(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > int port, > - int action, > + virFirewallAction action, > int tcp) > { > g_autofree char *portstr = g_strdup_printf("%d", port); > > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_OUT", > "--out-interface", iface, > "--protocol", tcp ? "tcp" : "udp", > @@ -203,7 +214,7 @@ iptablesForwardAllowOut(virFirewall *fw, > unsigned int prefix, > const char *iface, > const char *physdev, > - int action) > + virFirewallAction action) > { > g_autofree char *networkstr = NULL; > virFirewallLayer layer = VIR_SOCKET_ADDR_FAMILY(netaddr) == AF_INET ? > @@ -215,7 +226,7 @@ iptablesForwardAllowOut(virFirewall *fw, > if (physdev && physdev[0]) > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWO", > "--source", networkstr, > "--in-interface", iface, > @@ -225,7 +236,7 @@ iptablesForwardAllowOut(virFirewall *fw, > else > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWO", > "--source", networkstr, > "--in-interface", iface, > @@ -245,7 +256,7 @@ iptablesForwardAllowRelatedIn(virFirewall *fw, > unsigned int prefix, > const char *iface, > const char *physdev, > - int action) > + virFirewallAction action) > { > virFirewallLayer layer = VIR_SOCKET_ADDR_FAMILY(netaddr) == AF_INET ? > VIR_FIREWALL_LAYER_IPV4 : VIR_FIREWALL_LAYER_IPV6; > @@ -257,7 +268,7 @@ iptablesForwardAllowRelatedIn(virFirewall *fw, > if (physdev && physdev[0]) > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWI", > "--destination", networkstr, > "--in-interface", physdev, > @@ -269,7 +280,7 @@ iptablesForwardAllowRelatedIn(virFirewall *fw, > else > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWI", > "--destination", networkstr, > "--out-interface", iface, > @@ -290,7 +301,7 @@ iptablesForwardAllowIn(virFirewall *fw, > unsigned int prefix, > const char *iface, > const char *physdev, > - int action) > + virFirewallAction action) > { > virFirewallLayer layer = VIR_SOCKET_ADDR_FAMILY(netaddr) == AF_INET ? > VIR_FIREWALL_LAYER_IPV4 : VIR_FIREWALL_LAYER_IPV6; > @@ -302,7 +313,7 @@ iptablesForwardAllowIn(virFirewall *fw, > if (physdev && physdev[0]) > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWI", > "--destination", networkstr, > "--in-interface", physdev, > @@ -312,7 +323,7 @@ iptablesForwardAllowIn(virFirewall *fw, > else > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWI", > "--destination", networkstr, > "--out-interface", iface, > @@ -326,11 +337,11 @@ void > iptablesForwardAllowCross(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > - int action) > + virFirewallAction action) > { > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWX", > "--in-interface", iface, > "--out-interface", iface, > @@ -343,11 +354,11 @@ void > iptablesForwardRejectOut(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > - int action) > + virFirewallAction action) > { > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWO", > "--in-interface", iface, > "--jump", "REJECT", > @@ -359,11 +370,11 @@ void > iptablesForwardRejectIn(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > - int action) > + virFirewallAction action) > { > virFirewallAddRule(fw, layer, > "--table", "filter", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_FWI", > "--out-interface", iface, > "--jump", "REJECT", > @@ -382,7 +393,7 @@ iptablesForwardMasquerade(virFirewall *fw, > virSocketAddrRange *addr, > virPortRange *port, > const char *protocol, > - int action) > + virFirewallAction action) > { > g_autofree char *networkstr = NULL; > g_autofree char *addrStartStr = NULL; > @@ -409,7 +420,7 @@ iptablesForwardMasquerade(virFirewall *fw, > if (protocol && protocol[0]) { > rule = virFirewallAddRule(fw, layer, > "--table", "nat", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_PRT", > "--source", networkstr, > "-p", protocol, > @@ -418,7 +429,7 @@ iptablesForwardMasquerade(virFirewall *fw, > } else { > rule = virFirewallAddRule(fw, layer, > "--table", "nat", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_PRT", > "--source", networkstr, > "!", "--destination", networkstr, > @@ -479,7 +490,7 @@ iptablesForwardDontMasquerade(virFirewall *fw, > unsigned int prefix, > const char *physdev, > const char *destaddr, > - int action) > + virFirewallAction action) > { > g_autofree char *networkstr = NULL; > virFirewallLayer layer = VIR_SOCKET_ADDR_FAMILY(netaddr) == AF_INET ? > @@ -491,7 +502,7 @@ iptablesForwardDontMasquerade(virFirewall *fw, > if (physdev && physdev[0]) > virFirewallAddRule(fw, layer, > "--table", "nat", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_PRT", > "--out-interface", physdev, > "--source", networkstr, > @@ -501,7 +512,7 @@ iptablesForwardDontMasquerade(virFirewall *fw, > else > virFirewallAddRule(fw, layer, > "--table", "nat", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_PRT", > "--source", networkstr, > "--destination", destaddr, > @@ -516,13 +527,13 @@ static void > iptablesOutputFixUdpChecksum(virFirewall *fw, > const char *iface, > int port, > - int action) > + virFirewallAction action) > { > g_autofree char *portstr = g_strdup_printf("%d", port); > > virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4, > "--table", "mangle", > - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", > + virIptablesActionTypeToString(action), > "LIBVIRT_PRT", > "--out-interface", iface, > "--protocol", "udp", > @@ -547,7 +558,7 @@ iptablesAddOutputFixUdpChecksum(virFirewall *fw, > const char *iface, > int port) > { > - iptablesOutputFixUdpChecksum(fw, iface, port, VIR_NETFILTER_INSERT); > + iptablesOutputFixUdpChecksum(fw, iface, port, VIR_FIREWALL_ACTION_INSERT); > } > > /** > @@ -564,5 +575,5 @@ iptablesRemoveOutputFixUdpChecksum(virFirewall *fw, > const char *iface, > int port) > { > - iptablesOutputFixUdpChecksum(fw, iface, port, VIR_NETFILTER_DELETE); > + iptablesOutputFixUdpChecksum(fw, iface, port, VIR_FIREWALL_ACTION_DELETE); > } > diff --git a/src/util/viriptables.h b/src/util/viriptables.h > index 6ea589121e..17f43a8fa8 100644 > --- a/src/util/viriptables.h > +++ b/src/util/viriptables.h > @@ -22,6 +22,7 @@ > > #include "virsocketaddr.h" > #include "virfirewall.h" > +#include "virnetfilter.h" > > /* These functions are (currently) called directly from the consumer > * (e.g. the network driver), and only when the iptables backend is > @@ -50,7 +51,7 @@ iptablesInput(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > int port, > - int action, > + virFirewallAction action, > int tcp); > > void > @@ -58,7 +59,7 @@ iptablesOutput(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > int port, > - int action, > + virFirewallAction action, > int tcp); > > int > @@ -67,7 +68,7 @@ iptablesForwardAllowOut(virFirewall *fw, > unsigned int prefix, > const char *iface, > const char *physdev, > - int action); > + virFirewallAction action); > > int > iptablesForwardAllowRelatedIn(virFirewall *fw, > @@ -75,7 +76,7 @@ iptablesForwardAllowRelatedIn(virFirewall *fw, > unsigned int prefix, > const char *iface, > const char *physdev, > - int action); > + virFirewallAction action); > > int > iptablesForwardAllowIn(virFirewall *fw, > @@ -83,26 +84,26 @@ iptablesForwardAllowIn(virFirewall *fw, > unsigned int prefix, > const char *iface, > const char *physdev, > - int action); > + virFirewallAction action); > > > void > iptablesForwardAllowCross(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > - int action); > + virFirewallAction action); > > void > iptablesForwardRejectOut(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > - int action); > + virFirewallAction action); > > void > iptablesForwardRejectIn(virFirewall *fw, > virFirewallLayer layer, > const char *iface, > - int action); > + virFirewallAction action); > > int > iptablesForwardMasquerade(virFirewall *fw, > @@ -112,7 +113,7 @@ iptablesForwardMasquerade(virFirewall *fw, > virSocketAddrRange *addr, > virPortRange *port, > const char *protocol, > - int action); > + virFirewallAction action); > > int > iptablesForwardDontMasquerade(virFirewall *fw, > @@ -120,4 +121,4 @@ iptablesForwardDontMasquerade(virFirewall *fw, > unsigned int prefix, > const char *physdev, > const char *destaddr, > - int action); > + virFirewallAction action); > diff --git a/src/util/virnetfilter.c b/src/util/virnetfilter.c > index efe2ca01dc..10c1a54e26 100644 > --- a/src/util/virnetfilter.c > +++ b/src/util/virnetfilter.c > @@ -59,7 +59,7 @@ virNetfilterAddTcpInput(virFirewall *fw, > const char *iface, > int port) > { > - iptablesInput(fw, layer, iface, port, VIR_NETFILTER_INSERT, 1); > + iptablesInput(fw, layer, iface, port, VIR_FIREWALL_ACTION_INSERT, 1); > } > > > @@ -78,7 +78,7 @@ virNetfilterRemoveTcpInput(virFirewall *fw, > const char *iface, > int port) > { > - iptablesInput(fw, layer, iface, port, VIR_NETFILTER_DELETE, 1); > + iptablesInput(fw, layer, iface, port, VIR_FIREWALL_ACTION_DELETE, 1); > } > > > @@ -97,7 +97,7 @@ virNetfilterAddUdpInput(virFirewall *fw, > const char *iface, > int port) > { > - iptablesInput(fw, layer, iface, port, VIR_NETFILTER_INSERT, 0); > + iptablesInput(fw, layer, iface, port, VIR_FIREWALL_ACTION_INSERT, 0); > } > > > @@ -116,7 +116,7 @@ virNetfilterRemoveUdpInput(virFirewall *fw, > const char *iface, > int port) > { > - iptablesInput(fw, layer, iface, port, VIR_NETFILTER_DELETE, 0); > + iptablesInput(fw, layer, iface, port, VIR_FIREWALL_ACTION_DELETE, 0); > } > > > @@ -135,7 +135,7 @@ virNetfilterAddTcpOutput(virFirewall *fw, > const char *iface, > int port) > { > - iptablesOutput(fw, layer, iface, port, VIR_NETFILTER_INSERT, 1); > + iptablesOutput(fw, layer, iface, port, VIR_FIREWALL_ACTION_INSERT, 1); > } > > > @@ -154,7 +154,7 @@ virNetfilterRemoveTcpOutput(virFirewall *fw, > const char *iface, > int port) > { > - iptablesOutput(fw, layer, iface, port, VIR_NETFILTER_DELETE, 1); > + iptablesOutput(fw, layer, iface, port, VIR_FIREWALL_ACTION_DELETE, 1); > } > > > @@ -173,7 +173,7 @@ virNetfilterAddUdpOutput(virFirewall *fw, > const char *iface, > int port) > { > - iptablesOutput(fw, layer, iface, port, VIR_NETFILTER_INSERT, 0); > + iptablesOutput(fw, layer, iface, port, VIR_FIREWALL_ACTION_INSERT, 0); > } > > > @@ -192,7 +192,7 @@ virNetfilterRemoveUdpOutput(virFirewall *fw, > const char *iface, > int port) > { > - iptablesOutput(fw, layer, iface, port, VIR_NETFILTER_DELETE, 0); > + iptablesOutput(fw, layer, iface, port, VIR_FIREWALL_ACTION_DELETE, 0); > } > > > @@ -217,7 +217,7 @@ virNetfilterAddForwardAllowOut(virFirewall *fw, > const char *physdev) > { > return iptablesForwardAllowOut(fw, netaddr, prefix, iface, physdev, > - VIR_NETFILTER_INSERT); > + VIR_FIREWALL_ACTION_INSERT); > } > > > @@ -242,7 +242,7 @@ virNetfilterRemoveForwardAllowOut(virFirewall *fw, > const char *physdev) > { > return iptablesForwardAllowOut(fw, netaddr, prefix, iface, physdev, > - VIR_NETFILTER_DELETE); > + VIR_FIREWALL_ACTION_DELETE); > } > > > @@ -267,7 +267,7 @@ virNetfilterAddForwardAllowRelatedIn(virFirewall *fw, > const char *physdev) > { > return iptablesForwardAllowRelatedIn(fw, netaddr, prefix, iface, physdev, > - VIR_NETFILTER_INSERT); > + VIR_FIREWALL_ACTION_INSERT); > } > > > @@ -292,7 +292,7 @@ virNetfilterRemoveForwardAllowRelatedIn(virFirewall *fw, > const char *physdev) > { > return iptablesForwardAllowRelatedIn(fw, netaddr, prefix, iface, physdev, > - VIR_NETFILTER_DELETE); > + VIR_FIREWALL_ACTION_DELETE); > } > > > @@ -317,7 +317,7 @@ virNetfilterAddForwardAllowIn(virFirewall *fw, > const char *physdev) > { > return iptablesForwardAllowIn(fw, netaddr, prefix, iface, physdev, > - VIR_NETFILTER_INSERT); > + VIR_FIREWALL_ACTION_INSERT); > } > > > @@ -342,7 +342,7 @@ virNetfilterRemoveForwardAllowIn(virFirewall *fw, > const char *physdev) > { > return iptablesForwardAllowIn(fw, netaddr, prefix, iface, physdev, > - VIR_NETFILTER_DELETE); > + VIR_FIREWALL_ACTION_DELETE); > } > > > @@ -362,7 +362,7 @@ virNetfilterAddForwardAllowCross(virFirewall *fw, > virFirewallLayer layer, > const char *iface) > { > - iptablesForwardAllowCross(fw, layer, iface, VIR_NETFILTER_INSERT); > + iptablesForwardAllowCross(fw, layer, iface, VIR_FIREWALL_ACTION_INSERT); > } > > > @@ -382,7 +382,7 @@ virNetfilterRemoveForwardAllowCross(virFirewall *fw, > virFirewallLayer layer, > const char *iface) > { > - iptablesForwardAllowCross(fw, layer, iface, VIR_NETFILTER_DELETE); > + iptablesForwardAllowCross(fw, layer, iface, VIR_FIREWALL_ACTION_DELETE); > } > > > @@ -401,7 +401,7 @@ virNetfilterAddForwardRejectOut(virFirewall *fw, > virFirewallLayer layer, > const char *iface) > { > - iptablesForwardRejectOut(fw, layer, iface, VIR_NETFILTER_INSERT); > + iptablesForwardRejectOut(fw, layer, iface, VIR_FIREWALL_ACTION_INSERT); > } > > /** > @@ -419,7 +419,7 @@ virNetfilterRemoveForwardRejectOut(virFirewall *fw, > virFirewallLayer layer, > const char *iface) > { > - iptablesForwardRejectOut(fw, layer, iface, VIR_NETFILTER_DELETE); > + iptablesForwardRejectOut(fw, layer, iface, VIR_FIREWALL_ACTION_DELETE); > } > > > @@ -438,7 +438,7 @@ virNetfilterAddForwardRejectIn(virFirewall *fw, > virFirewallLayer layer, > const char *iface) > { > - iptablesForwardRejectIn(fw, layer, iface, VIR_NETFILTER_INSERT); > + iptablesForwardRejectIn(fw, layer, iface, VIR_FIREWALL_ACTION_INSERT); > } > > > @@ -457,7 +457,7 @@ virNetfilterRemoveForwardRejectIn(virFirewall *fw, > virFirewallLayer layer, > const char *iface) > { > - iptablesForwardRejectIn(fw, layer, iface, VIR_NETFILTER_DELETE); > + iptablesForwardRejectIn(fw, layer, iface, VIR_FIREWALL_ACTION_DELETE); > } > > > @@ -485,7 +485,7 @@ virNetfilterAddForwardMasquerade(virFirewall *fw, > { > return iptablesForwardMasquerade(fw, netaddr, prefix, > physdev, addr, port, protocol, > - VIR_NETFILTER_INSERT); > + VIR_FIREWALL_ACTION_INSERT); > } > > > @@ -513,7 +513,7 @@ virNetfilterRemoveForwardMasquerade(virFirewall *fw, > { > return iptablesForwardMasquerade(fw, netaddr, prefix, > physdev, addr, port, protocol, > - VIR_NETFILTER_DELETE); > + VIR_FIREWALL_ACTION_DELETE); > } > > > @@ -539,7 +539,8 @@ virNetfilterAddDontMasquerade(virFirewall *fw, > const char *destaddr) > { > return iptablesForwardDontMasquerade(fw, netaddr, prefix, > - physdev, destaddr, VIR_NETFILTER_INSERT); > + physdev, destaddr, > + VIR_FIREWALL_ACTION_INSERT); > } > > > @@ -566,5 +567,5 @@ virNetfilterRemoveDontMasquerade(virFirewall *fw, > { > return iptablesForwardDontMasquerade(fw, netaddr, prefix, > physdev, destaddr, > - VIR_NETFILTER_DELETE); > + VIR_FIREWALL_ACTION_DELETE); > } > diff --git a/src/util/virnetfilter.h b/src/util/virnetfilter.h > index c75f7eccbd..c8b91f16eb 100644 > --- a/src/util/virnetfilter.h > +++ b/src/util/virnetfilter.h > @@ -23,11 +23,6 @@ > #include "virsocketaddr.h" > #include "virfirewall.h" > > -enum { > - VIR_NETFILTER_INSERT = 0, > - VIR_NETFILTER_DELETE > -}; > - > void virNetfilterAddTcpInput (virFirewall *fw, > virFirewallLayer layer, > const char *iface, > -- > 2.39.2 > 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 :|