Re: [Xen-devel] [PATCH 3/5] hotplug/linux: Improve iptables logic

[Sylvain Munaut <s.munaut@xxxxxxxxxxxxxxxxxxxx>]

Hi,

> AFIACT the duplicate entries are simply because
>
>> -  iptables "$c" FORWARD -w $dev_in_match "$dev" \
>> -    "$@" -j ACCEPT 2>/dev/null &&
>> -  iptables "$c" FORWARD -w $dev_out_match "$dev" \
>> -    -j ACCEPT 2>/dev/null
>
> this second line, which does not contain "$@", is invoked once for
> each source pattern.

Well yes, obviously.


> Surely instead it would be better to lift this out.

"better" ?

Not creating this rules several times is exactly what this patch does.
Previously frob_iptables was called independently for each source IP,
in which case in that function you have no idea if you need to add the
output rule or not since you don't know if the rule is already there
or not. So now it's called with the full list of allowed IPs so it can
make smarter choices.
And just moving the 'out' rule outside of frob_iptables alltogether
seems hackish to me, especially when you add IPv6 later on because you
have iptables manipulations spread around.

Here you have a clear separation with frob_iptables generating all the
v4 rules and frob_ip6tables generating all the v6 rules.


> I'm not sure I like the change in the handling of any.

What change exactly ?

The only change in the resulting rules are :
 * No duplicating -> Don't really see how this could be a bad thing.
 * Don't add the DHCP rule if you allow all packets -> Again, this
avoid inserting a useless rules, how can that be bad ?

and same goes for the v6 version, trying to minimize the rules added
and never add useless ones.


Cheers,

   Sylvain

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel