INTERFACE_NAMSIZ is just a redefine of IFNAMSIZ and IFNAMSIZ
is the standard for interface name length on all platforms
that FRR currently compiles on.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Include a zclient value in the hash and tree key computations
for iprules in zebra: clients may collide without this.
Signed-off-by: Mark Stapp <mjs@labn.net>
DSCP and ECN matching are configured independently. Maintain
these values in independent fields in pbrd, zapi, and zebra.
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
Subset: zebra dataplane
Add new vlan filter fields. No kernel dataplane
implementation yet (linux does not support).
Changes by:
Josh Werner <joshuawerner@mitre.org>
Eli Baum <ebaum@mitre.org>
G. Paul Ziemba <paulz@labn.net>
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
The rib_process_dplane_results function was having each
sub function handler process the results and then
free the ctx. Lot's of functionality that needs to remember
to free the context. Let's just free it in the main loop.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
PBR rules are installed as match, action rules in most dataplanes. This
requires the action to be resolved via a GW. And the GW to be subsequently
resolved to {SMAC, DMAC}.
Signed-off-by: Anuradha Karuppiah <anuradhak@nvidia.com>
The usage of zebra dplane makes the job asyncronous which implies
that a given job will try to add an iptable, while the second job
will not know that its iptable is the same as the former one.
The below exabgp rules stand for two bgp flowspec rules sent to
the bgp device:
flow {
route {match {
source 185.228.172.73/32;
destination 0.0.0.0/0;
source-port >=49156&<=49159;
}then {redirect 213.242.114.113;}}
route {match {
source 185.228.172.73/32;
destination 0.0.0.0/0;
source-port >=49160&<=49163;
}then {redirect 213.242.114.113;}}
}
This rule creates a single iptable, but in fact, the same iptable
name is appended twice. This results in duplicated entries in the
iptables context. This also results in contexts not flushed, when
BGP session or 'flush' operation is performed.
iptables-save:
[..]
-A PREROUTING -m set --match-set match0x55baf4c25cb0 src,src -g match0x55baf4c25cb0
-A PREROUTING -m set --match-set match0x55baf4c25cb0 src,src -g match0x55baf4c25cb0
-A match0x55baf4c25cb0 -j MARK --set-xmark 0x100/0xffffffff
-A match0x55baf4c25cb0 -j ACCEPT
-A match0x55baf4c25cb0 -j MARK --set-xmark 0x100/0xffffffff
-A match0x55baf4c25cb0 -j ACCEPT
[..]
This commit addresses this issue, by checking that an iptable
context is not already being processed. A flag is added in the
original iptable context, and a check is done if the iptable
context is not already being processed for install or uinstall.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The dplane_ctx_get_pbr_ipset_entry function only
failed when the caller did not pass in a valid
usable pointer. Change the code to assert on
a pointer not being passed in and remove the
bool return
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The only time this function ever failed is when
the developer does not pass in a usable pointer
to place the data in. Change it to an assert
to signify to the end developer that is what
we want and then remove all the if checks
for failure
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The function call dplane_ctx_get_pbr_ipset only
returns false when the calling function fails to
pass in a valid ipset pointer. This should
be an assertion issue since it's a programming
issue as opposed to an actual run time issue.
Change the function call parameter to not return
a bool on success/fail for a compile time decision.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This one also needed a bit of shuffling around, but MTYPE_RE is the only
one left used across file boundaries now.
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>
like it has been done for iptable contexts, a zebra dplane context is
created for each ipset/ipset entry event. The zebra_dplane_ctx job is
then enqueued and processed by separate thread. Like it has been done
for zebra_pbr_iptable context, the ipset and ipset entry contexts are
encapsulated into an union of structures in zebra_dplane_ctx.
There is a specificity in that when storing ipset_entry structure, there
was a backpointer pointer to the ipset structure that is necessary
to get some complementary information before calling the hook. The
proposal is to use an ipset_entry_info structure next to the ipset_entry,
in the zebra_dplane context. That information is used for ipset_entry
processing. The ipset name and the ipset type are the only fields
necessary.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The iptable processing was not handled in remote dataplane, and was
directly processed by the thread in charge of zapi calls. Now that call
can be handled in the zebra_dplane separate thread. once a
zebra_dplane_ctx is allocated for iptable handling, the hook call is
performed later. Subsequently, a return code may be triggered to zclient
interface if any problem occurs when calling the hook call.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Move the pbr hash creation to be after the update release
and dplane install. Now that rules are installed in a separate
dplane pthread, we can have scenarios where we have an interface
flapping and we install/remove rules sufficiently fast enough we
could issue what we think is an update for an identical rule and
end up releasing the rule right after we created it and sent it to
the dplane. This solves the problem of recving duplicate rules
during interface flapping.
Signed-off-by: Stephen Worley <sworley@nvidia.com>
When installing rules pass by the interface name across
zapi.
This is being changed because we have a situation where
if you quickly create/destroy ephermeal interfaces under
linux the upper level protocol may be trying to add
a rule for a interface that does not quite exist
at the moment. Since ip rules actually want the
interface name ( to handle just this sort of situation )
convert over to passing the interface name and storing
it and using it in zebra.
Ticket: CM-31042
Signed-off-by: Stephen Worley <sworley@nvidia.com>
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
in order to create appropriate policy route, family attribute is stored
in ipset and iptable zapi contexts. This commit also adds the flow label
attribute in iptables, for further usage.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This commit avoids freeing the iptable context, once created. the case
where there is an error when reading zapi stream simply needs to free
the zpi context.
Fixes: ("8b5c4dce07e6 zebra: fix iptable memleak, fix free funcs")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
* Implement new dataplane operations
* Convert existing code to use dataplane context object
* Modify function preparing netlink message to use dataplane
context object
Signed-off-by: Jakub Urbańczyk <xthaid@gmail.com>
- Fix 1 byte overflow when showing GR info in bgpd
- Use PATH_MAX for path buffers
- Use unsigned specifiers for uint16_t's in zebra pbr
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Replace sprintf with snprintf where straightforward to do so.
- sprintf's into local scope buffers of known size are replaced with the
equivalent snprintf call
- snprintf's into local scope buffers of known size that use the buffer
size expression now use sizeof(buffer)
- sprintf(buf + strlen(buf), ...) replaced with snprintf() into temp
buffer followed by strlcat
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Define some explicit rule replace code paths into the dataplane
code and improve the handling around it/releasing the the old
rule from the hash table.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
My previous patch to fix a memory leak, caused by not properly freeing
the iptable iface list on stream parse failure, created/exposed a heap
use after free because we were not doing a deep copy
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
- Fix iptable freeing code to free malloc'd list
- malloc iptable in zapi handler and use those functions to free it when
done to fix a linked list memleak
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
If we only really use the ifp for the name, then
don't bother referencing the ifp. If that ifp is
freed, we don't expect zebra to handle the rules that
use it (that's pbrd's job), so it is going to be
pointing to unintialized memory when we decide to remove
that rule later. Thus, just keep the name in the data
and dont mess with pointer refs.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Use the ifindex value as a primary hash key/identifier, not
the ifp pointer. It is possible for that data to be freed
and then we would not be able to hash and find the rule entry
anymore. Using the ifindex, we can still find the rule even
if the interface is removed.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
We were seeing a double free on shutdown if the
hash release fails here due to the interface state
changing. We probably shouldn't free the data if its
still being handled in the table so adding a check there
and a debug message.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
in addition to support for tcpflags, it is possible to filter on any
protocol. the filtering can then be based with iptables.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
It doesn't make much sense for a hash function to modify its argument,
so const the hash input.
BGP does it in a couple places, those cast away the const. Not great but
not any worse than it was.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The _wrap_script inclusion implies a certain end functionality
of which we don't care. We just care that the hooks are called.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
