Names are not necessarily portable across implementations and this would be a major change to make this late in the cycle. At this point in the cycle, we need to focus on ensuring fixes minimize disruption.
mark On Sep 11, 2013, at 6:03 PM, "Arvind Somya (asomya)" <aso...@cisco.com> wrote: > Ok, those were some good points. I personally like the approach of letting > each implementation specify it's own set of supported protocols. > > I'll change my patch to simply convert all protocols to names (more > readable). > > > Thanks > Arvind > > On 9/11/13 3:06 PM, "Justin Hammond" <justin.hamm...@rackspace.com> wrote: > >> I agree with you. Plugin was a mere example and it does make sense to >> allow the provider to define custom protocols. >> >> +1 >> >> On 9/11/13 12:46 PM, "Akihiro Motoki" <amot...@gmail.com> wrote: >> >>> 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 >> >> >> _______________________________________________ >> 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 _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev