diff options
author | Igor Ryzhov <iryzhov@nfware.com> | 2021-06-04 16:47:32 +0200 |
---|---|---|
committer | Igor Ryzhov <iryzhov@nfware.com> | 2021-06-05 17:25:01 +0200 |
commit | 3eec4ee0773786b7ba366bc7499ed6bbd28fdf79 (patch) | |
tree | f97dc395533728e1885c4fea5b09b44ac9afc646 /ospfd | |
parent | Merge pull request #8776 from anlancs/fix-ospf-cli-passive-interface (diff) | |
download | frr-3eec4ee0773786b7ba366bc7499ed6bbd28fdf79.tar.xz frr-3eec4ee0773786b7ba366bc7499ed6bbd28fdf79.zip |
ospfd: fix passive interface configuration
Currently, passive interface flag is configured from the router node
using "passive-interface IFNAME". There are multiple problems with this
command:
- it is not in line with all other interface-related commands - other
parameters are configured from the interface node using "ip ospf"
prefix
- it is not in line with OSPFv3 - passive flag is configured from the
interface node using "ipv6 ospf6 passive" command
- most importantly, it doesn't work correctly when the interface is in
a different VRF - when using VRF-lite, it incorrectly changes the
vrf_id of the interface and it becomes desynced with the actual state;
when using netns, it creates a new fake interface and configures it
instead of configuring the necessary interface
To fix all the problems, this commit adds a new command to the interface
configuration node - "ip ospf passive". The purpose of the command is
completely the same, but it works correctly in a multi-VRF environment.
The old command is preserved for the backward compatibility, but the
warning is added that it is deprecated because it doesn't work correctly
with VRFs.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Diffstat (limited to 'ospfd')
-rw-r--r-- | ospfd/ospf_vty.c | 303 | ||||
-rw-r--r-- | ospfd/ospfd.c | 10 |
2 files changed, 167 insertions, 146 deletions
diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index fb2d79053..3186eabea 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -362,88 +362,79 @@ DEFPY (no_ospf_router_id, } -static void ospf_passive_interface_default(struct ospf *ospf, uint8_t newval) +static void ospf_passive_interface_default_update(struct ospf *ospf, + uint8_t newval) { - struct vrf *vrf = vrf_lookup_by_id(ospf->vrf_id); struct listnode *ln; - struct interface *ifp; struct ospf_interface *oi; ospf->passive_interface_default = newval; - FOR_ALL_INTERFACES (vrf, ifp) { - if (ifp && OSPF_IF_PARAM_CONFIGURED(IF_DEF_PARAMS(ifp), - passive_interface)) - UNSET_IF_PARAM(IF_DEF_PARAMS(ifp), passive_interface); - } - for (ALL_LIST_ELEMENTS_RO(ospf->oiflist, ln, oi)) { - if (OSPF_IF_PARAM_CONFIGURED(oi->params, passive_interface)) - UNSET_IF_PARAM(oi->params, passive_interface); - /* update multicast memberships */ + /* update multicast memberships */ + for (ALL_LIST_ELEMENTS_RO(ospf->oiflist, ln, oi)) ospf_if_set_multicast(oi); - } } -static void ospf_passive_interface_update_addr(struct ospf *ospf, - struct interface *ifp, - struct ospf_if_params *params, - uint8_t value, - struct in_addr addr) +static void ospf_passive_interface_update(struct interface *ifp) { - uint8_t dflt; + struct route_node *rn; - params->passive_interface = value; - if (params != IF_DEF_PARAMS(ifp)) { - if (OSPF_IF_PARAM_CONFIGURED(IF_DEF_PARAMS(ifp), - passive_interface)) - dflt = IF_DEF_PARAMS(ifp)->passive_interface; - else - dflt = ospf->passive_interface_default; + /* + * XXX We should call ospf_if_set_multicast on exactly those + * interfaces for which the passive property changed. It is too much + * work to determine this set, so we do this for every interface. + * This is safe and reasonable because ospf_if_set_multicast uses a + * record of joined groups to avoid systems calls if the desired + * memberships match the current memership. + */ - if (value != dflt) - SET_IF_PARAM(params, passive_interface); - else - UNSET_IF_PARAM(params, passive_interface); + for (rn = route_top(IF_OIFS(ifp)); rn; rn = route_next(rn)) { + struct ospf_interface *oi = rn->info; - ospf_free_if_params(ifp, addr); - ospf_if_update_params(ifp, addr); + if (oi) + ospf_if_set_multicast(oi); } + + /* + * XXX It is not clear what state transitions the interface needs to + * undergo when going from active to passive and vice versa. Fixing + * this will require precise identification of interfaces having such a + * transition. + */ } -static void ospf_passive_interface_update(struct ospf *ospf, - struct interface *ifp, - struct ospf_if_params *params, - uint8_t value) +DEFUN (ospf_passive_interface_default, + ospf_passive_interface_default_cmd, + "passive-interface default", + "Suppress routing updates on an interface\n" + "Suppress routing updates on interfaces by default\n") { - params->passive_interface = value; - if (params == IF_DEF_PARAMS(ifp)) { - if (value != ospf->passive_interface_default) - SET_IF_PARAM(params, passive_interface); - else - UNSET_IF_PARAM(params, passive_interface); - } + VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); + + ospf_passive_interface_default_update(ospf, OSPF_IF_PASSIVE); + + return CMD_SUCCESS; } -DEFUN (ospf_passive_interface, +DEFUN_HIDDEN (ospf_passive_interface_addr, ospf_passive_interface_addr_cmd, - "passive-interface <IFNAME [A.B.C.D]|default>", + "passive-interface IFNAME [A.B.C.D]", "Suppress routing updates on an interface\n" "Interface's name\n" - "IPv4 address\n" - "Suppress routing updates on interfaces by default\n") + "IPv4 address\n") { VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); int idx_ipv4 = 2; struct interface *ifp = NULL; struct in_addr addr = {.s_addr = INADDR_ANY}; - int ret; struct ospf_if_params *params; - struct route_node *rn; + int ret; + + vty_out(vty, + "This command is deprecated, because it is not VRF-aware.\n"); + vty_out(vty, + "Please, use \"ip ospf passive\" on an interface instead.\n"); - if (strmatch(argv[1]->text, "default")) { - ospf_passive_interface_default(ospf, OSPF_IF_PASSIVE); - return CMD_SUCCESS; - } if (ospf->vrf_id != VRF_UNKNOWN) ifp = if_get_by_name(argv[1]->arg, ospf->vrf_id); @@ -452,8 +443,6 @@ DEFUN (ospf_passive_interface, return CMD_WARNING_CONFIG_FAILED; } - params = IF_DEF_PARAMS(ifp); - if (argc == 3) { ret = inet_aton(argv[idx_ipv4]->arg, &addr); if (!ret) { @@ -464,45 +453,39 @@ DEFUN (ospf_passive_interface, params = ospf_get_if_params(ifp, addr); ospf_if_update_params(ifp, addr); - ospf_passive_interface_update_addr(ospf, ifp, params, - OSPF_IF_PASSIVE, addr); + } else { + params = IF_DEF_PARAMS(ifp); } - ospf_passive_interface_update(ospf, ifp, params, OSPF_IF_PASSIVE); + params->passive_interface = OSPF_IF_PASSIVE; + SET_IF_PARAM(params, passive_interface); - /* XXX We should call ospf_if_set_multicast on exactly those - * interfaces for which the passive property changed. It is too much - * work to determine this set, so we do this for every interface. - * This is safe and reasonable because ospf_if_set_multicast uses a - * record of joined groups to avoid systems calls if the desired - * memberships match the current memership. - */ + ospf_passive_interface_update(ifp); - for (rn = route_top(IF_OIFS(ifp)); rn; rn = route_next(rn)) { - struct ospf_interface *oi = rn->info; + return CMD_SUCCESS; +} - if (oi && (OSPF_IF_PARAM(oi, passive_interface) - == OSPF_IF_PASSIVE)) - ospf_if_set_multicast(oi); - } - /* - * XXX It is not clear what state transitions the interface needs to - * undergo when going from active to passive. Fixing this will - * require precise identification of interfaces having such a - * transition. - */ +DEFUN (no_ospf_passive_interface_default, + no_ospf_passive_interface_default_cmd, + "no passive-interface default", + NO_STR + "Allow routing updates on an interface\n" + "Allow routing updates on interfaces by default\n") +{ + VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); + + ospf_passive_interface_default_update(ospf, OSPF_IF_ACTIVE); return CMD_SUCCESS; } -DEFUN (no_ospf_passive_interface, +DEFUN_HIDDEN (no_ospf_passive_interface, no_ospf_passive_interface_addr_cmd, - "no passive-interface <IFNAME [A.B.C.D]|default>", + "no passive-interface IFNAME [A.B.C.D]", NO_STR "Allow routing updates on an interface\n" "Interface's name\n" - "IPv4 address\n" - "Allow routing updates on interfaces by default\n") + "IPv4 address\n") { VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf); int idx_ipv4 = 3; @@ -510,12 +493,11 @@ DEFUN (no_ospf_passive_interface, struct in_addr addr = {.s_addr = INADDR_ANY}; struct ospf_if_params *params; int ret; - struct route_node *rn; - if (strmatch(argv[2]->text, "default")) { - ospf_passive_interface_default(ospf, OSPF_IF_ACTIVE); - return CMD_SUCCESS; - } + vty_out(vty, + "This command is deprecated, because it is not VRF-aware.\n"); + vty_out(vty, + "Please, use \"no ip ospf passive\" on an interface instead.\n"); if (ospf->vrf_id != VRF_UNKNOWN) ifp = if_get_by_name(argv[2]->arg, ospf->vrf_id); @@ -525,8 +507,6 @@ DEFUN (no_ospf_passive_interface, return CMD_WARNING_CONFIG_FAILED; } - params = IF_DEF_PARAMS(ifp); - if (argc == 4) { ret = inet_aton(argv[idx_ipv4]->arg, &addr); if (!ret) { @@ -534,30 +514,22 @@ DEFUN (no_ospf_passive_interface, "Please specify interface address by A.B.C.D\n"); return CMD_WARNING_CONFIG_FAILED; } - params = ospf_lookup_if_params(ifp, addr); if (params == NULL) return CMD_SUCCESS; - ospf_passive_interface_update_addr(ospf, ifp, params, - OSPF_IF_ACTIVE, addr); + } else { + params = IF_DEF_PARAMS(ifp); } - ospf_passive_interface_update(ospf, ifp, params, OSPF_IF_ACTIVE); - - /* XXX We should call ospf_if_set_multicast on exactly those - * interfaces for which the passive property changed. It is too much - * work to determine this set, so we do this for every interface. - * This is safe and reasonable because ospf_if_set_multicast uses a - * record of joined groups to avoid systems calls if the desired - * memberships match the current memership. - */ - for (rn = route_top(IF_OIFS(ifp)); rn; rn = route_next(rn)) { - struct ospf_interface *oi = rn->info; - if (oi - && (OSPF_IF_PARAM(oi, passive_interface) == OSPF_IF_ACTIVE)) - ospf_if_set_multicast(oi); + params->passive_interface = OSPF_IF_ACTIVE; + UNSET_IF_PARAM(params, passive_interface); + if (params != IF_DEF_PARAMS(ifp)) { + ospf_free_if_params(ifp, addr); + ospf_if_update_params(ifp, addr); } + ospf_passive_interface_update(ifp); + return CMD_SUCCESS; } @@ -9094,6 +9066,82 @@ DEFUN (no_ip_ospf_area, return CMD_SUCCESS; } +DEFUN (ip_ospf_passive, + ip_ospf_passive_cmd, + "ip ospf passive [A.B.C.D]", + "IP Information\n" + "OSPF interface commands\n" + "Suppress routing updates on an interface\n" + "Address of interface\n") +{ + VTY_DECLVAR_CONTEXT(interface, ifp); + int idx_ipv4 = 3; + struct in_addr addr; + struct ospf_if_params *params; + int ret; + + if (argc == 4) { + ret = inet_aton(argv[idx_ipv4]->arg, &addr); + if (!ret) { + vty_out(vty, + "Please specify interface address by A.B.C.D\n"); + return CMD_WARNING_CONFIG_FAILED; + } + params = ospf_get_if_params(ifp, addr); + ospf_if_update_params(ifp, addr); + } else { + params = IF_DEF_PARAMS(ifp); + } + + params->passive_interface = OSPF_IF_PASSIVE; + SET_IF_PARAM(params, passive_interface); + + ospf_passive_interface_update(ifp); + + return CMD_SUCCESS; +} + +DEFUN (no_ip_ospf_passive, + no_ip_ospf_passive_cmd, + "no ip ospf passive [A.B.C.D]", + NO_STR + "IP Information\n" + "OSPF interface commands\n" + "Enable routing updates on an interface\n" + "Address of interface\n") +{ + VTY_DECLVAR_CONTEXT(interface, ifp); + int idx_ipv4 = 4; + struct in_addr addr; + struct ospf_if_params *params; + int ret; + + if (argc == 5) { + ret = inet_aton(argv[idx_ipv4]->arg, &addr); + if (!ret) { + vty_out(vty, + "Please specify interface address by A.B.C.D\n"); + return CMD_WARNING_CONFIG_FAILED; + } + params = ospf_lookup_if_params(ifp, addr); + if (params == NULL) + return CMD_SUCCESS; + } else { + params = IF_DEF_PARAMS(ifp); + } + + params->passive_interface = OSPF_IF_ACTIVE; + UNSET_IF_PARAM(params, passive_interface); + if (params != IF_DEF_PARAMS(ifp)) { + ospf_free_if_params(ifp, addr); + ospf_if_update_params(ifp, addr); + } + + ospf_passive_interface_update(ifp); + + return CMD_SUCCESS; +} + DEFUN (ospf_redistribute_source, ospf_redistribute_source_cmd, "redistribute " FRR_REDIST_STR_OSPFD " [{metric (0-16777214)|metric-type (1-2)|route-map WORD}]", @@ -11865,6 +11913,14 @@ static int config_write_interface_one(struct vty *vty, struct vrf *vrf) vty_out(vty, "\n"); } + if (OSPF_IF_PARAM_CONFIGURED(params, + passive_interface)) { + vty_out(vty, " ip ospf passive"); + if (params != IF_DEF_PARAMS(ifp) && rn) + vty_out(vty, " %pI4", &rn->p.u.prefix4); + vty_out(vty, "\n"); + } + /* LDP-Sync print */ if (params && params->ldp_sync_info) ospf_ldp_sync_if_write_config(vty, params); @@ -12312,10 +12368,6 @@ static int config_write_ospf_distance(struct vty *vty, struct ospf *ospf) static int ospf_config_write_one(struct vty *vty, struct ospf *ospf) { - struct vrf *vrf = vrf_lookup_by_id(ospf->vrf_id); - struct interface *ifp; - struct ospf_interface *oi; - struct listnode *node = NULL; int write = 0; /* `router ospf' print. */ @@ -12418,33 +12470,6 @@ static int ospf_config_write_one(struct vty *vty, struct ospf *ospf) vty_out(vty, " no proactive-arp\n"); } - FOR_ALL_INTERFACES (vrf, ifp) - if (OSPF_IF_PARAM_CONFIGURED(IF_DEF_PARAMS(ifp), - passive_interface) - && IF_DEF_PARAMS(ifp)->passive_interface - != ospf->passive_interface_default) { - vty_out(vty, " %spassive-interface %s\n", - IF_DEF_PARAMS(ifp)->passive_interface ? "" - : "no ", - ifp->name); - } - for (ALL_LIST_ELEMENTS_RO(ospf->oiflist, node, oi)) { - if (!OSPF_IF_PARAM_CONFIGURED(oi->params, passive_interface)) - continue; - if (OSPF_IF_PARAM_CONFIGURED(IF_DEF_PARAMS(oi->ifp), - passive_interface)) { - if (oi->params->passive_interface - == IF_DEF_PARAMS(oi->ifp)->passive_interface) - continue; - } else if (oi->params->passive_interface - == ospf->passive_interface_default) - continue; - - vty_out(vty, " %spassive-interface %s %pI4\n", - oi->params->passive_interface ? "" : "no ", - oi->ifp->name, &oi->address->u.prefix4); - } - /* TI-LFA print. */ if (ospf->ti_lfa_enabled) { if (ospf->ti_lfa_protection_type == OSPF_TI_LFA_NODE_PROTECTION) @@ -12639,6 +12664,10 @@ static void ospf_vty_if_init(void) install_element(INTERFACE_NODE, &ip_ospf_area_cmd); install_element(INTERFACE_NODE, &no_ip_ospf_area_cmd); + /* "ip ospf passive" commands. */ + install_element(INTERFACE_NODE, &ip_ospf_passive_cmd); + install_element(INTERFACE_NODE, &no_ip_ospf_passive_cmd); + /* These commands are compatibitliy for previous version. */ install_element(INTERFACE_NODE, &ospf_authentication_key_cmd); install_element(INTERFACE_NODE, &ospf_message_digest_key_cmd); @@ -12798,7 +12827,9 @@ void ospf_vty_init(void) install_element(OSPF_NODE, &no_ospf_router_id_cmd); /* "passive-interface" commands. */ + install_element(OSPF_NODE, &ospf_passive_interface_default_cmd); install_element(OSPF_NODE, &ospf_passive_interface_addr_cmd); + install_element(OSPF_NODE, &no_ospf_passive_interface_default_cmd); install_element(OSPF_NODE, &no_ospf_passive_interface_addr_cmd); /* "ospf abr-type" commands. */ diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c index 38c0ca2b6..2f088aa81 100644 --- a/ospfd/ospfd.c +++ b/ospfd/ospfd.c @@ -692,7 +692,6 @@ static void ospf_finish_final(struct ospf *ospf) struct route_node *rn; struct ospf_nbr_nbma *nbr_nbma; struct ospf_lsa *lsa; - struct interface *ifp; struct ospf_interface *oi; struct ospf_area *area; struct ospf_vl_data *vl_data; @@ -740,15 +739,6 @@ static void ospf_finish_final(struct ospf *ospf) if (ospf->vrf_id == VRF_DEFAULT) ospf_ldp_sync_gbl_exit(ospf, true); - /* Remove ospf interface config params: only passive-interface */ - FOR_ALL_INTERFACES (vrf, ifp) { - struct ospf_if_params *params; - - params = IF_DEF_PARAMS(ifp); - if (OSPF_IF_PARAM_CONFIGURED(params, passive_interface)) - UNSET_IF_PARAM(params, passive_interface); - } - /* Reset interface. */ for (ALL_LIST_ELEMENTS(ospf->oiflist, node, nnode, oi)) ospf_if_free(oi); |