When add allowed-address-pair 0.0.0.0/0 to one port, it will
unexpectedly open all others' protocol under same security
group. IPv6 has the same problem.
The root cause is the openflow rules calculation of the
security group, it will unexpectedly allow all IP(4&6)
traffic to get through.
For openvswitch openflow firewall, this patch adds a source
mac address match for the allowed-address-pair which has
prefix lenght 0, that means all ethernet packets from this
mac will be accepted. It exactly will meet the request of
accepting any IP address from the configured VM.
Test result shows that the remote security group and
allowed address pair works:
1. Port has 0.0.0.0/0 allowed-address-pair clould send any
IP (src) packet out.
2. Port has x.x.x.x/y allowed-address-pair could be accepted
for those VMs under same security group.
3. Ports under same network can reach each other (remote
security group).
4. Protocol port number could be accessed only when there
has related rule.
Closes-bug: #1867119
Change-Id: I2e3aa7c400d7bb17cc117b65faaa160b41013dde
(cherry picked from commit 00298fe6e8)
There is a lock synchronized created by Neutron IpsetManager
with common name 'neutron-ipset'. Some deployments
could use multiple IpsetManagers object in separated
namespaces. Because of this on heavy-loaded environments
with a lot of ports and related security groups
it could end with not-consumed rabbitmq queue on security
groups notification. To prevent that this lock should be
parametrized by namespace name.
Change-Id: I319cbd9b3b9713340859f362913ceb21ff514cb0
Reduces E128 warnings by ~260 to just ~900,
no way we're getting rid of all of them at once (or ever).
Files under neutron/tests still have a ton of E128 warnings.
Change-Id: I9137150ccf129bf443e33428267cd4bc9c323b54
Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
neutron-lib contains the synchronized lockutils decorator as well as
the SYNCHRONIZED_PREFIX global. This patch consumes them from
neutron-lib and removes them from neutron.
NeutronLibImpact
Change-Id: I729da348e340509f2d09f8a6436716e2398f1583
Through [1] ipset members are updated in update_security_group_members
instead of updating during firewall apply. In the same way, we will
delete conntrack entries immediately after deleting remote ipset
members(in update_security_group_members) instead of deleting them after
firewall apply.
As explained in [2], this change partially fixes bug #1580377 i.e it
deletes conntrack entries on remote hosts for a removed port.
[1] https://review.openstack.org/#/c/347068/
[2] https://bugs.launchpad.net/neutron/+bug/1580377/comments/13
Co-Authored-By:shihanzhang <shihanzhang@huawei.com>
Partial-Bug: #1580377
Change-Id: Iea3344a24e2a068b794c44796b4c945432379c13
All the callers of set_exists() already know the set name,
so there's no reason to re-calculate it based on the id and
ethertype. Renamed to set_name_exists() to be clear on what
it is checking.
Change-Id: Ifd2a2c7877c81a440067f9e8a654b49e63199f8c
ipset was locking on every set_members call with an external
filesystem lock. This was expensive when lots of ports that
were a part of the same security group were on the same agent.
This patch adjusts it to check if it needs to make a change before
acquiring the semaphore.
Closes-Bug: #1502930
Change-Id: I2553ab74b7d0fbada5d573246194f83d58bd7d56
When l2 agent execute ipset command, we should pass
parameter 'check_exit_code' to self.execute acording to
action.
The intention is to safely fail if we try to destroy a
non existing set, or delete a non existing member.
Otherwise the agent gets stuck in a loop if such situation
happens. Such kind of event would be logged as errors.
Change-Id: If67330523d114d6da13d0280851e7138a51d08f7
Closes-bug: #1497074
Previously, the ipset_manager would pass in 0.0.0.0/0 or ::/0 if
these addresses were inputted as allowed address pairs. This causes
ipset to raise an error as it does not work with zero prefix sizes.
To solve this problem we use two ipset rules to represent this:
Ipv4: 0.0.0.0/1 and 128.0.0.1/1
IPv6: ::/1' and '8000::/1
All of this logic is handled via _sanitize_addresses() in the ipset_manager
which is called to convert the input.
Change-Id: I8c6a08e0cf3b5b5386fe03af9f2174c666b8ac75
Closes-bug: 1461054
The new NET prefix introduced by I8177699b157cd3eac46e2f481f47b5d966c49b07
increases collision chances by trimming the sg_id by 3 more chars.
This patch reduces the prefix to 1 single char and also reduces the
swap suffix to reduce the chances of collision.
Change-Id: I8a1559e173a05b2297c5cd2efa9fee7627b88a4f
Related-Bug: #1439817
Related-Bug: #1444397
The previous hash type was 'ip' and this caused a major
issue with the allowed address pairs extension since it
results in CIDRs being passed to ipset. When the hash type
is 'ip', a CIDR is completely enumerated into all of its
addresses so 10.100.0.0/16 results in ~65k entries. This
meant a single allowed_address_pairs entry could easily
exhaust an entire set.
This patch changes the hash type to 'net', which is designed
to handle a CIDRs as a single entry.
This patch also changes the names of the ipsets because
creating an ipset with different parameters will cause an
error and our ipset manager code isn't robust enough to handle
that at this time. There is another ongoing patch to fix
that but it won't be ready in time.[1]
The related bug was closed by increasing the set limit, which
did alleviate the problem. However, this change would also
address the issue because the gate tests run an allowed address
pairs extension test with the CIDR mentioned above.
1. I59e2e1c090cb95ee1bd14dbb53b6ff2c5e2713fd
Related-Bug: #1439817
Closes-Bug: #1444397
Change-Id: I8177699b157cd3eac46e2f481f47b5d966c49b07
This reverts commit b5b919a7a3.
The current ipset manager code isn't robust enough to handle
ipsets that already exist with different parameters. This reverts
the ability to change the parameters so we don't break upgrades
to Kilo.
Change-Id: I538714df52424f0502cb75daea310517d1142c42
Closes-Bug: #1444201
Change ipset_manager _refresh_set() to make a copy of the list of
IPs when creating a set, instead of using a reference, else any
change to the set could update the caller's data.
Also made the IpsetManagerTestCase classes always pass maxelem and
hashsize to the parent class.
Change-Id: I45fc716ab0952b80363b0c7dabae29cda05604dc
Closes-bug: #1442377
Recently, these messages have been noticed in both tempest
logs, as well as reported by downstream users syslog:
Set IPv4915d358d-2c5b-43b5-9862 is full, maxelem 65536 reached
So the default of 64K is not sufficient enough.
This change adds two config options to control both the number
of elements as well as the hashsize, since they should be
tuned together for best performance. Slightly different
formats were required for 'ipset create' and 'ipset restore'.
The default values for these are now set to 131072 (maxelem) and
2048 (hashsize), which is an increase over their typical default values
of 65536/1024 (respectively), in order to fix the errors seen in
the tempest tests.
DocImpact
Change-Id: Ic0b5b38a840e737dc6be938230f4052974c8620f
Closes-bug: #1439817
Refactor the IpsetManager to move all the low level
knowledge about the ipset behaviour and performance
considerations from the firewall driver to the manager,
reduce redundant function names, and change missleading
variables talking about ipset chains, where they
should say ipset sets.
No logical changes to behaviour, just responsibilities
moved from one class to another.
Unit testing is move from iptables_firewall to ipset_manager
to test the new responsibilities of the class.
Implements: blueprint ipset-manager-refactor
Change-Id: I93000a37a71cd22753b32edbd0a5f3c9cb8b0bde
Add functional testing to the ipset_manager module to verify
it works as expected in combination with iptables matching.
Implements: blueprint add-ipset-to-security
Change-Id: Iec791ec30f87f6c00805f1d52c23b84aa7bc19de
Iptables chain is linear storage and filtering, when iptables rules are
large, the load of l2 agent is heavy, this patch introduces ipset to
security group for improving the security group performance.
Change-Id: I6ff0ac519d0b9034d3bb5270885ed3cc1805674d
Implements: blueprint add-ipset-to-security
DocImpact
Slawek Kaplonski