[RFE] Improved validation mechanism for QoS rules with port types

[Slawek Kaplonski]

Currently neutron API can return what rules are supported, in the context of ML2: by returning the minimum subset of rules supported by all mechanism drivers.

Mechanism drivers in ML2 plugin can specify a list of QoS rule types which are supported. For example openvswitch mech_driver will report "bandwidth_limit_rule" (which is only for egress traffic in fact) and "dscp_marking" rule.

There are two problems with such simple check:

 - if rules are created or policy is applied to port there is no validation at all. It means that for example policy with dscp_marking rule can be applied to port bound with linuxbridge or sriov mech_drivers without any errors.

 - there is no possibility that mechanism driver will support some rules only with specific parameters. For example bandwidth_limit_rule could have parameter "direction" with values "ingress" and "egress" and some of mechanism drivers could only supports bandwidth limit with direction=egress.

We propose changes to how supported rule types are calculated and how validation works.

Incompatible configurations or settings should be spotted as soon as possible, and stopped at API level with a clear explanation of what's going on. On corner cases where this can't be determined until bind time, binding should fail so the administrator won't have the wrong perception of the all the policy rules being applied, where it's not the case. Such Validation of rules should be made during events like:
* port_update
* network_update
* policy_update
* rule_create
* rule_update

there will be called method from neutron.services.qos.notification_drivers.qos_base:QosServiceNotificationDriverBase.validate_policy_for_{port,network} which will raise exception if policy can't be applied on port or one of ports.
For ML2 plugin it will raise exception if policy will contain any rule which is not supported by mechanism driver used for port binding or by mechanism drivers used by specific binding:vnic_type if port is unbound.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/323474

[Armando Migliaccio (armando-migliaccio)]

I appreciate the desire of making the system behave like the user expect. My suggestion would be to tackle a small set of use cases where lack of validation leads to system error and iterate from there. To this aim, enumerating a series of examples where you see the system fails today and how you would like to see the response change

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/328655

[Miguel Angel Ajo (mangelajo)]

@armax, a few examples would be:

Current:
a) If you set an sr-iov port (vnic type direct) to a policy with DSCP rules, the request would be accepted, and but the sr-iov agent would ignore the request.

Expected after RFE: The request is denied with HTTP conflict, because we know a sr-iov port can't handle it now.

b) Same thing for a port which is bound to linuxbridge.

Expected after RFE: The request is denied, as for SR-IOV

c) If we have sr-iov ports attached to a policy, and we modify this policy to add a dscp rule, the SR-IOV agent will ignore the rule.

Expected after RFE: The request is denied with HTTP conflict, returning at least the ID of a port which is associated to the policy, but what can't take the new rule.

SR-IOV & Linuxbridge could eventually implement DSCP, so it's not the perfect example but it's current.

Future:
d) if we introduce a direction parameter in bandwidth rule: SR-IOV can't implement "ingress", so we may, as for the c example, deny any rule introduction, or modification to set the direction field to ingress, when we know there's a port not able to handle it.

e) If we ever get to have traffic classifiers, again SR-IOV is unable to deliver such feature, we should expect that attaching any traffic classifier to a rule will be denied for policies which have SR-IOV (or any other form of incompatible port) attached to it.

[Ihar Hrachyshka (ihar-hrachyshka)]

Miguel, cases a-c can be handled by enforcing supported_rule_types that we already calculate; cases d-e seem like examples of API extensions that would require new /extensions entries to indicate to users they are supported for a plugin in question.

Would it solve the issue?

[Assaf Muller (amuller)]

http://eavesdrop.openstack.org/irclogs/%23openstack-meeting/%23openstack-meeting.2016-06-16.log.html#t2016-06-16T22:32:44

Further discussion needed, I didn't feel like we reached a strong consensus in any direction.

[Miguel Angel Ajo (mangelajo)]

From what I see in the drivers meeting log, and my conversation with Ihar on IRC http://eavesdrop.openstack.org/irclogs/%23openstack-neutron/%23openstack-neutron.2016-06-17.log.html#t2016-06-17T11:05:55

We seem have consensus on doing the validation part, while leaving the extra reporting ("$ neutron qos-rule-support-per-port-type") for a separate RFE if we still find it necessary and we find a reasonable way to do it (vnic type vs binding type vs tenant visibility of binding type)

[Miguel Angel Ajo (mangelajo)]

