Skip to content
This repository has been archived by the owner on Feb 10, 2018. It is now read-only.

fix multihop ttl to its default value in get_bgp_config() #215

Merged
merged 1 commit into from
Oct 11, 2017

Conversation

ckishimo
Copy link
Contributor

This fixes the multihop_ttl value in get_bgp_config() when the multihop command is configured but without an specific ttl value. So the following configuration:

group internal {
    type internal;
    multihop;
    local-address 10.99.1.2;
    neighbor 10.10.10.10 {
        description routerA;
        cluster 2.2.2.2;
    }
    neighbor 20.20.20.20 {
        description routerB;
    }
}

returns a multihop_ttl value of 0

{
    "internal": {
        "apply_groups": [],
        "description": "",
        "export_policy": "",
        "import_policy": "",
        "local_address": "10.99.1.2",
        "local_as": 0,
        "multihop_ttl": 0,
        "multipath": false,
        "neighbors": {
                .....
}

when I believe it should be set to its default value. Now the problem is how to determine its default value (I ignore if that changes across platforms and versions), so I set it to the default value 64 (according to Junos documentation). Please let me know. Thanks

    "internal": {
        "apply_groups": [],
        "description": "",
        "export_policy": "",
        "import_policy": "",
        "local_address": "10.99.1.2",
        "local_as": 0,
        "multihop_ttl": 64,
        "multipath": false,
        "neighbors": {

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 78.55% when pulling 7efaa94 on ckishimo:devel-hop into b087b8b on napalm-automation:develop.

1 similar comment
@coveralls
Copy link

coveralls commented Oct 11, 2017

Coverage Status

Coverage decreased (-0.2%) to 78.55% when pulling 7efaa94 on ckishimo:devel-hop into b087b8b on napalm-automation:develop.

Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

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

Looks good

@mirceaulinic mirceaulinic merged commit 171bd50 into napalm-automation:develop Oct 11, 2017
@mirceaulinic
Copy link
Member

Thanks @ckishimo

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants