From: Salvatore Orlando <salv.orlando@gmail.com>
Date: Thu, 19 Jun 2014 12:49:18 +0000 (-0700)
Subject: NSX: return 400 if dscp set for trusted queue
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f24482b18428a4935cbda97f96532e7c0876d9bd;p=openstack-build%2Fneutron-build.git

NSX: return 400 if dscp set for trusted queue

If qos_marking is set to True, the code currently silently
drops the dscp setting and continues.

It is preferrable instead to notify the user of the invalid
setting returning a 400 error.

Change-Id: I7db1867712bd20f2030c56a42baac20e6ae76d0d
Closes-Bug: 1332062
---

diff --git a/neutron/plugins/vmware/dbexts/qos_db.py b/neutron/plugins/vmware/dbexts/qos_db.py
index b094a2293..b01050201 100644
--- a/neutron/plugins/vmware/dbexts/qos_db.py
+++ b/neutron/plugins/vmware/dbexts/qos_db.py
@@ -288,8 +288,11 @@ class QoSDbMixin(qos.QueuePluginBase):
                 raise qos.DefaultQueueCreateNotAdmin()
         if qos_queue.get('qos_marking') == 'trusted':
             dscp = qos_queue.pop('dscp')
+            if dscp:
+                # must raise because a non-zero dscp was provided
+                raise qos.QueueInvalidMarking()
             LOG.info(_("DSCP value (%s) will be ignored with 'trusted' "
-                     "marking"), dscp)
+                       "marking"), dscp)
         max = qos_queue.get('max')
         min = qos_queue.get('min')
         # Max can be None
diff --git a/neutron/plugins/vmware/extensions/qos.py b/neutron/plugins/vmware/extensions/qos.py
index 45b343a1e..8eca2ca88 100644
--- a/neutron/plugins/vmware/extensions/qos.py
+++ b/neutron/plugins/vmware/extensions/qos.py
@@ -45,6 +45,11 @@ class QueueInvalidDscp(qexception.InvalidInput):
                 " between 0 and 63.")
 
 
+class QueueInvalidMarking(qexception.InvalidInput):
+    message = _("The qos marking cannot be set to 'trusted' "
+                "when the DSCP field is set")
+
+
 class QueueMinGreaterMax(qexception.InvalidInput):
     message = _("Invalid bandwidth rate, min greater than max.")
 
@@ -88,16 +93,19 @@ def convert_to_unsigned_int_or_none_max_63(val):
 # As per NSX API, if a queue is trusted, DSCP must be omitted; if a queue is
 # untrusted, DSCP must be specified. Whichever default values we choose for
 # the tuple (qos_marking, dscp), there will be at least one combination of a
-# request with conflicting values: for instance, with the following default:
-#
-# qos_marking = 'untrusted', dscp = '0'
-#
-# requests with qos_marking = 'trusted' and a default dscp will fail. Since
-# it is convoluted to ask the admin to specify a None value for dscp when
-# qos_marking is 'trusted', it is best to ignore the dscp value, regardless
-# of whether it has been specified or not. This preserves the chosen default
-# and keeps backward compatibility with the API. A warning will be logged, as
-# the server is overriding a potentially conflicting request from the admin
+# request with conflicting values: for instance given the default values below,
+# requests with qos_marking = 'trusted' and the default dscp value will fail.
+# In order to avoid API users to explicitly specify a setting for clearing
+# the DSCP field when a trusted queue is created, the code serving this API
+# will adopt the following behaviour when qos_marking is set to 'trusted':
+# - if the DSCP attribute is set to the default value (0), silently drop
+#   its value
+# - if the DSCP attribute is set to anything than 0 (but still a valid DSCP
+#   value) return a 400 error as qos_marking and DSCP setting conflict.
+# TODO(salv-orlando): Evaluate whether it will be possible from a backward
+# compatibility perspective to change the default value for DSCP in order to
+# avoid this peculiar behaviour
+
 RESOURCE_ATTRIBUTE_MAP = {
     'qos_queues': {
         'id': {'allow_post': False, 'allow_put': False,
diff --git a/neutron/tests/unit/vmware/extensions/test_qosqueues.py b/neutron/tests/unit/vmware/extensions/test_qosqueues.py
index 53e82f52f..6b5d53ba6 100644
--- a/neutron/tests/unit/vmware/extensions/test_qosqueues.py
+++ b/neutron/tests/unit/vmware/extensions/test_qosqueues.py
@@ -227,6 +227,13 @@ class TestQoSQueue(test_nsx_plugin.NsxPluginV2TestCase):
         res = self._create_qos_queue('json', body)
         self.assertEqual(res.status_int, 400)
 
+    def test_dscp_value_with_qos_marking_trusted_returns_400(self):
+        body = {'qos_queue': {'tenant_id': 'admin', 'dscp': '1',
+                              'qos_marking': 'trusted',
+                              'name': 'foo', 'min': 20, 'max': 20}}
+        res = self._create_qos_queue('json', body)
+        self.assertEqual(res.status_int, 400)
+
     def test_non_admin_cannot_create_queue(self):
         body = {'qos_queue': {'tenant_id': 'not_admin',
                               'name': 'foo', 'min': 20, 'max': 20}}