[libvirt] [PATCH v3] nwfilter: check for inverted ctdir
Linux netfilter at some point (Linux 2.6.39) inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We check for the kernel version and assume that all Linux kernels with version 2.6.39 have the newer inverted logic. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v2->v3: - using uname now to check for Linux kernel version number v1->v2: - using virSocketAddrParseIPv4 --- src/nwfilter/nwfilter_ebiptables_driver.c | 54 ++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -27,6 +27,7 @@ #include <string.h> #include <sys/stat.h> #include <fcntl.h> +#include <sys/utsname.h> #include "internal.h" @@ -85,6 +86,17 @@ static char *iptables_cmd_path; static char *ip6tables_cmd_path; static char *grep_cmd_path; +/* + * --ctdir original vs. --ctdir reply's meaning was inverted in netfilter + * at some point (Linux 2.6.39) + */ +enum ctdirStatus { + CTDIR_STATUS_UNKNOWN = 0, + CTDIR_STATUS_CORRECTED = 1, + CTDIR_STATUS_OLD = 2, +}; +static enum ctdirStatus iptables_ctdir_corrected; + #define PRINT_ROOT_CHAIN(buf, prefix, ifname) \ snprintf(buf, sizeof(buf), "libvirt-%c-%s", prefix, ifname) #define PRINT_CHAIN(buf, prefix, ifname, suffix) \ @@ -1262,6 +1274,17 @@ iptablesEnforceDirection(int directionIn virNWFilterRuleDefPtr rule, virBufferPtr buf) { + switch (iptables_ctdir_corrected) { + case CTDIR_STATUS_UNKNOWN: + /* could not be determined or s.th. is seriously wrong */ + return; + case CTDIR_STATUS_CORRECTED: + directionIn = !directionIn; + break; + case CTDIR_STATUS_OLD: + break; + } + if (rule->tt != VIR_NWFILTER_RULE_DIRECTION_INOUT) virBufferAsprintf(buf, " -m conntrack --ctdir %s", (directionIn) ? "Original" @@ -4304,6 +4327,34 @@ ebiptablesDriverTestCLITools(void) return ret; } +static void +ebiptablesDriverProbeCtdir(void) +{ + struct utsname utsname; + unsigned int major, minor, micro; + + iptables_ctdir_corrected = CTDIR_STATUS_UNKNOWN; + + if (uname(&utsname) < 0) { + VIR_ERROR(_("Call to utsname failed: %d"), errno); + return; + } + + /* following Linux lxr, the logic was inverted in 2.6.39 */ + if (sscanf(utsname.release, "%u.%u.%u", &major, &minor, µ) != 3) { + VIR_ERROR(_("Could not detect kernel version from string %s"), + utsname.release); + return; + } + + if (major >= 3 || + (major == 2 && minor == 6 && micro >= 39)) + iptables_ctdir_corrected = CTDIR_STATUS_CORRECTED; + else + iptables_ctdir_corrected = CTDIR_STATUS_OLD; + +} + static int ebiptablesDriverInit(bool privileged) { @@ -4341,6 +4392,9 @@ ebiptablesDriverInit(bool privileged) return -ENOTSUP; } + if (iptables_cmd_path) + ebiptablesDriverProbeCtdir(); + ebiptables_driver.flags = TECHDRV_FLAG_INITIALIZED; return 0;
On 05/14/2013 03:58 PM, Stefan Berger wrote:
Linux netfilter at some point (Linux 2.6.39) inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We check for the kernel version and assume that all Linux kernels with version 2.6.39 have the newer inverted logic.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- v2->v3: - using uname now to check for Linux kernel version number
v1->v2: - using virSocketAddrParseIPv4
--- src/nwfilter/nwfilter_ebiptables_driver.c | 54 ++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -27,6 +27,7 @@ #include <string.h> #include <sys/stat.h> #include <fcntl.h> +#include <sys/utsname.h>
#include "internal.h"
@@ -85,6 +86,17 @@ static char *iptables_cmd_path; static char *ip6tables_cmd_path; static char *grep_cmd_path;
+/* + * --ctdir original vs. --ctdir reply's meaning was inverted in netfilter + * at some point (Linux 2.6.39) + */ +enum ctdirStatus { + CTDIR_STATUS_UNKNOWN = 0, + CTDIR_STATUS_CORRECTED = 1, + CTDIR_STATUS_OLD = 2, +}; +static enum ctdirStatus iptables_ctdir_corrected; + #define PRINT_ROOT_CHAIN(buf, prefix, ifname) \ snprintf(buf, sizeof(buf), "libvirt-%c-%s", prefix, ifname) #define PRINT_CHAIN(buf, prefix, ifname, suffix) \ @@ -1262,6 +1274,17 @@ iptablesEnforceDirection(int directionIn virNWFilterRuleDefPtr rule, virBufferPtr buf) { + switch (iptables_ctdir_corrected) { + case CTDIR_STATUS_UNKNOWN: + /* could not be determined or s.th. is seriously wrong */ + return; + case CTDIR_STATUS_CORRECTED: + directionIn = !directionIn; + break; + case CTDIR_STATUS_OLD: + break; + } + if (rule->tt != VIR_NWFILTER_RULE_DIRECTION_INOUT) virBufferAsprintf(buf, " -m conntrack --ctdir %s", (directionIn) ? "Original" @@ -4304,6 +4327,34 @@ ebiptablesDriverTestCLITools(void) return ret; }
+static void +ebiptablesDriverProbeCtdir(void) +{ + struct utsname utsname; + unsigned int major, minor, micro; + + iptables_ctdir_corrected = CTDIR_STATUS_UNKNOWN; + + if (uname(&utsname) < 0) { + VIR_ERROR(_("Call to utsname failed: %d"), errno); + return; + } + + /* following Linux lxr, the logic was inverted in 2.6.39 */ + if (sscanf(utsname.release, "%u.%u.%u", &major, &minor, µ) != 3) {
Since these functions are only ever compiled on Linux, I think it's fine that you're only checking release, and not looking at sysname (we already know what it is. I'm not a fan of sscanf(), and would prefer it was completely eliminated from libvirt, but it is still in use in a few places, so I guess this could go in too. But I would prefer something similar to the way we parse the qemu version number at the top of virQEMUCapsParseHelpStr(). Does anyone else have an opinion one way or another on sscanf? At any rate, ACK either way.
+ VIR_ERROR(_("Could not detect kernel version from string %s"), + utsname.release); + return; + } + + if (major >= 3 || + (major == 2 && minor == 6 && micro >= 39)) + iptables_ctdir_corrected = CTDIR_STATUS_CORRECTED; + else + iptables_ctdir_corrected = CTDIR_STATUS_OLD; + +} + static int ebiptablesDriverInit(bool privileged) { @@ -4341,6 +4392,9 @@ ebiptablesDriverInit(bool privileged) return -ENOTSUP; }
+ if (iptables_cmd_path) + ebiptablesDriverProbeCtdir(); + ebiptables_driver.flags = TECHDRV_FLAG_INITIALIZED;
return 0;
On 05/14/2013 03:10 PM, Laine Stump wrote:
On 05/14/2013 03:58 PM, Stefan Berger wrote:
Linux netfilter at some point (Linux 2.6.39) inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We check for the kernel version and assume that all Linux kernels with version 2.6.39 have the newer inverted logic.
+ + /* following Linux lxr, the logic was inverted in 2.6.39 */ + if (sscanf(utsname.release, "%u.%u.%u", &major, &minor, µ) != 3) {
Since these functions are only ever compiled on Linux, I think it's fine that you're only checking release, and not looking at sysname (we already know what it is.
I'm not a fan of sscanf(), and would prefer it was completely eliminated from libvirt, but it is still in use in a few places, so I guess this could go in too. But I would prefer something similar to the way we parse the qemu version number at the top of virQEMUCapsParseHelpStr().
Use virParseVersionString (search for "uts\.release" in tools/virt-host-validate-common.c for an example usage).
Does anyone else have an opinion one way or another on sscanf?
At any rate, ACK either way.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/14/2013 03:58 PM, Stefan Berger wrote:
Linux netfilter at some point (Linux 2.6.39) inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We check for the kernel version and assume that all Linux kernels with version 2.6.39 have the newer inverted logic.
BTW, can you add a comment to the commit log pointing out that any distro that backports the netfilter --ctdir "fix" to a pre-2.6.39 kernel will need to also carry a patch against libvirt that changes ebiptablesDriverProbeCtdir to detect their kernel version as being "corrected"?
participants (3)
-
Eric Blake -
Laine Stump -
Stefan Berger