From b58bd4844a0f1a30ccae9ae8cd4cb85ed4a7fee2 Mon Sep 17 00:00:00 2001 From: karthik s Date: Fri, 10 Feb 2017 13:29:11 +0530 Subject: [PATCH] OVS2.6 changes for DPDK Configuration of DPDK parameters pmd-cpu-mask, dpdk-lcore-mask, dpdk-socket-mem are addressed as per OVS2.6, while maintaining backward compatibility with OVS 2.5 Implements: blueprint ovs-2-6-dpdk Change-Id: I3227189691df85f265cf84bd4115d8d4c9f979f3 Signed-off-by: karthik s --- Rakefile | 1 + lib/puppet/provider/vs_config/ovs.rb | 40 ++++- lib/puppet/type/vs_config.rb | 16 ++ manifests/dpdk.pp | 114 ++++++++----- manifests/params.pp | 2 + .../ovs2.6_support-cb3b2b5e75fde8ab.yaml | 9 + spec/classes/vswitch_dpdk_spec.rb | 156 +++++++++++------- spec/unit/puppet/lib/type/vs_config_spec.rb | 30 ++++ 8 files changed, 265 insertions(+), 103 deletions(-) create mode 100644 releasenotes/notes/ovs2.6_support-cb3b2b5e75fde8ab.yaml diff --git a/Rakefile b/Rakefile index 168d1081..6bd7cc29 100644 --- a/Rakefile +++ b/Rakefile @@ -1 +1,2 @@ require 'puppet-openstack_spec_helper/rake_tasks' +PuppetLint.configuration.send('disable_quoted_booleans') diff --git a/lib/puppet/provider/vs_config/ovs.rb b/lib/puppet/provider/vs_config/ovs.rb index 91a2f671..25ca7a8d 100644 --- a/lib/puppet/provider/vs_config/ovs.rb +++ b/lib/puppet/provider/vs_config/ovs.rb @@ -4,8 +4,6 @@ Puppet::Type.type(:vs_config).provide(:ovs) do commands :vsctl => 'ovs-vsctl' - mk_resource_methods - def self.munge_array_value(value) "[#{value[1..-2].split(',').map(&:strip).sort.join(",")}]" end @@ -81,7 +79,12 @@ Puppet::Type.type(:vs_config).provide(:ovs) do end def exists? - @property_hash[:ensure] == :present + # if skip_if_version matches ovs_version(), then skip the configuration by faking exists + if @resource[:skip_if_version].eql? ovs_version() + return true + else + @property_hash[:ensure] == :present + end end def destroy @@ -94,14 +97,39 @@ Puppet::Type.type(:vs_config).provide(:ovs) do end def _set - vsctl("set", "Open_vSwitch", ".", "#{@resource[:name]}=#{@resource[:value]}") + if @resource[:wait] == :false + vsctl("--no-wait", "set", "Open_vSwitch", ".", "#{@resource[:name]}=#{@resource[:value]}") + else + vsctl("set", "Open_vSwitch", ".", "#{@resource[:name]}=#{@resource[:value]}") + end end def create - _set + if @resource[:value].nil? or @resource[:value].empty? + destroy + else + _set + end + end + + def value + # if skip_if_version matches ovs_version(), then skip the configuration by returning the same value + if @resource[:skip_if_version].eql? ovs_version() + @resource[:value] + else + @property_hash[:value] + end + end + + def ovs_version + vsctl("--version")[/.*ovs-vsctl\s+\(Open\s+vSwitch\)\s+(\d+\.\d+)/][/(\d+\.\d+)/].chomp() end def value=(value) - _set + if @resource[:value].nil? or @resource[:value].empty? + destroy + else + _set + end end end diff --git a/lib/puppet/type/vs_config.rb b/lib/puppet/type/vs_config.rb index 2660869f..68ef2bbb 100644 --- a/lib/puppet/type/vs_config.rb +++ b/lib/puppet/type/vs_config.rb @@ -15,6 +15,22 @@ Puppet::Type.newtype(:vs_config) do end end + newparam(:skip_if_version) do + desc 'Skip setting the value when ovs version matches' + validate do |value| + unless value.is_a?(String) and value =~ /^\d+.\d+$/ + raise ArgumentError, "Invalid format for #{value}. Requires a String with format \d+.\d+, not a #{value.class}" + end + end + end + + newparam(:wait) do + desc 'Should it wait for ovs-vswitchd to reconfigure itself before it exits' + + newvalues(:true, :false) + defaultto :true + end + newproperty(:value) do desc 'Configuration value for the paramter' diff --git a/manifests/dpdk.pp b/manifests/dpdk.pp index 7733120b..46af0caa 100644 --- a/manifests/dpdk.pp +++ b/manifests/dpdk.pp @@ -4,15 +4,17 @@ # === Parameters # # [*memory_channels*] -# (required) The number of memory channels to use as an integer +# (optional) The number of memory channels to use as an integer. # # [*driver_type*] # (Optional) The DPDK Driver type # Defaults to 'vfio-pci' +# This parameter is required only for OVS versions <= 2.5. # # [*host_core_list*] -# (optional) The list of host cores to be used by the DPDK Poll Mode Driver -# The host_core_list is a string with format as [-c2][,c3[-c4],...] where c1, c2, etc are core indexes between 0 and 128 +# (optional) The list of cores to be used by the lcore threads. +# The host_core_list is a string with format as [-c2][,c3[-c4],...] +# where c1, c2, etc are core indexes between 0 and 128. # For example, to configure 3 cores the value should be "0-2" # # [*package_ensure*] @@ -20,18 +22,20 @@ # Defaults to 'present'. # # [*pmd_core_list*] -# (optional) The list of cores to be used by the OVS Agent -# The pmd_core_list is a string with format as [-c2][,c3[-c4],...] where c1, c2, etc are core indexes between 0 and 128 +# (optional) The list of cores to be used by the DPDK PMD threads. +# The pmd_core_list is a string with format as [-c2][,c3[-c4],...] where +# c1, c2, etc are core indexes between 0 and 128 # For example, to configure 3 cores the value should be "0-2" # # [*socket_mem*] # (Optional) Set the memory to be allocated on each socket -# The socket_mem is a string with comma separated memory list in MB in the order of socket numbers. -# For example, to allocate memory of 1GB for socket 1 and no allocation for socket 0, the value should be "0,1024" +# The socket_mem is a string with comma separated memory list in MB in the +# order of socket numbers. For example, to allocate memory of 1GB for +# socket 1 and no allocation for socket 0, the value should be "0,1024" # Defaults to undef. # class vswitch::dpdk ( - $memory_channels, + $memory_channels = undef, $driver_type = 'vfio-pci', $host_core_list = undef, $package_ensure = 'present', @@ -40,58 +44,90 @@ class vswitch::dpdk ( ) { include ::vswitch::params - kmod::load { 'vfio-pci': } + if $::osfamily != 'Redhat' { + fail( "${::osfamily} not yet supported for dpdk installation by puppet-vswitch") + } + package { $::vswitch::params::ovs_dpdk_package_name: ensure => $package_ensure, before => Service['openvswitch'], tag => 'openvswitch', } - # Set DPDK_OPTIONS to openvswitch + # DEPRECATED support for OVS 2.5 + # DPDK_OPTIONS is no longer used in ovs 2.6, since it was a distribution + # specific hack to the ovs-ctl scripts. Instead dpdk information is + # pulled from the ovsdb. if $socket_mem { $socket_string = "--socket-mem ${socket_mem}" } - + else { + $socket_string = undef + } if $driver_type { $pci_list = inline_template('<%= Facter.value("pci_address_driver_#@driver_type") %>') - unless empty($pci_list) { + if empty($pci_list) { + $white_list = undef + } + else { $white_list = inline_template('-w <%= @pci_list.gsub(",", " -w ") %>') } } - if !$host_core_list { - fail('host_core_list must be set for ovs agent when DPDK is enabled') - } $options = "DPDK_OPTIONS = \"-l ${host_core_list} -n ${memory_channels} ${socket_string} ${white_list}\"" - if $pmd_core_list { + file_line { '/etc/sysconfig/openvswitch': + path => '/etc/sysconfig/openvswitch', + match => '^DPDK_OPTIONS.*', + line => $options, + require => Package[$::vswitch::params::ovs_dpdk_package_name], + before => Service['openvswitch'], + } + + if $pmd_core_list and !empty($pmd_core_list){ $pmd_core_list_updated = inline_template('<%= @pmd_core_list.split(",").map{|c| c.include?("-")?(c.split("-").map(&:to_i)[0]..c.split("-").map(&:to_i)[1]).to_a.join(","):c}.join(",") %>') $pmd_core_mask = inline_template('<%= @pmd_core_list_updated.split(",").map{|c| 1<') - vs_config { 'other_config:pmd-cpu-mask': - value => $pmd_core_mask, - require => Service['openvswitch'], - } } - case $::osfamily { - 'Redhat': { - file_line { '/etc/sysconfig/openvswitch': - notify => Service['openvswitch'], - path => '/etc/sysconfig/openvswitch', - match => '^DPDK_OPTIONS.*', - line => $options, - require => Package[$::vswitch::params::ovs_dpdk_package_name], - before => Service['openvswitch'], - } + else { + $pmd_core_mask = undef + } - service { 'openvswitch': - ensure => true, - enable => true, - name => $::vswitch::params::ovs_service_name, - } - } - default: { - fail( "${::osfamily} not yet supported for dpdk installation by puppet-vswitch") - } + + if $host_core_list and !empty($host_core_list) { + $host_core_list_updated = inline_template('<%= @host_core_list.split(",").map{|c| c.include?("-")?(c.split("-").map(&:to_i)[0]..c.split("-").map(&:to_i)[1]).to_a.join(","):c}.join(",") %>') + $dpdk_lcore_mask = inline_template('<%= @host_core_list_updated.split(",").map{|c| 1<') } + else { + $dpdk_lcore_mask = undef + } + + if $memory_channels and !empty($memory_channels) { + $memory_channels_conf = "-n ${memory_channels}" + } + else { + $memory_channels_conf = undef + } + + $dpdk_configs = { + 'other_config:dpdk-extra' => { value => $memory_channels_conf, skip_if_version => '2.5'}, + 'other_config:dpdk-init' => { value => 'true', skip_if_version => '2.5'}, + 'other_config:dpdk-socket-mem' => { value => $socket_mem, skip_if_version => '2.5'}, + 'other_config:dpdk-lcore-mask' => { value => $dpdk_lcore_mask, skip_if_version => '2.5'}, + 'other_config:pmd-cpu-mask' => { value => $pmd_core_mask}, + } + + $dpdk_dependencies = { + wait => false, + require => Service['openvswitch'], + } + + service { 'openvswitch': + ensure => true, + enable => true, + name => $::vswitch::params::ovs_service_name, + } + + create_resources ('vs_config', $dpdk_configs, $dpdk_dependencies) } + diff --git a/manifests/params.pp b/manifests/params.pp index 8750224e..91ce31f1 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -17,6 +17,8 @@ class vswitch::params { $ovs_dpdk_package_name = 'openvswitch' $ovs_dkms_package_name = undef $ovs_service_name = 'openvswitch' + $ovs_service_hasstatus = undef + $ovs_status = undef $provider = 'ovs_redhat' } 'Debian': { diff --git a/releasenotes/notes/ovs2.6_support-cb3b2b5e75fde8ab.yaml b/releasenotes/notes/ovs2.6_support-cb3b2b5e75fde8ab.yaml new file mode 100644 index 00000000..1b35835e --- /dev/null +++ b/releasenotes/notes/ovs2.6_support-cb3b2b5e75fde8ab.yaml @@ -0,0 +1,9 @@ +upgrade: + - Support for OVS-DPDK is being extended for OvS 2.6 while retaining the + backward compatibility with OvS 2.5. In OvS 2.5 the DPDK parameters are + written as DPDK_OPTIONS to a sysconfig file, while in OvS 2.6 the DPDK + parameters are configured as ovsdb entries + - Parameter 'memory_channels' is required for OvS 2.5, while in OvS 2.6 its + optional + - parameter 'driver_type' is required only for OvS 2.5. Shall be deprecated + when OvS 2.5 support is discontinued diff --git a/spec/classes/vswitch_dpdk_spec.rb b/spec/classes/vswitch_dpdk_spec.rb index 6217093d..6804b519 100644 --- a/spec/classes/vswitch_dpdk_spec.rb +++ b/spec/classes/vswitch_dpdk_spec.rb @@ -4,7 +4,6 @@ describe 'vswitch::dpdk' do let :default_params do { :package_ensure => 'present', - :memory_channels => '2', } end @@ -21,15 +20,7 @@ describe 'vswitch::dpdk' do shared_examples_for 'vswitch::dpdk on RedHat' do let(:params) { default_params } - context 'should raise error when not passing host_core_list' do - it_raises 'a Puppet::Error', /host_core_list must be set for ovs agent when DPDK is enabled/ - end - - context 'basic parameters' do - before :each do - params.merge!(:host_core_list => '1,2') - end - + context 'shall write DPDK_OPTIONS as well as ovsdb params' do it 'include the class' do is_expected.to contain_class('vswitch::dpdk') end @@ -43,8 +34,6 @@ describe 'vswitch::dpdk' do :ensure => true, :enable => true, :name => platform_params[:ovs_service_name], - :hasstatus => platform_params[:service_hasstatus], - :status => platform_params[:service_status], ) end @@ -59,36 +48,106 @@ describe 'vswitch::dpdk' do it 'should have dpdk driver modules file' do is_expected.to contain_kmod__load('vfio-pci') end + it 'configures dpdk options with socket memory' do + is_expected.to contain_file_line('/etc/sysconfig/openvswitch') - it 'configures dpdk options for ovs' do - is_expected.to contain_file_line('/etc/sysconfig/openvswitch').with( - :path => '/etc/sysconfig/openvswitch', - :match => '^DPDK_OPTIONS.*', - :line => 'DPDK_OPTIONS = "-l 1,2 -n 2 "', - :before => 'Service[openvswitch]', + is_expected.to contain_vs_config('other_config:dpdk-init').with( + :value => 'true', :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:pmd-cpu-mask').with( + :value => nil, :wait => false, + ) + is_expected.to contain_vs_config('other_config:dpdk-socket-mem').with( + :value => nil, :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:dpdk-lcore-mask').with( + :value => nil, :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:dpdk-extra').with( + :value => nil, :wait => false, :skip_if_version => "2.5", ) end end - context 'when passing socket mem' do + + context 'when passing all empty params' do before :each do - params.merge!(:host_core_list => '1,2') - params.merge!(:socket_mem => '1024') + params.merge!(:host_core_list => '') + params.merge!(:socket_mem => '') + params.merge!(:memory_channels => '' ) + params.merge!(:pmd_core_list => '') end - it 'configures dpdk options with socket memory' do + it 'configures dpdk options' do + is_expected.to contain_file_line('/etc/sysconfig/openvswitch').with( + :path => '/etc/sysconfig/openvswitch', + :match => '^DPDK_OPTIONS.*', + :before => 'Service[openvswitch]', + ) + is_expected.to contain_vs_config('other_config:dpdk-init').with( + :value => 'true', :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:pmd-cpu-mask').with( + :value => nil, :wait => false, + ) + is_expected.to contain_vs_config('other_config:dpdk-socket-mem').with( + :value => '', :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:dpdk-lcore-mask').with( + :value => nil, :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:dpdk-extra').with( + :value => nil, :wait => false, :skip_if_version => "2.5", + ) + + end + end + + + context 'when passing all params' do + before :each do + params.merge!(:host_core_list => '1,2') + params.merge!(:socket_mem => '1024') + params.merge!(:memory_channels => 2) + params.merge!(:pmd_core_list => '22,23,24,25,66,67,68,69') + end + it 'configures dpdk options' do is_expected.to contain_file_line('/etc/sysconfig/openvswitch').with( :path => '/etc/sysconfig/openvswitch', :match => '^DPDK_OPTIONS.*', :line => 'DPDK_OPTIONS = "-l 1,2 -n 2 --socket-mem 1024 "', :before => 'Service[openvswitch]', ) + is_expected.to contain_vs_config('other_config:dpdk-init').with( + :value => 'true', :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:pmd-cpu-mask').with( + :value => '3c0000000003c00000', :wait => false, + ) + is_expected.to contain_vs_config('other_config:dpdk-socket-mem').with( + :value => '1024', :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:dpdk-lcore-mask').with( + :value => '6', :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:dpdk-extra').with( + :value => '-n 2', :wait => false, :skip_if_version => "2.5", + ) end end context 'when providing valid driver type facts' do + let :facts do + OSDefaults.get_facts({ + :osfamily => 'Redhat', + :operatingsystem => 'RedHat', + :ovs_version => '2.5.1', + :pci_address_driver_test => '0000:00:05.0,0000:00:05.1' + }) + end + before :each do - params.merge!(:host_core_list => '1,2') - params.merge!(:driver_type => 'test') - facts.merge!({ :pci_address_driver_test => '0000:00:05.0,0000:00:05.1' }) + params.merge!(:host_core_list => '1,2') + params.merge!(:driver_type => 'test') + params.merge!(:memory_channels => 2) end it 'configures dpdk options with pci address for driver test' do is_expected.to contain_file_line('/etc/sysconfig/openvswitch').with( @@ -97,37 +156,19 @@ describe 'vswitch::dpdk' do :line => 'DPDK_OPTIONS = "-l 1,2 -n 2 -w 0000:00:05.0 -w 0000:00:05.1"', :before => 'Service[openvswitch]', ) - end - end + is_expected.to contain_vs_config('other_config:dpdk-init').with( + :value => 'true', :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:pmd-cpu-mask').with( + :value => nil, :wait => false, + ) + is_expected.to contain_vs_config('other_config:dpdk-socket-mem').with( + :value => nil, :wait => false, :skip_if_version => "2.5", + ) + is_expected.to contain_vs_config('other_config:dpdk-lcore-mask').with( + :value => '6', :wait => false, :skip_if_version => "2.5", + ) - context 'when passing pmd core list with comma seperator for ovs' do - before :each do - params.merge!(:host_core_list => '1,2') - params.merge!(:pmd_core_list => '22,23,24,25,66,67,68,69') - end - it 'configures pmd core mask for ovs' do - is_expected.to contain_vs_config('other_config:pmd-cpu-mask').with( - :value => '3c0000000003c00000', - ) - end - end - context 'when passing pmd core list with range for ovs' do - before :each do - params.merge!(:host_core_list => '1,2') - params.merge!(:pmd_core_list => '22-25,66,67,68,69') - end - it 'configures pmd core mask for ovs' do - is_expected.to contain_vs_config('other_config:pmd-cpu-mask').with( - :value => '3c0000000003c00000', - ) - end - end - context 'when not passing pmd core list for ovs' do - before :each do - params.merge!(:host_core_list => '1,2') - end - it 'should not configure pmd core mask for ovs' do - is_expected.to_not contain_vs_config('other_config:pmd-cpu-mask') end end end @@ -137,9 +178,8 @@ describe 'vswitch::dpdk' do }).each do |os,facts| context "on #{os}" do let (:facts) do - facts.merge!(OSDefaults.get_facts({ :ovs_version => '1.4.2' })) + facts.merge!(OSDefaults.get_facts({ :ovs_version => '2.6.1' })) end - let (:platform_params) do case facts[:osfamily] when 'Debian' @@ -151,10 +191,10 @@ describe 'vswitch::dpdk' do :ovs_dpdk_package_name => 'openvswitch', :ovs_service_name => 'openvswitch', :provider => 'ovs_redhat', + :ovsdb_service_name => 'ovsdb-server', } end end - it_behaves_like "vswitch::dpdk on #{facts[:osfamily]}" end end diff --git a/spec/unit/puppet/lib/type/vs_config_spec.rb b/spec/unit/puppet/lib/type/vs_config_spec.rb index 0ca37d76..67aa265d 100644 --- a/spec/unit/puppet/lib/type/vs_config_spec.rb +++ b/spec/unit/puppet/lib/type/vs_config_spec.rb @@ -13,6 +13,36 @@ describe Puppet::Type.type(:vs_config) do end.to_not raise_error end + it "wait property should accept boolean values" do + expect do + described_class.new({:name => "foo", :value => "[2, 1, 3, 0]", :ensure => :present, :wait => true}) + end.to_not raise_error + end + + it "wait property should throw error for non boolean values" do + expect do + described_class.new({:name => "foo", :value => "123", :ensure => :present, :wait => "abc"}) + end.to raise_error(Puppet::Error) + end + + it "skip_if_version param should accept string values of format \d+.\d+" do + expect do + described_class.new({:name => "foo", :value => "[2, 1, 3, 0]", :ensure => :present, :skip_if_version => '2.5'}) + end.to_not raise_error + expect do + described_class.new({:name => "foo", :value => "[2, 1, 3, 0]", :ensure => :present, :skip_if_version => 'a2.5'}) + end.to raise_error(Puppet::Error) + expect do + described_class.new({:name => "foo", :value => "[2, 1, 3, 0]", :ensure => :present, :skip_if_version => '2.5a'}) + end.to raise_error(Puppet::Error) + end + + it "skip_if_version param should not accept non string values" do + expect do + described_class.new({:name => "foo", :value => "[2, 1, 3, 0]", :ensure => :present, :skip_if_version => 2.5}) + end.to raise_error(Puppet::Error) + end + it "should have a :value parameter" do expect(described_class.attrtype(:value)).to eq(:property) end