NAS-128574 / 24.10 / Allow HeaderDigest and DataDigest to be configured for iSCSI targets

src/middlewared/middlewared/alembic/versions/24.10/2024-04-25_19-46_iscsi_target_options.py

@@ -0,0 +1,26 @@
+"""Add iSCSI target options
+
+Revision ID: fc912643baa2
+Revises: 4f11cc19bb9c
+Create Date: 2024-04-25 19:46:37.356476+00:00
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'fc912643baa2'
+down_revision = '4f11cc19bb9c'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    with op.batch_alter_table('services_iscsitarget', schema=None) as batch_op:
+        batch_op.add_column(sa.Column('iscsi_target_options', sa.TEXT(), nullable=True))
+
+
+def downgrade():
+    with op.batch_alter_table('services_iscsitarget', schema=None) as batch_op:
+        batch_op.drop_column('iscsi_target_options')

src/middlewared/middlewared/etc_files/scst.conf.mako

@@ -212,6 +212,11 @@
             if set(devices).issubset(clustered_extents):
                 return True
         return False
+
+    def option_value(v):
+        if type(v) is bool:
+            return "Yes" if v else "No"
+        return v
 %>\
 ##
 ## If we are on a HA system then write out a cluster name, we'll hard-code
@@ -415,6 +420,12 @@ TARGET_DRIVER iscsi {
 %   if mutual_chap:
         OutgoingUser "${mutual_chap}"
 %   endif
+##
+## Add target options
+##
+% for k,v in target.get('options', {}).items():
+        ${k} ${option_value(v)}
+% endfor
 
         GROUP security_group {
 %   for access_control in initiator_portal_access:

src/middlewared/middlewared/plugins/iscsi_/scst.py

@@ -103,3 +103,25 @@ def set_node_optimized(self, node):
         else:
             pathlib.Path(SCST_CONTROLLER_A_TARGET_GROUPS_STATE).write_text("nonoptimized\n")
             pathlib.Path(SCST_CONTROLLER_B_TARGET_GROUPS_STATE).write_text("active\n")
+
+    def reset_target_parameter(self, iqn, param):
+        """Reset the specified parameter to its default value."""
+        # Do some sanity checking
+        valid_parm_names = [
+            'DataDigest',
+            'HeaderDigest',
+            'InitialR2T',
+            'ImmediateData',
+            'MaxRecvDataSegmentLength',
+            'MaxXmitDataSegmentLength',
+            'MaxBurstLength',
+            'FirstBurstLength',
+            'MaxOutstandingR2T'
+        ]
+        if param not in valid_parm_names:
+            raise ValueError('Invalid parameter name supplied', param)
+        try:
+            return pathlib.Path(f'{SCST_BASE}/targets/iscsi/{iqn}/{param}').write_text(':default:\n')
+        except FileNotFoundError:
+            # If we're not running, that's OK
+            pass

src/middlewared/middlewared/plugins/iscsi_/targets.py

@@ -1,4 +1,5 @@
 import asyncio
+import enum
 import errno
 import os
 import pathlib
@@ -10,13 +11,41 @@
 from middlewared.schema import accepts, Bool, Dict, Int, IPAddr, List, Patch, Str
 from middlewared.service import CallError, CRUDService, private, ValidationErrors
 from middlewared.utils import UnexpectedFailure, run
+from middlewared.validators import Range
 from collections import defaultdict
 
 from .utils import AUTHMETHOD_LEGACY_MAP
 
 RE_TARGET_NAME = re.compile(r'^[-a-z0-9\.:]+$')
 
 
+class DigestEnum(str, enum.Enum):
+    NONE = 'None'
+    NONE_CRC32C = 'None,CRC32C'
+    CRC32C = 'CRC32C'
+
+    def choices():
+        return [x.value for x in DigestEnum]
+
+
+# These are taken from iSCSI rfc-3720 / rfc-7143
+MAX_RECV_DATA_SEGMENT_LENGTH_MIN = 512
+MAX_RECV_DATA_SEGMENT_LENGTH_MAX = 2**24 - 1
+MAX_BURST_LENGTH_MIN = 512
+MAX_BURST_LENGTH_MAX = 2**24 - 1
+FIRST_BURST_LENGTH_MIN = 512
+FIRST_BURST_LENGTH_MAX = 2**24 - 1
+MAX_OUTSTANDING_R2T_MIN = 1
+MAX_OUTSTANDING_R2T_MAX = 65535
+
+# MaxXmitDataSegmentLength is not documented in iSCSI RFCs, but
+# it is present in SCST and other iSCSI software (incl upstream
+# linux, where struct iscsi_conn_ops has a comment showing it
+# has the same range as for MaxRecvDataSegmentLength.)
+MAX_XMIT_DATA_SEGMENT_LENGTH_MIN = 512
+MAX_XMIT_DATA_SEGMENT_LENGTH_MAX = 2**24 - 1
+
+
 class iSCSITargetModel(sa.Model):
     __tablename__ = 'services_iscsitarget'
 
@@ -26,6 +55,7 @@ class iSCSITargetModel(sa.Model):
     iscsi_target_mode = sa.Column(sa.String(20), default='iscsi')
     iscsi_target_auth_networks = sa.Column(sa.JSON(list))
     iscsi_target_rel_tgt_id = sa.Column(sa.Integer(), unique=True)
+    iscsi_target_options = sa.Column(sa.JSON(), nullable=True)
 
 
 class iSCSITargetGroupModel(sa.Model):
@@ -97,6 +127,18 @@ async def extend(self, data):
             ),
         ]),
         List('auth_networks', items=[IPAddr('ip', network=True)]),
+        Dict(
+            'options',
+            Str('DataDigest', enum=DigestEnum.choices()),
+            Str('HeaderDigest', enum=DigestEnum.choices()),
+            Bool('InitialR2T'),
+            Bool('ImmediateData'),
+            Int('MaxRecvDataSegmentLength', validators=[Range(min_=MAX_RECV_DATA_SEGMENT_LENGTH_MIN, max_=MAX_RECV_DATA_SEGMENT_LENGTH_MAX)]),
+            Int('MaxXmitDataSegmentLength', validators=[Range(min_=MAX_XMIT_DATA_SEGMENT_LENGTH_MIN, max_=MAX_XMIT_DATA_SEGMENT_LENGTH_MAX)]),
+            Int('MaxBurstLength', validators=[Range(min_=MAX_BURST_LENGTH_MIN, max_=MAX_BURST_LENGTH_MAX)]),
+            Int('FirstBurstLength', validators=[Range(min_=FIRST_BURST_LENGTH_MIN, max_=FIRST_BURST_LENGTH_MAX)]),
+            Int('MaxOutstandingR2T', validators=[Range(min_=MAX_OUTSTANDING_R2T_MIN, max_=MAX_OUTSTANDING_R2T_MAX)]),
+        ),
         register=True
     ), audit='Create iSCSI target', audit_extended=lambda data: data["name"])
     async def do_create(self, data):
@@ -322,9 +364,23 @@ async def do_update(self, audit_callback, id_, data):
         await self._service_change('iscsitarget', 'reload', options={'ha_propagate': False})
 
         # Then process the BACKUP config if we are HA and ALUA is enabled.
-        if await self.middleware.call("iscsi.global.alua_enabled") and await self.middleware.call('failover.remote_connected'):
+        run_on_peer = await self.middleware.call("iscsi.global.alua_enabled") and await self.middleware.call('failover.remote_connected')
+        if run_on_peer:
             await self.middleware.call('failover.call_remote', 'service.reload', ['iscsitarget'])
 
+        # NOTE: Any options whose keys are omitted will be removed from the config i.e. we will deliberately revert removed items
+        # to SCST defaults
+        old_options = set(old.get('options', {}).keys())
+        if old_options:
+            removed_options = old_options - set(new.get('options', {}).keys())
+            if removed_options:
+                global_basename = (await self.middleware.call('iscsi.global.config'))['basename']
+                iqn = f'{global_basename}:{new["name"]}'
+                for param in removed_options:
+                    await self.middleware.call('iscsi.scst.reset_target_parameter', iqn, param)
+                    if run_on_peer:
+                        await self.middleware.call('failover.call_remote', 'iscsi.scst.reset_target_parameter', [iqn, param])
+
         return await self.get_instance(id_)
 
     @accepts(Int('id'),

tests/api2/test_261_iscsi_cmd.py

@@ -21,7 +21,7 @@
 
 from middlewared.service_exception import ValidationError, ValidationErrors
 from middlewared.test.integration.assets.pool import dataset, snapshot
-from middlewared.test.integration.utils import call
+from middlewared.test.integration.utils import call, ssh
 from auto_config import ha, hostname, isns_ip, pool_name
 from functions import SSH_TEST
 from protocols import (initiator_name_supported, iscsi_scsi_connection,
@@ -2728,6 +2728,142 @@ def test_target_sizes(ipaddr):
                                 test_target_sizes(controller2_ip)
 
 
+class TwoTargetFixture:
+    """Abstract two-target fixture class"""
+    # Keeping it separate for possible future reuse, although we can also just
+    # add additional tests under TestFixtureTargetOptions.
+
+    name1 = f"{target_name}x1"
+    name2 = f"{target_name}x2"
+    iqn1 = f'{basename}:{name1}'
+    iqn2 = f'{basename}:{name2}'
+
+    @pytest.fixture(scope='class')
+    def create_two_targets(self):
+        with initiator_portal() as config:
+            with configured_target(config, self.name1, 'VOLUME') as config1:
+                with configured_target(config, self.name2, 'FILE') as config2:
+                    yield config1, config2
+
+
+class TestFixtureTargetOptions(TwoTargetFixture):
+
+    int_options = ['MaxRecvDataSegmentLength', 'MaxXmitDataSegmentLength', 'MaxBurstLength', 'FirstBurstLength', 'MaxOutstandingR2T']
+    bool_options = ['InitialR2T', 'ImmediateData']
+
+    # mapped, meaning with some post-processing performed e.g. "No" -> False
+    scst_default_mapped_values = {
+        'DataDigest': 'None',
+        'HeaderDigest': 'None',
+        'InitialR2T': False,
+        'ImmediateData': True,
+        'MaxRecvDataSegmentLength': 1048576,
+        'MaxXmitDataSegmentLength': 1048576,
+        'MaxBurstLength': 1048576,
+        'FirstBurstLength': 65536,
+        'MaxOutstandingR2T': 32,
+    }
+
+    def read_target_value(self, iqn, name):
+        return ssh(f'head -1 /sys/kernel/scst_tgt/targets/iscsi/{iqn}/{name}').strip()
+
+    def read_mapped_target_value(self, iqn, optionname):
+        value = self.read_target_value(iqn, optionname)
+        if optionname in self.int_options:
+            return int(value)
+        elif optionname in self.bool_options:
+            if value.lower() == "yes":
+                return True
+            elif value.lower() == "no":
+                return False
+            raise ValueError("Unhandled boolean value", value)
+        else:
+            return value
+
+    def read_target_values(self, iqn):
+        values = {}
+        for k in self.scst_default_mapped_values:
+            values[k] = self.read_mapped_target_value(iqn, k)
+        return values
+
+    options_values = [
+        {'DataDigest': 'None,CRC32C'},
+        {'DataDigest': 'None,CRC32C', 'HeaderDigest': 'CRC32C'},
+        {'HeaderDigest': 'None,CRC32C'},
+        {'InitialR2T': True},
+        {'ImmediateData': False},
+        {'MaxRecvDataSegmentLength': 8192},
+        {'MaxXmitDataSegmentLength': 16384},
+        {'MaxRecvDataSegmentLength': 16384, 'MaxXmitDataSegmentLength': 8192},
+        {'MaxBurstLength': 32768},
+        {'FirstBurstLength': 49152},
+        {'MaxOutstandingR2T': 48},
+    ]
+
+    options_values_parms = [pytest.param({}, id='{} - Initial empty options')] + \
+        [pytest.param(options, id=str(options)) for options in options_values] + \
+        [pytest.param({}, id='{} - Final empty options')]
+
+    @pytest.mark.parametrize('options', options_values_parms)
+    def test_33_target_options(self, request, create_two_targets, options):
+        """Test some target options."""
+        depends(request, ["iscsi_cmd_00"], scope="session")
+
+        config1, config2 = create_two_targets
+        # For the time being libiscsi is limited, so cannot test DataDigest with it.
+        # We will at least check the middleware-2-scst linkage here, and can perform
+        # some additional manual testing (for now).
+
+        newdata = self.scst_default_mapped_values.copy()
+        newdata.update(options)
+
+        newalias = ''.join(random.choices(string.ascii_uppercase) + random.choices(string.ascii_lowercase + string.digits, k=10))
+        modify_target(config1['target']['id'], {'options': options})
+        modify_target(config2['target']['id'], {'alias': newalias})
+
+        assert newdata == self.read_target_values(self.iqn1)
+        assert self.scst_default_mapped_values == self.read_target_values(self.iqn2)
+        assert newalias == self.read_target_value(self.iqn2, 'alias')
+
+    def test_34_target_header_digest(self, request, create_two_targets):
+        # Currently, by default python-scsi uses "None,CRC32C", so what gets negociated
+        # depends on the target setting.
+        config1, _ = create_two_targets
+
+        # Check that both HeaderDigest and DataDigest are negociated as None by default
+        with iscsi_scsi_connection(ip, self.iqn1):
+            # Validate that the two sessions are reported correctly
+            data = get_iscsi_sessions([['target', '=', self.iqn1]], 1)[0]
+            assert data['header_digest'] is None, data
+            assert data['data_digest'] is None, data
+
+        # Check that when HeaderDigest is set to "None,CRC32C", CRC32C is negociated
+        modify_target(config1['target']['id'], {'options': {'HeaderDigest': 'None,CRC32C'}})
+        with iscsi_scsi_connection(ip, self.iqn1):
+            # Validate that the two sessions are reported correctly
+            data = get_iscsi_sessions([['target', '=', self.iqn1]], 1)[0]
+            assert data['header_digest'] == 'CRC32C', data
+            assert data['data_digest'] is None, data
+
+        # Check that when HeaderDigest is set to "CRC32C", CRC32C is negociated
+        modify_target(config1['target']['id'], {'options': {'HeaderDigest': 'CRC32C'}})
+        with iscsi_scsi_connection(ip, self.iqn1):
+            # Validate that the two sessions are reported correctly
+            data = get_iscsi_sessions([['target', '=', self.iqn1]], 1)[0]
+            assert data['header_digest'] == 'CRC32C', data
+            assert data['data_digest'] is None, data
+
+        # Check that when HeaderDigest is set to "None" and DataDigest is set to CRC32C,
+        # None is negociated for HeaderDigest
+        modify_target(config1['target']['id'], {'options': {'DataDigest': 'CRC32C'}})
+        with iscsi_scsi_connection(ip, self.iqn1):
+            # Validate that the two sessions are reported correctly
+            data = get_iscsi_sessions([['target', '=', self.iqn1]], 1)[0]
+            assert data['header_digest'] is None, data
+            # Currently libiscsi, cython-iscsi and python-scsi cannot drive DataDigest
+            # assert data['data_digest'] == 'CRC32C', data
+
+
 def test_99_teardown(request):
     # Disable iSCSI service
     depends(request, ["iscsi_cmd_00"])