util/tests: enable locking on iptables/ebtables commandlines by default

iptables and ip6tables have had a "-w" commandline option to grab a
systemwide lock that prevents two iptables invocations from modifying
the iptables chains since 2013 (upstream commit 93587a04 in
iptables-1.4.20).  Similarly, ebtables has had a "--concurrent"
commandline option for the same purpose since 2011 (in the upstream
ebtables commit f9b4bcb93, which was present in ebtables-2.0.10.4).

Libvirt added code to conditionally use the commandline option for
iptables/ip6tables in upstream commit ba95426d6f (libvirt-1.2.0,
November 2013), and for ebtables in upstream commit dc33e6e4a5
(libvirt-1.2.11, November 2014) (the latter actually *re*-added the
locking for iptables/ip6tables, as it had accidentally been removed
during a refactor of firewall code in the interim).

I say "conditionally" because a check was made during firewall module
initialization that tried executing a test command with the
-w/--concurrent option, and only continued using it for actual
commands if that test command completed successfully. At the time the
code was added this was a reasonable thing to do, as it had been less
than a year since introduction of -w to iptables, so many distros
supported by libvirt were still using iptables (and possibly even
ebtables) versions too old to have the new commandline options.

It is now 2020, and as far as I can discern from repology.org (and
manually examining a RHEL7.9 system), every version of every distro
that is supported by libvirt now uses new enough versions of both
iptables and ebtables that they all have support for -w/--concurrent.
That means we can finally remove the conditional code and simply
always use them.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This commit is contained in:
Laine Stump 2020-11-16 20:02:43 -05:00
parent e66451f685
commit 0a867cd895
7 changed files with 3 additions and 72 deletions

View File

@ -2158,7 +2158,6 @@ virFirewallRuleAddArgList;
virFirewallRuleAddArgSet;
virFirewallRuleGetArgCount;
virFirewallSetBackend;
virFirewallSetLockOverride;
virFirewallStartRollback;
virFirewallStartTransaction;

View File

@ -96,59 +96,6 @@ virFirewallOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virFirewall);
static bool iptablesUseLock;
static bool ip6tablesUseLock;
static bool ebtablesUseLock;
static bool lockOverride; /* true to avoid lock probes */
void
virFirewallSetLockOverride(bool avoid)
{
lockOverride = avoid;
if (avoid) {
/* add the lock option to all commands */
iptablesUseLock = true;
ip6tablesUseLock = true;
ebtablesUseLock = true;
}
}
static void
virFirewallCheckUpdateLock(bool *lockflag,
const char *const*args)
{
int status; /* Ignore failed commands without logging them */
g_autoptr(virCommand) cmd = virCommandNewArgs(args);
if (virCommandRun(cmd, &status) < 0 || status) {
VIR_INFO("locking not supported by %s", args[0]);
} else {
VIR_INFO("using locking for %s", args[0]);
*lockflag = true;
}
}
static void
virFirewallCheckUpdateLocking(void)
{
const char *iptablesArgs[] = {
IPTABLES_PATH, "-w", "-L", "-n", NULL,
};
const char *ip6tablesArgs[] = {
IP6TABLES_PATH, "-w", "-L", "-n", NULL,
};
const char *ebtablesArgs[] = {
EBTABLES_PATH, "--concurrent", "-L", NULL,
};
if (lockOverride)
return;
virFirewallCheckUpdateLock(&iptablesUseLock,
iptablesArgs);
virFirewallCheckUpdateLock(&ip6tablesUseLock,
ip6tablesArgs);
virFirewallCheckUpdateLock(&ebtablesUseLock,
ebtablesArgs);
}
static int
virFirewallValidateBackend(virFirewallBackend backend)
{
@ -196,8 +143,6 @@ virFirewallValidateBackend(virFirewallBackend backend)
currentBackend = backend;
virFirewallCheckUpdateLocking();
return 0;
}
@ -359,15 +304,12 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
switch (rule->layer) {
case VIR_FIREWALL_LAYER_ETHERNET:
if (ebtablesUseLock)
ADD_ARG(rule, "--concurrent");
break;
case VIR_FIREWALL_LAYER_IPV4:
if (iptablesUseLock)
ADD_ARG(rule, "-w");
break;
case VIR_FIREWALL_LAYER_IPV6:
if (ip6tablesUseLock)
ADD_ARG(rule, "-w");
break;
case VIR_FIREWALL_LAYER_LAST:

View File

@ -111,6 +111,4 @@ void virFirewallStartRollback(virFirewallPtr firewall,
int virFirewallApply(virFirewallPtr firewall);
void virFirewallSetLockOverride(bool avoid);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virFirewall, virFirewallFree);

View File

@ -179,8 +179,6 @@ mymain(void)
ret = -1; \
} while (0)
virFirewallSetLockOverride(true);
if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) < 0) {
if (!hasNetfilterTools()) {
fprintf(stderr, "iptables/ip6tables/ebtables tools not present");

View File

@ -503,8 +503,6 @@ mymain(void)
{
int ret = 0;
virFirewallSetLockOverride(true);
if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) < 0) {
if (!hasNetfilterTools()) {
fprintf(stderr, "iptables/ip6tables/ebtables tools not present");

View File

@ -457,8 +457,6 @@ mymain(void)
ret = -1; \
} while (0)
virFirewallSetLockOverride(true);
if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) < 0) {
if (!hasNetfilterTools()) {
fprintf(stderr, "iptables/ip6tables/ebtables tools not present");

View File

@ -1076,8 +1076,6 @@ mymain(void)
RUN_TEST_DIRECT(name, method); \
RUN_TEST_FIREWALLD(name, method)
virFirewallSetLockOverride(true);
RUN_TEST("single group", testFirewallSingleGroup);
RUN_TEST("remove rule", testFirewallRemoveRule);
RUN_TEST("many groups", testFirewallManyGroups);