NsxV3: Fine grained logging for security-groups

Also migrates security group logging for NSXv to new model

Change-Id: I0d6a90e0d8531156e06817cba431c72db0c81bde

etc/nsx.ini

@@ -408,3 +408,11 @@
 # If True, an internal metadata network will be created for a router only when
 # the router is attached to a DHCP-disabled subnet.
 # metadata_on_demand = False
+
+# (Optional) Indicates whether distributed-firewall rule for security-groups
+# blocked traffic is logged.
+# log_security_groups_blocked_traffic = False
+
+# (Optional) Indicates whether distributed-firewall security-groups rules are
+# logged.
+# log_security_groups_allowed_traffic = False

vmware_nsx/common/config.py

@@ -256,6 +256,14 @@ nsx_v3_opts = [
                 help=_("If true, an internal metadata network will be created "
                        "for a router only when the router is attached to a "
                        "DHCP-disabled subnet.")),
+    cfg.BoolOpt('log_security_groups_blocked_traffic',
+                default=False,
+                help=_("Indicates whether distributed-firewall rule for "
+                       "security-groups blocked traffic is logged")),
+    cfg.BoolOpt('log_security_groups_allowed_traffic',
+                default=False,
+                help=_("Indicates whether distributed-firewall "
+                       "security-groups rules are logged")),
 ]
 
 DEFAULT_STATUS_CHECK_INTERVAL = 2000

vmware_nsx/db/extended_security_group.py

@@ -0,0 +1,75 @@
+# Copyright 2016 VMware, Inc.
+# All Rights Reserved
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import sqlalchemy as sa
+from sqlalchemy import orm
+
+from neutron.db import db_base_plugin_v2
+from neutron.db import model_base
+from neutron.db import securitygroups_db
+from neutron.extensions import securitygroup as ext_sg
+
+from vmware_nsx.extensions import securitygrouplogging as sg_logging
+
+
+class NsxExtendedSecurityGroupProperties(model_base.BASEV2):
+    __tablename__ = 'nsx_extended_security_group_properties'
+
+    security_group_id = sa.Column(sa.String(36),
+                                  sa.ForeignKey('securitygroups.id',
+                                                ondelete="CASCADE"),
+                                  primary_key=True)
+    logging = sa.Column(sa.Boolean, default=False, nullable=False)
+    security_group = orm.relationship(
+        securitygroups_db.SecurityGroup,
+        backref=orm.backref('ext_properties', lazy='joined',
+                            uselist=False, cascade='delete'))
+
+
+class ExtendedSecurityGroupPropertiesMixin(object):
+
+    def _process_security_group_properties_create(self, context,
+                                                  sg_res, sg_req):
+        with context.session.begin(subtransactions=True):
+            properties = NsxExtendedSecurityGroupProperties(
+                security_group_id=sg_res['id'],
+                logging=sg_req.get(sg_logging.LOGGING, False))
+            context.session.add(properties)
+        sg_res[sg_logging.LOGGING] = sg_req.get(sg_logging.LOGGING, False)
+
+    def _get_security_group_properties(self, context, security_group_id):
+        return context.session.query(
+            NsxExtendedSecurityGroupProperties).filter_by(
+                security_group_id=security_group_id).one()
+
+    def _process_security_group_properties_update(self, context,
+                                                  sg_res, sg_req):
+        if (sg_logging.LOGGING in sg_req
+                and sg_req[sg_logging.LOGGING] != sg_res[sg_logging.LOGGING]):
+            prop = self._get_security_group_properties(context, sg_res['id'])
+            with context.session.begin(subtransactions=True):
+                prop.update({sg_logging.LOGGING: sg_req[sg_logging.LOGGING]})
+            sg_res[sg_logging.LOGGING] = sg_req[sg_logging.LOGGING]
+
+    def _is_security_group_logged(self, context, security_group_id):
+        prop = self._get_security_group_properties(context, security_group_id)
+        return prop.logging
+
+    db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs(
+        ext_sg.SECURITYGROUPS, ['_extend_security_group_with_properties'])
+
+    def _extend_security_group_with_properties(self, sg_res, sg_db):
+        if sg_db.ext_properties:
+            sg_res[sg_logging.LOGGING] = sg_db.ext_properties.logging

