[libvirt] [PATCH] nwfilter: Use -m conntrack rather than -m state
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue Aug 6 23:44:23 UTC 2013
On 08/06/2013 07:10 PM, John Ferlan wrote:
> On 08/06/2013 12:43 PM, Stefan Berger wrote:
>> On 08/06/2013 11:20 AM, John Ferlan wrote:
>>> On 08/06/2013 09:52 AM, Stefan Berger wrote:
>>>> Since iptables version 1.4.16 '-m state --state NEW' is converted to
>>>> '-m conntrack --ctstate NEW'. Therefore, when encountering this or later
>>>> versions of iptables use '-m conntrack --ctstate'.
>>>>
>>>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>>>>
>>>> ---
>>>> src/nwfilter/nwfilter_ebiptables_driver.c | 50
>>>> +++++++++++++++++++++++++++++-
>>>> 1 file changed, 49 insertions(+), 1 deletion(-)
>>>>
>>>> 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
>>>> @@ -188,6 +188,9 @@ static const char ebiptables_script_set_
>>>> static const char *m_state_out_str = "-m state --state
>>>> NEW,ESTABLISHED";
>>>> static const char *m_state_in_str = "-m state --state ESTABLISHED";
>>>> +static const char *m_state_out_str_new = "-m conntrack --ctstate
>>>> NEW,ESTABLISHED";
>>>> +static const char *m_state_in_str_new = "-m conntrack --ctstate
>>>> ESTABLISHED";
>>>> +
>>>> static const char *m_physdev_in_str = "-m physdev " PHYSDEV_IN;
>>>> static const char *m_physdev_out_str = "-m physdev " PHYSDEV_OUT;
>>>> static const char *m_physdev_out_old_str = "-m physdev "
>>>> PHYSDEV_OUT_OLD;
>>>> @@ -4353,6 +4356,49 @@ ebiptablesDriverProbeCtdir(void)
>>>> iptables_ctdir_corrected = CTDIR_STATUS_OLD;
>>>> }
>>>> +static void
>>>> +ebiptablesDriverProbeStateMatch(void)
>>>> +{
>>>> + virBuffer buf = VIR_BUFFER_INITIALIZER;
>>>> + char *cmdout = NULL, *version;
>>>> + unsigned long thisversion;
>>>> +
>>>> + NWFILTER_SET_IPTABLES_SHELLVAR(&buf);
>>>> +
>>>> + virBufferAsprintf(&buf,
>>>> + "$IPT --version");
>>>> +
>>>> + if (ebiptablesExecCLI(&buf, NULL, &cmdout) < 0) {
>>>> + VIR_ERROR(_("Testing of iptables command failed: %s"),
>>>> + cmdout);
>>>> + return;
>>> Probably should just goto cleanup since we'll need to free buf
>> ebiptablesExecCLI already takes care of freeing the buffer.
>>
>>>> + }
>>>> +
>>>> + /*
>>>> + * we expect output in the format
>>>> + * iptables v1.4.16
>>>> + */
>>>> + if (!(version = strchr(cmdout, 'v')) &&
>>>> + virParseVersionString(version + 1, &thisversion, true) < 0) {
>>>> + VIR_ERROR(_("Could not determine iptables version from
>>>> string %s"),
>>>> + cmdout);
>>>> + goto cleanup;
>>>> + }
>>>> +
>>>> + /*
>>>> + * since version 1.4.16 '-m state --state ...' will be converted to
>>>> + * '-m conntrack --ctstate ...'
>>>> + */
>>>> + if (thisversion > 1 * 1000000 + 4 * 1000 + 16) {
>>>> + m_state_out_str = m_state_out_str_new;
>>>> + m_state_in_str = m_state_in_str_new;
>>>> + }
>>>> +
>>>> +cleanup:
>>> Need to free 'buf' too right?
>> Should not be needed due to the reason above.
>>
> Ahh, I see... I must dig deeper :-) virCommandAddArgBuffer() does the
> magic free...
>
> Does the thisversion check need to be >= or is > proper? Reading the
> comment makes me believe it was added as of 1.4.16, thus a >= rather
> than >.
Good catch... Sending v2 fixing also the && above...
Thanks.
Stefan
More information about the libvir-list
mailing list