WIP: Record `no bgp no-rib` override of implicit `-n` when using `-l`

[KrisShannon]

We are running the bgpd daemon with -l BIND_IP options but we don't want the implicit -n behaviour.

Whenever we make a change and write memory the config it will lose the required no bgp no-rib command.

This is probably not the correct answer to the problem but it works for us.

[NetDEF-CI]

Continuous Integration Result: SUCCESSFUL

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-6673/

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.

[ton31337]

I'm not sure what the reason was for implicitly adding NO_RIB when specifying listen addresses... Maybe we can disable this behavior? @donaldsharp any thoughts?

		case 'l':
			listnode_add_sort_nodup(addresses, optarg);
		/* listenon implies -n */
		/* fallthru */
		case 'n':
			no_fib_flag = 1;
			break;

[donaldsharp]

well let's actually get this PR against master first then I'll worry about it. It's a no go from me going staight into 8.3

[ton31337]

@KrisShannon could you create a PR against the master branch?

[donaldsharp]

my recollection is that listeon on implied a route reflector and thus no need to communicate routes to zebra

[NetDEF-CI]

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

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

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: Failed

Checkout code: Failed (click for details)

PullReq merge failed. Please rebase your branch: see merge log in attachment https://ci1.netdef.org/browse/FRR-PULLREQ2-6734/artifact/CHECKOUT/ErrorLog/log_merge.txt

[KrisShannon]

Hi,

I've updated to master and formatted the commit message according to the guidelines.

I don't actually think this is really the right solution, it's basically just the hack we're using ourselves.

Possibly the right answer is to instead remove the implicit fallthrough and document it as a breaking change.

[KrisShannon]

I'm guessing my drop and run commit against stable confused frrbot.

I think the only labels which are really relevant are bgp and vtysh

[LabN-CI]

Outdated results 🚧

Basic BGPD CI results: Partial FAILURE, 1 tests failed

_	_
Result	SUCCESS git merge/11688 0d75510c
Date	07/31/2022
Start	22:21:19
Finish	22:47:51
Run-Time	26:32
Total	1816
Pass	1815
Fail	1
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2022-07-31-22:21:19.txt
Log	autoscript-2022-07-31-22:22:36.log.bz2
Memory	554 546 458

For details, please contact louberger

[NetDEF-CI]

Continuous Integration Result: SUCCESSFUL

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-6736/

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.

[NetDEF-CI]

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-6735/

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.

[donaldsharp]

During today's tech meeting this was discussed and the general consensus is that -l should not imply the -n behavior anymore. All in all this patch is ok with that but if @KrisShannon you would like to introduce a patch that actually removes the fallthrough that woudl be ok too

[donaldsharp]

@louberger -> I don't see anything that should cause your CI to fail, can you help us understand what went wrong?

[louberger]

@louberger -> I don't see anything that should cause your CI to fail, can you help us understand what went wrong?

agree - rerunning

[ton31337]

This is already not relevant, this behavior is fixed here https://github.com/FRRouting/frr/pull/11769?