isisd: added l1 to l2 distibution #14610

Sashhkaa · 2023-10-17T14:33:38Z

added adding prefixes with the best metric from the level-1 db to the level-2 db on l1_l2 end system

Signed-off-by: Sososhas [email protected]

ton31337 · 2023-10-17T14:35:28Z

Please check frrbot (Github check) failures.

odd22

Code need some review. In addition, the code is not correctly indented. Please have a look to automatic check style result.

odd22 · 2023-10-19T17:04:02Z

isisd/isis_lsp.c

+	_lsp_regenerate_schedule(area, lsp->level, 0, false, __func__, __FILE__,
+				 __LINE__);
+


You should not use the _lsp_regenerate_schedule() function but instead the lsp_regenerate_schedule() macro. Please have a look to existing call to this function.

odd22 · 2023-10-19T17:06:00Z

isisd/isis_lsp.c

+                        struct isis_lsp* lsp_tmp_ip;
+                        struct isis_lsp* lsp_tmp_ipv6;
+                        struct lspdb_head *head_tmp_ip = &area->lspdb[0];
+                        struct lspdb_head *head_tmp_ipv6 = &area->lspdb[0];


Please use same variables for both IP and IPv6. There are temporary pointer and there is no reason to duplicate them.

riw777

looks okay ... waiting on other review comments

riw777

looks good ... waiting on other reviewers

odd22

Iteration functions over IP extended reachability should be rewritten.
All new functions should be declared as static and not publish through isis_lsp.h if there are not used in another file

odd22 · 2023-11-13T17:21:28Z

isisd/isis_lsp.h

+void iteration_in_lspdb(struct isis_area *area, struct isis_lsp *lsp);
+void adding_new_prefix(struct isis_area *area, struct isis_lsp *lsp,
+		       struct isis_lsp *lsp_tmp);
+void iteration_in_lsp_ip(struct isis_extended_ip_reach *i, struct isis_lsp *lsp,
+			 int *count);
+void iteration_in_lsp_ipv6(struct isis_ipv6_reach *i, struct isis_lsp *lsp,
+			   int *count);


This is not necessary if these functions are not used outside isis_lsp.c file. You should rather declare the functions as static in isis_lsp.c file

odd22 · 2023-11-13T17:23:08Z

isisd/isis_lsp.c

@@ -1070,6 +1071,80 @@ static struct isis_lsp *lsp_next_frag(uint8_t frag_num, struct isis_lsp *lsp0,
 * Builds the LSP data part. This func creates a new frag whenever
 * area->lsp_frag_threshold is exceeded.
 */
+void iteration_in_lsp_ip(struct isis_extended_ip_reach *r, struct isis_lsp *lsp,


It is preferable to declare a function that return the count instead of void and returning the result with a variable. In particular, in the function, you don't check if the count variable has been allocated or not i.e. if it is not NULL.

odd22 · 2023-11-13T17:23:42Z

isisd/isis_lsp.c

+		if (IPV4_ADDR_SAME(&r->prefix.prefix, &rt->prefix.prefix)) {
+			if ((r->metric > rt->metric) ||
+			    (r->metric == rt->metric))
+				*count += 1;


use a local variable instead and later use return count

odd22 · 2023-11-13T17:26:59Z

isisd/isis_lsp.c

+			if ((r->metric > rt->metric) ||
+			    (r->metric == rt->metric))
+				*count += 1;
+			break;


Here you code terminate the loop once a first occurrence has been founded. Thus, the value of count will be 0 or 1. May be returning a boolean should be sufficient if it is really the goal of this function. If the objective is to count the number of extended IP reachability TLV that have the same IPv4 prefix, you should remove the break instruction.

odd22 · 2023-11-13T17:27:25Z

isisd/isis_lsp.c

+void iteration_in_lsp_ipv6(struct isis_ipv6_reach *r, struct isis_lsp *lsp,
+			   int *count)
+{
+	for (struct isis_item *l = lsp->tlvs->ipv6_reach.head; l; l = l->next) {
+		struct isis_ipv6_reach *rt = (struct isis_ipv6_reach *)l;
+
+		if (IPV6_ADDR_SAME(&r->prefix.prefix.s6_addr,
+				   &rt->prefix.prefix.s6_addr)) {
+			if ((r->metric > rt->metric) ||
+			    (r->metric == rt->metric))
+				*count += 1;
+			break;
+		}
+	}
+}
+


Same comments as for IPv4 function

riw777 · 2024-01-16T13:38:13Z

@Sashhkaa still working on this?

github-actions · 2024-04-27T16:18:38Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

frrbot bot added the isis label Oct 17, 2023

github-actions bot added master size/M rebase PR needs rebase labels Oct 17, 2023

Sashhkaa force-pushed the multiarea_isis branch from 25f0001 to 3cac7ef Compare October 19, 2023 10:58

odd22 requested changes Oct 19, 2023

View reviewed changes

Sashhkaa force-pushed the multiarea_isis branch from 3cac7ef to 1ea5326 Compare October 19, 2023 20:30

github-actions bot added size/L and removed size/M labels Oct 19, 2023

Sashhkaa force-pushed the multiarea_isis branch from 1ea5326 to e398825 Compare October 19, 2023 20:48

github-actions bot added size/M and removed size/L labels Oct 19, 2023

Sashhkaa force-pushed the multiarea_isis branch 3 times, most recently from fcfbe69 to eb4217b Compare October 23, 2023 13:05

Sashhkaa requested a review from odd22 October 23, 2023 13:13

Sashhkaa force-pushed the multiarea_isis branch 3 times, most recently from 79b41ad to c15c161 Compare October 24, 2023 09:37

github-actions bot added size/XL and removed size/M labels Oct 24, 2023

riw777 reviewed Oct 24, 2023

View reviewed changes

Sashhkaa force-pushed the multiarea_isis branch 2 times, most recently from 6d25240 to be16d01 Compare October 24, 2023 11:58

frrbot bot added bgp documentation labels Oct 24, 2023

github-actions bot added size/L and removed size/XL labels Oct 24, 2023

Sashhkaa force-pushed the multiarea_isis branch from fc794ee to 70a24a1 Compare October 24, 2023 12:01

Sashhkaa force-pushed the multiarea_isis branch from 70a24a1 to 97a869a Compare October 24, 2023 12:06

github-actions bot added size/M and removed size/L labels Oct 24, 2023

Sashhkaa force-pushed the multiarea_isis branch 7 times, most recently from 74c32fe to 252fdb4 Compare October 26, 2023 03:09

github-actions bot added size/L and removed size/M labels Oct 26, 2023

Sashhkaa force-pushed the multiarea_isis branch from 252fdb4 to dfde882 Compare October 26, 2023 03:17

github-actions bot added size/M and removed size/L labels Oct 26, 2023

riw777 approved these changes Nov 7, 2023

View reviewed changes

odd22 requested changes Nov 13, 2023

View reviewed changes

squirrelking57 force-pushed the multiarea_isis branch from dfde882 to 5cb2fc2 Compare April 27, 2024 15:58

github-actions bot added size/XXL and removed size/M labels Apr 27, 2024

squirrelking57 deleted the multiarea_isis branch April 27, 2024 16:00

squirrelking57 restored the multiarea_isis branch April 27, 2024 16:01

squirrelking57 force-pushed the multiarea_isis branch from 5cb2fc2 to 8cfa3b5 Compare April 27, 2024 16:18

github-actions bot added conflicts size/XS and removed size/XXL labels Apr 27, 2024

ton31337 merged commit 48805d6 into FRRouting:master Apr 27, 2024
32 of 33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

isisd: added l1 to l2 distibution #14610

isisd: added l1 to l2 distibution #14610

Sashhkaa commented Oct 17, 2023

ton31337 commented Oct 17, 2023

odd22 left a comment

odd22 Oct 19, 2023

odd22 Oct 19, 2023

riw777 left a comment •

edited

Loading

riw777 left a comment

odd22 left a comment

odd22 Nov 13, 2023

odd22 Nov 13, 2023

odd22 Nov 13, 2023

odd22 Nov 13, 2023

odd22 Nov 13, 2023

riw777 commented Jan 16, 2024

github-actions bot commented Apr 27, 2024

		_lsp_regenerate_schedule(area, lsp->level, 0, false, __func__, __FILE__,
		__LINE__);

isisd: added l1 to l2 distibution #14610

isisd: added l1 to l2 distibution #14610

Conversation

Sashhkaa commented Oct 17, 2023

ton31337 commented Oct 17, 2023

odd22 left a comment

Choose a reason for hiding this comment

odd22 Oct 19, 2023

Choose a reason for hiding this comment

odd22 Oct 19, 2023

Choose a reason for hiding this comment

riw777 left a comment • edited Loading

Choose a reason for hiding this comment

riw777 left a comment

Choose a reason for hiding this comment

odd22 left a comment

Choose a reason for hiding this comment

odd22 Nov 13, 2023

Choose a reason for hiding this comment

odd22 Nov 13, 2023

Choose a reason for hiding this comment

odd22 Nov 13, 2023

Choose a reason for hiding this comment

odd22 Nov 13, 2023

Choose a reason for hiding this comment

odd22 Nov 13, 2023

Choose a reason for hiding this comment

riw777 commented Jan 16, 2024

github-actions bot commented Apr 27, 2024

riw777 left a comment •

edited

Loading