yahoo-eng-team team mailing list archive

[Robert Kukura <1367391@xxxxxxxxxxxxxxxxxx>]

Public bug reported:

Support for distributed port bindings was added to ML2 in order to
enable the same DVR port to be bound simultaneously on multiple hosts.
This was implemented by:

* Adding a new ml2_dvr_port_bindings table similar to the ml2_port_bindings table, but with the host column as part of the primary key.
* Adding a new DvrPortContext class the overrides several functions in PortContext.
* Adding DVR-specific internal functions to Ml2Plugin, _process_dvr_port_binding and _commit_dvr_port_binding, that are modified copies of existing functions.
* In about 8 places, making code conditional on "port['device_owner'] == const.DEVICE_OWNER_DVR_INTERFACE" to handle DVR ports using the above models, classes and functions instead of the normal ones.

This duplication of schema and code adds significant technical debt to
the ML2 plugin implementation, requiring developers and reviewers to
evaluate for all changes whether they need to apply to both the normal
and DVR-specific copies. In addition, copied code is certain to diverge
over time, making the effort to keep the copies as synchronized as
possible become more and more difficult.

This unnecessary duplication of schema and code should be significantly
reduced or completely eliminated by treating a normal non-distributed
port as a special case of a distributed port that happens to only bind
on a single host.

The schema would be unified by replacing the existing ml2_port_bindings
and ml2_dvr_port_bindings tables with two new non-overlapping tables.
One would contain the port state that is the same for all hosts on which
the port binds, including the values of the binding:host,
binding:vnic_type, and binding:profile attributes. The other would
contain the port state that differs among host-specific bindings, such
as the binding:vif_type and binding:vif_details attribute values, and
the bound driver and segment (until these two move to a separate table
for hierarchical port binding).

Also, the basic idea of distributed port bindings is not specific to
DVR, and could be used for DHCP and other services, so the schema and
code could be made more generic as the distributed and normal schema and
code are unified.

** Affects: neutron
     Importance: High
     Assignee: Robert Kukura (rkukura)
         Status: New


** Tags: ml2

** Description changed:

  Support for distributed port bindings was added to ML2 in order to
  enable the same DVR port to be bound simultaneously on multiple hosts.
  This was implemented by:
  
  * Adding a new ml2_dvr_port_bindings table similar to the ml2_port_bindings table, but with the host column as part of the primary key.
  * Adding a new DvrPortContext class the overrides several functions in PortContext.
  * Adding DVR-specific internal functions to Ml2Plugin, _process_dvr_port_binding and _commit_dvr_port_binding, that are modified copies of existing functions.
  * In about 8 places, making code conditional on "port['device_owner'] == const.DEVICE_OWNER_DVR_INTERFACE" to handle DVR ports using the above models, classes and functions instead of the normal ones.
  
  This duplication of schema and code adds significant technical debt to
  the ML2 plugin implementation, requiring developers and reviewers to
  evaluate for all changes whether they need to apply to both the normal
  and DVR-specific copies. In addition, copied code is certain to diverge
- over time, making the the effort to keep the copies as synchronized as
+ over time, making the effort to keep the copies as synchronized as
  possible become more and more difficult.
  
  This unnecessary duplication of schema and code should be significantly
  reduced or completely eliminated by treating a normal non-distributed
  port as a special case of a distributed port that happens to only bind
  on a single host.
  
  The schema would be unified by replacing the existing ml2_port_bindings
  and ml2_dvr_port_bindings tables with two new tables. One would contain
  the port state that is the same for all hosts on which the port binds,
  including the values of the binding:host, binding:vnic_type, and
  binding:profile attributes. The other would contain the port state that
  differs among host-specific bindings, such as the binding:vif_type and
  binding:vif_details attribute values, and the bound driver and segment
  (until these two move to a separate table for hierarchical port
  binding).
  
  Also, the basic idea of distributed port bindings is not specific to
  DVR, and could be used for DHCP and other services, so the schema and
  code could be made more generic as the distributed and normal schema and
  code are unified.

