On 04/03/2017 09:23 PM, Ghanshyam Mann wrote:
On Sun, Mar 26, 2017 at 9:31 PM, Sridhar Gaddam <sgad...@redhat.com
<mailto:sgad...@redhat.com>> wrote:
On Fri, Mar 24, 2017 at 7:27 PM, Brian Haley <haleyb....@gmail.com
<mailto:haleyb....@gmail.com>> wrote:
On 03/24/2017 06:41 AM, Ghanshyam Mann wrote:
Hi All,
Tempest is testing SG rule creation and pinging scenario
tests with
ethertype='IPv6' and protocol='icmp' [0].
In case of ethertype='IPv6', currently neutron accept
protocol type
as 'icmp', 'icmpv6' and 'ipv6-icmp' which again seems like
duplication
of SG rules bug on neutron side but not sure [1]
But it seems like some driver does not work with 'icmp' on
IPv6, at
least ODL as mentioned in bug [2]. Where few others like ML2/OVS
iptables driver convert 'icmp' to 'icmpv6' when
ethertype='IPv6' and had
no issue with 'icmp'.
IMO neutron should keep accepting 'icmp' for IPv6 for backward
compatibility and legacy usage and tempest should test
'icmp' also along
with other protocol type.
But we need more feedback on that what is right way (as per
backward
compatibility pov) and recommended way for having best
behaviour for SG
rules on IPv6. What best can work for all plugins also?
Thanks for raising this issue. Let me just restate it a little
so it's clear.
1. One can create an IPv6 rule using protocol value "icmp"
today, and the base security group code does the right thing
changing the rule to be correct for the underlying
implementation, for example, "ipv6-icmp" for iptables. It
doesn't look like all other drivers handle this properly.
Well, let the Neutron API accept multiple values like
"icmp/icmpv6/ipv6-icmp", but IMO it should store a single Security
Group rule in the DB and raise "Duplicate error when similar rule is
configured once again".
Currently, Neutron treats each of them as a different Security Group
rule and stores them as separate entries in the DB.
However, IPtables Firewall driver in Neutron is converting[1] the
"ethertype=IPv6 and protocol=icmp" as a request to ICMPv6 and
applying the necessary ip-table rule.
https://github.com/openstack/neutron/blob/stable/newton/neutron/agent/linux/iptables_firewall.py#L373
<https://github.com/openstack/neutron/blob/stable/newton/neutron/agent/linux/iptables_firewall.py#L373>
Since this is not a documented behavior, other firewall drivers
(which I guess could be an issue even with OVS firewall driver) may
be missing this info.
++ for this, documentation could have helped this better way.
2. The neutron API will accept multiple values - "icmp",
"ipv6-icmp" and "icmpv6" for an IPv6 rule, but it will create
unique database entries for each (I just verified that). While
that shouldn't create multiple entries in the base iptables
code, it will probably generate a warning in the logs about a
duplicate being suppressed.
So there are a few things that could be done:
1. Drivers need to accept "icmp" in order to be
backwards-compatible with the current code.
2. Duplicates should be detected and generate 409 (?) errors.
3. We should add a migration (IMO) where any duplicates are
squashed.
Agree with your points.
I just created https://review.openstack.org/453346 as a start at #2,
will try and add code to squash existing duplicates too (#3).
-Brian
Yes, 409 on same type again make sense. And it can be documented
properly that 'icmp' will be treated same as other protocol type for
Ethertype=IPv6 and user will get 409 if they try to create and expect 2
different SG rules with those different protocol_type.
This is something change in current behavior where both requests('icmp'
and 'icmpv6') are treated as 2 separate rules. And so this new Tempest
tests will fail [1] which can be modified later.
With those points and current situation, I feel Tempest should tests the
current behavior proposed in [1] which will discover the drivers for
point 1. Later based on bug [2] consensus we can change the tests
accordingly.
I want to merge Tempest changes so that while changing anything on
neutron side, we know exactly what behavior we are going to change and
its impact etc.
Please leave comment on patch if any more feedback.
The open question is, do we want to change the DB to have a
different value here, like "icmpv6" ? We could obviously add a
migration where we update the value. The problem is that flag
day could pose a problem if out-of-tree drivers don't support
the new value. I think we should leave it "icmp" for that
reason, thoughts from others?
IMO, the out-of-tree drivers should be supporting the icmpv6
protocol rule properly. There is a possibility that they may hit
this issue[https://bugs.launchpad.net/tempest/+bug/1671366
<https://bugs.launchpad.net/tempest/+bug/1671366>] for not being
aware that a request for "
Ethertype=IPv6 and protocol=icmp" should be treated as ICMPv6 SG rule.
Please see the DB entries for the two SG rules
- http://paste.openstack.org/show/604190/
<http://paste.openstack.org/show/604190/>
..1
https://review.openstack.org/#/c/431276/16/tempest/api/network/test_security_groups.py
..2 https://bugs.launchpad.net/neutron/+bug/1582500
-gmann
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
