October 2024 - gerrit-log - lists.osmocom.org

[L] Change in libosmo-sigtran[master]: ss7 vty: Fix insert order of routes with higher priority
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email ) Change subject: ss7 vty: Fix insert order of routes with higher priority ...................................................................... Patch Set 2: Code-Review+1 (2 comments) File src/osmo_ss7.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375/comment/7e78466e_884e7… : PS2, Line 664: noy typo: not File tests/vty/osmo_stp_route_prio.vty: https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375/comment/b5885aad_cca7a… : PS2, Line 71: Os This comment should be removed now? -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218 Gerrit-Change-Number: 38375 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 09 Oct 2024 19:01:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[M] Change in libosmo-sigtran[master]: tests/vty: Show bug inserting routes with different priority
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38374?usp=email ) Change subject: tests/vty: Show bug inserting routes with different priority ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38374?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I125046fcbc7563c3275f42ff9de2aa351ae616c1 Gerrit-Change-Number: 38374 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 09 Oct 2024 18:40:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in libosmo-sigtran[master]: tests/vty/Makefile.am: Allow testing with specific vty file
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: daniel, laforge, pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38373?usp=email ) Change subject: tests/vty/Makefile.am: Allow testing with specific vty file ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38373?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: Ia0d53007334aaa3df35d1c821c520af4952b7843 Gerrit-Change-Number: 38373 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Wed, 09 Oct 2024 18:37:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in libosmo-sigtran[master]: ss7 vty: Fix insert order of routes with higher priority
by pespin 09 Oct '24

09 Oct '24

pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email ) Change subject: ss7 vty: Fix insert order of routes with higher priority ...................................................................... Patch Set 2: (1 comment) File tests/vty/osmo_stp_route_prio.vty: https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375/comment/e906cc78_1e043… : PS2, Line 87: 3.2.1/14 0 as3 ? ? ? As it can be seen, the default prio (0) when no prio is selected is the highest one. Now comes the question whether this is desired, or we should be setting prio 0xffffff by default (the lower one), or some intermediate value like 0xff00000. This would also mean changing old behavior, but in this case we know for sure the priority stuff was not being used at all anywhere so it shouldn't be a problem. -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218 Gerrit-Change-Number: 38375 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Wed, 09 Oct 2024 18:14:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in libosmo-sigtran[master]: ss7 vty: Fix insert order of routes with higher priority
by pespin 09 Oct '24

09 Oct '24

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: ss7 vty: Fix insert order of routes with higher priority ...................................................................... ss7 vty: Fix insert order of routes with higher priority Route lookup ((hmrt_message_for_routing() => osmo_ss7_route_lookup() => osmo_ss7_route_find_dpc_mask())) iterates the list of routes assuming they are ordered by mask length (so it doesn't need to iterate the full list every time). Similary, it implicitly seems to assume that the entries are also sorted by priority. However, priority is not currently taken into account when being inserted in the routing table, so the route ends up in the incorrect place. The VTY "update route" command used to first insert the route and only *after* that tweak some of its fields, like priority or qos-class. This commit adds a new set of osmo_ss7_route API which allows users to *first* allocate a route and configure it completelly, and *second* insert it once it contains all the information. This way, the insert algorithm can take into account the configured route priority and place the route entry in the routing table at the proper place. Related: SYS#7112 Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218 --- M TODO-RELEASE M include/osmocom/sigtran/osmo_ss7.h M src/osmo_ss7.c M src/osmo_ss7_vty.c M tests/vty/osmo_stp_route_prio.vty 5 files changed, 200 insertions(+), 55 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/75/38375/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218 Gerrit-Change-Number: 38375 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de>

1 0

[L] Change in libosmo-sigtran[master]: ss7 vty: Fix insert order of routes with higher priority
by Jenkins Builder 09 Oct '24

09 Oct '24

Jenkins Builder has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email ) Change subject: ss7 vty: Fix insert order of routes with higher priority ...................................................................... Patch Set 1: (2 comments) File src/osmo_ss7.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-19108): https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375/comment/72d85dc6_86140… : PS1, Line 849: if (!rt->cfg.linkset_name) { braces {} are not necessary for single statement blocks Robot Comment from checkpatch (run ID jenkins-gerrit-lint-19108): https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375/comment/8cb42d66_e4ed9… : PS1, Line 916: if (ss7_route_inserted(rt)) { suspect code indent for conditional statements (8, 24) -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218 Gerrit-Change-Number: 38375 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Wed, 09 Oct 2024 18:04:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in libosmo-sigtran[master]: ss7 vty: Fix insert order of routes with higher priority
by pespin 09 Oct '24

09 Oct '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email ) Change subject: ss7 vty: Fix insert order of routes with higher priority ...................................................................... ss7 vty: Fix insert order of routes with higher priority Route lookup ((hmrt_message_for_routing() => osmo_ss7_route_lookup() => osmo_ss7_route_find_dpc_mask())) iterates the list of routes assuming they are ordered by mask length (so it doesn't need to iterate the full list every time). Similary, it implicitly seems to assume that the entries are also sorted by priority. However, priority is not currently taken into account when being inserted in the routing table, so the route ends up in the incorrect place. The VTY "update route" command used to first insert the route and only *after* that tweak some of its fields, like priority or qos-class. This commit adds a new set of osmo_ss7_route API which allows users to *first* allocate a route and configure it completelly, and *second* insert it once it contains all the information. This way, the insert algorithm can take into account the configured route priority and place the route entry in the routing table at the proper place. Related: SYS#7112 Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218 --- M TODO-RELEASE M include/osmocom/sigtran/osmo_ss7.h M src/osmo_ss7.c M src/osmo_ss7_vty.c M tests/vty/osmo_stp_route_prio.vty 5 files changed, 199 insertions(+), 55 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/75/38375/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index 935cda8..410bb81 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -8,3 +8,4 @@ # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line libosmo-sigtran API change struct osmo_ss7_instance has new member 'secondary_pc' +libosmo-sigtran new API osmo_ss7_route_alloc,osmo_ss7_route_set_linkset,osmo_ss7_route_insert diff --git a/include/osmocom/sigtran/osmo_ss7.h b/include/osmocom/sigtran/osmo_ss7.h index 049c7d8..d525dd9 100644 --- a/include/osmocom/sigtran/osmo_ss7.h +++ b/include/osmocom/sigtran/osmo_ss7.h @@ -230,6 +230,8 @@ }; struct osmo_ss7_route * +osmo_ss7_route_alloc(struct osmo_ss7_route_table *rtbl, uint32_t pc, uint32_t mask); +struct osmo_ss7_route * osmo_ss7_route_find_dpc(struct osmo_ss7_route_table *rtbl, uint32_t dpc); struct osmo_ss7_route * osmo_ss7_route_find_dpc_mask(struct osmo_ss7_route_table *rtbl, uint32_t dpc, @@ -238,7 +240,9 @@ osmo_ss7_route_lookup(struct osmo_ss7_instance *inst, uint32_t dpc); struct osmo_ss7_route * osmo_ss7_route_create(struct osmo_ss7_route_table *rtbl, uint32_t dpc, - uint32_t mask, const char *linkset_name); + uint32_t mask, const char *linkset_name); +int osmo_ss7_route_set_linkset(struct osmo_ss7_route *rt, const char *linkset_name); +int osmo_ss7_route_insert(struct osmo_ss7_route *rt); void osmo_ss7_route_destroy(struct osmo_ss7_route *rt); const char *osmo_ss7_route_print(const struct osmo_ss7_route *rt); const char *osmo_ss7_route_name(struct osmo_ss7_route *rt, bool list_asps); diff --git a/src/osmo_ss7.c b/src/osmo_ss7.c index 55d3d02..cd479a6 100644 --- a/src/osmo_ss7.c +++ b/src/osmo_ss7.c @@ -657,6 +657,100 @@ * SS7 Routes ***********************************************************************/ +/*! \brief Allocate a route entry + * \param[in] rtbl Routing Table where the route belongs + * \param[in] pc Point Code of the destination of the route + * \param[in] mask Mask of the destination Point Code \ref pc + * \returns Allocated route (noy yet insertd into its rtbl), NULL on error + * + * The returned route has no linkset associated yet, user *must* associate it + * using API osmo_ss7_route_set_linkset() before inserting the route into its + * routing table. + * + * Fields priority and qos_class may be set *before* inserting the route into + * its routing table: + * - A default priority of 0 is configured on the route. + * - A default qos-class of 0 is configured on the route. + * + * Use API osmo_ss7_route_insert() to insert the route into its routing table. + * + * The route entry allocated with this API can be destroyed/freed at any point using API + * osmo_ss7_route_destroy(), regardless of it being already insertd or not in + * its routing table. + */ +struct osmo_ss7_route * +osmo_ss7_route_alloc(struct osmo_ss7_route_table *rtbl, uint32_t pc, uint32_t mask) +{ + struct osmo_ss7_route *rt; + + OSMO_ASSERT(ss7_initialized); + + rt = talloc_zero(rtbl, struct osmo_ss7_route); + if (!rt) + return NULL; + + /* Mark it as not being inserted yet in rtbl */ + INIT_LLIST_HEAD(&rt->list); + rt->rtable = rtbl; + /* truncate mask to maximum. Let's avoid callers specifying arbitrary large + * masks to ensure we don't fail duplicate detection with longer mask lengths */ + rt->cfg.mask = osmo_ss7_pc_normalize(&rtbl->inst->cfg.pc_fmt, mask); + rt->cfg.pc = osmo_ss7_pc_normalize(&rtbl->inst->cfg.pc_fmt, pc); + return rt; +} + +/*! \brief Check whether route has already been inserted into its routing table. + * \returns true if already inserted, false if not. + */ +static bool ss7_route_inserted(const struct osmo_ss7_route *rt) +{ + return !llist_empty(&rt->list); +} + +/*! \brief Set linkset on route entry + * \param[in] rt Route to be configured + * \param[in] linkset_name string name of the linkset to be used + * \returns 0 on success, negative on error. + */ +int +osmo_ss7_route_set_linkset(struct osmo_ss7_route *rt, const char *linkset_name) +{ + struct osmo_ss7_linkset *lset; + struct osmo_ss7_as *as = NULL; + struct osmo_ss7_route_table *rtbl = rt->rtable; + + if (rt->cfg.linkset_name) { + LOGSS7(rtbl->inst, LOGL_ERROR, "Attempt setting linkset on route already configured!\n"); + return -EBUSY; + } + + if (ss7_route_inserted(rt)) { + LOGSS7(rtbl->inst, LOGL_ERROR, "Attempt setting linkset on route already in the routing table!\n"); + return -EALREADY; + } + + lset = osmo_ss7_linkset_find_by_name(rtbl->inst, linkset_name); + if (!lset) { + as = osmo_ss7_as_find_by_name(rtbl->inst, linkset_name); + if (!as) + return -ENODEV; + } + + rt->cfg.linkset_name = talloc_strdup(rt, linkset_name); + if (lset) { + rt->dest.linkset = lset; + LOGSS7(rtbl->inst, LOGL_INFO, "Creating route: pc=%u=%s mask=0x%x via linkset '%s'\n", + rt->cfg.pc, osmo_ss7_pointcode_print(rtbl->inst, rt->cfg.pc), + rt->cfg.mask, lset->cfg.name); + } else { + rt->dest.as = as; + LOGSS7(rtbl->inst, LOGL_INFO, "Creating route: pc=%u=%s mask=0x%x via AS '%s'\n", + rt->cfg.pc, osmo_ss7_pointcode_print(rtbl->inst, rt->cfg.pc), + rt->cfg.mask, as->cfg.name); + } + return 0; +} + /*! \brief Find a SS7 route for given destination point code in given table */ struct osmo_ss7_route * osmo_ss7_route_find_dpc(struct osmo_ss7_route_table *rtbl, uint32_t dpc) @@ -709,6 +803,7 @@ /* insert the route in the ordered list of routes. The list is sorted by * mask length, so that the more specific (longer mask) routes are * first, while the less specific routes with shorter masks are last. + * Within the same mask length, the routes are ordered by priority. * Hence, the first matching route in a linear iteration is the most * specific match. */ static void route_insert_sorted(struct osmo_ss7_route_table *rtbl, @@ -717,73 +812,93 @@ struct osmo_ss7_route *rt; llist_for_each_entry(rt, &rtbl->routes, list) { + if (rt->cfg.mask == cmp->cfg.mask && + rt->cfg.priority > cmp->cfg.priority) { + /* insert before the current entry */ + llist_add(&cmp->list, rt->list.prev); + return; + } if (rt->cfg.mask < cmp->cfg.mask) { /* insert before the current entry */ llist_add(&cmp->list, rt->list.prev); return; } } - /* not added, i.e. no smaller mask length found: we are the - * smallest mask and thus should go last */ + /* not added, i.e. no smaller mask length and priority found: we are the + * smallest mask and priority and thus should go last */ llist_add_tail(&cmp->list, &rtbl->routes); } +/*! \brief Insert route into its routing table + * \param[in] rt Route to be inserted into its routing table + * \returns 0 on success, negative on error + * + * A route is only really used once it has been inserted into its routing table. + */ +int +osmo_ss7_route_insert(struct osmo_ss7_route *rt) +{ + struct osmo_ss7_route *prev_rt; + struct osmo_ss7_route_table *rtbl = rt->rtable; + + if (ss7_route_inserted(rt)) { + LOGSS7(rtbl->inst, LOGL_ERROR, "Attempt insert of route already in the routing table!\n"); + return -EALREADY; + } + + if (!rt->cfg.linkset_name) { + return -EINVAL; + } + + /* check for duplicates */ + prev_rt = osmo_ss7_route_find_dpc_mask(rtbl, rt->cfg.pc, rt->cfg.mask); + if (prev_rt && !strcmp(prev_rt->cfg.linkset_name, rt->cfg.linkset_name)) { + LOGSS7(rtbl->inst, LOGL_ERROR, "Refusing to create duplicate route: " + "pc=%u=%s mask=0x%x via linkset/AS '%s'\n", + rt->cfg.pc, osmo_ss7_pointcode_print(rtbl->inst, rt->cfg.pc), + rt->cfg.mask, rt->cfg.linkset_name); + return -EADDRINUSE; + } + + route_insert_sorted(rtbl, rt); + return 0; +} + /*! \brief Create a new route in the given routing table * \param[in] rtbl Routing Table in which the route is to be created * \param[in] pc Point Code of the destination of the route * \param[in] mask Mask of the destination Point Code \ref pc * \param[in] linkset_name string name of the linkset to be used - * \returns caller-allocated + initialized route, NULL on error + * \returns callee-allocated + initialized route, NULL on error + * + * The route allocated and returned by this API is already inserted into the + * routing table, with priority and qos-class set to 0. + * If you plan to use different values for priority and qos-class, avoid using + * this API and use osmo_ss7_route_alloc() + osmo_ss7_route_set_linkset() + + * osmo_ss7_route_insert() instead. */ struct osmo_ss7_route * osmo_ss7_route_create(struct osmo_ss7_route_table *rtbl, uint32_t pc, uint32_t mask, const char *linkset_name) { struct osmo_ss7_route *rt; - struct osmo_ss7_linkset *lset; - struct osmo_ss7_as *as = NULL; + int rc; - /* truncate mask to maximum. Let's avoid callers specifying arbitrary large - * masks to ensure we don't fail duplicate detection with longer mask lengths */ - mask = osmo_ss7_pc_normalize(&rtbl->inst->cfg.pc_fmt, mask); - pc = osmo_ss7_pc_normalize(&rtbl->inst->cfg.pc_fmt, pc); - - OSMO_ASSERT(ss7_initialized); - lset = osmo_ss7_linkset_find_by_name(rtbl->inst, linkset_name); - if (!lset) { - as = osmo_ss7_as_find_by_name(rtbl->inst, linkset_name); - if (!as) - return NULL; - } - - /* check for duplicates */ - rt = osmo_ss7_route_find_dpc_mask(rtbl, pc, mask); - if (rt && !strcmp(rt->cfg.linkset_name, linkset_name)) { - LOGSS7(rtbl->inst, LOGL_ERROR, "Refusing to create duplicate route: " - "pc=%u=%s mask=0x%x via linkset/AS '%s'\n", - pc, osmo_ss7_pointcode_print(rtbl->inst, pc), mask, linkset_name); - return rt; - } - - rt = talloc_zero(rtbl, struct osmo_ss7_route); + rt = osmo_ss7_route_alloc(rtbl, pc, mask); if (!rt) return NULL; - rt->cfg.pc = pc; - rt->cfg.mask = mask; - rt->cfg.linkset_name = talloc_strdup(rt, linkset_name); - if (lset) { - rt->dest.linkset = lset; - LOGSS7(rtbl->inst, LOGL_INFO, "Creating route: pc=%u=%s mask=0x%x via linkset '%s'\n", - pc, osmo_ss7_pointcode_print(rtbl->inst, pc), mask, lset->cfg.name); - } else { - rt->dest.as = as; - LOGSS7(rtbl->inst, LOGL_INFO, "Creating route: pc=%u=%s mask=0x%x via AS '%s'\n", - pc, osmo_ss7_pointcode_print(rtbl->inst, pc), mask, as->cfg.name); + if (osmo_ss7_route_set_linkset(rt, linkset_name) < 0) { + talloc_free(rt); + return NULL; } - rt->rtable = rtbl; - route_insert_sorted(rtbl, rt); + rc = osmo_ss7_route_insert(rt); + /* Keep old behavior, return already existing route: */ + if (rc == -EADDRINUSE) { + talloc_free(rt); + return osmo_ss7_route_find_dpc_mask(rtbl, rt->cfg.pc, rt->cfg.mask); + } return rt; } @@ -795,10 +910,16 @@ OSMO_ASSERT(ss7_initialized); - LOGSS7(rtbl->inst, LOGL_INFO, "Destroying route: pc=%u=%s mask=0x%x via linkset/ASP '%s'\n", - rt->cfg.pc, osmo_ss7_pointcode_print(rtbl->inst, rt->cfg.pc), rt->cfg.mask, rt->cfg.linkset_name); + if (!rt) + return; - llist_del(&rt->list); + if (ss7_route_inserted(rt)) { + LOGSS7(rtbl->inst, LOGL_INFO, + "Destroying route: pc=%u=%s mask=0x%x via linkset/ASP '%s'\n", + rt->cfg.pc, osmo_ss7_pointcode_print(rtbl->inst, rt->cfg.pc), + rt->cfg.mask, rt->cfg.linkset_name); + llist_del(&rt->list); + } talloc_free(rt); } diff --git a/src/osmo_ss7_vty.c b/src/osmo_ss7_vty.c index 0dd8a58..01eafff 100644 --- a/src/osmo_ss7_vty.c +++ b/src/osmo_ss7_vty.c @@ -361,6 +361,7 @@ int mask = osmo_ss7_pointcode_parse_mask_or_len(rtable->inst, argv[1]); const char *ls_name = argv[2]; unsigned int argind; + int rc; if (dpc < 0) { vty_out(vty, "%% Invalid point code (%s)%s", argv[0], VTY_NEWLINE); @@ -372,16 +373,9 @@ return CMD_WARNING; } - rt = osmo_ss7_route_create(rtable, dpc, mask, ls_name); - if (!rt) { - vty_out(vty, "%% Cannot create route %s/%s to %s%s", - argv[0], argv[1], argv[2], VTY_NEWLINE); - return CMD_WARNING; - } - switch (argc) { case 3: - return CMD_SUCCESS; + break; /* Continue below */ case 5: if (strcmp(argv[3], "priority") != 0 && strcmp(argv[3], "qos-class") != 0) @@ -397,6 +391,17 @@ return CMD_WARNING; } + rt = osmo_ss7_route_alloc(rtable, dpc, mask); + if (!rt) { + vty_out(vty, "%% Cannot allocate new route%s", VTY_NEWLINE); + return CMD_WARNING; + } + + if ((rc = osmo_ss7_route_set_linkset(rt, ls_name)) < 0) { + vty_out(vty, "%% Cannot find linkset %s%s", ls_name, VTY_NEWLINE); + goto destroy_warning; + } + argind = 3; if (argc > argind && !strcmp(argv[argind], "priority")) { argind++; @@ -408,7 +413,20 @@ rt->cfg.qos_class = atoi(argv[argind++]); } + if ((rc = osmo_ss7_route_insert(rt)) < 0) { + char buf_err[128]; + strerror_r(-rc, buf_err, sizeof(buf_err)); + vty_out(vty, "%% Cannot insert route %s/%s to %s: %s (%d)%s", + argv[0], argv[1], argv[2], + buf_err, rc, VTY_NEWLINE); + goto destroy_warning; + } + return CMD_SUCCESS; + +destroy_warning: + osmo_ss7_route_destroy(rt); + return CMD_WARNING; } DEFUN_ATTR(cs7_rt_rem, cs7_rt_rem_cmd, diff --git a/tests/vty/osmo_stp_route_prio.vty b/tests/vty/osmo_stp_route_prio.vty index 178f9be..90f9b2e 100644 --- a/tests/vty/osmo_stp_route_prio.vty +++ b/tests/vty/osmo_stp_route_prio.vty @@ -74,9 +74,9 @@ cs7 instance 0 ... route-table system - update route 3.2.1 7.255.7 linkset as1 priority 5 update route 3.2.1 7.255.7 linkset as3 update route 3.2.1 7.255.7 linkset as2 priority 2 + update route 3.2.1 7.255.7 linkset as1 priority 5 ... OsmoSTP(config-cs7-rt)# do show cs7 instance 0 route Routing table = system @@ -84,6 +84,6 @@ Destination C Q P Linkset Name Linkset Non-adj Route ---------------------- - - - ------------------- ------- ------- ------- -3.2.1/14 5 as1 ? ? ? 3.2.1/14 0 as3 ? ? ? 3.2.1/14 2 as2 ? ? ? +3.2.1/14 5 as1 ? ? ? -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218 Gerrit-Change-Number: 38375 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[M] Change in libosmo-sigtran[master]: tests/vty: Show bug inserting routes with different priority
by pespin 09 Oct '24

09 Oct '24

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38374?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: tests/vty: Show bug inserting routes with different priority ...................................................................... tests/vty: Show bug inserting routes with different priority route lookup ((hmrt_message_for_routing() => osmo_ss7_route_lookup() => osmo_ss7_route_find_dpc_mask())) iterates the list of routes assuming they are ordered by mask length (so it doesn't need to iterate the full list every time). Similary, it implicitly seems to assume that the entries are also sorted by priority. However, priority is not currently taken into account when being inserted in the routing table, so the route ends up in the incorrect place. This commit adds a VTY test showing the wrong insert. Related: SYS#7112 Change-Id: I125046fcbc7563c3275f42ff9de2aa351ae616c1 --- M tests/vty/Makefile.am A tests/vty/osmo_stp_route_prio.vty 2 files changed, 90 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/74/38374/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38374?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I125046fcbc7563c3275f42ff9de2aa351ae616c1 Gerrit-Change-Number: 38374 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder

1 0

[S] Change in libosmo-sigtran[master]: tests/vty/Makefile.am: Allow testing with specific vty file
by pespin 09 Oct '24

09 Oct '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38373?usp=email ) Change subject: tests/vty/Makefile.am: Allow testing with specific vty file ...................................................................... tests/vty/Makefile.am: Allow testing with specific vty file Change-Id: Ia0d53007334aaa3df35d1c821c520af4952b7843 --- M tests/vty/Makefile.am 1 file changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/73/38373/1 diff --git a/tests/vty/Makefile.am b/tests/vty/Makefile.am index 3d35c25..4157bcb 100644 --- a/tests/vty/Makefile.am +++ b/tests/vty/Makefile.am @@ -28,17 +28,23 @@ vty-python-test: $(top_builddir)/stp/osmo-stp $(srcdir)/vty_test_runner.py -w $(abs_top_builddir) -v +# Run a specific test with: 'make vty-test VTY_TEST_STP=osmo_stp_test.vty' +VTY_TEST_STP ?= osmo_stp_*.vty + vty-transcript-test-stp: $(top_builddir)/stp/osmo-stp osmo_verify_transcript_vty.py -v \ -n OsmoSTP -p 4239 \ -r "$(top_builddir)/stp/osmo-stp -c $(top_srcdir)/doc/examples/osmo-stp-multihome.cfg" \ - $(U) $(srcdir)/osmo_stp_*.vty + $(U) $(srcdir)/$(VTY_TEST_STP) + +# Run a specific test with: 'make vty-test VTY_TEST_ASP=ss7_asp_test.vty' +VTY_TEST_ASP ?= ss7_asp_*.vty vty-transcript-test-asp: ss7_asp_vty_test osmo_verify_transcript_vty.py -v \ -p 42043 \ -r "$(builddir)/ss7_asp_vty_test" \ - $(U) $(srcdir)/ss7_asp_*.vty + $(U) $(srcdir)/$(VTY_TEST_ASP) # To update the VTY script from current application behavior, # pass -u to osmo_verify_transcript_vty.py by doing: -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38373?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: Ia0d53007334aaa3df35d1c821c520af4952b7843 Gerrit-Change-Number: 38373 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[M] Change in libosmo-sigtran[master]: tests/vty: Show bug inserting routes with different priority
by pespin 09 Oct '24

09 Oct '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38374?usp=email ) Change subject: tests/vty: Show bug inserting routes with different priority ...................................................................... tests/vty: Show bug inserting routes with different priority route lookup ((hmrt_message_for_routing() => osmo_ss7_route_lookup() => osmo_ss7_route_find_dpc_mask())) iterates the list of routes assuming they are ordered by mask length (so it doesn't need to iterate the full list every time). Similary, it implicitly seems to assume that the entries are also sorted by priority. However, priority is not currently taken into account when being inserted in the routing table, so the route ends up in the incorrect place. This commit adds a VTY test showing the wrong insert. Related: SYS#7112 Change-Id: I125046fcbc7563c3275f42ff9de2aa351ae616c1 --- M tests/vty/Makefile.am A tests/vty/osmo_stp_route_prio.vty 2 files changed, 71 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/74/38374/1 diff --git a/tests/vty/Makefile.am b/tests/vty/Makefile.am index 4157bcb..4b09f6d 100644 --- a/tests/vty/Makefile.am +++ b/tests/vty/Makefile.am @@ -7,6 +7,7 @@ EXTRA_DIST = \ osmo_stp_test.vty \ + osmo_stp_route_prio.vty \ ss7_asp_test.vty \ vty_test_runner.py \ $(NULL) diff --git a/tests/vty/osmo_stp_route_prio.vty b/tests/vty/osmo_stp_route_prio.vty new file mode 100644 index 0000000..661b3ad --- /dev/null +++ b/tests/vty/osmo_stp_route_prio.vty @@ -0,0 +1,70 @@ +OsmoSTP> enable +OsmoSTP# configure terminal +OsmoSTP(config)# cs7 instance 0 + +OsmoSTP(config-cs7)# ! CREATE 2 AS with 1 ASP EACH: +OsmoSTP(config-cs7)# asp asp1 2905 54321 m3ua +OsmoSTP(config-cs7-asp)# remote-ip 127.0.0.100 +OsmoSTP(config-cs7-asp)# local-ip 27.0.0.1 +OsmoSTP(config-cs7-asp)# exit +OsmoSTP(config-cs7)# asp asp2 2905 54321 m3ua +OsmoSTP(config-cs7-asp)# remote-ip 127.0.0.200 +OsmoSTP(config-cs7-asp)# local-ip 27.0.0.2 +OsmoSTP(config-cs7-asp)# exit +OsmoSTP(config-cs7)# as as1 m3ua +OsmoSTP(config-cs7-as)# asp asp1 +OsmoSTP(config-cs7-as)# routing-key 56 0.1.2 +OsmoSTP(config-cs7-as)# exit +OsmoSTP(config-cs7)# as as2 m3ua +OsmoSTP(config-cs7-as)# asp asp2 +OsmoSTP(config-cs7-as)# routing-key 57 0.2.4 +OsmoSTP(config-cs7-as)# exit +OsmoSTP(config-cs7)# show running-config +... +cs7 instance 0 + xua rkm routing-key-allocation dynamic-permitted + asp asp1 2905 54321 m3ua + local-ip 27.0.0.1 + remote-ip 127.0.0.100 + role sg + sctp-role server + asp asp2 2905 54321 m3ua + local-ip 27.0.0.2 + remote-ip 127.0.0.200 + role sg + sctp-role server + as as1 m3ua + asp asp1 + routing-key 56 0.1.2 + as as2 m3ua + asp asp2 + routing-key 57 0.2.4 + route-table system + listen m3ua 2905 + local-ip 127.0.0.2 + local-ip 127.0.0.1 + local-ip ::1 + accept-asp-connections dynamic-permitted +end + +OsmoSTP(config-cs7)# ! ADD 2 ROUTES WITH DECREASING PRIORITIES. THEY SHOULD END UP REORDERED BY PRIORITY: +OsmoSTP(config-cs7)# route-table system +OsmoSTP(config-cs7-rt)# update route 3.2.1 7.255.7 linkset as1 priority 5 +OsmoSTP(config-cs7-rt)# update route 3.2.1 7.255.7 linkset as2 priority 2 +OsmoSTP(config-cs7-rt)# ! FIXME! THIS IS WRONG ORDER: +OsmoSTP(config-cs7-rt)# show running-config +... +cs7 instance 0 +... + route-table system + update route 3.2.1 7.255.7 linkset as1 priority 5 + update route 3.2.1 7.255.7 linkset as2 priority 2 +... +OsmoSTP(config-cs7-rt)# do show cs7 instance 0 route +Routing table = system +C=Cong Q=QoS P=Prio + +Destination C Q P Linkset Name Linkset Non-adj Route +---------------------- - - - ------------------- ------- ------- ------- +3.2.1/14 5 as1 ? ? ? +3.2.1/14 2 as2 ? ? ? -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38374?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I125046fcbc7563c3275f42ff9de2aa351ae616c1 Gerrit-Change-Number: 38374 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0