ospf6d: some cleaning #10315

ckishimo · 2022-01-10T14:25:50Z

Some cleaning mostly in the NSSA part. See individual commits

2f2bf3a - ospf6d: fix typos
0341564 - ospf6d: use _func_
8eaf7c4 - ospf6d: remove duplicated code
0c70068 - ospf6d: remove double htons
b80cebb - ospf6d: remove duplicated log

When running topotest ospf6_topo2 we can see a log message with wrong lsa id 2021/12/20 23:14:40.330 OSPF6: [V8P0C-HB5Z2] ASBR[default:Status:3]: Update 2021/12/20 23:14:40.330 OSPF6: [Z489N-JAJ6P] ASBR[default:Status:3]: Already ASBR 2021/12/20 23:14:40.330 OSPF6: [Z9D0B-12SBJ] Redistribute 2001:db8:2::/64 (connected) 2021/12/20 23:14:40.330 OSPF6: [N66XP-ANN4G] Advertise as AS-External Id:8.70.41.177 prefix 2001:db8:2::/64 metric 2 (**) 2021/12/20 23:14:40.330 OSPF6: [K4Y9R-C22T6] Advertise new AS-External Id:0.0.0.3 prefix 2001:db8:2::/64 metric 2 2021/12/20 23:14:40.330 OSPF6: [PKX0N-KNRQR] Originate AS-External-LSA for 2001:db8:2::/64 This PR removes the log (instead of fixing it) as same information is printed in the following entry Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

NetDEF-CI · 2022-01-10T16:35:08Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2594/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings

Report for ospf6_nssa.c | 2 issues
===============================================
< WARNING: line over 80 characters
< #1024: FILE: /tmp/f1-16502/ospf6_nssa.c:1024:

idryzhov · 2022-01-10T16:39:04Z

