summaryrefslogtreecommitdiffstats
path: root/bfdd (follow)
Commit message (Collapse)AuthorAgeFilesLines
...
* *: Create and use infrastructure to show debugs in libDonald Sharp2022-10-071-0/+2
| | | | | | | | | There are lib debugs being set but never show up in `show debug` commands because there was no way to show that they were being used. Add a bit of infrastructure to allow this and then use it for `debug route-map` Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bfdd: changes for code maintainabilitysri-mohan12022-09-052-20/+20
| | | | | | these changes are for improving the code maintainability Signed-off-by: sri-mohan1 <sri.mohan@samsung.com>
* Merge pull request #11668 from rampxxxx/bfd_rtt_in_echo_pktRafael Zalamena2022-08-094-11/+103
|\ | | | | BFDD: Add RTT to BFD IPV4 Echo packet processing
| * BFDD: Add RTT to BFD IPV4 Echo packet processinglynnemorrison2022-08-024-11/+103
| | | | | | | | | | | | | | | | | | | | Add a send time into the BFD Echo packet. When the BFD Echo packet is received back store time it took in usec. When user issues a show bfd peer(s) command calculate and display minimum, average, and max time it took for the BFD Echo packet to be looped back. Signed-off-by: Lynne Morrison <lynne.morrison@ibm.com>
* | bfdd: Some interfaces don't have mac addressesDonald Sharp2022-08-061-2/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When an interface does not have a mac address, don't try to retrieve the mac address ( for it to just fail ). Example interface: sharpd@eva [2]> ip link show tun100 21: tun100@NONE: <POINTOPOINT,NOARP,UP,LOWER_UP> mtu 1480 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000 link/ipip 192.168.119.224 peer 192.168.119.120 Let's just notice that there is a NOARP flag and abort the call. Fixes: #11733 Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* | Merge pull request #11565 from pguibert6WIND/bfd_vrf_lite_supportRafael Zalamena2022-07-222-34/+77
|\ \ | |/ |/| bfdd: allow l3vrf bfd sessions without udp leaking
| * bfdd: allow l3vrf bfd sessions without udp leakingPhilippe Guibert2022-07-192-34/+77
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now, when in vrf-lite mode, the BFD implementation creates a single UDP socket and relies on the following sysctl value to 1: echo 1 > /proc/sys/net/ipv4/udp_l3mdev_accept With this setting, the incoming BFD packets from a given vrf, would leak to the default vrf, and would match the UDP socket. The drawback of this solution is that udp packets received on a given vrf may leak to an other vrf. This may be a security concern. The commit addresses this issue by avoiding this leak mechanism. An UDP socket is created for each vrf, and each socket uses new setsockopt option: SO_REUSEADDR + SO_REUSEPORT. With this option, the incoming UDP packets are distributed on the available sockets. The impact of those options with l3mdev devices is unknown. It has been observed that this option is not needed, until the default vrf sockets are created. To ensure the BFD packets are correctly routed to the appropriate socket, a BPF filter has been put in place and attached to the sockets : SO_ATTACH_REUSEPORT_CBPF. This option adds a criterium to force the packet to choose a given socket. If initial criteria from the default distribution algorithm were not good, at least two sockets would be available, and the CBPF would force the selection to the same socket. This would come to the situation where an incoming packet would be processed on a different vrf. The bpf code is the following one: struct sock_filter code[] = { { BPF_RET | BPF_K, 0, 0, 0 }, }; struct sock_fprog p = { .len = sizeof(code)/sizeof(struct sock_filter), .filter = code, }; if (setsockopt(sd, SOL_SOCKET, SO_ATTACH_REUSEPORT_CBPF, &p, sizeof(p))) { zlog_warn("unable to set SO_ATTACH_REUSEPORT_CBPF on socket: %s", strerror(errno)); return -1; } Some tests have been done with by creating vrf contexts, and by using the below vtysh configuration: ip route 2.2.2.2/32 10.126.0.2 vrf vrf2 ip route 2.2.2.2/32 10.126.0.2 ! interface ntfp2 ip address 10.126.0.1/24 ! interface ntfp3 vrf vrf4 ip address 10.126.0.1/24 ! interface ntfp2 vrf vrf1 ip address 10.126.0.1/24 ! interface ntfp2.100 vrf vrf2 ip address 10.126.0.1/24 ! interface ntfp2.200 vrf vrf3 ip address 10.126.0.1/24 ! line vty ! bfd peer 10.126.0.2 vrf vrf2 ! peer 10.126.0.2 vrf vrf3 ! peer 10.126.0.2 ! peer 10.126.0.2 vrf vrf4 ! peer 2.2.2.2 multihop local-address 1.1.1.1 ! peer 2.2.2.2 multihop local-address 1.1.1.1 vrf vrf2 transmit-interval 1500 receive-interval 1500 ! The results showed no issue related to packets received by the wrong vrf. Even changing the udp_l3mdev_accept flag to 1 did not change the test results. Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
* | BFDD: Cleanup warninglynnemorrison2022-07-191-1/+1
|/ | | | Signed-off-by: Lynne Morrison <lynne.morrison@ibm.com>
* bfdd: fix coverity memory overrunRafael Zalamena2022-07-061-2/+2
| | | | | | | | | | | Use the destination for the operator `sizeof()` instead of the source which could (and is) be bigger than destination. We are not truncating any data here it just happens that the zebra interface data structure hardware address can be bigger due to different types of interface. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
* bfdd: fix coverity scan resource leakRafael Zalamena2022-07-061-0/+2
| | | | | | Close the descriptor if something fails and we don't return it. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
* bfdd: add IPv4 BFD Echo support that matches RFClynnemorrison2022-06-274-10/+442
| | | | | | | | | | | | | Modify the existing BFD Echo code to send an Echo message that will be looped in the peers forwarding plane. The existing Echo code only works with other FRR implementations because the Echo packet must go up to BFD to be turned around and forwarded back to the local router. The new BFD Echo code sets the src/dst IP of the packet to be the local router's IP and sets the dest MAC to be the peers MAC address. The peer receives the packet and because it is not it's IP address it forwards it back to the local router. Signed-off-by: Lynne Morrison <lynne.morrison@ibm.com>
* bfdd: Prevent coverity from thinking values are uninitedDonald Sharp2022-05-121-0/+2
| | | | | | | | | Coverity is claiming that bfdd is able got have bglobal.bg_use_dplane can be true, while dplane_addr can be uninitialized. Not really possible since global variables are initialized to all 0's. In any event. Force it to think it can't go there. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bfdd: fix override between sessionsanlan_cs2022-05-061-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After two single-hop sessions (*no local address are configured*) on two interfaces are UP, remove one address of one interface, both of them (actually, quite independent sessions) come to be DOWN, not just one. Consider two boxes: A with `a1` and `a2` adddress on two interfaces, and B with `b1` and `b2`. Two sessions are set up and ok: `s1` with <a1,b1> and `s2` with <a2,b2>. After `a1` of A is removed, there is an unhappy coincidence: 1) On A: `s1` changes local address, and sends <a2,b1> packets with help of route. 2) On B: wrongly regarded <a2,b1> packets with non-zero remote descriminator as part of `s2`, and are dropped for mismatched remote remote descriminator. 3) On A: `s1` sends <a2,b1> packets with zero remote descriminator to initialize this session. 4) On B: wrongly regarded <a2,b1> packets with zero remote descriminator as part of `s2`. Then `s2` will vibrate. So the good sessions are overridden. In this case, the <a2,b1> packets with zero remote descriminator won't take effect until the current good sessions become bad. Since single-hop sessions are allowed to be set without bound inteface in current code, this commit adds one check in `bfd_recv_cb()` to avoid wrong override. Signed-off-by: anlan_cs <vic.lan@pica8.com>
* Merge pull request #11137 from opensourcerouting/if-name-len-fixesDonald Sharp2022-05-041-2/+2
|\ | | | | *: standardize interface name maximum length
| * *: use FRR interface name definition everywhereRafael Zalamena2022-05-021-2/+2
| | | | | | | | | | | | | | Don't rely on the OS interface name length definition and use the FRR definition instead. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
* | bfdd: remove "local_address" of bfd sessionewlumpkin2022-05-023-19/+6
|/ | | | | | | | | | The "local_address" of bfd is only used in `show bfd peers brief` for single hop sessions which are configured without "local address". Since it is set by destination address of received packet, not completely correct, so remove it. Signed-off-by: ewlumpkin <ewlumpkin@gmail.com> Signed-off-by: anlan_cs <vic.lan@pica8.com>
* bfdd: fix broken FSM in active modeanlan_cs2022-04-241-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With the simple BFD configuration - (active mode, single hop, without other parameters) ``` ! bfd peer 11.11.11.11 exit ! ``` The interface with 11.11.11.0/24 is a *virtual* interface, which can be deleted. After BFD FSM is created and session is ok, do these things: 1) delete this interface 2) create this interface 3) set same ip address in this interface Now, everything seems completely restored because all configuration is same. But bad thing happens, BFD session hang on "down" status - ``` root# show bfd peer 11.11.11.11 BFD Peer: peer 11.11.11.11 vrf default ID: 638815827 Remote ID: 0 Active mode Status: down Downtime: 3 second(s) Diagnostics: path down <- caused by destroyed interface Remote diagnostics: ok ``` With the interface creating, `bfdd_sessions_enable_interface()` wrongly compares added interface with the created, even key of this `bfd_session` isn't binded with any interface. So this `bfd_session` will hang on "down" status for ever. So skip the compare in this case (no interface in key) to wake up this `bfd_session`. Signed-off-by: anlan_cs <vic.lan@pica8.com>
* *: Fix spelling of overridenDonald Sharp2022-04-192-2/+2
| | | | Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* *: Change thread->func to return void instead of intDonald Sharp2022-02-245-67/+44
| | | | | | | The int return value is never used. Modify the code base to just return a void instead. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bfdd: Fix overflow possibility with time statementsDonald Sharp2022-02-221-7/+7
| | | | | | | | | | | If time ( a uint64_t ) is large enough doing division and subtraction can still lead to situations where the resulting number is greater than a uint32_t. Just use uint32_t as an intermediate storage spot. This is unlikely to every occur in a time frame I could possibly care about but makes Coverity happy. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bfdd: Use AF_UNSPEC instead of comparing to 0Donald Sharp2022-02-071-1/+1
| | | | Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* Merge pull request #10388 from anlancs/bfd-fsm-passiveIgor Ryzhov2022-02-021-41/+12
|\ | | | | bfdd: fix broken FSM in passive mode
| * bfdd: fix broken FSM in passive modeanlan_cs2022-02-021-41/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: One is with active mode, the other is with passive mode. Sometimes the one with active mode is in `Down` stauts, but the other one with passive mode is unluckily stuck in `Init` status: It doesn't answer its peer with any packets, even receiving continuous `Down` packets. Root Cause: bfdd with passive mode answers its peer only *one* packet in `Down` status, then it enters into `Init` status and ignores subsequent `Down` packets. Unluckily that *one* answered packet is lost, at that moment its peer with active mode can only have to send `Down` packets. Fix: 1) With passive mode, bfdd should start xmittimer after received `Down` packet. Refer to RFC5880: "A system taking the Passive role MUST NOT begin sending BFD packets for a particular session until it has received a BFD packet for that session, and thus has learned the remote system's discriminator value." 2) Currently this added xmittimer for passive mode can be safely removed except receiving `AdminDown` packet: - `bfd_session_enable/bfd_set_passive_mode` doesn't start xmittimer - `ptm_bfd_sess_dn/bfd_set_shutdown` can remove xmittimer Per RFC5880, receiving `AdminDown` packet should be also regarded as `Down`, so just call `ptm_bfd_sess_dn`, which will safely remove the added xmittimer for passive mode. In summary, call `ptm_bfd_sess_dn` for two status changes on receiving `AdminDown`: `Init`->`Down` and `Up`->`Down`. Signed-off-by: anlan_cs <vic.lan@pica8.com>
* | bfdd,yang: optimize nb with YANGanlan_cs2022-01-251-86/+15
| | | | | | | | | | | | | | | | | | A few optimizations for bfd NB: - Remove unuseful checks for parameters with the same values - Replace checking values of bfd parameters with YANG's "range" - Append "required-echo-receive-interval" with 0 for it can be disabled Signed-off-by: anlan_cs <vic.lan@pica8.com>
* | bfdd: correct one word of commentanlan_cs2022-01-241-1/+1
|/ | | | Signed-off-by: anlan_cs <vic.lan@pica8.com>
* Merge pull request #10363 from anlancs/bfd-move-counterSantosh P K2022-01-191-2/+2
|\ | | | | bfdd: fix the possibly wrong counter of control packets
| * bfdd: fix the possibly wrong counter of control packetsanlan_cs2022-01-181-2/+2
| | | | | | | | | | | | | | Since control packets may be dropped by ttl check, the counter operation should be put after all check including ttl check. Signed-off-by: anlan_cs <vic.lan@pica8.com>
* | Merge pull request #10183 from idryzhov/rework-vrf-renameRafael Zalamena2022-01-173-78/+1
|\ \ | |/ |/| *: rework renaming the default VRF
| * *: rework renaming the default VRFIgor Ryzhov2021-12-213-78/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, it is possible to rename the default VRF either by passing `-o` option to zebra or by creating a file in `/var/run/netns` and binding it to `/proc/self/ns/net`. In both cases, only zebra knows about the rename and other daemons learn about it only after they connect to zebra. This is a problem, because daemons may read their config before they connect to zebra. To handle this rename after the config is read, we have some special code in every single daemon, which is not very bad but not desirable in my opinion. But things are getting worse when we need to handle this in northbound layer as we have to manually rewrite the config nodes. This approach is already hacky, but still works as every daemon handles its own NB structures. But it is completely incompatible with the central management daemon architecture we are aiming for, as mgmtd doesn't even have a connection with zebra to learn from it. And it shouldn't have it, because operational state changes should never affect configuration. To solve the problem and simplify the code, I propose to expand the `-o` option to all daemons. By using the startup option, we let daemons know about the rename before they read their configs so we don't need any special code to deal with it. There's an easy way to pass the option to all daemons by using `frr_global_options` variable. Unfortunately, the second way of renaming by creating a file in `/var/run/netns` is incompatible with the new mgmtd architecture. Theoretically, we could force daemons to read their configs only after they connect to zebra, but it means adding even more code to handle a very specific use-case. And anyway this won't work for mgmtd as it doesn't have a connection with zebra. So I had to remove this option. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
* | bfdd: Clean up some white space snafu'sDonald Sharp2022-01-081-2/+2
|/ | | | | | | Found some extra spaces during code inspection. Let's get them cleaned up. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* Merge pull request #10186 from idryzhov/bfd-receive-timer-cbMark Stapp2021-12-071-8/+0
|\ | | | | bfdd: remove unnecessary receive timer restart
| * bfdd: remove unnecessary receive timer restartIgor Ryzhov2021-12-061-8/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the detection time expires, we put the session down and restart the timer. As the comment in the code says, it's needed to zero the remote discriminator after the second expiration. But the RFC clearly says that this must be done on the first expiration: bfd.RemoteDiscr The remote discriminator for this BFD session. This is the discriminator chosen by the remote system, and is totally opaque to the local system. This MUST be initialized to zero. If a period of a Detection Time passes without the receipt of a valid, authenticated BFD packet from the remote system, this variable MUST be set to zero. And we actually already do it in `ptm_bfd_sess_dn`, so there's no need to reset the timer and wait for it twice. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
* | Merge pull request #10120 from idryzhov/bfd-detect-toRuss White2021-12-072-24/+18
|\ \ | |/ |/| bfdd: fix detection timeout update
| * bfdd: fix detection timeout updateIgor Ryzhov2021-12-032-24/+18
| | | | | | | | | | | | | | | | | | | | | | Per RFC 5880 section 6.8.12, the use of a Poll Sequence is not necessary when the Detect Multiplier is changed. Currently, we update the Detection Timeout only when a Poll Sequence is terminated, therefore we ignore the Detect Multiplier change if it's not accompanied with RX/TX timer change. To fix the problem, we should update the Detection Timeout on every received packet. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
* | bfdd: Convert vty_out to vty_json for JSONDonatas Abraitis2021-11-251-10/+5
|/ | | | Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
* *: cleanup ifp->vrf_idIgor Ryzhov2021-11-222-8/+6
| | | | | | | Since f60a1188 we store a pointer to the VRF in the interface structure. There's no need anymore to store a separate vrf_id field. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
* *: Convert quagga_signal_X to frr_signal_XDonald Sharp2021-11-111-1/+1
| | | | | | | Naming functions/data structures more appropriately for the project we are actually in. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bfdd: fix coverity warningsIgor Ryzhov2021-11-101-1/+4
| | | | | | | | | show/clear DEFUNs always require either peer label or IP address to be specified, so if `label` is NULL then `peer_str` is definitely not NULL. But Coverity doesn't know about that, so it complains about possible NULL dereference of `peer_str`. This commit should make Coverity happy. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
* Merge pull request #9833 from idryzhov/cleanup-if-by-index-all-vrfRuss White2021-11-053-24/+16
|\ | | | | *: fix usage of if_lookup_by_index_all_vrf
| * bfdd: cleanup vrf handling in packet receiveIgor Ryzhov2021-10-143-24/+16
| | | | | | | | | | | | | | | | | | | | | | We get the pointer to the interface on which the packet was received right at the beginning of bfd_recv_cb. So let's use this pointer and don't perform additional interface lookups. Also explain in more detail how we process VRF id with different backends. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
* | Merge pull request #9837 from idryzhov/cleanup-if-by-name-vrf-allRuss White2021-10-271-24/+7
|\ \ | | | | | | *: fix usage of if_lookup_by_name_all_vrf
| * | bfdd: cleanup bfd_session_enableIgor Ryzhov2021-10-151-24/+7
| |/ | | | | | | | | | | | | | | | | | | | | | | | | Well, there are some weird and duplicated checks there... All we need is two simple checks: - VRF existence. We must have it to enable the session. - Interface existence. If it's configured for the session, we have to bind the session to the interface. This commit implements these checks and removes unnecessary duplication. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
* | Merge pull request #9854 from opensourcerouting/zapi-call-tableRuss White2021-10-261-14/+17
|\ \ | | | | | | *: convert zclient callbacks to table
| * | *: convert zclient callbacks to tableDavid Lamparter2021-10-201-14/+17
| |/ | | | | | | | | | | | | | | | | | | | | | | | | This removes a giant `switch { }` block from lib/zclient.c and harmonizes all zclient callback function types to be the same (some had a subset of the args, some had a void return, now they all have ZAPI_CALLBACK_ARGS and int return.) Apart from getting rid of the giant switch, this is a minor security benefit since the function pointers are now in a `const` array, so they can't be overwritten by e.g. heap overflows for code execution anymore. Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* | Merge pull request #9824 from idryzhov/nb-cli-const-lyd-nodeDonald Sharp2021-10-252-41/+42
|\ \ | |/ |/| lib: northbound cli show/cmd functions must not modify data nodes
| * lib: northbound cli show/cmd functions must not modify data nodesIgor Ryzhov2021-10-132-41/+42
| | | | | | | | | | | | | | | | To ensure this, add a const modifier to functions' arguments. Would be great do this initially and avoid this large code change, but better late than never. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
* | Merge pull request #9684 from opensourcerouting/printfrr-false-positiveDonald Sharp2021-10-141-17/+14
|\ \ | |/ |/| *: `frr-format` with unmodified GCC
| * *: `frr-format` with unmodified GCCDavid Lamparter2021-09-281-17/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Since there's very few locations where the `frr-format` actually prints false positive warnings, consensus seems to be to just work around the false positives even if the code is correct. In fact, there is only one pattern of false positives currently, in `bfdd/dplane.c` which does `vty_out("%"PRIu64, (uint64_t)be64toh(...))`. The workaround/fix for this is a replacement `be64toh` whose type is always `uint64_t` regardless of what OS we're on, making the cast unnecessary. Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* | bfdd: Do not explicitly set the thread pointer to NULLDonatas Abraitis2021-10-041-1/+0
|/ | | | | | | | FRR should only ever use the appropriate THREAD_ON/THREAD_OFF semantics. This is espacially true for the functions we end up calling the thread for. Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
* Merge pull request #9331 from idryzhov/explicit-exitChristian Hopps2021-08-261-0/+2
|\ | | | | *: explicitly print "exit" at the end of every node config
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-30 09:46:11 +0100