pimd's include files are very interdependent. Let's chop that down a
bit to gain some flexibility.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Problem
======
In pim_msg_send_frame api, the while loop was executed only once.
Fix
===
while is changed to if, as in the code flow
the while part is getting executed only once.
Signed-off-by: Sai Gomathi <nsaigomathi@vmware.com>
Problem :
=======
When all the groups from Ixia are stopped,
groups still keep refreshing and not getting timeout
RCA:
====
IGMP Report is coming in include mode without any source address, this problem will come.
Fix :
===
If the requested filter mode is INCLUDE *and* the requested
source list is empty, then the entry corresponding to the
requested interface and multicast address is deleted if present.
If no such entry is present, the request is ignored.
When an interface receives the IGMP report without any source, then the group is deleted.
Signed-off-by: Sai Gomathi <nsaigomathi@vmware.com>
There is a possibility that the same line can be matched as a command in
some node and its parent node. In this case, when reading the config,
this line is always executed as a command of the child node.
For example, with the following config:
```
router ospf
network 193.168.0.0/16 area 0
!
mpls ldp
discovery hello interval 111
!
```
Line `mpls ldp` is processed as command `mpls ldp-sync` inside the
`router ospf` node. This leads to a complete loss of `mpls ldp` node
configuration.
To eliminate this issue and all possible similar issues, let's print an
explicit "exit" at the end of every node config.
This commit also changes indentation for a couple of existing exit
commands so that all existing commands are on the same level as their
corresponding node-entering commands.
Fixes#9206.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
While defaults are good picks for "reasonable" guesses, min and max
range values really aren't. Operators and experimenters often like to
configure "unreasonable" values to stress test, tests boundary
conditions and explore innovations.
With that in mind, change all ranges to 1..max (of type).
While we're here add optional ignored values in the "no" CLI forms.
Signed-off-by: Christian Hopps <chopps@labn.net>
Problem Statement:
==================
IGMP query is sent at irregular intervals
(more than 30 seconds) when "ip igmp query-max-response-time 100"
command is executed multiple times.
RCA:
=================
When "ip igmp query-max-response-time 100" is executed, the timers
are reset resulting in the delay of sending the query.
Fix:
=================
When there is no change in the config value, we should not reset
the timers.
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
When we decide that we do not need a item on the partial_bsrp_list
don't just drop the memory on the floor, free it up.
This was happening when we decided that a pending item has
a hold time of 0.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Some no commands were not accepting values and left us in
a situation where a cut-n-paste of the non-no line would
not be properly accepted.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The only difference in daemons' interface node definition is the config
write function. No need to define the node in every daemon, just pass
the callback as an argument to a library function and define the node
there.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This commit is to correct the order in which the fields are
accessed while verifying it. First the fields should be
verified, and if it is valid then access it.
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
Allow the join-prune interval to be as small as 5 seconds instead
of limiting the value to 60.
This can and will come at a price of being able to converge less
mroutes.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
IGMPv3 packets with invalid TOS should be dropped.
Test Case ID: 4.10
TEST_DESCRIPTION
Every IGMP message described in this document is sent with
IP Precedence of Internetwork Control (e.g., Type of Service
0xc0)
(Tests that IGMPv3 Membership Query Message conforms to
above statement)
TEST_REFERENCE
NEGATIVE: RFC 3376, IGMP Version 3, s4 p7 Message Formats
Issue: #9071
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
IGMPv3 packets with invalid TTL should be dropped.
Test Case ID: 4.10
TEST_DESCRIPTION
Every IGMP message described in this document is sent with an IP
Time-to-Live of 1 (Tests that IGMPv3 Membership Report Message
conforms to above statement)
TEST_REFERENCE
NEGATIVE: RFC 3376, IGMP Version 3, s4 p7 Message Formats
Issue: #9070
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
Problem Statement:
==================
valgrind shows memleaks in rp_table, when pimd shuts down gracefully.
2020-05-05 22:09:29,451 ERROR: Memory leaks in router [r4] for daemon [pimd]
2020-05-05 22:09:29,451 ERROR: Memory leaks in router [r4] for daemon [zebra]
2020-05-05 22:09:29,637 ERROR: Found memory leak in module pimd
2020-05-05 22:09:29,638 ERROR: ==6178== 184 (56 direct, 128 indirect) bytes in 1 blocks are definitely lost in loss record 21 of 21
2020-05-05 22:09:29,638 ERROR: ==6178== at 0x4C2FFAC: calloc (vg_replace_malloc.c:762)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x4E855EE: qcalloc (memory.c:111)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x4EAA43C: route_table_init_with_delegate (table.c:52)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x1281A1: pim_rp_init (pim_rp.c:114)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x11D0F8: pim_instance_init (pim_instance.c:117)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x11D0F8: pim_vrf_new (pim_instance.c:150)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x4EB1BEC: vrf_get (vrf.c:209)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x4EB2B2F: vrf_init (vrf.c:493)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x11D227: pim_vrf_init (pim_instance.c:217)
2020-05-05 22:09:29,638 ERROR: ==6178== by 0x11BBAB: main (pim_main.c:121)
Fix:
====
rp_info is allocated in pim_rp_init API. rp_info pointer is present
in rp_list and rp_table. In rp_list cleanup, the memory for rp_info
gets freed. rp_table clean up should be done first and then rp_list.
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
Problem:
S,G entry has iif = oif in FHR is LHR case.
Setup:-
R11-----R2----R4
R11 :- FHR and LHR
R2 :- RP
R4 :- LHR
Issue :-
1) shut mapped interface in R11
2) wait for 5 min
3) do FRR restart
5) No shut of mapped interface
OIL is added for local interface also where OIL is same as IIF
and duplicate traffic observed on R4 receives in Ixia
RCA:
pim_ifchannel_local_membership_add adds inherited oif from starg when iif for
SG is unavailable.
When rpf for that SG resolves to this inherited oif from starg, iif is also in oif.
This results in dup traffic.
Fix:
If iif is not available, do not inherit from starg.
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
Problem Statement:
In Prune pending state, when Join is received, and there is hold timer change
the Expiry timer is not getting updated with new Hold timer.
Root Cause:
When thread_add_timer function is called and the thread is already in the list
the thread api just returns, it does not modify the timer value.
So when we want to change the timer, we need to first call THREAD_OFF and then
call thread_add_timer.
The Expiry timer thread is not cancelled in PIM_IFJOIN_PRUNE_PENDING state,
therefore the timer change is not taking effect.
Fix:
Call THREAD_OFF in that flow.
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
Making the interface holdtime range to 3.5 times the hello-time
As per 7761, Section 4.11:
The Holdtime in a Hello message should be set to
(3.5 * Hello_Period), giving a default value of 105 seconds.
Therefore providing the user also to configure max upto 3.5 times
the hello timer interval.
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
When we have a "192.0.2.1 peer 192.0.2.2/32" address on an interface, we
need to (a) recognize the local address as being on the link for our own
packets, and (b) do the IGMP socket lookup with the proper local address
rather than the peer prefix.
Fixes: efe6f18 ("pimd: fix IGMP receive handling")
Cc: Nathan Bahr <nbahr@atcorp.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
1. Add the querierIP object to igmp_sock datastruct to save the IP address of the querier.
Management of the querierIP object is added.
2. To show the querier IP address in the CLI "show ip igmp interface".
3. To add the json object querierIP for querier IP address in the json CLI "show ip igmp interface json".
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
... the PIM code is kinda misusing prefix lists to match addresses.
Considering the weird semantics of access-lists, I can't fault it.
However, prefix lists aren't great at matching addresses by default,
since they try to match the prefix length too. So, here's an "address
match mode" for prefix lists to get that to work more reasonably.
Fixes: #8492
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
There's an IGMP socket per interface, so they should be bound to that
interface. Which also makes IGMP work in VRFs.
Fixes: #7889
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Rename functions (`pim_msdp_peer_new` => `pim_msdp_peer_add` and
`pim_msdp_peer_do_del` => `pim_msdp_peer_del`) to keep consistency and
update the `pim_msdp_peer_add` documentation to tell users that this is
also used for non meshed group peers.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Fully utilize the northbound to hold pointers to our private data
instead of searching for data structures every time we need to change a
configuration.
Highlights:
* Support multiple mesh groups per PIM instance (instead of one)
* Use DEFPY instead of DEFUN to reduce code complexity
* Use northbound private pointers to store data structures
* Reduce callback names size
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
When running pim on an interface and that interface has
state and we move that interface into a different vrf
there exists a call path where we have not created the pimreg
device yet. Prevent a crash in this rare situation.
Ticket: #2552763
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Compile with v2.0.0 tag of `libyang2` branch of:
https://github.com/CESNET/libyang
staticd init load time of 10k routes now 6s vs ly1 time of 150s
Signed-off-by: Christian Hopps <chopps@labn.net>
VRF creation can happen from either cli or from
knowledged about the vrf learned from zebra.
In the case where we learn about the vrf from
the cli, the vrf id is UNKNOWN. Upon actual
creation of the vrf, lib/vrf.c touches up the vrf_id
and calls pim_vrf_enable to turn it on properly.
At this point in time we have a pim->vrf_id of
UNKNOWN and the vrf->vrf_id of the right value.
There is no point in duplicating this data. So just
remove all pim->vrf_id and use the vrf->vrf_id instead
since we keep a copy of the pim->vrf pointer.
This will remove some crashes where we expect the
pim->vrf_id to be usable and it's not.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When creating configuration for a vrf *Before* the vrf has been
created, pim will not properly create the pimreg device and
we will promptly crash when we try to pass data.
Put some code checks in place to ensure that the pimreg is
created for vrf's.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The router->register_suppress_time is used to derive the
rp_keep_alive_time, but when the suppress time was changed, pim was
not recalculating the rp_keep_alive_time and left it at the old value.
This fix applies the changes when a new suppress_time is entered
(or removed.)
Signed-off-by: Don Slice <dslice@nvidia.com>
Problem reported that when certain pim commands were entered, they
showed up duplicated in the configuration both under default instance
and every vrf (whether pim was used there or not.) This was because
these particular parameters are global only and the function doing
the display would repeat for each vrf. This fix only displays those
in the default case (and removes them from the help for entering
under a vrf.)
Signed-off-by: Don Slice <dslice@nvidia.com>
Just some cleanup before I touch this code; switching to typesafe list
macros & putting the data directly inline.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
... yes we may need it later, but if and when that happens we can put it
back there. No point carrying around unused things.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Display the MSDP peer configuration in `show running-config` so it can
be saved on configuration write.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
encoding signed int as unsigned is bad practice; since we want to do
it here lets at least be explicit about it
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
`config.h` has all the defines from autoconf, which may include things
that switch behavior of other included headers (e.g. _GNU_SOURCE
enabling prototypes for additional functions.)
So, the first include in any `.c` file must be either `config.h` (with
the appropriate guard) or `zebra.h` (which includes `config.h` first
thing.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
... by referencing all autogenerated headers relative to the root
directory. (90% of the changes here is `version.h`.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Most of these are many, many years out of date. All of them vary
randomly in quality. They show up by default in packages where they
aren't really useful now that we use integrated config. Remove them.
The useful ones have been moved to the docs.
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
Issue:
User is allowed to configure only hello without hold timer but when undo
config, the hold timer is mandatory as shown below:
FRR-4(config-if)# ip pim hello 10
<cr>
(1-180) Time in seconds for Hold Interval
FRR-4(config-if)# ip pim hello 10
FRR-4(config-if)# no ip pim hello 10
(1-180) Time in seconds for Hold Interval
FRR-4(config-if)# no ip pim hello 10
% Command incomplete: no ip pim hello 20
Fix:
Making the hold timer as optional when undo config.
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
Also included display of hold time in CLI 'show ip pim int <intf>' cmd
and json commands.
Issue:
PIM neighbor not coming up if hold time is less than hello timer
since hello is sent every 4 sec and hold is 1 sec,
because of this nbr is flapping
Fix:
Do not allow configuration of hold timer less than hello timer
Also reset the value of hold timer to 3.5 times to hello whenever
only hello is modified so that the relationship holds good.
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
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>
There are places in the code where function nb_running_get_entry is used
with abort_if_not_found set to true during the config validation stage.
This is incorrect because when used in transactional CLI, the running
entry won't be set until the apply stage, and such usage leads to crash.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Recent change in commit: 6b73800ba2
Caused this error to pop up in pim_igmp_mtrace.c:
error: taking address of packed member 'rsp_addr' of class or structure 'igmp_mtrace' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member]
Follow the pattern used in the code to solve this problem for clang
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
