Open Stack

As it seems the review is no longer the place for this discussion, I will
copy/paste my inline comments here:

I dislike the idea of passing magical numbers around to define protocols
(defined or otherwise). I believe there should be a common set of
protocols with their numbers mapped (such as this constants business) and
a well defined way to validate/list said common constants. If a plugin
wishes to add support for a protocol outside of the common case, it should
be added to the list in a pluggable manner.
Ex: common defines the constants 1, 6, 17 to be valid but my_cool_plugin
wants to support 42. It should be my plugin's responsibility to add 42 to
the list of valid protocols by appending to the list given a pluggable
interface to do so. I do not believe plugins should continue to update the
common.constants file with new protocols, but I do believe explicitly
stating which protocols are valid is better than allowing users to
possibly submit protocols erroneously.
If the plugins use a system such as this, it is possible that new, common,
protocols can be found to be core. See NETWORK_TYPE constants.

tl;dr: magic constants are no good, but values should be validated in a
pluggable and explicit manner.



On 9/11/13 10:40 AM, "Akihiro Motoki" <amotoki at gmail.com> wrote:

>Hi all,
>
>Arvind, thank you for initiate the discussion about the ip protocol in
>security group rules.
>I think the discussion point can be broken down into:
>
>(a) how to specify ip protocol : by name, number, or both
>(b) what ip protocols can be specified: known protocols only, all
>protocols (or some subset of protocols including unknown protocols)
>     where "known protocols" is defined as a list in Neutron (a list
>of constants or a configurable list)
>
>------
>(b) is the main topic Arvind and I discussed in the review.
>If only known protocols are allowed, we cannot allow protocols which
>are not listed in the known protocol list.
>For instance, if "tcp", "udp" and "icmp" are registered as known
>protocols (this is the current neutron implementation),
>a tenant cannot allow "stcp" or "gre".
>
>Pros of "known protocols only" is the infrastructure provider can
>control which protocols are allowed.
>Cons is that users cannot use ip protocols not listed in a known list
>and a provider needs to maintain a known protocol list.
>Pros and cons of "all protocols allowed" is vice versa.
>
>If a list of known protocols is configurable, we can cover both cases,
>e.g., an empty list or a list ["ANY"] means all protocols are allowed.
>The question in this case is what is the best default value.
>
>My preference is to allow all protocols. At least a list of known
>protocols needs to be configurable.
>In my principle, a virtual network should be able to convery any type
>of IP protocols in a virtual network. This is the reason of my
>preference.
>
>-----
>Regarding (a), if a name and a number refer to a same protocol, it
>should be considered as identical.
>For example, ip protocol number 6 is "tcp", so ip protocol number 6
>and protocol name "tcp" should be regarded as same.
>My preference is to allow both name and number of IP protocol. This
>will be achieved by Arvind's patch under the review.
>"name" representation is easy to understand in general, but
>maintaining all protocol names is a tough work.
>This is the reason of my preference.
>
>
>I understand there is a topic whether a list of known protocols should
>contain name only or accepts both name and number.
>I don't discuss it here because it is a simple question once we have a
>consensus on the above two topic.
>
>Thanks,
>Akihiro
>
>On Wed, Sep 11, 2013 at 11:15 PM, Arvind Somya (asomya)
><asomya at cisco.com> wrote:
>> Hello all
>>
>> I have a patch in review where  Akihiro made some comments about only
>> restricting protocols by names and allowing all protocol numbers when
>> creating security group rules. I personally disagree with this approach
>>as
>> names and numbers are just a textual/integer representation of a common
>> protocol. The end result is going to be the same in both cases.
>>
>> https://review.openstack.org/#/c/43725/
>>
>> Akihiro suggested a community discussion around this issue before the
>>patch
>> is accepted upstream. I hope this e-mail gets the ball rolling on that.
>>I
>> would like to hear the community's opinion on this issue and any
>> pros/cons/pitfalls of either approach.
>>
>> Thanks
>> Arvind
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
>
>
>-- 
>Akihiro MOTOKI <amotoki at gmail.com>
>
>_______________________________________________
>OpenStack-dev mailing list
>OpenStack-dev at lists.openstack.org
>http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev