ospf6d: add PtMP interface mode & related bits #9198
Conversation
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/09771a37b1672fea61fd072fd6d5379d/raw/e14ce802b22b89674844f0d1450746d3c97f7580/cr_9198_1627402927.diff | git apply
diff --git a/ospf6d/ospf6_interface.c b/ospf6d/ospf6_interface.c
index f4fadbd68..766fb98af 100644
--- a/ospf6d/ospf6_interface.c
+++ b/ospf6d/ospf6_interface.c
@@ -56,8 +56,8 @@ DEFINE_HOOK(ospf6_interface_change,
unsigned char conf_debug_ospf6_interface = 0;
const char *const ospf6_interface_state_str[] = {
- "None", "Down", "Loopback", "Waiting", "PointToPoint",
- "PtMultipoint", "DROther", "BDR", "DR", NULL};
+ "None", "Down", "Loopback", "Waiting", "PointToPoint",
+ "PtMultipoint", "DROther", "BDR", "DR", NULL};
struct ospf6_interface *ospf6_interface_lookup_by_ifindex(ifindex_t ifindex,
vrf_id_t vrf_id)
@@ -2458,16 +2458,13 @@ DEFUN (no_ipv6_ospf6_advertise_prefix_list,
return CMD_SUCCESS;
}
-DEFUN (ipv6_ospf6_network,
- ipv6_ospf6_network_cmd,
- "ipv6 ospf6 network <broadcast|point-to-point|point-to-multipoint>",
- IP6_STR
- OSPF6_STR
- "Network type\n"
- "Specify OSPF6 broadcast network\n"
- "Specify OSPF6 point-to-point network\n"
- "Specify OSPF6 point-to-multipoint network\n"
- )
+DEFUN(ipv6_ospf6_network, ipv6_ospf6_network_cmd,
+ "ipv6 ospf6 network <broadcast|point-to-point|point-to-multipoint>",
+ IP6_STR OSPF6_STR
+ "Network type\n"
+ "Specify OSPF6 broadcast network\n"
+ "Specify OSPF6 point-to-point network\n"
+ "Specify OSPF6 point-to-multipoint network\n")
{
VTY_DECLVAR_CONTEXT(interface, ifp);
int idx_network = 3;
@@ -2542,14 +2539,11 @@ DEFUN (no_ipv6_ospf6_network,
return CMD_SUCCESS;
}
-DEFPY (ipv6_ospf6_p2xp_only_cfg_neigh,
- ipv6_ospf6_p2xp_only_cfg_neigh_cmd,
- "[no] ipv6 ospf6 p2p-p2mp config-neighbors-only",
- NO_STR
- IP6_STR
- OSPF6_STR
- "Point-to-point and Point-to-Multipoint parameters\n"
- "Only form adjacencies with explicitly configured neighbors\n")
+DEFPY(ipv6_ospf6_p2xp_only_cfg_neigh, ipv6_ospf6_p2xp_only_cfg_neigh_cmd,
+ "[no] ipv6 ospf6 p2p-p2mp config-neighbors-only",
+ NO_STR IP6_STR OSPF6_STR
+ "Point-to-point and Point-to-Multipoint parameters\n"
+ "Only form adjacencies with explicitly configured neighbors\n")
{
VTY_DECLVAR_CONTEXT(interface, ifp);
struct ospf6_interface *oi = ifp->info;
@@ -2569,14 +2563,12 @@ DEFPY (ipv6_ospf6_p2xp_only_cfg_neigh,
return CMD_SUCCESS;
}
-DEFPY (ipv6_ospf6_p2xp_no_multicast_hello,
- ipv6_ospf6_p2xp_no_multicast_hello_cmd,
- "[no] ipv6 ospf6 p2p-p2mp disable-multicast-hello",
- NO_STR
- IP6_STR
- OSPF6_STR
- "Point-to-point and Point-to-Multipoint parameters\n"
- "Do not send multicast hellos\n")
+DEFPY(ipv6_ospf6_p2xp_no_multicast_hello,
+ ipv6_ospf6_p2xp_no_multicast_hello_cmd,
+ "[no] ipv6 ospf6 p2p-p2mp disable-multicast-hello",
+ NO_STR IP6_STR OSPF6_STR
+ "Point-to-point and Point-to-Multipoint parameters\n"
+ "Do not send multicast hellos\n")
{
VTY_DECLVAR_CONTEXT(interface, ifp);
struct ospf6_interface *oi = ifp->info;
@@ -2596,16 +2588,13 @@ DEFPY (ipv6_ospf6_p2xp_no_multicast_hello,
return CMD_SUCCESS;
}
-DEFPY (ipv6_ospf6_p2xp_connected_pfx,
- ipv6_ospf6_p2xp_connected_pfx_cmd,
- "[no] ipv6 ospf6 p2p-p2mp connected-prefixes <include$incl|exclude$excl>",
- NO_STR
- IP6_STR
- OSPF6_STR
- "Point-to-point and Point-to-Multipoint parameters\n"
- "Adjust handling of directly connected prefixes\n"
- "Advertise prefixes and own /128 (default for PtP)\n"
- "Ignore, only advertise own /128 (default for PtMP)\n")
+DEFPY(ipv6_ospf6_p2xp_connected_pfx, ipv6_ospf6_p2xp_connected_pfx_cmd,
+ "[no] ipv6 ospf6 p2p-p2mp connected-prefixes <include$incl|exclude$excl>",
+ NO_STR IP6_STR OSPF6_STR
+ "Point-to-point and Point-to-Multipoint parameters\n"
+ "Adjust handling of directly connected prefixes\n"
+ "Advertise prefixes and own /128 (default for PtP)\n"
+ "Ignore, only advertise own /128 (default for PtMP)\n")
{
VTY_DECLVAR_CONTEXT(interface, ifp);
struct ospf6_interface *oi = ifp->info;
@@ -2633,14 +2622,11 @@ DEFPY (ipv6_ospf6_p2xp_connected_pfx,
return CMD_SUCCESS;
}
-ALIAS (ipv6_ospf6_p2xp_connected_pfx,
- no_ipv6_ospf6_p2xp_connected_pfx_cmd,
- "no ipv6 ospf6 p2p-p2mp connected-prefixes",
- NO_STR
- IP6_STR
- OSPF6_STR
- "Point-to-point and Point-to-Multipoint parameters\n"
- "Adjust handling of directly connected prefixes\n")
+ALIAS(ipv6_ospf6_p2xp_connected_pfx, no_ipv6_ospf6_p2xp_connected_pfx_cmd,
+ "no ipv6 ospf6 p2p-p2mp connected-prefixes",
+ NO_STR IP6_STR OSPF6_STR
+ "Point-to-point and Point-to-Multipoint parameters\n"
+ "Adjust handling of directly connected prefixes\n")
static int config_write_ospf6_interface(struct vty *vty, struct vrf *vrf)
diff --git a/ospf6d/ospf6_interface.h b/ospf6d/ospf6_interface.h
index 970af8fce..d7ed60040 100644
--- a/ospf6d/ospf6_interface.h
+++ b/ospf6d/ospf6_interface.h
@@ -172,16 +172,16 @@ struct ospf6_interface {
DECLARE_QOBJ_TYPE(ospf6_interface);
/* interface state */
-#define OSPF6_INTERFACE_NONE 0
-#define OSPF6_INTERFACE_DOWN 1
-#define OSPF6_INTERFACE_LOOPBACK 2
-#define OSPF6_INTERFACE_WAITING 3
-#define OSPF6_INTERFACE_POINTTOPOINT 4
-#define OSPF6_INTERFACE_POINTTOMULTIPOINT 5
-#define OSPF6_INTERFACE_DROTHER 6
-#define OSPF6_INTERFACE_BDR 7
-#define OSPF6_INTERFACE_DR 8
-#define OSPF6_INTERFACE_MAX 9
+#define OSPF6_INTERFACE_NONE 0
+#define OSPF6_INTERFACE_DOWN 1
+#define OSPF6_INTERFACE_LOOPBACK 2
+#define OSPF6_INTERFACE_WAITING 3
+#define OSPF6_INTERFACE_POINTTOPOINT 4
+#define OSPF6_INTERFACE_POINTTOMULTIPOINT 5
+#define OSPF6_INTERFACE_DROTHER 6
+#define OSPF6_INTERFACE_BDR 7
+#define OSPF6_INTERFACE_DR 8
+#define OSPF6_INTERFACE_MAX 9
extern const char *const ospf6_interface_state_str[];
diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c
index 13b9df23f..86550da0b 100644
--- a/ospf6d/ospf6_neighbor.c
+++ b/ospf6d/ospf6_neighbor.c
@@ -715,14 +715,11 @@ static void p2xp_neigh_refresh(struct ospf6_neighbor *on, uint32_t prev_cost)
#include "ospf6d/ospf6_neighbor_clippy.c"
#endif
-DEFPY (ipv6_ospf6_p2xp_neigh,
- ipv6_ospf6_p2xp_neigh_cmd,
- "[no] ipv6 ospf6 neighbor X:X::X:X",
- NO_STR
- IP6_STR
- OSPF6_STR
- "Configure static neighbor\n"
- "Neighbor link-local address\n")
+DEFPY(ipv6_ospf6_p2xp_neigh, ipv6_ospf6_p2xp_neigh_cmd,
+ "[no] ipv6 ospf6 neighbor X:X::X:X",
+ NO_STR IP6_STR OSPF6_STR
+ "Configure static neighbor\n"
+ "Neighbor link-local address\n")
{
VTY_DECLVAR_CONTEXT(interface, ifp);
struct ospf6_interface *oi = ifp->info;
@@ -760,16 +757,13 @@ DEFPY (ipv6_ospf6_p2xp_neigh,
return CMD_SUCCESS;
}
-DEFPY (ipv6_ospf6_p2xp_neigh_cost,
- ipv6_ospf6_p2xp_neigh_cost_cmd,
- "[no] ipv6 ospf6 neighbor X:X::X:X cost (1-65535)",
- NO_STR
- IP6_STR
- OSPF6_STR
- "Configure static neighbor\n"
- "Neighbor link-local address\n"
- "Outgoing metric for this neighbor\n"
- "Outgoing metric for this neighbor\n")
+DEFPY(ipv6_ospf6_p2xp_neigh_cost, ipv6_ospf6_p2xp_neigh_cost_cmd,
+ "[no] ipv6 ospf6 neighbor X:X::X:X cost (1-65535)",
+ NO_STR IP6_STR OSPF6_STR
+ "Configure static neighbor\n"
+ "Neighbor link-local address\n"
+ "Outgoing metric for this neighbor\n"
+ "Outgoing metric for this neighbor\n")
{
VTY_DECLVAR_CONTEXT(interface, ifp);
struct ospf6_interface *oi = ifp->info;
@@ -841,16 +835,14 @@ static int p2xp_unicast_hello_send(struct thread *thread)
return 0;
}
-DEFPY (ipv6_ospf6_p2xp_neigh_poll_interval,
- ipv6_ospf6_p2xp_neigh_poll_interval_cmd,
- "[no] ipv6 ospf6 neighbor X:X::X:X poll-interval (1-65535)",
- NO_STR
- IP6_STR
- OSPF6_STR
- "Configure static neighbor\n"
- "Neighbor link-local address\n"
- "Send unicast hellos to neighbor when down\n"
- "Unicast hello interval when down (seconds)\n")
+DEFPY(ipv6_ospf6_p2xp_neigh_poll_interval,
+ ipv6_ospf6_p2xp_neigh_poll_interval_cmd,
+ "[no] ipv6 ospf6 neighbor X:X::X:X poll-interval (1-65535)",
+ NO_STR IP6_STR OSPF6_STR
+ "Configure static neighbor\n"
+ "Neighbor link-local address\n"
+ "Send unicast hellos to neighbor when down\n"
+ "Unicast hello interval when down (seconds)\n")
{
VTY_DECLVAR_CONTEXT(interface, ifp);
struct ospf6_interface *oi = ifp->info;
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20579/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsCLANG Static Analyzer Summary
1 Static Analyzer issues remaining.See details at |
| #define OSPF6_INTERFACE_BDR 6 | ||
| #define OSPF6_INTERFACE_DR 7 | ||
| #define OSPF6_INTERFACE_MAX 8 | ||
| #define OSPF6_INTERFACE_NONE 0 |
There was a problem hiding this comment.
POINTTOMULTIPOINT is longer than the other names… without the whitespace change it's this:
#define OSPF6_INTERFACE_WAITING 3
#define OSPF6_INTERFACE_POINTTOPOINT 4
#define OSPF6_INTERFACE_POINTTOMULTIPOINT 5
#define OSPF6_INTERFACE_DROTHER 6
#define OSPF6_INTERFACE_BDR 7
⇒ I'd say the whitespace change is appropriate.
|
topotests and SA issue introduced |
| - distinct costs can be set for each pair of routers and direction | ||
|
|
||
| The main downside is less efficient flooding on networks with a large number | ||
| of OSPFv3 routers. |
There was a problem hiding this comment.
There's actually another downside, but I don't know if it's worth going into in the comments. :-) With the DR, all the routers on a single broadcast interface look like a hub-and-spoke rather than a full mesh. This reduces the size of the spt, so spf can calculate just a hair faster. It's not a "thing" in most networks and on most processors now, but way back in the day when all our server room floors were dirt, it made a difference. :-)
There was a problem hiding this comment.
That's kinda listed as an advantage because the very same thing allows you to control metrics in a n×m manner 😄 … but yeah that's another reason why I put the "advanced options, make sure you know what you're doing" above.
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)PullReq merge failed. Please rebase your branch: |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
|
(I'm aware this needs to be rebased. It's a DRAFT. I pushed it out for visibility and to avoid duplicate work. Due to priorities/resource allocations, this is probably going to sit here for a bit.) |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21424/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsCLANG Static Analyzer Summary
1 Static Analyzer issues remaining.See details at |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This function is not implemented anywhere. Not sure it ever existed. Signed-off-by: David Lamparter <[email protected]>
Both for virtual links and correct PtMP operation, advertising local addresses as Intra-Prefix with LA set is a prerequisite. Add the appropriate entries. Signed-off-by: David Lamparter <[email protected]>
For PtMP the cost may need to be recalculated when the LL addr changes (since neighbors are configured by LL addr and a different entry with a different cost may match.) Signed-off-by: David Lamparter <[email protected]>
Add a list of configured neighbors for each interface. Only stores cost (and "existence") for now. Signed-off-by: David Lamparter <[email protected]>
This adds a knob to refuse forming adjacencies with neighbors not listed in the config. Only works on PtP/PtMP of course, otherwise the DR/BDR machinery gets broken. Signed-off-by: David Lamparter <[email protected]>
With the configured neighbor list, unicast hellos can be sent. Allow disabling multicast hellos for that scenario. Signed-off-by: David Lamparter <[email protected]>
Some lower layers still don't handle multicast correctly (or efficiently.) Add option to send unicast hellos on explicitly configured neighbors for PtP/PtMP. Signed-off-by: David Lamparter <[email protected]>
This adds the PtMP interface type, which is effectively identical to PtP except that all the database flooding & updates are unicast. Signed-off-by: David Lamparter <[email protected]>
To announce connected prefixes, or not to announce connected prefixes, that is the question... Signed-off-by: David Lamparter <[email protected]>
Update & add docs for all the stuff in the previous 10-ish commits. Signed-off-by: David Lamparter <[email protected]>
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-9625/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-9625/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-9625/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-9625/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-9625/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-9625/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundCLANG Static Analyzer Summary
1 Static Analyzer issues remaining.See details at |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
A newer version of this PR was merged in #14546 so this is no longer needed. |
This implements:
LAbit set (this is a prerequisite for implementing virtual link support too, since this is how the IPv6 address for the other router is found)ipv6 ospf6 neighbor X:X::X:Xlist under each interface, withcostandpoll-intervalsettings eachipv6 ospf6 p2p-p2mp disable-multicast-helloknob does what the label saysipv6 ospf6 p2p-p2mp config-neighbors-onlyrefuses becoming neighbor/adjacent with routers that aren't in theipv6 ospf6 neighborlistipv6 ospf6 p2p-p2mp connected-prefixes <include|exclude>the semantically trickiest one (though code-wise extremely simple) controls whether addresses on the ptp/ptmp interface are announced as /128 or as /128 + (/64 or other prefix len)ipv6 ospf6 network point-to-multipoint, finally, just changes from multicast to unicast for LSDB flooding & updatesUser doc should explain everything.
Not fully tested yet, hence draft PR. Tests coming up soon™.