parse_topology function doesn't correctly process vertex types with
spaces. Therefore the reference topology files are completely messed up,
we have values in incorrect fields - types in metrics, metrics in
parents, etc.
This commit fixes the parsing function and the reference files.
The same fix was done for isis-topo1-vrf in #8365.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Problem Statement:
=================
In scale setup BGP sessions start flapping.
RCA:
====
In virtualized environment there are multiple places where
MTU need to be set. If there are some places were MTU is not set
properly then there is chances that BGP packets get fragmented,
in scale setup this will lead to BGP session flap.
Fix:
====
A new tcp option is provided as part of this implementation,
which can be configured per neighbor and helps to set the TCP
max segment size. User need to derive the path MTU between the BGP
neighbors and set that value as part of tcp-mss setting.
1. CLI Configuration:
[no] neighbor <A.B.C.D|X:X::X:X|WORD> tcp-mss (1-65535)
2. Running config
frr# show running-config
router bgp 100
neighbor 198.51.100.2 tcp-mss 150 => new entry
neighbor 2001:DB8::2 tcp-mss 400 => new entry
3. Show command
frr# show bgp neighbors 198.51.100.2
BGP neighbor is 198.51.100.2, remote AS 100, local AS 100, internal link
Hostname: frr
Configured tcp-mss is 150, synced tcp-mss is 138 => new display
4. Show command json output
frr# show bgp neighbors 2001:DB8::2 json
{
"2001:DB8::2":{
"remoteAs":100,
"bgpTimerKeepAliveIntervalMsecs":60000,
"bgpTcpMssConfigured":400, => new entry
"bgpTcpMssSynced":388, => new entry
Risk:
=====
Low - This is a config driven feature and it sets the max segment
size for the TCP session between BGP peers.
Tests Executed:
===============
Have done manual testing with three router topology.
1. Executed basic config and un config scenarios
2. Verified if the config is updated in running config
during config and no config operation
3. Verified the show command output in both CLI format and
JSON format.
4. Verified if TCP SYN messages carry the max segment size
in their initial packets.
5. Verified the behaviour during clear bgp session.
6. done packet capture to see if the new segment size
takes effect.
Signed-off-by: Abhinay Ramesh <rabhinay@vmware.com>
Not having scapy in the docker image leads to very obtuse failures in
the pim bsm tests (obtuse, as in, it just fails without any hint as to
why...)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The previous method, using zassert.h and hoping nothing includes
assert.h (which, on glibc at least, just does "#undef assert" and puts
its own definition in...) was fragile - and actually broke undetected.
Just provide our own assert.h and control overriding by putting it in a
separate directory to add to the include path (or not.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Individual tests must not depend on each other. In particular, a test
can't be sure that the previous test config is applied or cleared.
It is definitely not true when a single test is executed, for example:
`test_bgp_auth.py::test_prefix_peer_remove_passwords`.
This commit makes all tests independent of each other. It also adds a
call to check_all_peers_established at the start of "remove_passwords"
tests to make sure that we not only block new peers with an incorrect
password, but also clean the existing peers.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
YANG model and CLI commands allow user to configure LDP-sync per area.
But the actual implementation is incorrect - all commands are changing
the config for the whole VRF instead of a single area. This commit fixes
this issue by actually implementing per area configuration.
Fixes#8578.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Currently we don't allow to configure the interface before the area is
configured. This approach has the following issues:
1. The area config can be deleted even when we have an interface config
relying on it. The code is not ready for that - we'll have a whole
bunch of stale pointers if user does that.
2. The code doesn't correctly process the event of changing the VRF for
an interface. There is no mechanism to ensure that the area exists
in the new VRF so currently the circuit still stays in the old VRF.
This commit allows an arbitrary order of area/interface configuration.
There is no more need to configure the area before configuring the
interface.
This change fixes both the issues.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Description:
DR information is missing under "show ip ospf interface [json]".
Added DR infomation to display in "show ip ospf interface".
Signed-off-by: Rajesh Girada <rgirada@vmware.com>
The current log prints maximum wait time which is not actually correct,
because it doesn't include the command execution time. We usually have
"failed after X seconds" log with X being far longer than this maximum.
Let's print the maximum number of tries instead.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This test is completely incorrect on test_bfd_loss_intermediate step.
It shuts down the interface and then "waiting" for the BGP session to
fail. But instead of the actual wait it compares the output of "show bfd
peers" with the "up" state. As it does this comparison right after the
interface shutdown, the BFD session has not yet failed and the comparison
is always successful except very rare cases when the command takes a lot
of time to execute (due to the heavy load on CI system I suppose).
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This function kills all processes that happen to have the same
name to frr processes and it was only ever used in the setup.
Setup should not be used to kill old runs. That should be a
separate process.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
`CFLAGS` is a "user variable", not intended to be controlled by
configure itself. Let's put all the "important" stuff in AC_CFLAGS and
only leave debug/optimization controls in CFLAGS.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This test establishes a binding between nbma ip of a spoke and its
protocol address. This information is pushed to hub.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Just another round of trying to add pytest.mark.bgpd. Not finished yet just
what I could stand doing for a few minutes.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Fixes:
/usr/lib/python3.9/site-packages/_pytest/config/__init__.py:1463: in getoption
val = getattr(self.option, name)
E AttributeError: 'Namespace' object has no attribute 'topology_only'
The above exception was the direct cause of the following exception:
/usr/lib/python3.9/site-packages/pluggy/manager.py:127: in register
hook._maybe_apply_history(hookimpl)
/usr/lib/python3.9/site-packages/pluggy/hooks.py:333: in _maybe_apply_history
res = self._hookexec(self, [method], kwargs)
/usr/lib/python3.9/site-packages/pluggy/manager.py:93: in _hookexec
return self._inner_hookexec(hook, methods, kwargs)
/usr/lib/python3.9/site-packages/pluggy/manager.py:84: in <lambda>
self._inner_hookexec = lambda hook, methods, kwargs: hook.multicall(
tests/topotests/conftest.py:62: in pytest_configure
if config.getoption("--topology-only"):
/usr/lib/python3.9/site-packages/_pytest/config/__init__.py:1474: in getoption
raise ValueError(f"no option named {name!r}") from e
E ValueError: no option named 'topology_only'
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
Description:
BGP session not established for ipv6 link local address with vrf config
Problem Description/Summary :
BGP session not established for ipv6 link local address with vrf configyy
1.Configure ipv6 link-local address fe80::1234/64 on dut1 and fe80::4567/64 on dut2
2.Configure BGP neighbors for ipv6 link-local on both dut1 and dut2
3.Verify BGP session is UP over link-local ipv6 address
4.Observed that bgp session not established for ipv6 link local address
Expected Behavior :
BGP session should be established for ipv6 link local address with vrf config
Signed-off-by: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
These are for string quoting (`%pSQ`) and string escaping (`%pSE`); the
sets / escape methods are currently rather "basic" and might be extended
in the future.
Signed-off-by: David Lamparter <equinox@diac24.net>
Analogous to Linux kernel `%pV` (but our mechanism expects 2 specifier
chars and `%pVA` is clearer anyway.)
Signed-off-by: David Lamparter <equinox@diac24.net>
... to suppress the warnings when using something that isn't quite ISO C
compatible and would otherwise cause compiler warnings from `-Wformat`.
Signed-off-by: David Lamparter <equinox@diac24.net>
Three new tests:
- OSPFv3 convergence using 'ipv6 ospf6 neighbor json'
- Default route functionality:
* Check that the LSA is present
* Check that the route was installed
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
This replaces `%n` with a safe, out-of-band option that simply records
the start and end offset of the output produced for each `%...`
specifier.
The old `%n` code is removed.
Signed-off-by: David Lamparter <equinox@diac24.net>
Allowing printfrr extensions to directly write to the output buffer has
a few advantages:
- there is no arbitrary length limit imposed (previously 64)
- the output doesn't need to be copied another time
- the extension can directly use bprintfrr() to put together pieces
The downside is that the theoretical length (regardless of available
buffer space) must be computed correctly.
Extended unit tests to test these paths a bit more thoroughly.
Signed-off-by: David Lamparter <equinox@diac24.net>
When "bgp bestpath peer-type multipath-relax" is enabled, multipaths
with both eBGP and iBGP learned routes may exist. It is not desirable
for the iBGP next hops to be discarded from the FIB because they are not
directly connected. When publishing a nexthop group to zebra, the
ZEBRA_FLAG_ALLOW_RECURSION flag is normally not set when the best path
is eBGP; when "bgp bestpath aspath multipath-relax" is configured, the
flag will now be set if any paths are from iBGP peers. This leaves
all-eBGP multipaths still requiring nexthops over connected routes.
Signed-off-by: Joanne Mikkelson <jmmikkel@arista.com>
This new BGP configuration is akin to "bgp bestpath aspath
multipath-relax". When applied, paths learned from different peer types
will be eligible to be considered for multipath (ECMP). Paths from all
of eBGP, iBGP, and confederation peers may be included in multipaths
if they are otherwise equal cost.
This change preserves the existing bestpath behavior of step 10's result
being returned, not the result from steps 8 and 9, in the case where
both 8+9 and 10 determine a winner.
Signed-off-by: Joanne Mikkelson <jmmikkel@arista.com>
This new test launches a small network composed by 4 OSPF routers with
Traffic Engineering and Segment Routing configuration. To assess the Link
State Traffic Engineering feature, the TED of each router is compared
against the reference TED which corresponds to the network topology.
Then a series of 6 steps, where Link, TE & SR configurations are modified
up to r4 shutwdown, are used to verify that the TED is correctly updated
on the 4 routers.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Changes:
- Decrease hello interval to avoid packet loss slow downs
- Decrease dead interval to converge faster
- Remove previous 'Full' state check that wasn't checking for all
peers (only one per router)
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
The previous, more complex mechanism failed to take into account that
git worktrees only have a stub .git file & copying the worktree itself
is not enough. Just extract a file list beforehand & don't bother with
git inside the container.
Signed-off-by: David Lamparter <equinox@diac24.net>
Back when I put this together in 2015, ISO C11 was still reasonably new
and we couldn't require it just yet. Without ISO C11, there is no
"good" way (only bad hacks) to require a semicolon after a macro that
ends with a function definition. And if you added one anyway, you'd get
"spurious semicolon" warnings on some compilers...
With C11, `_Static_assert()` at the end of a macro will make it so that
the semicolon is properly required, consumed, and not warned about.
Consistently requiring semicolons after "file-level" macros matches
Linux kernel coding style and helps some editors against mis-syntax'ing
these macros.
Signed-off-by: David Lamparter <equinox@diac24.net>
The following error is shown when running the OSPFv3 tests
2021-03-16 23:37:44,792 INFO: Function returned global name 'data_rid' is not defined
2021-03-16 23:37:44,792 INFO: Retry [#1] after sleeping for 2s
2021-03-16 23:37:46,794 INFO: Verifying OSPF6 neighborship on router r1:
2021-03-16 23:37:46,993 INFO: Output for command [ show ipv6 ospf6 neighbor ] on router r1:
Neighbor ID Pri DeadTime State/IfState Duration I/F[State]
2.2.2.2 1 00:00:03 Full/PointToPoint 00:00:01 r1-r2-eth0[PointToPoint]
Fix the "data_rid" warning by using the correct variable
Signed-off-by: ckishimo <carles.kishimoto@gmail.com>
Currently there is a single interval for both RX and TX echo functions.
This commit introduces separate RX and TX timers for echo packets.
The main advantage is to be able to set the receive interval to zero
when we don't want to receive echo packets from the remote system.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Additional example usage of iproute2_is_vrf_capable check in
isis-topo1-vrf topotest.
Signed-off-by: David Schweizer <dschweizer@opensourcerouting.org>
Example usage of iproute2_is_vrf_capable check in bgp_multi_vrf_topo1
and bgp_multi_vrf_topo2 topotests.
Signed-off-by: David Schweizer <dschweizer@opensourcerouting.org>
The test has been failing often recently and it is causing some false
positives for unrelated PRs.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
We version the tests with the source code so we should no longer attempt
to support old versions in development branch.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
The new ospf-sr-topo2 tests are much broader and detailed,
hence it makes no sense to keep the old ospf-sr-topo1
tests.
Signed-off-by: GalaxyGorilla <sascha@netdef.org>
1. Improved error meesage logging.
2. No functionality changes only put some meaningfull error messages.
Signed-off-by: Kuldeep Kashyap <kashyapk@vmware.com>
Avoid undocumented topotest dependency on installing en_US locale.
With this change dependency is removed.
Signed-off-by: Christian Hopps <chopps@labn.net>
Didn't test this but it's already randomly broken so cant be worse
Hopefully fixes:
raise InvalidCLIError("%s" % output)
InvalidCLIError: line 2: % Command incomplete[4]: bgp
large-community-list standard Test1 permit
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
Sleeping when convergence is not guaranteed in 60 seconds
and then testing the rib to see if it has the data is
not a great way to have a test complete all the time.
Modify the code so that we check for convergence
and if we have converged then look in the rib.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
1. There were few tests where routes were configured with blackhole and
non-blackhole nexthops simultaneously, enhanced tests accordingly and
verified in master branch and with PR #8158 changes.
Signed-off-by: Kuldeep Kashyap <kashyapk@vmware.com>
Add some pytest.mark.bgpd. This is about all I could stomach doing
in one patch. I'll do another pass at another time.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When the last SID in the TI-LFA repair list is an Adj-SID from the
penultimate hop router towards the final hop, the No-PHP flag of the
original Prefix-SID must be honored in the repair list itself since
the penultimate hop router won't have a chance to process that SID
and pop it if necessary.
Reported-by: Fredi Raspall <fredi@voltanet.io>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
