| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
interface (VPP-935)
The logic in hash ACL bitmask update was using the vector
of ACLs applied to the interface to rebuild the hash lookup mask.
However, in transient cases (like doing group manipulation with
hash ACLs), that will not hold true. Thus, make
a local copy of for which ACL indices the hash_acl_apply
was called previously, and maintain that one local
to the hash_lookup.c file logic.
Change-Id: I30187d68febce8bba2ab6ffbb1eee13b5c96a44b
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 1de7d7044434196610190011ebb431f054701259)
|
|
traffic (VPP-910/VPP-929)
The commit fixing the VPP-910 and separating the memory operations
into separate heaps has missed setting the MHEAP_FLAG_THREAD_SAFE,
which quite obviously caused the issues in the multithread setup.
Fix that.
Also, add the debug CLIs
"set acl-plugin heap {main|hash} {validate|trace} {1|0}"
to toggle the memory instrumentation, in case we ever need it
in the future.
Change-Id: I8bd4f7978613f5ea75a030cfb90674dac34ae7bf
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit e6423bef32ca2ffcfcd7a092eb4673badd53ea4c)
|
|
The packet that was creating the session was not tracked,
consequently the TCP flags seen within the session record
never got the value for the session to get treated as
being in the established state.
Test-escape, so add the TCP tests which test the
three phases of the TCP session life and make them all pass.
Change-Id: Ib048bc30c809a7f03be2de7e8361c2c281270348
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 754370f1b55d4102d21dd94676f2bda3170c7df0)
|
|
It was uncaught by make test because the corresponding tests are not there yet - part of 17.10 deliverables
Change-Id: I55456f1874ce5665a06ee411c7abf37cd19ed814
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 58013b73509521789608f24a79a00177797ff9b1)
|
|
Change-Id: I2e71aef1aa745e85ad3234b0b708cdc50f335a75
Signed-off-by: Neale Ranns <neale.ranns@cisco.com>
|
|
(VPP-910)
The further prolonged testing from testbed that reported VPP-910
has uncovered a couple of deeper issues with optimization from
7384, and the usage of subscripts rather than vec_elt_at_index()
allowed to hide a couple of further errors in the code.
Also, the current acl-plugin behavior of using the global
heap for its dynamic data is problematic - it makes
the troubleshooting much harder by potentially spreading
the problem around.
Based on this experience, this commits makes a few changes to fix
the issues seen, also improving the serviceability of the acl-plugin
code for the future:
- Use separate mheaps for any ACL-related control plane
operations and separate for the hash lookup datastructures,
to compartmentalize any memory-related issues for the ACL plugin.
- Ensure vec_elt_at_index() usage throughout the hash_lookup.c file.
- Use vectors rather than raw memory for storing the "ordinary" ACL rules.
- Rework the optimization from 7384 to use a separate tail pointer
rather than overloading the "prev" field.
- Make get_session_ptr() more conservative and adjust is_valid_session_ptr
accordingly
Change-Id: Ifda85193f361de5ed3782a4acd39622bd33c5830
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit bd9c5ffe39e9ce61db95d74d150e07d738f24da1)
|
|
(VPP-912)
Fix several threading-related issues uncovered by the CSIT scale/performance test:
- make the per-interface add/del counters per-thread
- preallocate the per-worker session pools rather than
attempting to resize them within the datapath
- move the bihash initialization to the moment of ACL
being applied rather than later during the connection creation
- adjust the connection cleaning logic to not require
the signaling from workers to main thread
- make the connection lists check in the main thread robust against workers
updating the list heads at the same time
- add more information to "show acl-plugin sessions" to aid in debugging
Change-Id: If82ef715e4993614df11db5e9afa7fa6b522d9bc
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 8e4222fc7e23a478b021930ade3cb7d20938e398)
|
|
Simplify L2 output feature infra to unify with L2 input feature
infra using the newly improved feature bitmap mechanism.
Updated all L2 features to use the more efficient infra functions.
Change-Id: If8f463826b0af0717129befe92a27ea8cfc40449
Signed-off-by: John Lo <loj@cisco.com>
|
|
applied as part of many (VPP-910)
change 7385 has added the code which has the first ACE's "prev" entry within the linked list of
shadowed ACEs pointing to the last ACE, in order to avoid the frequent linear list traversal.
That change was not complete and did not update this "prev" entry whenever the last ACE was deleted.
As a result the changes within the applied ACLs which caused the calls to hash_acl_unapply/hash_acl_apply
may result in hitting assert which does the sanity check. The solution is to add the missing update logic.
Change-Id: I9cbe9a7c68b92fa3a22a8efd11b679667d38f186
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 45fe7399152f5ca511ba0b03fee3d5a3dffd1897)
|
|
When applying ACEs, in the new hash-based scheme, for each ACE
the lookup in the hash table is done, and either that ACE is added
to the end of the existing list if there is a match,
or a new list is created if there is no match.
Usually ACEs do not overlap, so this operation is fast, however,
the fragment-permit entries in case of a large number of ACLs
create a huge list which needs to be traversed for every other
ACE being added, slowing down the process dramatically.
The solution is to add an explicit flag to denote the first
element of the chain, and use the "prev" index of that
element to point to the tail element. The "next" field
of the last element is still ~0 and if we touch that
one, we do the linear search to find the first one,
but that is a relatively infrequent operation.
Change-Id: I352a3becd7854cf39aae65f0950afad7d18a70aa
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 204cf74aed51ca07933df7c606754abb4b26fd82)
|
|
The "acl_plugin" tests has one of the tests sporadically fail with the following traceback:
r.reply.decode().rstrip('\x00') UnicodeDecodeError: 'ascii' codec can't decode byte
0xd8 in position 20666: ordinal not in range(128)
This occurs in the newly added "show acl-plugin table" debug CLI.
This CLI has only the numeric outputs, so the conclusion is that it is
the incorrect termination (trailing zero) that might be most probably
causing it. The other acl-plugins show commands also
lack the zero-termination termination, so fix all of them.
The particularity of this command vs. the other acl-plugin debug CLIs
is that the accumulator is freed and allocated multiple times,
this might explain the issue is not seen with them.
Change-Id: I87b5c0d6152fbebcae9c7d0ce97155c1ae6666db
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit be055bd719559fc79d8a4c06479497c4c0bfae93)
|
|
interface
Multiple subsequent calls to vnet_feature_enable_disable() to enable the feature
cause the feature to be inserted into the processing graph multiple times in a row.
This might be argued to be a bug in that function, but enabling already enabled feature
is suboptimal anyway, so avoid that. The existing tests already catch this issue whenever
the ASSERT() part of this patch was added.
Change-Id: Ia2c06f7dc87bbe05795c2c7b7d19ea06270ce150
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
custom-dump and in VAT (VPP-885)
Add the missing function to convert the entire array of rules in the respective _endian functions,
rather than just the first rule.
Change-Id: Ic057f27ff7ec20150595efca1a48b74e5850f52b
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
It is useful to have the CLI to clear the existing sessions.
There was a work-in-progress CLI but it did not work properly.
Fix it and split into a separate "clear acl-plugin sessions",
and add a unit test into the extended connection-oriented tests.
Change-Id: I55889165ebcee139841fdac88747390903a05394
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
ethernet header
When extracting the 5-tuple, use the ethernet_buffer_header_size() so we can correctly
handle the case of subinterfaces, etc.
Change-Id: Ied73fde98d6b313e9eeab2aff4f22daa50a6cbbf
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Remove the unnecessary variable assignment which coverity detected.
Change-Id: I66ac20a8495400ac59192ddb72f16c95f6b4d03c
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
A typo resulted in a value being overwritten and flagged as unused, fix the typo.
Change-Id: I512ba94321afb80d12c71ebbb0eec42d9fa6f299
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
The code path which sets the sw_if_index aimed to restrict the output
did not set the flag to trigger that output.
Change-Id: I0a1a3977fdddbce9a276960df43fed745d099ca0
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
n_input == 0
The logic to print the line " output " is wrong for the case of n_input == 0,
and the applied ACLs are printed as if they were applied on input.
One may still figure out the truth by looking at the n_input value above,
but it is confusing.
Change-Id: I7b4a4d548e569994678dd1e139eb829456548b88
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Add a bihash-based ACL lookup mechanism and make it a new default.
This changes the time required to lookup a 5-tuple match
from O(total_N_entries) to O(total_N_mask_types), where
"mask type" is an overall mask on the 5-tuple required
to represent an ACE.
For testing/comparison there is a temporary debug CLI
"set acl-plugin use-hash-acl-matching {0|1}", which,
when set to 0, makes the plugin use the "old" linear lookup,
and when set to 1, makes it use the hash-based lookup.
Based on the discussions on vpp-dev mailing list,
prevent assigning the ACL index to an interface,
when the ACL with that index is not defined,
also prevent deleting an ACL if that ACL is applied.
Also, for the easier debugging of the state, there are
new debug CLI commands to see the ACL plugin state at
several layers:
"show acl-plugin acl [index N]" - show a high-level
ACL representation, used for the linear lookup and
as a base for building the hashtable-based lookup.
Also shows if a given ACL is applied somewhere.
"show acl-plugin interface [sw_if_index N]" - show
which interfaces have which ACL(s) applied.
"show acl-plugin tables" - a lower-level debug command
used to see the state of all of the related data structures
at once. There are specifiers possible, which make
for a more focused and maybe augmented output:
"show acl-plugin tables acl [index N]"
show the "bitmask-ready" representations of the ACLs,
we well as the mask types and their associated indices.
"show acl-plutin tables mask"
show the derived mask types and their indices only.
"show acl-plugin tables applied [sw_if_index N]"
show the table of all of the ACEs applied for a given
sw_if_index or all interfaces.
"show acl-plugin tables hash [verbose N]"
show the 48x8 bihash used for the ACL lookup.
Change-Id: I89fff051424cb44bcb189e3cee04c1b8f76efc28
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
A bihash-per-interface is convenient, but turns out tricky difficult from
the maintenance standpoint with the large number of interfaces.
This patch makes the sessions reside in a single hash table for all the interfaces,
adding the lower 16 bit of sw_if_index as part of the key into the previously
unused space.
There is a tradeoff, that a session with an identical 5-tuple and the same
sw_if_index modulo 65536 will match on either of the interfaces.
The probability of that is deemed sufficiently small to not worry about it.
In case it still happens before the heat death of the universe,
there is a clib_warning and the colliding packet will be dropped,
at which point we will need to bump the hash key size by another u64,
but rather not pay the cost of doing that right now.
Change-Id: I2747839cfcceda73e597cbcafbe1e377fb8f1889
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
The previous iteration of the code used the L2 classifier tables for session storage,
as a result, the table allocations were pretty big. The new ACL plugin
datapath uses the tables just as a redirection mechanism, without adding any
entries. Thus, the tables can be much smaller.
Change-Id: Ieec4a5abf0abda6e513ab4e675f912f14d47e671
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Change-Id: Ibd57cb617642e42d4c4d64f186479702b8a81900
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Change-Id: Ic3c9a914a588824b8abd6668961f731432083c4f
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
code use it
This fixes the undesirable pause in the dump commands in case there is nothing to dump.
Change-Id: I0554556c9e442038aa2a1ed8c88234f21f7fe9b9
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Add the logic to be able to use stateful ACLs in a multithreaded setup.
Change-Id: I3b0cfa6ca4ea8f46f61648611c3e97b00c3376b6
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Attempting to supply within a VAT CLI to add the ACLs a rule count
override with no rules to add would result in null pointer dereference
as we attempt to copy those rules to the message.
Add the check to avoid copy if the source pointer is null
(i.e. if there are no rules to copy from).
This commit fixes coverity errors 166797 and 166792.
Change-Id: Icabe060d961ba07dc41f63b8e17fca12ff82aa29
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Prior to commit bfd9227e6da567e0e19e026afe94cd4c0b65f725, there was
no clean way to check the lower-level message length as supplied
by the client, so there was no option but to trust that the client
does the right thing and allocates memory correctly.
The absence of checks makes it hard for a misbehaving client
to spot the problem - because everything "appears" to work
correctly for the specific erroneous message exchange.
This commit ensures the message received is at least
as big as we expect, and complains loudly if it is not.
Change-Id: I806eaac7c7f1ab3c64cb2bfa6939ce27da9a2b44
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
(HC2VPP-137 is the client behavior triggering this)
If the user does not unapply the ACLs off the interface,
but deletes the interface, the subsequent reuse of the
sw_if_index might find itself with the datapath
hooked up for ACL processing even though there is
no ACL configured. The fix is to unapply any ACLs
in the callback which is called upon the sw_if_index
addition/deletion.
Change-Id: Icea413d7fbf1ef891844a4818626e1b34fe79cbf
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 4c72e629e5ace392390a9d6109594254525064f7)
|
|
Change-Id: I87495d95dc6c5a36ff6d2ae05203e22e43403bf6
Signed-off-by: Jon Loeliger <jdl@netgate.com>
|
|
32-bit code still can use crc32c instructions, but it operates
on 32 registers
Change-Id: I9bb6b0b59635d6ea6a753584676ebcf59c8f6584
Signed-off-by: Damjan Marion <damarion@cisco.com>
|
|
Fix a logic error related to timing out of the connections
following the active one. To avoid this class of issue in
the future, create corresponding testcases, as well as some
trivial sanity testcases for both IPv4 and IPv6.
Since these tests are timing-dependent and take up time,
mark them as extended tests.
Change-Id: I2c72bad5efda7db8aa9cb05801fe47928dc47927
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Change-Id: I72298aaae7d172082ece3a8edea4217c11b28d79
Signed-off-by: Dave Barach <dave@barachs.net>
|
|
Change-Id: I3d64d5ced38a68f3fa208be00c49d20c4e6d4d0e
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Change-Id: Ide4f9bd6158fb64d069540fb43f4e593e39d6ff3
Signed-off-by: Pavel Kotucek <pkotucek@cisco.com>
|
|
Change-Id: Ie2ab844ad27b5ddb552bad9b19e7029cf91e4071
Signed-off-by: Pavel Kotucek <pkotucek@cisco.com>
|
|
(VPP-682)
This fixes the previously-implicit "drop all non-first fragments" behavior
to be more in line with security rules: a non-first fragment is treated
for the purposes of matching the ACL as a packet with the port
match succeeding. This allows to change the behavior to permit
the fragmented packets for the default "permit specific rules"
ruleset, but also gives the flexibility to block the non-initial
fragments by inserting into the begining a bogus rule
which would deny the L4 traffic.
Also, add a knob which allows to potentially turn this behavior off
in case of a dire need (and revert to dropping all non-initial fragments),
via a debug CLI.
Change-Id: I546b372b65ff2157d9c68b1d32f9e644f1dd71b4
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 9fc0c26c6b28fd6c8b8142ea52f52eafa7e8c7ac)
|
|
Change-Id: Id15b401223aabe7dacb7566c871ebefc17fbb1fc
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 7fd3f513c7df198c45204eba0a3e9a3abe509593)
|
|
- use the counters in a private struct rather than node error counters
- ensure the timer for the non-idle connections is restarted
- fix the deletion of conn at the current tail the list
Change-Id: I632f63574d2ced95fb75c5e7fb588c78fb3cce1c
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
(cherry picked from commit 097051a3bd1f63a177c0728f15375afd84a68918)
|
|
Change-Id: I88b322a5d602f3d6d3310e971479180a89430e0e
Signed-off-by: Damjan Marion <damarion@cisco.com>
|
|
L3 path support, L2+L3 unified processing node, skip IPv6 EH support.
Change-Id: Iac37a466ba1c035e5c2997b03c0743bfec5c9a08
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
Change-Id: I08ab1fd0abdd1db4aff11a38c9c0134b01368e11
Signed-off-by: Eyal Bari <ebari@cisco.com>
|
|
Change-Id: I95113a277b94cce5ff332fcf9f57ec6f385acec0
Signed-off-by: Pavel Kotucek <pkotucek@cisco.com>
|
|
via l2output_main.next_nodes
Before this commit, several output features that happen to be the
last in the list of features to be executed, send the packets directly
to <interfaceName>-output. To do this, they use l2_output_dispatch,
which builds a list of sw_if_index to next index mappings.
When interfaces are deleted and the new interfaces are created,
these mappings become stale, and cause the packets being sent to wrong
interface output nodes.
This patch (thanks John Lo for the brilliant idea!) adds a feature node "output",
whose sole purpose is dispatching the packets to the correct interface output
nodes. To do that, it uses the l2output_main.next_nodes, which is already
taken care of for the case of the sw_if_index reuse, so this makes the dependent
features all work correctly.
Since this changes the packet path, for the features that were always the last ones
it has triggered a side problem of the output feat_next_node_index not being properly
initalized. These two users are l2-output-classify node and the output nodes belonging
to the acl-plugin.
For the first one the less invasive fix is just to initialize that field.
For the acl-plugin nodes, rewrite the affected part of the code to use
feat_bitmap_get_next_node_index since this is essentially what the conditional
in l2_output_dispatch does, and fix the compiler warnings generated.
This fix was first made in stable/1701 under commit e7dcee4027854b0ad076101471afdfff67eb9011.
Change-Id: I32e876ab1e1d498cf0854c19c6318dcf59a93805
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
|
|
This patch replaces requirement for vlib_plugin_register function
in the plugin so file and introduces new macro:
VLIB_PLUGIN_REGISTER () = {
.version = "version string",
.version_required = "requred version",
.default_disabled = 1,
.early_init = "early_init_function_name",
};
Plugin will nor be loaded if .default_disabled is set to 1
unless explicitely enabled in startup.conf.
If .verstion_required is set, plugin will not be loaded if there
is version mismatch between plugin and vpp. This can be bypassed
by setting "skip-version-check" for specific plugin.
If .early-init string is present, plugin loader will try to resolve
this specific symbol in the plugin namespace and make a function call.
Following startup.conf configuration is added:
plugins {
path /path/to/plugin/directory
plugin ila_plugin.so { enable skip-version-check }
plugin acl_plugin.so { disable }
}
Change-Id: I706c691dd34d94ffe9e02b59831af8859a95f061
Signed-off-by: Damjan Marion <damarion@cisco.com>
|
|
Instead, have them accept and assign a return paramter leaving
the return control flow up to the caller. Clean up otherwise
misleading returns present even after "NOT REACHED" comments.
Change-Id: I0861921f73ab65d55b95eabd27514f0129152723
Signed-off-by: Jon Loeliger <jdl@netgate.com>
|
|
Rather than rely on an unbound variable, explicitly introduce
the timeout variable within the 'do { ... } while (0)' construct
as a block-local variable.
Change-Id: I6e78635290f9b5ab3f56b7f116c5fa762c88c9e9
Signed-off-by: Jon Loeliger <jdl@netgate.com>
|
|
Rather than blindly assume an unbound, fixed message parameter
explicilty pass it as a paramter to the S() macro.
Change-Id: Ieea1f1815cadd2eec7d9240408d69acdc3caa49a
Signed-off-by: Jon Loeliger <jdl@netgate.com>
|
|
Rather than maintain (?) an unused second parameter, t, and pull
an unbound message pointer, mp, out of context, explicitly list
the message pointer as the second parameter.
Change-Id: I92143efda6211cdf6b935470f8c71579742a6b64
Signed-off-by: Jon Loeliger <jdl@netgate.com>
|
|
Change-Id: I422a3f168bd483e011cfaf54af022cb79b78db02
Signed-off-by: Dave Barach <dave@barachs.net>
|
Andrew Yourtchenko
Neale Ranns
John Lo
Jon Loeliger
Damjan Marion
Dave Barach
Pavel Kotucek
Eyal Bari