ospf6d/ospf6_nssa.c

 			type5 = ospf6_lsdb_lookup(
-				htons(type), lsa->external_lsa_id,
+				htons(OSPF6_LSTYPE_AS_EXTERNAL), lsa->external_lsa_id,


There are a couple of missing htons in this file on lines 578 and 667. Could you fix them as well, please?

And also fix the style warnings, please.

@idryzhov style warnings and htons in line 578 were fixed. The one in 667 was already updated in a different PR #10158 that is still under review....

Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

The OSPF6_LSA_UNAPPROVED flag is set in the function above ospf6_lsa_translated_nssa_new, so there is no need to set the flag again Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

NetDEF-CI · 2022-01-10T18:58:15Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2595/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

mobash-rasool

The changes looks good but can you fix the style errors.

Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

ckishimo · 2022-01-11T10:07:36Z

sorry 🙏 just fixed I hope...thanks

NetDEF-CI · 2022-01-11T12:15:33Z

Continuous Integration Result: FAILED

Test incomplete. See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2615/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Incomplete

Topotests Ubuntu 18.04 amd64 part 8: Incomplete

(check logs for details)

Successful on other platforms/tests

Topotests debian 10 amd64 part 1
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 arm8 part 1
Topotests debian 10 amd64 part 6
Fedora 29 rpm pkg check
Topotests Ubuntu 18.04 amd64 part 7
Topotests Ubuntu 18.04 i386 part 5
Topotests Ubuntu 18.04 i386 part 0
Topotests Ubuntu 18.04 amd64 part 4
CentOS 7 rpm pkg check
Addresssanitizer topotests part 2
Topotests Ubuntu 18.04 amd64 part 0
Topotests Ubuntu 18.04 i386 part 4
Topotests Ubuntu 18.04 i386 part 9
Topotests debian 10 amd64 part 7
Debian 9 deb pkg check
Addresssanitizer topotests part 8
Topotests debian 10 amd64 part 5
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 arm8 part 8
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 arm8 part 6
Addresssanitizer topotests part 6
Topotests Ubuntu 18.04 amd64 part 1
Topotests Ubuntu 18.04 i386 part 2
Ubuntu 18.04 deb pkg check
Topotests Ubuntu 18.04 amd64 part 2
Addresssanitizer topotests part 5
Addresssanitizer topotests part 4
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 i386 part 8
IPv6 protocols on Ubuntu 18.04
Addresssanitizer topotests part 0
Topotests debian 10 amd64 part 8
Topotests debian 10 amd64 part 3
Topotests debian 10 amd64 part 4
Topotests Ubuntu 18.04 arm8 part 9
Topotests Ubuntu 18.04 arm8 part 4
Topotests Ubuntu 18.04 arm8 part 3
Topotests Ubuntu 18.04 amd64 part 3
Topotests debian 10 amd64 part 9
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 arm8 part 7
Static analyzer (clang)
IPv4 protocols on Ubuntu 18.04
Addresssanitizer topotests part 9
Topotests debian 10 amd64 part 0
IPv4 ldp protocol on Ubuntu 18.04
Ubuntu 16.04 deb pkg check
Topotests Ubuntu 18.04 arm8 part 0
Topotests Ubuntu 18.04 amd64 part 9
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 arm8 part 2
Topotests Ubuntu 18.04 i386 part 1
Debian 10 deb pkg check
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 arm8 part 5
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 amd64 part 5
Ubuntu 20.04 deb pkg check

riw777

looks kay modulo the responses to previous comments ... will wait for those to be verified

idryzhov · 2022-01-11T22:18:26Z

ci:rerun

NetDEF-CI · 2022-01-12T00:17:06Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2639/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

frrbot bot added bugfix ospfv3 labels Jan 10, 2022

idryzhov reviewed Jan 10, 2022

View reviewed changes

ckishimo added 2 commits January 10, 2022 17:57

ospf6d: remove double htons

1852968

Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

ospf6d: remove duplicated code

3a53b2a

The OSPF6_LSA_UNAPPROVED flag is set in the function above ospf6_lsa_translated_nssa_new, so there is no need to set the flag again Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

ckishimo force-pushed the ospf6d_cleaning branch from 2f2bf3a to 98ec37c Compare January 10, 2022 17:05

mobash-rasool requested changes Jan 11, 2022

View reviewed changes

ckishimo added 3 commits January 11, 2022 11:03

ospf6d: use __func__

d8da8a4

Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

ospf6d: fix typos

416721d

Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

ospf6d: add missing htons

bc36c05

Signed-off-by: ckishimo <carles.kishimoto@gmail.com>

ckishimo force-pushed the ospf6d_cleaning branch from 98ec37c to bc36c05 Compare January 11, 2022 10:04

riw777 approved these changes Jan 11, 2022

View reviewed changes

idryzhov approved these changes Jan 12, 2022

View reviewed changes

idryzhov merged commit 499841d into FRRouting:master Jan 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ospf6d: some cleaning #10315

ospf6d: some cleaning #10315

ckishimo commented Jan 10, 2022

NetDEF-CI commented Jan 10, 2022 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

idryzhov Jan 10, 2022

idryzhov Jan 10, 2022

ckishimo Jan 10, 2022

NetDEF-CI commented Jan 10, 2022 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

mobash-rasool left a comment

ckishimo commented Jan 11, 2022

NetDEF-CI commented Jan 11, 2022 •

edited

Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Incomplete

riw777 left a comment

idryzhov commented Jan 11, 2022

NetDEF-CI commented Jan 12, 2022

ospf6d: some cleaning #10315

ospf6d: some cleaning #10315

Conversation

ckishimo commented Jan 10, 2022

NetDEF-CI commented Jan 10, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

idryzhov Jan 10, 2022

Choose a reason for hiding this comment

idryzhov Jan 10, 2022

Choose a reason for hiding this comment

ckishimo Jan 10, 2022

Choose a reason for hiding this comment

NetDEF-CI commented Jan 10, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

mobash-rasool left a comment

Choose a reason for hiding this comment

ckishimo commented Jan 11, 2022

NetDEF-CI commented Jan 11, 2022 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Incomplete

riw777 left a comment

Choose a reason for hiding this comment

idryzhov commented Jan 11, 2022

NetDEF-CI commented Jan 12, 2022

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Jan 10, 2022 •

edited

Loading

NetDEF-CI commented Jan 10, 2022 •

edited

Loading

NetDEF-CI commented Jan 11, 2022 •

edited

Loading