Based on the previous meeting conversation, and IRC conversation with Ihar, we have reduced the scope of the RFE to just extended validation to cover all case and combinations, but we won't be adding any API to report the rule support on the scope of this RFE.

Please approve if there is consensus, since this will unblock the other already approved QoS RFEs for this cycle.

This one should have priority over the already approved ones for that reason, there is already a working patch on flight: https://review.openstack.org/#/c/319694/

[Miguel Angel Ajo (mangelajo)]

Since this was previously discussed on the drivers meeting, and there was no consensus due to one of the points I'm moving it to Triaged. The conflicting point has been removed (Extended reporting of mechanism/port type capabilities).

I have raised Importance because this is blocking the two other qos RFEs which have already been approved and have code in flight.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.openstack.org/328655
Reason: this part was removed from rfe bug and we will not want to do it

[Armando Migliaccio (armando-migliaccio)]

Status update:

http://eavesdrop.openstack.org/meetings/neutron_drivers/2016/neutron_drivers.2016-07-14-22.01.log.html#l-148

[Ihar Hrachyshka (ihar-hrachyshka)]

This proposal is now properly scoped, and does not suggest to introduce API changes. It's merely a way to 1) relax supported rule types API; 2) at the same time, introduce validation that would forbid incorrect parameters used for a backend, so that the relaxation of rules from 1. does not result in incorrect bindings.

[Armando Migliaccio (armando-migliaccio)]

https://review.openstack.org/#/c/319694/

[Armando Migliaccio (armando-migliaccio)]

https://review.openstack.org/#/c/323474/

[Armando Migliaccio (armando-migliaccio)]

Status update:

http://eavesdrop.openstack.org/meetings/neutron_drivers/2016/neutron_drivers.2016-07-21-22.00.log.html#l-191

[Armando Migliaccio (armando-migliaccio)]

The team is close, we should try and pull it off for Newton.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/323474
Committed: https://git.openstack.org/cgit/openstack/neutron-specs/commit/?id=d573df321ee73e40d114332a3098948eb1b5669f
Submitter: Jenkins
Branch: master

commit d573df321ee73e40d114332a3098948eb1b5669f
Author: Miguel Angel Ajo <email address hidden>
Date: Tue May 31 18:09:01 2016 +0200

    QoS Improved validation mechanism for rules

    This spec describes the implementation of a more granular validation
    of QoS policy rules. With two purposes in mind:
    1st) Supporting heterogeneous deployments which provide several types
         of vnics and binding mechanisms.

    2nd) Allowing a more natural expansion of QoS rules, where some
         parameters would be supported in some mechanisms, and not in others
         while that's detected and notified to the caller at API level.

    Co-Authored-By: Slawek Kaplonski <email address hidden>

    Change-Id: I44fa91eab08d3c80511c0354153f15fa41d53918
    Related-Bug: 1586056

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/426946

[OpenStack Infra (hudson-openstack)]

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.openstack.org/319694
Reason: It will be now done again from scratch with refactored QoS notifications driver: https://review.openstack.org/#/c/426946/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/426946
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=e2226fe373dfde74e8fc9e64ad65100e1c5de98b
Submitter: Jenkins
Branch: master

commit e2226fe373dfde74e8fc9e64ad65100e1c5de98b
Author: Sławek Kapłoński <email address hidden>
Date: Mon Jan 30 21:56:38 2017 +0000

    Improve validation of supported QoS rules

    New way of validation of QoS policy can be applied on port/network based on
    port_types. It can validate if port can support rule by rule_type and also by
    rule parameters or some values of parameters.

    For example bandwidth_limit_rule can have parameter "direction" with
    possible values "ingress" and "egress". Some of mechanism drivers can support
    only one of those directions.

    If user will try to apply policy with rule unsupported by port_type then it
    will get error message and policy will not be applied.
    Such validation is made during:
    * port create
    * port update
    * network update
    * QoS rule create
    * QoS rule update

    Co-Authored-By: Miguel Angel Ajo <email address hidden>

    Implements blueprint qos-rules-validation
    Closes-Bug: #1586056

    Change-Id: I75bd18b3a1875daa5639dd141fb7bbd6e1c54118

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/460460

[OpenStack Infra (hudson-openstack)]

Change abandoned by Rodolfo Alonso Hernandez (<email address hidden>) on branch: master
Review: https://review.openstack.org/460460

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 11.0.0.0b2 development milestone.