vmware_nsx/db/migration/alembic_migrations/versions/CONTRACT_HEAD

@@ -1 +1 @@
-3c88bdea3054
+5ed1ffbc0d2a

vmware_nsx/db/migration/alembic_migrations/versions/EXPAND_HEAD

@@ -1 +1 @@
-2c87aedb206f
+3e4dccfe6fb4

vmware_nsx/db/migration/alembic_migrations/versions/newton/contract/5ed1ffbc0d2a_nsx_security_group_logging.py

@@ -0,0 +1,57 @@
+# Copyright 2016 OpenStack Foundation
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+"""nsxv_security_group_logging
+
+Revision ID: 5ed1ffbc0d2a
+Revises: 3e4dccfe6fb4
+Create Date: 2016-03-24 06:06:06.680092
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '5ed1ffbc0d2a'
+down_revision = '3c88bdea3054'
+depends_on = ('3e4dccfe6fb4',)
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    secgroup_prop_table = sa.Table(
+        'nsx_extended_security_group_properties',
+        sa.MetaData(),
+        sa.Column('security_group_id', sa.String(36), nullable=False),
+        sa.Column('logging', sa.Boolean(), nullable=False))
+
+    op.bulk_insert(secgroup_prop_table, get_values())
+    op.drop_column('nsxv_security_group_section_mappings', 'logging')
+
+
+def get_values():
+    values = []
+    session = sa.orm.Session(bind=op.get_bind())
+    section_mapping_table = sa.Table('nsxv_security_group_section_mappings',
+                                     sa.MetaData(),
+                                     sa.Column('neutron_id', sa.String(36)),
+                                     sa.Column('logging', sa.Boolean(),
+                                               nullable=False))
+
+    for row in session.query(section_mapping_table).all():
+        values.append({'security_group_id': row.neutron_id,
+                       'logging': row.logging})
+    session.commit()
+    return values

vmware_nsx/db/migration/alembic_migrations/versions/newton/expand/3e4dccfe6fb4_nsx_security_group_logging.py

@@ -0,0 +1,40 @@
+# Copyright 2016 VMware, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""NSXv add dns search domain to subnets
+
+Revision ID: 3e4dccfe6fb4
+Revises: 2c87aedb206f
+Create Date: 2016-03-20 07:28:35.369938
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '3e4dccfe6fb4'
+down_revision = '2c87aedb206f'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.create_table(
+        'nsx_extended_security_group_properties',
+        sa.Column('security_group_id', sa.String(36), nullable=False),
+        sa.Column('logging', sa.Boolean(), nullable=False),
+        sa.ForeignKeyConstraint(['security_group_id'],
+                                ['securitygroups.id'],
+                                ondelete='CASCADE'),
+        sa.PrimaryKeyConstraint('security_group_id')
+    )

vmware_nsx/db/nsxv_db.py

@@ -366,10 +366,10 @@ def delete_nsxv_internal_edge(session, ext_ip_address):
                 filter_by(ext_ip_address=ext_ip_address).delete())
 
 
-def add_neutron_nsx_section_mapping(session, neutron_id, section_id, logging):
+def add_neutron_nsx_section_mapping(session, neutron_id, section_id):
     with session.begin(subtransactions=True):
         mapping = nsxv_models.NsxvSecurityGroupSectionMapping(
-            neutron_id=neutron_id, ip_section_id=section_id, logging=logging)
+            neutron_id=neutron_id, ip_section_id=section_id)
         session.add(mapping)
     return mapping
 

vmware_nsx/db/nsxv_models.py

@@ -114,7 +114,6 @@ class NsxvSecurityGroupSectionMapping(model_base.BASEV2):
                                          ondelete="CASCADE"),
                            primary_key=True)
     ip_section_id = sa.Column(sa.String(100))
-    logging = sa.Column(sa.Boolean, default=False, nullable=False)
 
 
 class NsxvRuleMapping(model_base.BASEV2):

vmware_nsx/extensions/securitygrouplogging.py

@@ -15,9 +15,11 @@
 from neutron.api import extensions
 from neutron.api.v2 import attributes
 
+LOGGING = 'logging'
+
 RESOURCE_ATTRIBUTE_MAP = {
     'security_groups': {
-        'logging': {
+        LOGGING: {
             'allow_post': True,
             'allow_put': True,
             'convert_to': attributes.convert_to_boolean,

vmware_nsx/nsxlib/v3/dfw_api.py

@@ -100,10 +100,12 @@ def list_nsgroups():
 
 
 @utils.retry_upon_exception_nsxv3(nsx_exc.StaleRevision)
-def update_nsgroup(nsgroup_id, display_name, description):
+def update_nsgroup(nsgroup_id, display_name=None, description=None):
     nsgroup = read_nsgroup(nsgroup_id)
-    nsgroup.update({'display_name': display_name,
+    if display_name is not None:
-                    'description': description})
+        nsgroup['display_name'] = display_name
+    if description is not None:
+        nsgroup['description'] = description
     return nsxclient.update_resource('ns-groups/%s' % nsgroup_id, nsgroup)
 
 
@@ -180,15 +182,25 @@ def create_empty_section(display_name, description, applied_tos, tags,
 
 
 @utils.retry_upon_exception_nsxv3(nsx_exc.StaleRevision)
-def update_section(section_id, display_name, description, applied_tos=None):
+def update_section(section_id, display_name=None, description=None,
+                   applied_tos=None, rules=None):
     resource = 'firewall/sections/%s' % section_id
     section = read_section(section_id)
-    section.update({'display_name': display_name,
+
-                    'description': description})
+    if rules is not None:
+        resource += '?action=update_with_rules'
+        section.update({'rules': rules})
+    if display_name is not None:
+        section['display_name'] = display_name
+    if description is not None:
+        section['description'] = description
     if applied_tos is not None:
         section['applied_tos'] = [get_nsgroup_reference(nsg_id)
                                   for nsg_id in applied_tos]
-    return nsxclient.update_resource(resource, section)
+    if rules is not None:
+        return nsxclient.create_resource(resource, section)
+    elif any(p is not None for p in (display_name, description, applied_tos)):
+        return nsxclient.update_resource(resource, section)
 
 
 def read_section(section_id):
@@ -219,14 +231,15 @@ def get_ip_cidr_reference(ip_cidr_block, ip_protocol):
 
 def get_firewall_rule_dict(display_name, source=None, destination=None,
                            direction=IN_OUT, ip_protocol=IPV4_IPV6,
-                           service=None, action=ALLOW):
+                           service=None, action=ALLOW, logged=False):
     return {'display_name': display_name,
             'sources': [source] if source else [],
             'destinations': [destination] if destination else [],
             'direction': direction,
             'ip_protocol': ip_protocol,
             'services': [service] if service else [],
-            'action': action}
+            'action': action,
+            'logged': logged}
 
 
 def add_rule_in_section(rule, section_id):
@@ -244,3 +257,8 @@ def add_rules_in_section(rules, section_id):
 def delete_rule(section_id, rule_id):
     resource = 'firewall/sections/%s/rules/%s' % (section_id, rule_id)
     return nsxclient.delete_resource(resource)
+
+
+def get_section_rules(section_id):
+    resource = 'firewall/sections/%s/rules' % section_id
+    return nsxclient.get_resource(resource)

vmware_nsx/nsxlib/v3/security.py

@@ -28,6 +28,7 @@ from vmware_nsx._i18n import _, _LW
 from vmware_nsx.common import exceptions as nsx_exc
 from vmware_nsx.common import utils
 from vmware_nsx.db import nsx_models
+from vmware_nsx.extensions import securitygrouplogging as sg_logging
 from vmware_nsx.nsxlib.v3 import dfw_api as firewall
 
 
@@ -91,7 +92,7 @@ def _decide_service(sg_rule):
                                       protocol_number=l4_protocol)
 
 
-def _get_fw_rule_from_sg_rule(sg_rule, nsgroup_id, rmt_nsgroup_id):
+def _get_fw_rule_from_sg_rule(sg_rule, nsgroup_id, rmt_nsgroup_id, logged):
     # IPV4 or IPV6
     ip_protocol = sg_rule['ethertype'].upper()
     direction = _get_direction(sg_rule)
@@ -115,10 +116,10 @@ def _get_fw_rule_from_sg_rule(sg_rule, nsgroup_id, rmt_nsgroup_id):
     return firewall.get_firewall_rule_dict(name, source,
                                            destination, direction,
                                            ip_protocol, service,
-                                           firewall.ALLOW)
+                                           firewall.ALLOW, logged)
 
 
-def create_firewall_rules(context, section_id, nsgroup_id,
+def create_firewall_rules(context, section_id, nsgroup_id, logging_enabled,
                           security_group_rules):
 
     # 1. translate rules
@@ -131,13 +132,43 @@ def create_firewall_rules(context, section_id, nsgroup_id,
             context, sg_rule, nsgroup_id)
 
         fw_rule = _get_fw_rule_from_sg_rule(
-            sg_rule, nsgroup_id, remote_nsgroup_id)
+            sg_rule, nsgroup_id, remote_nsgroup_id, logging_enabled)
 
         firewall_rules.append(fw_rule)
 
     return firewall.add_rules_in_section(firewall_rules, section_id)
 
 
+def _process_firewall_section_rules_logging_for_update(section_id,
+                                                       logging_enabled):
+    rules = firewall.get_section_rules(section_id).get('results', [])
+    update_rules = False
+    for rule in rules:
+        if rule['logged'] != logging_enabled:
+            rule['logged'] = logging_enabled
+            update_rules = True
+    return rules if update_rules else None
+
+
+def set_firewall_rule_logging_for_section(section_id, logging):
+    rules = _process_firewall_section_rules_logging_for_update(section_id,
+                                                               logging)
+    firewall.update_section(section_id, rules=rules)
+
+
+def update_security_group_on_backend(context, security_group):
+    nsgroup_id, section_id = get_sg_mappings(context.session,
+                                             security_group['id'])
+    name = get_nsgroup_name(security_group)
+    description = security_group['description']
+    logging = (cfg.CONF.nsx_v3.log_security_groups_allowed_traffic or
+               security_group[sg_logging.LOGGING])
+    rules = _process_firewall_section_rules_logging_for_update(section_id,
+                                                               logging)
+    firewall.update_nsgroup(nsgroup_id, name, description)
+    firewall.update_section(section_id, name, description, rules=rules)
+
+
 def get_nsgroup_name(security_group):
     # NOTE(roeyc): We add the security-group id to the NSGroup name,
     # for usability purposes.
@@ -228,36 +259,37 @@ def _init_default_section(name, description, nested_groups):
     fw_sections = firewall.list_sections()
     for section in fw_sections:
         if section['display_name'] == name:
-            firewall.update_section(section['id'],
-                                    name, section['description'],
-                                    applied_tos=nested_groups)
             break
     else:
         tags = utils.build_v3_api_version_tag()
         section = firewall.create_empty_section(
             name, description, nested_groups, tags)
-        block_rule = firewall.get_firewall_rule_dict(
-            'Block All', action=firewall.DROP)
-        # TODO(roeyc): Add additional rules to allow IPV6 NDP.
-        dhcp_client = firewall.get_nsservice(firewall.L4_PORT_SET_NSSERVICE,
-                                             l4_protocol=firewall.UDP,
-                                             source_ports=[67],
-                                             destination_ports=[68])
-        dhcp_client_rule_in = firewall.get_firewall_rule_dict(
-            'DHCP Reply', direction=firewall.IN, service=dhcp_client)
 
-        dhcp_server = (
+    block_rule = firewall.get_firewall_rule_dict(
-            firewall.get_nsservice(firewall.L4_PORT_SET_NSSERVICE,
+        'Block All', action=firewall.DROP,
-                                   l4_protocol=firewall.UDP,
+        logged=cfg.CONF.nsx_v3.log_security_groups_blocked_traffic)
-                                   source_ports=[68],
+    # TODO(roeyc): Add additional rules to allow IPV6 NDP.
-                                   destination_ports=[67]))
+    dhcp_client = firewall.get_nsservice(firewall.L4_PORT_SET_NSSERVICE,
-        dhcp_client_rule_out = firewall.get_firewall_rule_dict(
+                                         l4_protocol=firewall.UDP,
-            'DHCP Request', direction=firewall.OUT, service=dhcp_server)
+                                         source_ports=[67],
+                                         destination_ports=[68])
+    dhcp_client_rule_in = firewall.get_firewall_rule_dict(
+        'DHCP Reply', direction=firewall.IN, service=dhcp_client)
 
-        firewall.add_rules_in_section([dhcp_client_rule_out,
+    dhcp_server = (
-                                       dhcp_client_rule_in,
+        firewall.get_nsservice(firewall.L4_PORT_SET_NSSERVICE,
-                                       block_rule],
+                               l4_protocol=firewall.UDP,
-                                      section['id'])
+                               source_ports=[68],
+                               destination_ports=[67]))
+    dhcp_client_rule_out = firewall.get_firewall_rule_dict(
+        'DHCP Request', direction=firewall.OUT, service=dhcp_server)
+
+    firewall.update_section(section['id'],
+                            name, section['description'],
+                            applied_tos=nested_groups,
+                            rules=[dhcp_client_rule_out,
+                                   dhcp_client_rule_in,
+                                   block_rule])
     return section['id']
 
 

vmware_nsx/plugins/nsx_v/plugin.py

@@ -71,6 +71,7 @@ from vmware_nsx.db import (
 from vmware_nsx.db import (
     routertype as rt_rtr)
 from vmware_nsx.db import db as nsx_db
+from vmware_nsx.db import extended_security_group as extended_secgroup
 from vmware_nsx.db import nsxv_db
 from vmware_nsx.db import vnic_index_db
 from vmware_nsx.extensions import (
@@ -80,6 +81,7 @@ from vmware_nsx.extensions import (
 from vmware_nsx.extensions import dns_search_domain as ext_dns_search_domain
 from vmware_nsx.extensions import routersize
 from vmware_nsx.extensions import secgroup_rule_local_ip_prefix as ext_loip
+from vmware_nsx.extensions import securitygrouplogging as sg_logging
 from vmware_nsx.plugins.nsx_v import managers
 from vmware_nsx.plugins.nsx_v import md_proxy as nsx_v_md_proxy
 from vmware_nsx.plugins.nsx_v.vshield.common import (
@@ -105,6 +107,7 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                    portsecurity_db.PortSecurityDbMixin,
                    extend_sg_rule.ExtendedSecurityGroupRuleMixin,
                    securitygroups_db.SecurityGroupDbMixin,
+                   extended_secgroup.ExtendedSecurityGroupPropertiesMixin,
                    vnic_index_db.VnicIndexDbMixin,
                    dns_db.DNSDbMixin):
 
@@ -294,17 +297,17 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
             context = n_context.get_admin_context()
             log_all_rules = cfg.CONF.nsxv.log_security_groups_allowed_traffic
 
-            for sg in self.get_security_groups(context, fields=['id']):
+            # If the section/sg is already logged, then no action is
-                fw_section = self._get_section(context.session, sg['id'])
+            # required.
-                # If the section/sg is already logged, then no action is
+            for sg in [sg for sg in self.get_security_groups(context)
-                # required.
+                       if sg[sg_logging.LOGGING] is False]:
-                if fw_section is None or fw_section['logging']:
+                section_uri = self._get_section_uri(context.session, sg['id'])
+                if section_uri is None:
                     continue
 
                 # Section/sg is not logged, update rules logging according to
                 # the 'log_security_groups_allowed_traffic' config option.
                 try:
-                    section_uri = fw_section['ip_section_id']
                     h, c = self.nsx_v.vcns.get_section(section_uri)
                     section = self.nsx_sg_utils.parse_section(c)
                     section_needs_update = (
@@ -1962,22 +1965,6 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
         if mapping is not None:
             return mapping['ip_section_id']
 
-    def _get_section(self, session, security_group_id):
-        return nsxv_db.get_nsx_section(session, security_group_id)
-
-    def _update_section_logging(self, session, section, section_db):
-        logging = not section_db['logging']
-        # Update the DB for the new logging settings.
-        with session.begin(subtransactions=True):
-            section_db['logging'] = logging
-        # Update section rules logging only if we are not already logging them.
-        log_all_rules = cfg.CONF.nsxv.log_security_groups_allowed_traffic
-        section_needs_update = False
-        if not log_all_rules:
-            section_needs_update = (
-                self.nsx_sg_utils.set_rules_logged_option(section, logging))
-        return section_needs_update
-
     def create_security_group(self, context, security_group,
                               default_sg=False):
         """Create a security group."""
@@ -2001,7 +1988,7 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
             h, nsx_sg_id = self.nsx_v.vcns.create_security_group(sg_dict)
 
             section_name = self.nsx_sg_utils.get_nsx_section_name(nsx_sg_name)
-            logging = sg_data.get('logging', False)
+            logging = sg_data.get(sg_logging.LOGGING, False)
             nsx_rules = []
             for rule in new_security_group['security_group_rules']:
                 nsx_rule = self._create_nsx_rule(
@@ -2022,7 +2009,9 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                 context.session, sg_id, nsx_sg_id)
             # Add database associations for fw section and rules
             nsxv_db.add_neutron_nsx_section_mapping(
-                context.session, sg_id, section_uri, logging)
+                context.session, sg_id, section_uri)
+            self._process_security_group_properties_create(
+                context, new_security_group, sg_data)
             for pair in rule_pairs:
                 # Save nsx rule id in the DB for future access
                 nsxv_db.add_neutron_nsx_rule_mapping(context.session,
@@ -2042,15 +2031,12 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                 self._delete_section(section_uri)
                 self._delete_nsx_security_group(nsx_sg_id)
                 LOG.exception(_LE('Failed to create security group'))
-        if context.is_admin:
-            new_security_group['logging'] = logging
         return new_security_group
 
     def update_security_group(self, context, id, security_group):
         s = security_group['security_group']
         nsx_sg_id = nsx_db.get_nsx_security_group_id(context.session, id)
-        section_db = self._get_section(context.session, id)
+        section_uri = self._get_section_uri(context.session, id)
-        section_uri = section_db['ip_section_id']
         section_needs_update = False
 
         sg_data = super(NsxVPluginV2, self).update_security_group(
@@ -2068,15 +2054,14 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
             section.attrib['name'] = section_name
             section_needs_update = True
         # Update the dfw section if security-group logging option has changed.
-        # TBD: enforce admin only?
+        log_all_rules = cfg.CONF.nsxv.log_security_groups_allowed_traffic
-        if 'logging' in s and s['logging'] != section_db['logging']:
+        self._process_security_group_properties_update(context, sg_data, s)
-            section_needs_update = self._update_section_logging(
+        if not log_all_rules and context.is_admin:
-                context.session, section, section_db)
+            section_needs_update |= self.nsx_sg_utils.set_rules_logged_option(
+                section, sg_data[sg_logging.LOGGING])
         if section_needs_update:
             self.nsx_v.vcns.update_section(
                 section_uri, self.nsx_sg_utils.to_xml_string(section), h)
-        if context.is_admin:
-            sg_data['logging'] = section_db['logging']
         return sg_data
 
     def delete_security_group(self, context, id):
@@ -2183,10 +2168,9 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
 
         self._validate_security_group_rules(context, security_group_rules)
 
-        # Fetching the the dfw section associated with the security-group
+        # Querying DB for associated dfw section id
-        section_db = self._get_section(context.session, sg_id)
+        section_uri = self._get_section_uri(context.session, sg_id)
-        section_uri = section_db['ip_section_id']
+        logging = self._is_security_group_logged(context, sg_id)
-        logging = section_db['logging']
         log_all_rules = cfg.CONF.nsxv.log_security_groups_allowed_traffic
 
         # Translating Neutron rules to Nsx DFW rules

vmware_nsx/plugins/nsx_v3/plugin.py

@@ -26,6 +26,7 @@ from neutron.callbacks import resources
 from neutron.common import rpc as n_rpc
 from neutron.common import topics
 from neutron.common import utils as neutron_utils
+from neutron import context as q_context
 from neutron.db import agents_db
 from neutron.db import agentschedulers_db
 from neutron.db import allowedaddresspairs_db as addr_pair_db
@@ -67,7 +68,9 @@ from vmware_nsx.common import locking
 from vmware_nsx.common import nsx_constants
 from vmware_nsx.common import utils
 from vmware_nsx.db import db as nsx_db
+from vmware_nsx.db import extended_security_group
 from vmware_nsx.dhcp_meta import rpc as nsx_rpc
+from vmware_nsx.extensions import securitygrouplogging as sg_logging
 from vmware_nsx.nsxlib import v3 as nsxlib
 from vmware_nsx.nsxlib.v3 import client as nsx_client
 from vmware_nsx.nsxlib.v3 import cluster as nsx_cluster
@@ -86,6 +89,7 @@ NSX_V3_DHCP_PROFILE_NAME = 'neutron_port_dhcp_profile'
 class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
                   db_base_plugin_v2.NeutronDbPluginV2,
                   securitygroups_db.SecurityGroupDbMixin,
+                  extended_security_group.ExtendedSecurityGroupPropertiesMixin,
                   external_net_db.External_net_db_mixin,
                   extraroute_db.ExtraRoute_db_mixin,
                   l3_gwmode_db.L3_NAT_db_mixin,
@@ -114,7 +118,8 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
                                    "router",
                                    "availability_zone",
                                    "network_availability_zone",
-                                   "subnet_allocation"]
+                                   "subnet_allocation",
+                                   "security-group-logging"]
 
     @resource_registry.tracked_resources(
         network=models_v2.Network,
@@ -141,6 +146,7 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
         self._port_client = nsx_resources.LogicalPort(self._nsx_client)
         self.nsgroup_manager, self.default_section = (
             self._init_nsgroup_manager_and_default_section_rules())
+        self._process_security_group_logging()
         self._router_client = nsx_resources.LogicalRouter(self._nsx_client)
         self._router_port_client = nsx_resources.LogicalRouterPort(
             self._nsx_client)
@@ -262,6 +268,21 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
                 tags=utils.build_v3_api_version_tag())
         return self._get_port_security_profile()
 
+    def _process_security_group_logging(self):
+        context = q_context.get_admin_context()
+        log_all_rules = cfg.CONF.nsx_v3.log_security_groups_allowed_traffic
+        secgroups = self.get_security_groups(context,
+                                             fields=['id', sg_logging.LOGGING])
+        for sg in [sg for sg in secgroups if sg[sg_logging.LOGGING] is False]:
+            _, section_id = security.get_sg_mappings(context.session, sg['id'])
+            try:
+                security.set_firewall_rule_logging_for_section(
+                    section_id, logging=log_all_rules)
+            except nsx_exc.ManagerError:
+                with excutils.save_and_reraise_exception():
+                    LOG.error(_LE("Failed to update firewall rule logging for "
+                                  "rule in section %s"), section_id)
+
     def _init_nsgroup_manager_and_default_section_rules(self):
         with locking.LockManager.get_lock('nsxv3_nsgroup_manager_init'):
             return security.init_nsgroup_manager_and_default_section_rules()
@@ -1701,7 +1722,6 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
         if not default_sg:
             tenant_id = secgroup['tenant_id']
             self._ensure_default_security_group(context, tenant_id)
-
         try:
             # NOTE(roeyc): We first create the nsgroup so that once the sg is
             # saved into db its already backed up by an nsx resource.
@@ -1726,6 +1746,10 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
                                           secgroup_db['id'],
                                           ns_group['id'],
                                           firewall_section['id'])
+
+                self._process_security_group_properties_create(context,
+                                                               secgroup_db,
+                                                               secgroup)
         except nsx_exc.ManagerError:
             with excutils.save_and_reraise_exception():
                 LOG.exception(_LE("Unable to create security-group on the "
@@ -1747,10 +1771,13 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
         try:
             sg_rules = secgroup_db['security_group_rules']
             # translate and creates firewall rules.
-            rules = security.create_firewall_rules(
-                context, firewall_section['id'], ns_group['id'], sg_rules)
-            security.save_sg_rule_mappings(context.session, rules['rules'])
 
+            logging = (cfg.CONF.nsx_v3.log_security_groups_allowed_traffic or
+                       secgroup.get(sg_logging.LOGGING, False))
+            rules = security.create_firewall_rules(
+                context, firewall_section['id'], ns_group['id'],
+                logging, sg_rules)
+            security.save_sg_rule_mappings(context.session, rules['rules'])
             self.nsgroup_manager.add_nsgroup(ns_group['id'])
         except nsx_exc.ManagerError:
             with excutils.save_and_reraise_exception():
@@ -1768,26 +1795,25 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
         return secgroup_db
 
     def update_security_group(self, context, id, security_group):
-        nsgroup_id, section_id = security.get_sg_mappings(context.session, id)
+        orig_secgroup = self.get_security_group(
-        original_security_group = self.get_security_group(
             context, id, fields=['id', 'name', 'description'])
-        updated_security_group = (
+        with context.session.begin(subtransactions=True):
-            super(NsxV3Plugin, self).update_security_group(context, id,
+            secgroup_res = (
-                                                           security_group))
+                super(NsxV3Plugin, self).update_security_group(context, id,
-        name = security.get_nsgroup_name(updated_security_group)
+                                                               security_group))
-        description = updated_security_group['description']
+            self._process_security_group_properties_update(
+                context, secgroup_res, security_group['security_group'])
         try:
-            firewall.update_nsgroup(nsgroup_id, name, description)
+            security.update_security_group_on_backend(context, secgroup_res)
-            firewall.update_section(section_id, name, description)
         except nsx_exc.ManagerError:
             with excutils.save_and_reraise_exception():
                 LOG.exception(_LE("Failed to update security-group %(name)s "
                                   "(%(id)s), rolling back changes in "
-                                  "Neutron."), original_security_group)
+                                  "Neutron."), orig_secgroup)
                 super(NsxV3Plugin, self).update_security_group(
-                    context, id, {'security_group': original_security_group})
+                    context, id, {'security_group': orig_secgroup})
 
-        return updated_security_group
+        return secgroup_res
 
     def delete_security_group(self, context, id):
         nsgroup_id, section_id = security.get_sg_mappings(context.session, id)
@@ -1807,9 +1833,12 @@ class NsxV3Plugin(addr_pair_db.AllowedAddressPairsMixin,
         sg_id = security_group_rules_db[0]['security_group_id']
         nsgroup_id, section_id = security.get_sg_mappings(context.session,
                                                           sg_id)
+        logging_enabled = (cfg.CONF.nsx_v3.log_security_groups_allowed_traffic
+                           or self._is_security_group_logged(context, sg_id))
         try:
-            rules = security.create_firewall_rules(
+            rules = security.create_firewall_rules(context, section_id,
-                context, section_id, nsgroup_id, security_group_rules_db)
+                                                   nsgroup_id, logging_enabled,
+                                                   security_group_rules_db)
         except nsx_exc.ManagerError:
             with excutils.save_and_reraise_exception():
                 for rule in security_group_rules_db:

vmware_nsx/tests/unit/nsx_v/test_plugin.py

@@ -2558,11 +2558,6 @@ class NsxVTestSecurityGroup(ext_sg.TestSecurityGroups,
         sg = self._plugin_update_security_group(_context, sg['id'], True)
         self.assertTrue(sg['logging'])
 
-    def test_security_group_logging_not_visible_for_user(self):
-        _context = context.Context('user', 'tenant_id')
-        sg = self._plugin_create_security_group(_context)
-        self.assertFalse('logging' in sg)
-
 
 class TestVdrTestCase(L3NatTest, L3NatTestCaseBase,
                       test_l3_plugin.L3NatDBIntTestCase,