** Description changed:

  Support for distributed port bindings was added to ML2 in order to
  enable the same DVR port to be bound simultaneously on multiple hosts.
  This was implemented by:
  
  * Adding a new ml2_dvr_port_bindings table similar to the ml2_port_bindings table, but with the host column as part of the primary key.
  * Adding a new DvrPortContext class the overrides several functions in PortContext.
  * Adding DVR-specific internal functions to Ml2Plugin, _process_dvr_port_binding and _commit_dvr_port_binding, that are modified copies of existing functions.
  * In about 8 places, making code conditional on "port['device_owner'] == const.DEVICE_OWNER_DVR_INTERFACE" to handle DVR ports using the above models, classes and functions instead of the normal ones.
  
  This duplication of schema and code adds significant technical debt to
  the ML2 plugin implementation, requiring developers and reviewers to
  evaluate for all changes whether they need to apply to both the normal
  and DVR-specific copies. In addition, copied code is certain to diverge
  over time, making the effort to keep the copies as synchronized as
  possible become more and more difficult.
  
  This unnecessary duplication of schema and code should be significantly
  reduced or completely eliminated by treating a normal non-distributed
  port as a special case of a distributed port that happens to only bind
  on a single host.
  
  The schema would be unified by replacing the existing ml2_port_bindings
- and ml2_dvr_port_bindings tables with two new tables. One would contain
- the port state that is the same for all hosts on which the port binds,
- including the values of the binding:host, binding:vnic_type, and
- binding:profile attributes. The other would contain the port state that
- differs among host-specific bindings, such as the binding:vif_type and
- binding:vif_details attribute values, and the bound driver and segment
- (until these two move to a separate table for hierarchical port
- binding).
+ and ml2_dvr_port_bindings tables with two new non-overlapping tables.
+ One would contain the port state that is the same for all hosts on which
+ the port binds, including the values of the binding:host,
+ binding:vnic_type, and binding:profile attributes. The other would
+ contain the port state that differs among host-specific bindings, such
+ as the binding:vif_type and binding:vif_details attribute values, and
+ the bound driver and segment (until these two move to a separate table
+ for hierarchical port binding).
  
  Also, the basic idea of distributed port bindings is not specific to
  DVR, and could be used for DHCP and other services, so the schema and
  code could be made more generic as the distributed and normal schema and
  code are unified.

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to neutron.
https://bugs.launchpad.net/bugs/1367391

Title:
  ML2 DVR port binding implementation unnecessarily duplicates schema
  and logic

Status in OpenStack Neutron (virtual network service):
  New

Bug description:
  Support for distributed port bindings was added to ML2 in order to
  enable the same DVR port to be bound simultaneously on multiple hosts.
  This was implemented by:

  * Adding a new ml2_dvr_port_bindings table similar to the ml2_port_bindings table, but with the host column as part of the primary key.
  * Adding a new DvrPortContext class the overrides several functions in PortContext.
  * Adding DVR-specific internal functions to Ml2Plugin, _process_dvr_port_binding and _commit_dvr_port_binding, that are modified copies of existing functions.
  * In about 8 places, making code conditional on "port['device_owner'] == const.DEVICE_OWNER_DVR_INTERFACE" to handle DVR ports using the above models, classes and functions instead of the normal ones.

  This duplication of schema and code adds significant technical debt to
  the ML2 plugin implementation, requiring developers and reviewers to
  evaluate for all changes whether they need to apply to both the normal
  and DVR-specific copies. In addition, copied code is certain to
  diverge over time, making the effort to keep the copies as
  synchronized as possible become more and more difficult.

  This unnecessary duplication of schema and code should be
  significantly reduced or completely eliminated by treating a normal
  non-distributed port as a special case of a distributed port that
  happens to only bind on a single host.

  The schema would be unified by replacing the existing
  ml2_port_bindings and ml2_dvr_port_bindings tables with two new non-
  overlapping tables. One would contain the port state that is the same
  for all hosts on which the port binds, including the values of the
  binding:host, binding:vnic_type, and binding:profile attributes. The
  other would contain the port state that differs among host-specific
  bindings, such as the binding:vif_type and binding:vif_details
  attribute values, and the bound driver and segment (until these two
  move to a separate table for hierarchical port binding).

  Also, the basic idea of distributed port bindings is not specific to
  DVR, and could be used for DHCP and other services, so the schema and
  code could be made more generic as the distributed and normal schema
  and code are unified.

To manage notifications about this bug go to:
https://bugs.launchpad.net/neutron/+bug/1367391/+subscriptions