-
Notifications
You must be signed in to change notification settings - Fork 484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
M #-: Add IP Spoofing support for NIC_ALIAS #4764
base: master
Are you sure you want to change the base?
M #-: Add IP Spoofing support for NIC_ALIAS #4764
Conversation
Add IP addresses of a NIC_ALIAS to the corresponding ipset if the parent NIC belongs to the "Bridged & Security Groups" network mode (fw) when a VM is instantiated or a NIC_ALIAS attached. Signed-off-by: Ricardo Diaz <[email protected]>
# Execute post-boot networking setup | ||
{ | ||
:driver => :vnm, | ||
:action => :post, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not clean action?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean action deletes the whole iptables chain included the rules for the parent NIC
nic_build_hash(nic_element, nic) | ||
|
||
if !VNMMAD.pre_action? | ||
nic.get_info(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these functions work/make sense for NIC_ALIAS? (when calling it as nics_build('TEMPLATE/NIC_ALIAS')
# @param element [String] the NIC_ID | ||
# @return [Hash] the NIC_ALIAS | ||
def nic_alias(id) | ||
if @nic_aliases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better use exit condition.
return nil if @nic_aliases.nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change it
@@ -334,6 +334,14 @@ def self.vars(vm, nic, sg_id = nil) | |||
vars[:set_sg_out] = "#{vars[:chain]}-#{sg_id}-o" | |||
end | |||
|
|||
vars[:nic_aliases] = [] | |||
|
|||
unless nic[:alias_ids].nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer if !nic[:alias_ids].nil? for single statements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change it
Hello. Such behaviour and delaying PRs with years is highly discouraging to do project contributions. Regards, |
Thanks @atodorov-storpool for this an other contributions to the project. As you see this PR is still in review and will include reference to the affected bugs. As per the implementation this is indeed inspired by your PR, but re-written to improve its design and better fit in the driver. Also note that design decisions evolve with the code and need to consider its relation with other new components. Finally, we need to prioritize our available development resources, that unfortunately force us to delay some contributions. Please note that these decisions are not easy for us, and by not means imply that we underestimate your contributions. Thanks again for your valuable contributions and hope you'll keep making them in the future. |
What is the status of this? It would be nice to have this included in opennebula... |
I have tested that this commit applies fine onto 5.12.0.3 and fixes the issue by adding/removing alias ips to the ipset. |
bdce88d
to
14635f2
Compare
339f9c9
to
1cf07c3
Compare
Add IP addresses of a NIC_ALIAS to the corresponding ipset if the parent
NIC belongs to the "Bridged & Security Groups" network mode (fw) when a
VM is instantiated or a NIC_ALIAS attached.
Signed-off-by: Ricardo Diaz [email protected]