review changes

docs/models/dcim/interface.md

@@ -125,7 +125,7 @@ The link aggregation mode of a LAG interface. (This field may be set only on int
 
 ### LAG Hash Policy
 
-The transmit hash policy used to distribute traffic across the members of a LAG interface: layer 2, layer 2+3, layer 3+4, encapsulated layer 2+3, or encapsulated layer 3+4. (This field may be set only on interfaces of type LAG.)
+The transmit hash policy used to distribute traffic across the members of a LAG interface: layer 2, layer 2+3, layer 3+4, encapsulated layer 2+3, or encapsulated layer 3+4. (This field may be set only on LAG interfaces whose mode is LACP, static, or balance-TLB, as other modes do not select members by hash.)
 
 ### PoE Mode
 

netbox/dcim/constants.py

@@ -2,7 +2,7 @@ import re
 
 from django.db.models import Q
 
-from .choices import InterfaceTypeChoices
+from .choices import InterfaceLAGModeChoices, InterfaceTypeChoices
 
 # Exclude SVG images (unsupported by PIL)
 DEVICETYPE_IMAGE_FORMATS = 'image/bmp,image/gif,image/jpeg,image/png,image/tiff,image/webp'
@@ -84,6 +84,13 @@ NONCONNECTABLE_IFACE_TYPES = VIRTUAL_IFACE_TYPES + WIRELESS_IFACE_TYPES + [
     InterfaceTypeChoices.TYPE_CHANNEL,
 ]
 
+# LAG modes which select a member interface by transmit hash (Linux bonding xmit_hash_policy)
+LAG_HASH_MODES = (
+    InterfaceLAGModeChoices.MODE_LACP,
+    InterfaceLAGModeChoices.MODE_STATIC,
+    InterfaceLAGModeChoices.MODE_BALANCE_TLB,
+)
+
 
 #
 # MAC blocks

netbox/dcim/forms/bulk_edit.py

@@ -2038,8 +2038,8 @@ class InterfaceBulkEditForm(
         FieldSet('vrf', 'wwn', name=_('Addressing')),
         FieldSet('vdcs', 'mtu', 'tx_power', 'enabled', 'mgmt_only', 'mark_connected', name=_('Operation')),
         FieldSet('poe_mode', 'poe_type', name=_('PoE')),
-        FieldSet('parent', 'channel_id', 'bridge', 'lag', name=_('Related Interfaces')),
         FieldSet('lag_mode', 'lag_hash_policy', name=_('LAG')),
+        FieldSet('parent', 'channel_id', 'bridge', 'lag', name=_('Related Interfaces')),
         FieldSet(
             'mode', 'vlan_group', 'untagged_vlan', 'qinq_svlan', 'vlan_translation_policy', name=_('802.1Q Switching')
         ),

netbox/dcim/forms/filtersets.py

@@ -2018,8 +2018,8 @@ class InterfaceFilterForm(PathEndpointFilterForm, DeviceComponentFilterForm):
             name=_('Attributes')
         ),
         FieldSet('vrf_id', 'l2vpn_id', 'mac_address', 'wwn', name=_('Addressing')),
-        FieldSet('lag_mode', 'lag_hash_policy', name=_('LAG')),
         FieldSet('poe_mode', 'poe_type', name=_('PoE')),
+        FieldSet('lag_mode', 'lag_hash_policy', name=_('LAG')),
         FieldSet('mode', 'vlan_translation_policy_id', name=_('802.1Q Switching')),
         FieldSet('rf_role', 'rf_channel', 'rf_channel_width', 'tx_power', name=_('Wireless')),
         FieldSet('region_id', 'site_group_id', 'site_id', 'location_id', 'rack_id', name=_('Location')),

netbox/dcim/forms/model_forms.py

@@ -2057,6 +2057,7 @@ class InterfaceForm(InterfaceCommonForm, ModularDeviceComponentForm):
         label=_('LAG mode'),
         choices=add_blank_choice(InterfaceLAGModeChoices),
         required=False,
+        widget=HTMXSelect(hx_target_id='lag-attributes'),
     )
     lag_hash_policy = TypedChoiceField(
         label=_('LAG hash policy'),
@@ -2187,8 +2188,8 @@ class InterfaceForm(InterfaceCommonForm, ModularDeviceComponentForm):
         FieldSet('vrf', 'mac_address', 'wwn', name=_('Addressing')),
         FieldSet('vdcs', 'mtu', 'tx_power', 'enabled', 'mgmt_only', 'mark_connected', name=_('Operation')),
         FieldSet('parent', 'channel_id', 'bridge', 'lag', name=_('Related Interfaces')),
-        FieldSet('lag_mode', 'lag_hash_policy', name=_('LAG')),
         FieldSet('poe_mode', 'poe_type', name=_('PoE')),
+        FieldSet('lag_mode', 'lag_hash_policy', name=_('LAG'), html_id='lag-attributes'),
         FieldSet(
             'mode', 'vlan_group', 'untagged_vlan', 'tagged_vlans', 'qinq_svlan', 'vlan_translation_policy',
             name=_('802.1Q Switching'),
@@ -2215,6 +2216,20 @@ class InterfaceForm(InterfaceCommonForm, ModularDeviceComponentForm):
             ),
         }
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        # A hash policy applies only to LAG modes which hash
+        if get_field_value(self, 'lag_mode') not in LAG_HASH_MODES:
+            del self.fields['lag_hash_policy']
+
+    def clean(self):
+        super().clean()
+
+        # Clear a hash policy left over from a previous LAG mode
+        if 'lag_hash_policy' not in self.fields:
+            self.instance.lag_hash_policy = None
+
 
 class FrontPortForm(FrontPortFormMixin, ModularDeviceComponentForm):
     type = ChoiceField(

netbox/dcim/graphql/filters.py

@@ -603,6 +603,12 @@ class InterfaceFilter(
     poe_type: BaseFilterLookup[Annotated['InterfacePoETypeEnum', strawberry.lazy('dcim.graphql.enums')]] | None = (
         strawberry_django.filter_field()
     )
+    lag_mode: BaseFilterLookup[Annotated['InterfaceLAGModeEnum', strawberry.lazy('dcim.graphql.enums')]] | None = (
+        strawberry_django.filter_field()
+    )
+    lag_hash_policy: (
+        BaseFilterLookup[Annotated['InterfaceLAGHashPolicyEnum', strawberry.lazy('dcim.graphql.enums')]] | None
+    ) = strawberry_django.filter_field()
     wireless_link: Annotated['WirelessLinkFilter', strawberry.lazy('wireless.graphql.filters')] | None = (
         strawberry_django.filter_field()
     )
@@ -685,12 +691,6 @@ class InterfaceTemplateFilter(ModularComponentTemplateFilterMixin, ChangeLoggedM
     poe_type: BaseFilterLookup[Annotated['InterfacePoETypeEnum', strawberry.lazy('dcim.graphql.enums')]] | None = (
         strawberry_django.filter_field()
     )
-    lag_mode: BaseFilterLookup[Annotated['InterfaceLAGModeEnum', strawberry.lazy('dcim.graphql.enums')]] | None = (
-        strawberry_django.filter_field()
-    )
-    lag_hash_policy: (
-        BaseFilterLookup[Annotated['InterfaceLAGHashPolicyEnum', strawberry.lazy('dcim.graphql.enums')]] | None
-    ) = strawberry_django.filter_field()
     rf_role: BaseFilterLookup[Annotated['WirelessRoleEnum', strawberry.lazy('wireless.graphql.enums')]] | None = (
         strawberry_django.filter_field()
     )

netbox/dcim/models/device_components.py

@@ -1326,6 +1326,10 @@ class Interface(
             raise ValidationError({'lag_mode': _("LAG mode may be set only on LAG interfaces.")})
         if self.lag_hash_policy and not self.is_lag:
             raise ValidationError({'lag_hash_policy': _("LAG hash policy may be set only on LAG interfaces.")})
+        if self.lag_hash_policy and self.lag_mode not in LAG_HASH_MODES:
+            raise ValidationError({
+                'lag_hash_policy': _("LAG hash policy may be set only for LACP, static, or balance-TLB LAG modes."),
+            })
 
         # Wireless validation
 

netbox/dcim/tests/test_api.py

@@ -4776,6 +4776,39 @@ class InterfaceTestCase(Mixins.FieldPrefetchMixin, Mixins.ComponentTraceMixin, A
         ):
             self._perform_interface_test_with_invalid_data(None, {field: value})
 
+    def test_lag_hash_policy_without_hash_mode(self):
+        self.add_permissions('dcim.add_interface')
+        data = {
+            'device': Device.objects.first().pk,
+            'name': 'bond0',
+            'type': InterfaceTypeChoices.TYPE_LAG,
+            'lag_mode': InterfaceLAGModeChoices.MODE_ACTIVE_BACKUP,
+            'lag_hash_policy': InterfaceLAGHashPolicyChoices.POLICY_LAYER3_4,
+        }
+
+        response = self.client.post(self._get_list_url(), data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+        self.assertIn('lag_hash_policy', response.json())
+
+    def test_graphql_filter_lag_attributes(self):
+        self.add_permissions('dcim.view_interface')
+        lag = Interface.objects.create(
+            device=Device.objects.get(name='Device 1'),
+            name='LAG 1',
+            type=InterfaceTypeChoices.TYPE_LAG,
+            lag_mode=InterfaceLAGModeChoices.MODE_LACP,
+            lag_hash_policy=InterfaceLAGHashPolicyChoices.POLICY_LAYER3_4,
+        )
+
+        for filters in ('lag_mode: {exact: MODE_LACP}', 'lag_hash_policy: {exact: POLICY_LAYER3_4}'):
+            with self.subTest(filters=filters):
+                query = f'{{ interface_list(filters: {{ {filters} }}) {{ id }} }}'
+                response = self.client.post(reverse('graphql'), data={'query': query}, format='json', **self.header)
+                self.assertHttpStatus(response, status.HTTP_200_OK)
+                data = response.json()
+                self.assertNotIn('errors', data)
+                self.assertEqual([row['id'] for row in data['data']['interface_list']], [str(lag.pk)])
+
     def test_list_objects_reuse_peer_serializers(self):
         """Link peers and connected endpoints share one nested serializer each per request."""
         self.add_permissions('dcim.view_interface')

netbox/dcim/tests/test_forms.py

@@ -10,6 +10,8 @@ from dcim.choices import (
     CableProfileChoices,
     DeviceFaceChoices,
     DeviceStatusChoices,
+    InterfaceLAGHashPolicyChoices,
+    InterfaceLAGModeChoices,
     InterfaceModeChoices,
     InterfaceTypeChoices,
     LinkStatusChoices,
@@ -1299,6 +1301,33 @@ class InterfaceTestCase(TestCase):
         self.assertNotIn('tagged_vlans', form.cleaned_data.keys())
         self.assertNotIn('qinq_svlan', form.cleaned_data.keys())
 
+    def test_lag_hash_policy_hidden_for_non_hash_mode(self):
+        """A LAG hash policy is offered only for hashing LAG modes, and is cleared when switching away from one."""
+        lag = Interface.objects.create(
+            device=self.device,
+            name='bond0',
+            type=InterfaceTypeChoices.TYPE_LAG,
+            lag_mode=InterfaceLAGModeChoices.MODE_LACP,
+            lag_hash_policy=InterfaceLAGHashPolicyChoices.POLICY_LAYER3_4,
+        )
+        self.assertIn('lag_hash_policy', InterfaceForm(instance=lag).fields)
+
+        data = {
+            'device': self.device.pk,
+            'name': 'bond0',
+            'type': InterfaceTypeChoices.TYPE_LAG,
+            'lag_mode': InterfaceLAGModeChoices.MODE_ACTIVE_BACKUP,
+            'lag_hash_policy': InterfaceLAGHashPolicyChoices.POLICY_LAYER3_4,
+        }
+        form = InterfaceForm(data, instance=lag)
+        self.assertNotIn('lag_hash_policy', form.fields)
+        self.assertTrue(form.is_valid(), form.errors)
+        form.save()
+
+        lag.refresh_from_db()
+        self.assertEqual(lag.lag_mode, InterfaceLAGModeChoices.MODE_ACTIVE_BACKUP)
+        self.assertIsNone(lag.lag_hash_policy)
+
     def test_child_interface_preselects_every_parent_vdc(self):
         """A child interface's form pre-selects every virtual device context of its parent."""
         vdcs = (

netbox/dcim/tests/test_models.py

@@ -3765,6 +3765,32 @@ class InterfaceLAGAttributesTestCase(TestCase):
             interface.full_clean()
         self.assertIn('lag_hash_policy', cm.exception.message_dict)
 
+    def test_lag_hash_policy_lag_mode(self):
+        for lag_mode, valid in (
+            (None, False),
+            (InterfaceLAGModeChoices.MODE_LACP, True),
+            (InterfaceLAGModeChoices.MODE_STATIC, True),
+            (InterfaceLAGModeChoices.MODE_BALANCE_TLB, True),
+            (InterfaceLAGModeChoices.MODE_ACTIVE_BACKUP, False),
+            (InterfaceLAGModeChoices.MODE_BALANCE_RR, False),
+            (InterfaceLAGModeChoices.MODE_BALANCE_ALB, False),
+            (InterfaceLAGModeChoices.MODE_BROADCAST, False),
+        ):
+            with self.subTest(lag_mode=lag_mode):
+                interface = Interface(
+                    device=self.device,
+                    name='bond0',
+                    type=InterfaceTypeChoices.TYPE_LAG,
+                    lag_mode=lag_mode,
+                    lag_hash_policy=InterfaceLAGHashPolicyChoices.POLICY_LAYER3_4,
+                )
+                if valid:
+                    interface.full_clean()
+                else:
+                    with self.assertRaises(ValidationError) as cm:
+                        interface.full_clean()
+                    self.assertIn('lag_hash_policy', cm.exception.message_dict)
+
 
 class VirtualDeviceContextTestCase(TestCase):