Hi Justin,
My point is what
On Thu, Sep 12, 2013 at 12:46 AM, Justin Hammond
<justin.hamm...@rackspace.com> wrote:
> 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.
I agree that value should be validated appropriately in general.
A configurable list of allowed protocols looks good to me.
> 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.
I think this is just a case a backend plugin defines allowed protocols.
I also see a different case: a cloud provider defines allowed protocols.
For example VLAN network type of OVS plugin can convey any type of packets
including GRE, STCP and so on if a provider wants to do so.
We need to allow a provider to configure the list.
Considering the above, what we need to do looks:
(a) to validate values properly,
(b) to allow a plugin to define what protocols should be allowed
(I think we need two types of lists: possible protocols and
default allowed protocols)
(c) to allow a cloud provider (deployer) to customize allow protocols.
(Of course (c) is a subnet of "possible protocols" in (b))
Does it make sense?
The above is just a start point of the discussion and some list can be omitted.
# Whether (c) is needed or not depends on the default list of (b).
# If it is wide enough (c) is not needed. The current list of (b) is
[tcp, udp, icmp]
# and it looks too small set to me, so it is better to have (c) too.
> 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.
I think the situation is a bit different. What network types are
allowed is tightly
coupled with a plugin implementation, and a cloud provider choose a plugin
based on their needs. Thus the mechanism of NETWORK_TYPE constants
make sense to me too.
> tl;dr: magic constants are no good, but values should be validated in a
> pluggable and explicit manner.
As I said above, I agree it is important to validate values properly in general.
Thanks,
Akihiro
>
>
>
> On 9/11/13 10:40 AM, "Akihiro Motoki" <amot...@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)
>><aso...@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@lists.openstack.org
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>
>>
>>
>>--
>>Akihiro MOTOKI <amot...@gmail.com>
>>
>>_______________________________________________
>>OpenStack-dev mailing list
>>OpenStack-dev@lists.openstack.org
>>http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
--
Akihiro MOTOKI <amot...@gmail.com>
_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
