On Wed, Jan 18, 2017 at 10:45 PM, Bernard Cafarelli <[email protected]> wrote:
> Hi neutrinos, > > I would like your feedback on the mentioned changeset in title[1] > (yes, added since Liberty). > > With this patch, we (should) skip ports with > port_security_enabled=False or with an empty list of security groups > when processing added ports [2]. But we found multiple problems here > > * Ports create with port_security_enabled=False > > This is the original bug that started this mail: if the FORWARD > iptables chain has a REJECT default policy/last rule, the traffic is > still blocked[3]. There is also a launchpad bug with similar details > [4] > The problem here: these ports must not be skipped, as we add specific > firewall rules to allow all traffic. These iptables rules have the > following comment: > "/* Accept all packets when port security is disabled. */" > > With the current code, any port created with port security will not > have these rules (and updates do not work). > I initially sent a patch to process these ports again [5], but there > is more (as detailed by some in the launchpad bug) > > * Ports with no security groups, current code > > There is a bug in the current agent code [6]: even with no security > groups, the check will return true as, the security_groups key exists > in the port details (with value "[]"). > So the port will not be skipped > > * Ports with no security groups, updated code > > Next step was to update checks (security groups list not empy, port > security True or None), and test again. The port this time was > skipped, but this showed up in openvswitch-agent.log: > 2017-01-18 16:19:56.780 7458 INFO > neutron.agent.linux.iptables_firewall > [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Attempted to > update port filter which is not filtered > c2c58f8f-3b76-4c00-b792-f1726b28d2fc > 2017-01-18 16:19:56.853 7458 INFO > neutron.plugins.ml2.drivers.openvswitch.agent.ovs_neutron_agent > [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Configuration for > devices up [u'c2c58f8f-3b76-4c00-b792-f1726b28d2fc'] and devices down > [] completed. > > Which is the kind of logs we saw in the first bug report. So as an > additional test, I tried to update this port, adding a security group. > New log entries: > 2017-01-18 17:36:53.164 7458 INFO neutron.agent.securitygroups_rpc > [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Refresh firewall > rules > 2017-01-18 17:36:55.873 7458 INFO > neutron.agent.linux.iptables_firewall > [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Attempted to > update port filter which is not filtered > 0f2eea88-0e6a-4ea9-819c-e26eb692cb25 > 2017-01-18 17:36:58.587 7458 INFO > neutron.plugins.ml2.drivers.openvswitch.agent.ovs_neutron_agent > [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Configuration for > devices up [u'0f2eea88-0e6a-4ea9-819c-e26eb692cb25'] and devices down > [] completed. > > And the iptables configuration did not change to show the newly allowed > ports. > > So with a fixed check, wend up back in the same buggy situation as the > first one. > > * Feedback > > So which course of action should we take? After checking these 3 cases > out, I am in favour of reverting this commit entirely, as in its > current state it does not help for ports without security groups, and > breaks ports with port security disabled. > > After having gone through the code and debugged the situation I'm also in favor of reverting the patch. We should explicitly setup a rule which allows traffic for that tap device exactly as we do when the port_security_enabled is switched from True to False. We can't relay on traffic to be implicitly allowed. Also, on the tests side, should we add more tests only using create > calls (port_security tests mostly update an existing port)? How to > make sure these iptables rules are correctly applied (the ping tests > are not enough, especially if the host system does not reject packets > by default)? Tests are incomplete so we should add either functional or fullstack/tempest tests that validate these cases (ports created with port_security_enabled set to False, ports created with no security groups, etc.). I can try to do that. > [1] https://review.openstack.org/#/c/210321/ > [2] https://github.com/openstack/neutron/blob/ > a66c27193573ce015c6c1234b0f2a1d86fb85a22/neutron/plugins/ > ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1640 > [3] https://bugzilla.redhat.com/show_bug.cgi?id=1406263 > [4] https://bugs.launchpad.net/neutron/+bug/1549443 > [5] https://review.openstack.org/#/c/421832/ > [6] https://github.com/openstack/neutron/blob/ > a66c27193573ce015c6c1234b0f2a1d86fb85a22/neutron/plugins/ > ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1521 > > Thanks! > > -- > Bernard Cafarelli > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: [email protected]?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
