[libvirt] [PATCH v2] nwfilters: support for TCP flags evaluation

[Eric Blake]

On 04/07/2011 01:47 PM, Stefan Berger wrote:
> This patch adds support for the evaluation of TCP flags in nwfilters.
> 
> It adds documentation to the web page and extends the tests as well.
> Also, the nwfilter schema is extended.
> 
> The following are some example for rules using the tcp flags:
> 
> <rule action='accept' direction='in'>
> <tcp state='NONE' flags='SYN/ALL' dsptportstart='80'/>
> </rule>
> <rule action='drop' direction='in'>
> <tcp state='NONE' flags='SYN/ALL'/>
> </rule>
> 
> 
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> 
> ---
>  docs/formatnwfilter.html.in               |   10 ++
>  docs/schemas/nwfilter.rng                 |   16 ++++
>  src/conf/nwfilter_conf.c                  |  115
> +++++++++++++++++++++++++++---
>  src/conf/nwfilter_conf.h                  |    9 ++
>  src/libvirt_private.syms                  |    1
>  src/nwfilter/nwfilter_ebiptables_driver.c |    9 ++
>  tests/nwfilterxml2xmlin/tcp-test.xml      |   12 +++
>  tests/nwfilterxml2xmlout/tcp-test.xml     |   12 +++
>  8 files changed, 174 insertions(+), 10 deletions(-)

ACK, looks reasonable to me, with one optimization nit fixed:

> +    int32_t mask = 0x1;
> 
> -    for (i = 0; int_map[i].val; i++) {
> -        if (last_attr != int_map[i].attr &&
> -            flags & int_map[i].attr) {
> -            if (c >= 1)
> -                virBufferVSprintf(buf, "%s", sep);
> -            virBufferVSprintf(buf, "%s", int_map[i].val);
> -            c++;
> +    while (mask) {
> +        if ((mask & flags)) {
> +            for (i = 0; int_map[i].val; i++) {
> +                if (mask == int_map[i].attr) {
> +                    if (c >= 1)
> +                        virBufferVSprintf(buf, "%s", sep);
> +                    virBufferVSprintf(buf, "%s", int_map[i].val);
> +                    c++;
> +                }
> +            }
> +            flags ^= mask;
> +            if (!flags)
> +                break;

This if condition should be after...

>          }

this brace; otherwise, if flags == 0 on entry, then you will
(needlessly) iterate through all 32 bits of mask, rather than breaking
on the first iteration.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110407/39c56b97/attachment-0001.sig>