Skip to content

Closes #20972: Add support for channelized subinterfaces#22647

Open
jeremystretch wants to merge 1 commit into
featurefrom
20972-channelized-subinterfaces
Open

Closes #20972: Add support for channelized subinterfaces#22647
jeremystretch wants to merge 1 commit into
featurefrom
20972-channelized-subinterfaces

Conversation

@jeremystretch

@jeremystretch jeremystretch commented Jul 8, 2026

Copy link
Copy Markdown
Member

Closes: #20972

  • New model fields:
    • Add channels and channel_id to Interface.
    • Add parent, channels, and channel_id to InterfaceTemplate.
  • Add a unique constraint for (parent, channel_id) to Interface and InterfaceTemplate.
  • Override set_cable_termination() and clear_cable_termination() on the Interface model to replicate cable assignment to channelized subinterfaces.
  • Adapt the cable tracing logic & rendering to support paths terminated to subinterfaces for a cable connected to the physical parent interface.

@jeremystretch
jeremystretch force-pushed the 20972-channelized-subinterfaces branch 5 times, most recently from 07ff11d to feab725 Compare July 15, 2026 16:59
@jeremystretch

Copy link
Copy Markdown
Member Author

@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration.

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jeremystretch's task in 5m 22s —— View job


PR Review: Channelized Subinterfaces

  • Gathering context and reading diff
  • Analyzing for issues of concern
  • Posting review

Issues of Concern

1. No validation that cable profile capacity covers all channel IDs

netbox/dcim/models/device_components.py:1259-1266 / netbox/dcim/cable_profiles.py:28-52

propagate_channel_cables() assigns each child cable_positions=[child.channel_id]. If the parent has more channels than the cable profile has positions (e.g. 8-channel parent on a 1C4P cable), channels 5–8 get position values that have no corresponding CableTermination. get_peer_termination() then returns None silently — the path tracer creates incomplete paths with no user-facing error or warning.

CableProfile.clean() validates the number of connectors (terminations), not positions per connector, so it doesn't catch this mismatch. A channelized interface can be cabled with a profile whose position count is lower than its channels value, and the resulting dead-end paths are invisible to the user.

Suggestion: validate in propagate_channel_cables() (or somewhere in Cable.clean() / CableTermination.save()) that parent.channels <= profile_position_count_for_the_relevant_connector. Fix this →


2. API test coverage gap for new fields

netbox/dcim/tests/test_api.py

channels and channel_id are added to the Interface and InterfaceTemplate serializers, but the existing test_api.py has no test data or assertions for these fields. The standard NetBox API test pattern exercises create/update/bulk operations on all serializer fields; the absence of these fields from the test fixtures means their serialization, filtering, and validation aren't exercised by CI.


3. N+1 queries in signal handler update_channelized_cable_paths

netbox/dcim/signals.py:202-205

for parent in Interface.objects.filter(pk__in=parent_ids, cable__isnull=False):
    if parent.channels:
        parent.propagate_channel_cables()
    rebuild_cable_paths(parent.cable)

propagate_channel_cables() accesses self.cable and self.cable.profile (line 1253). Because the query above has no select_related('cable__profile'), each iteration issues two additional queries (one to fetch the Cable, one for its profile). For a parent with a large number of simultaneously-moving channel subinterfaces this is minor, but adding .select_related('cable__profile') to the filter on line 202 would eliminate the extra round-trips.


4. Intermediate invalid DB state during device/module instantiation

netbox/dcim/utils.py:169-184 / netbox/dcim/models/device_component_templates.py:569-582

InterfaceTemplate.instantiate() passes channel_id=self.channel_id when building the Interface object. That interface is saved immediately (without a parent, since parents are wired in the second pass by update_interface_parents()). Because save() does not call full_clean(), the DB briefly contains channel-type interfaces with channel_id set but parent=NULL — a state that fails model validation. If an exception interrupts the process between the first save and update_interface_parents(), those orphaned interfaces remain.

This is a low-risk concern since both passes should be wrapped in a transaction, but the intermediate state bypasses the invariant enforced by InterfaceValidationMixin.clean(). One approach: defer saving channel subinterface templates until after their parents exist, following the two-pass order used for update_interface_bridges.

@jeremystretch

Copy link
Copy Markdown
Member Author
  1. I'm going to keep this as-is for now, as it can be argued that it's desirable behavior. We can re-evaluate once we get feedback from the v4.7 beta.
  2. Addressed
  3. Addressed (by adding selected_related('cable'))
  4. This seems like a narrow, acceptable risk. Going to leave it for now, as mitigating it would likely require a substantial change to the instantiation mechanism.

@jeremystretch
jeremystretch marked this pull request as ready for review July 15, 2026 17:42
@jeremystretch
jeremystretch requested review from a team and jnovinger and removed request for a team July 15, 2026 17:43
@jeremystretch
jeremystretch force-pushed the 20972-channelized-subinterfaces branch from feab725 to 26896da Compare July 16, 2026 13:51
@jeremystretch
jeremystretch requested review from bctiemann and removed request for jnovinger July 20, 2026 15:06
@jeremystretch
jeremystretch force-pushed the 20972-channelized-subinterfaces branch from 26896da to e1d0b38 Compare July 21, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant