[RFE] FWaaS: Using Netlink instead of conntrack-tools to improve performance

python-conntrack is unmaintained
pynetfilter_conntrack seems more complete just not very active (does that mean it's stable?)

Either way, are there other options?

1) Are we duplicating any conntrack calls?
2) Are we running a lot of commands when a different, one-liner will work?

But feel free to create a patch and send it out for review based on what you wrote above as well.

@Brian Haley:
Thanks for discussing, I would like to response as below:
1) Are we duplicating any conntrack calls?
>> The conntrack command in FWaaS filter conntrack entries by "protocol", "ip version", "src_port" and "dst_port". In my test case, it is not duplicated.
2) Are we running a lot of commands when a different, one-liner will work?
>> Running one-liner may affect to other unexpected connection.

I will create the patch for my concept. Thank for your attention.

@Brian Haley:
Thanks for considering, I have uploaded my patch as PoC in [1]. Could you please review it? Would you mind if I add you as reviewer.

[1] https://review.openstack.org/389654

I think I'd like to see this implemented and improved upon in FWaaS before we adopt it for our SG implementation for a few reasons.

1. It's significantly more code for us to maintain, even without the functional tests the patch is missing.
2. It depends on privsep, which we haven't actually adopted yet in Neutron.
3. It's not clear to me if this ctype use is too runtime specific for deployer flexibility. https://review.openstack.org/#/c/389654/10/neutron_fwaas/privileged/nl_lib.py@86

Hi Kevin Benton,
I have seen that the privsep has already been adopted in Neutron in [1]. My FWaaS patch [2] with Netlink solution is depended on [3] within Neutron. Could you please take a look at it?

[1] https://review.openstack.org/394741
[2] https://review.openstack.org/389654
[3] https://review.openstack.org/392014

hi kevin, this patch has merged, what do you think adopting it for our SG in Pike?

Let's bring this for discussion.

Dear all,
This patch has been reverted[1], we are proposing another patch for it [2][3][4]. They are enhanced with namespace solution [5], unittests and functional tests.
[1]: https://review.openstack.org/434181
[2]: https://review.openstack.org/#/c/433598/
[3]: https://review.openstack.org/#/c/437311/
[4]: https://review.openstack.org/#/c/438445/
[5]: https://review.openstack.org/433633

My suggestion is to track this as a regular bug. Even though this is similar to bug 1492714, in reality we spearheaded efforts in neutron, so fwaas can just mirror what has been done already for adopting privsep.

Fix proposed to branch: master
Review: https://review.openstack.org/469028

Change abandoned by Cuong Nguyen (<email address hidden>) on branch: master
Review: https://review.openstack.org/469028
Reason: will upload other ps

Fix proposed to branch: master
Review: https://review.openstack.org/470912

No one noticed that the step "_apply_synchronized()" at neutron/agent/linux/iptables_manager.py was time consuming when there were many firewall rules(>70), especially in function "changes = _generate_path_between_rules(old_rules, new_rules)" and "_generate_chain_diff_iptables_commands()"?
I have test the performance for function "_generate_path_between_rules()"， it consumed about 30s when generating param "changes" for command iptable-restore with 130 firewall rules. So I think this is where we need to improve.

Can you elaborate your steps/tools used for that performance test?
And we welcome any code contribution for this bug on gerrit.

Fix proposed to branch: master
Review: https://review.openstack.org/483864

hi cuongnv, I only tested func performance of "_generate_chain_diff_iptables_commands" and tool difflib in my local vm, and I found the reason of time consuming is the different sequences of iptables params between old_chain_rules and new_chain_rules, for example:
one rule in old_chain_rules(which extracting from router namespace using iptables-save cmd) is：
-A neutron-vpn-agen-ov4c70fcfc5 -p tcp -m tcp --dport 220 --sport 220 -s 192.168.1.0/24 -d 192.168.3.0/24 -j ACCEPT
but the same rule in new_chain_rules may be is:
-A neutron-vpn-agen-ov4c70fcfc5 -m tcp --dport 220 --sport 220 -p tcp -s 192.168.1.0/24 -d 192.168.3.0/24 -j ACCEPT

and tool difflib could not able to identify it as a same rule. when there are many rules(>70 or even more)，the process of "_generate_chain_diff_iptables_commands"（time complexity is O(n^2)） may consume lots of times.

I don't know whether you can figure out my explations, and please forgive me my bad english, have a good day.

Hi Song,

I'm not sure that the thread you started at comment #13 belongs in this bug - can you please open another with that information if you haven't already?

If there is a difference in two iptables rules generated - '-p tcp -m tcp' versus '-m tcp' that could just be a bug somewhere in the code as it shouldn't happen.

Thanks, Brian

Hi Song,

There is a specific option you can turn on to detect when the internal representation of rules does not match the system representation. debug_iptables_rules http://git.openstack.org/cgit/openstack/neutron/tree/neutron/conf/agent/common.py#n73

As Brian said, it shouldn't happen and if you find a case like that we need to fix the way we form the rule.

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/483864
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

The below is the info for performance testing:

Number of changed rules Time (s) Conntrack-tool times Performance improvement
500 1.040600538 19.11228633 94.555332
1000 2.515271902 31.07810974 91.90661233
5000 88.87204766 141.193157 37.05640589
10000 114.2987695 269.072845 57.52125434

Detail can be found here: https://docs.google.com/spreadsheets/d/1-eSf_NblcHqE2YqKSSe_vtTitQ1ouvNf5x8oW-qbsRQ/edit#gid=0

Related fix proposed to branch: master
Review: https://review.openstack.org/537654

I changed the bug in the related fix I proposed in the last comment.

This bug has had a related patch abandoned and has been automatically un-assigned due to inactivity. Please re-assign yourself if you are continuing work or adjust the state as appropriate if it is no longer valid.

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.openstack.org/470912
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.