Skip to content

Add the ability to specify PCI address in dpdk-bond-mappings.#956

Draft
PanosKostopoulos wants to merge 1 commit intojuju:masterfrom
PanosKostopoulos:master
Draft

Add the ability to specify PCI address in dpdk-bond-mappings.#956
PanosKostopoulos wants to merge 1 commit intojuju:masterfrom
PanosKostopoulos:master

Conversation

@PanosKostopoulos
Copy link
Copy Markdown

Since mac addresses change often in deployment, an option for specifying PCI address is very useful.

Since mac addresses change often in deployment, an option for
specifying PCI address is very useful.

Signed-off-by: Panos Kostopoulos Kyrimis <panos.kostopouloskyrimis@canonical.com>

return resolved_devices

PCI_REGEX = r'^([0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-7]$'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The purpose of the resolve_pci_from_mapping_config function is to translate MAC into PCI addresses and as such I'd not modify it for your purpose.
    Instead, I'd suggest to create a new function that can be called from somewhere around here:
    def __call__(self):
    """Populate context.
    :returns: context
    :rtype: Dict[str,Union[str,collections.OrderedDict[str,str]]]
    """
    driver = config(self.driver_key)
    if driver is None:
    return {}
    # Resolve PCI devices for both directly used devices (_bridges)
    # and devices for use in dpdk bonds (_bonds)
    pci_devices = resolve_pci_from_mapping_config(self.bridges_key)
    pci_devices.update(resolve_pci_from_mapping_config(self.bonds_key))
    return {'devices': pci_devices,
    'driver': driver}
  2. Regular expressions are prohibitively difficult to read and maintain, please make use of the rich Python string helpers instead, there is some potential inspiration in the parse_* functions here:
    def parse_mappings(mappings, key_rvalue=False):
    """By default mappings are lvalue keyed.
    If key_rvalue is True, the mapping will be reversed to allow multiple
    configs for the same lvalue.
    """
    parsed = {}
    if mappings:
    mappings = mappings.split()
    for m in mappings:
    p = m.partition(':')
    if key_rvalue:
    key_index = 2
    val_index = 0
    # if there is no rvalue skip to next
    if not p[1]:
    continue
    else:
    key_index = 0
    val_index = 2
    key = p[key_index].strip()
    parsed[key] = p[val_index].strip()
    return parsed
    def parse_bridge_mappings(mappings):
    """Parse bridge mappings.
    Mappings must be a space-delimited list of provider:bridge mappings.
    Returns dict of the form {provider:bridge}.
    """
    return parse_mappings(mappings)
    def parse_data_port_mappings(mappings, default_bridge='br-data'):
    """Parse data port mappings.
    Mappings must be a space-delimited list of bridge:port.
    Returns dict of the form {port:bridge} where ports may be mac addresses or
    interface names.
    """
    # NOTE(dosaboy): we use rvalue for key to allow multiple values to be
    # proposed for <port> since it may be a mac address which will differ
    # across units this allowing first-known-good to be chosen.
    _mappings = parse_mappings(mappings, key_rvalue=True)
    if not _mappings or list(_mappings.values()) == ['']:
    if not mappings:
    return {}
    # For backwards-compatibility we need to support port-only provided in
    # config.
    _mappings = {mappings.split()[0]: default_bridge}
    ports = _mappings.keys()
    if len(set(ports)) != len(ports):
    raise Exception("It is not allowed to have the same port configured "
    "on more than one bridge")
    return _mappings
    def parse_vlan_range_mappings(mappings):
    """Parse vlan range mappings.
    Mappings must be a space-delimited list of provider:start:end mappings.
    The start:end range is optional and may be omitted.
    Returns dict of the form {provider: (start, end)}.
    """
    _mappings = parse_mappings(mappings)
    return {p: tuple(r.split(':')) for p, r in _mappings.items()}

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.

2 participants