Some discussion inline.

On Jan 7, 2011, at 4:31 AM, Soren Hansen wrote:

>> Also, have you tested with --use_nova_chains as well?
> 
> I'm having trouble understanding that code, to be honest.
> 
> If use_nova_chains is enabled (I don't understand why it's configurable?), we do this:
>    At the end of FLAGS.input_chain (which I can't see getting DEFINE'd anywhere?) we add a nova_input chain.
>    At the end of FORWARD, we add a nova_forward chain.
>    At the end of OUTPUT, we add a nova_output chain.
>    At the end of nat:PREROUTING, we add nova_prerouting.
>    At the end of nat:POSTROUTING, we add nova_postrouting and nova_snatting (in that order).
>    At the end of nat:OUTPUT, we add nova_output.
> 
> If use_nova_chains isn't enabled, we add a new chain, called SNATTING, to the end of nat:POSTROUTING.

splitting postrouting into two chains allows us to ensure the right ordering for the postrouting rules.

> 
> Regardless of the use_nova_chains setting, we now add a rule to the SNATTING chain, even though AFAICT it only exists if use_nova_chains is False?

if nova chains is true, the rule will be rewritten in confirm_rule and it will be added to nova_snatting instead

> 
> In nova.network.linux_net.ensure_bridge:
> 
> If use_nova_chains is enabled, we create nova_forward (again?) and add it to the end of FORWARD.

It creates nova_forward if it hasn't been created yet

> 
> We then prepend the ACCEPT rules for the bridge to the FORWARD chain (rather than the nova_forward chain?).

once again the rule will go into the nova_forward chain

> 
> I'm at a bit of a loss as to where to integrate my stuff. What I need is a place to put some forwarding rules that get applied before the ACCEPT rules for the bridge. I could simply just leave it where it is in the current patch (which is at the very top of the FORWARD chain), and that would totally work with use_nova_chains as far as I can see, but I'm not sure if that's what is indended in the use_nova_chains paradigm?

The idea here is that all of the rules go into nova specific chains instead of the default chains so that sysadmins can add and remove rules by dealing with the shorter default chains.  It is mostly for so that the chains remain relatively clean. and that all of the rules added by nova are in one place so that they are easy to flush/remove if necessary.  For example, we tend to have a pretty locked down set of rules, so we have custom exceptions for a bunch of non-nova services.  There have been many cases where there are extraneous due to a misconfigured flag or some such.  Once the flag is changed, the iptables -D doesn't delete the old rule.  With use nova_chains you can just flush the nova_xxxx chain and restart the network worker instead of manually removing bad rules from the input chain.

I don't know that your code needs to follow the same pattern, although it could.  I just wanted to make sure it wouldn't break with use_nova_chains, since we use it.  If all of your rules are in forward, you could have a conditional based on --use_nova_chains that ensures the chain exists and then adds them to nova-forward instead.  Or you could have a specific new nova_xxxxx chain before nova_forward.
> 
> -- 
> https://code.launchpad.net/~soren/nova/iptables-security-groups/+merge/43767
> You are reviewing the proposed merge of lp:~soren/nova/iptables-security-groups into lp:nova.