<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)"><br></div><div class="gmail_extra">
<br><div class="gmail_quote">On Sun, Mar 26, 2017 at 9:31 PM, Sridhar Gaddam <span dir="ltr"><<a href="mailto:sgaddam@redhat.com" target="_blank">sgaddam@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:tahoma,sans-serif"><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 24, 2017 at 7:27 PM, Brian Haley <span dir="ltr"><<a href="mailto:haleyb.dev@gmail.com" target="_blank">haleyb.dev@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_-6449706251555358467gmail-">On 03/24/2017 06:41 AM, Ghanshyam Mann wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hi All,<br>
<br>
Tempest is testing SG rule creation and pinging scenario tests with<br>
ethertype='IPv6' and protocol='icmp' [0].<br>
In case of ethertype='IPv6', currently neutron accept protocol type<br>
as 'icmp', 'icmpv6' and 'ipv6-icmp' which again seems like duplication<br>
of SG rules bug on neutron side but not sure [1]<br>
<br>
But it seems like some driver does not work with 'icmp' on IPv6, at<br>
least ODL as mentioned in bug [2]. Where few others like ML2/OVS<br>
iptables driver convert 'icmp' to 'icmpv6' when ethertype='IPv6' and had<br>
no issue with 'icmp'.<br>
<br>
IMO neutron should keep accepting 'icmp' for IPv6 for backward<br>
compatibility and legacy usage and tempest should test 'icmp' also along<br>
with other protocol type.<br>
But we need more feedback on that what is right way (as per backward<br>
compatibility pov) and recommended way for having best behaviour for SG<br>
rules on IPv6. What best can work for all plugins also?<br>
</blockquote>
<br></span>
Thanks for raising this issue. Let me just restate it a little so it's clear.<br>
<br>
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.<br>
<br></blockquote><div><div style="font-family:tahoma,sans-serif">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".</div><div style="font-family:tahoma,sans-serif">Currently, Neutron treats each of them as a different Security Group rule and stores them as separate entries in the DB.</div><div style="font-family:tahoma,sans-serif">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.</div><div><font face="tahoma, sans-serif"><a href="https://github.com/openstack/neutron/blob/stable/newton/neutron/agent/linux/iptables_firewall.py#L373" target="_blank">https://github.com/openstack/<wbr>neutron/blob/stable/newton/<wbr>neutron/agent/linux/iptables_<wbr>firewall.py#L373</a></font><br></div><div><font face="tahoma, sans-serif"><br></font></div><div style="font-family:tahoma,sans-serif">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.</div></div></div></div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">++ for this, documentation could have helped this better way. </div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
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.<br>
<br>
<br>
So there are a few things that could be done:<br>
<br>
1. Drivers need to accept "icmp" in order to be backwards-compatible with the current code.<br>
<br>
2. Duplicates should be detected and generate 409 (?) errors.<br>
<br>
3. We should add a migration (IMO) where any duplicates are squashed.<br>
<br></blockquote><div style="font-family:tahoma,sans-serif">Agree with your points.</div></div></div></div></blockquote><div><br></div><div><br></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">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 <div class="gmail_default" style="display:inline"></div><span style="color:rgb(34,34,34);font-family:tahoma,sans-serif">Ethertype=IPv6</span> and user will get 409 if they try to create and expect 2 different SG rules with those different protocol_type.</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">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. </div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">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.</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">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. </div></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">Please leave comment on patch if any more feedback. </div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)"><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div style="font-family:tahoma,sans-serif"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
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?<br>
<br></blockquote><div><div style="font-family:tahoma,sans-serif">IMO, the out-of-tree drivers should be supporting the icmpv6 protocol rule properly. There is a possibility that they may hit this issue[<a href="https://bugs.launchpad.net/tempest/+bug/1671366" target="_blank">https://bugs.launchpad.<wbr>net/tempest/+bug/1671366</a>] for not being aware that a request for "<div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0);display:inline"></div>Ethertype=IPv6 and protocol=icmp" should be treated as ICMPv6 SG rule.</div><div style="font-family:tahoma,sans-serif"><br></div><div style="font-family:tahoma,sans-serif">Please see the DB entries for the two SG rules - <a href="http://paste.openstack.org/show/604190/" target="_blank">http://paste.openstack.org/<wbr>show/604190/</a></div><br></div><div><div style="font-family:tahoma,sans-serif"><br></div></div></div></div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">..1 <a href="https://review.openstack.org/#/c/431276/16/tempest/api/network/test_security_groups.py">https://review.openstack.org/#/c/431276/16/tempest/api/network/test_security_groups.py</a></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">..2 <a href="https://bugs.launchpad.net/neutron/+bug/1582500">https://bugs.launchpad.net/neutron/+bug/1582500</a> </div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0)">-gmann</div><br></div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div style="font-family:tahoma,sans-serif"></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-Brian<br>
<br>
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
</blockquote></div><br></div></div>
<br>______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.<wbr>openstack.org?subject:<wbr>unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/<wbr>cgi-bin/mailman/listinfo/<wbr>openstack-dev</a><br>
<br></blockquote></div><br></div></div>