From 45ec97a708582c7223c1cb1b7ae3ec0738228105 Mon Sep 17 00:00:00 2001 From: Yasuhito Takamiya Date: Wed, 10 Jun 2015 09:38:46 +0900 Subject: [PATCH] Fix PacketIn accessor methods (raw_data = VLAN tagged UDP). --- CHANGELOG.md | 5 +++ Gemfile | 2 +- lib/pio/ipv4_header.rb | 2 - lib/pio/open_flow/message.rb | 72 ++++++++------------------------ lib/pio/open_flow10/packet_in.rb | 2 +- lib/pio/version.rb | 2 +- pio.gemspec | 16 +++---- spec/pio/mac_spec.rb | 4 +- spec/pio/packet_out_spec.rb | 3 +- 9 files changed, 37 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fded05d6..293b8acb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ ## develop (unreleased) +## 0.20.1 (6/10/2015) +### Bugs fixed +* [#167](https://github.com/trema/pio/pull/167): Fix PacketIn accessor methods (raw_data = VLAN tagged UDP). + + ## 0.20.0 (4/22/2015) ### New features * [#138](https://github.com/trema/pio/pull/138): Add new class `Pio::Features::Request`. diff --git a/Gemfile b/Gemfile index 53d59f39..67e3cb67 100644 --- a/Gemfile +++ b/Gemfile @@ -3,5 +3,5 @@ source 'https://rubygems.org' gemspec development_group: :test group :development do - gem 'byebug', '~> 4.0.5', platforms: :ruby_20 + gem 'byebug', '~> 5.0.0', platforms: :ruby_20 end diff --git a/lib/pio/ipv4_header.rb b/lib/pio/ipv4_header.rb index 8fbd3c41..6783759e 100644 --- a/lib/pio/ipv4_header.rb +++ b/lib/pio/ipv4_header.rb @@ -13,7 +13,6 @@ module ProtocolNumber include Payload # rubocop:disable MethodLength - # rubocop:disable AbcSize # This method smells of :reek:TooManyStatements def self.included(klass) def klass.ipv4_header(options = {}) @@ -33,7 +32,6 @@ def klass.ipv4_header(options = {}) end end # rubocop:enable MethodLength - # rubocop:enable AbcSize private diff --git a/lib/pio/open_flow/message.rb b/lib/pio/open_flow/message.rb index 6891f6df..d64deeee 100644 --- a/lib/pio/open_flow/message.rb +++ b/lib/pio/open_flow/message.rb @@ -6,20 +6,12 @@ module Pio module OpenFlow # Defines shortcuts to OpenFlow header fields. + # rubocop:disable MethodLength class Message def self.factory(klass, message_type, &block) klass.extend Forwardable klass.module_eval(&block) if block - klass.module_eval _format_class(klass, message_type) - klass.module_eval(&_define_open_flow_accessors) - klass.module_eval(&_define_self_read) - klass.module_eval(&_define_initialize) - klass.module_eval(&_define_to_binary) - end - - # rubocop:disable MethodLength - def self._format_class(klass, message_type) - %( + klass.module_eval <<-EOT, __FILE__, __LINE__ class Format < BinData::Record endian :big @@ -36,13 +28,7 @@ class Format < BinData::Record def self.format const_get :Format end - ) - end - # rubocop:enable MethodLength - # rubocop:disable MethodLength - def self._define_open_flow_accessors - proc do def_delegators :@format, :snapshot def_delegators :snapshot, :open_flow_header def_delegators :open_flow_header, :ofp_version @@ -53,61 +39,39 @@ def self._define_open_flow_accessors def_delegators :snapshot, :body def_delegator :snapshot, :body, :user_data - end - end - # rubocop:enable MethodLength - def self._define_self_read - proc do def self.read(raw_data) allocate.tap do |message| message.instance_variable_set(:@format, format.read(raw_data)) end rescue BinData::ValidityError message_name = name.split('::')[1..-1].join(' ') - raise Pio::ParseError, "Invalid #{message_name} message." + raise Pio::ParseError, "Invalid \#{message_name} message." end - end - end - # rubocop:disable MethodLength - # rubocop:disable AbcSize - def self._define_initialize - proc do def initialize(user_options = {}) header_options = OpenFlowHeader::Options.parse(user_options) - body_options = parse_body_options(user_options) + body_options = if user_options.respond_to?(:fetch) + user_options.delete :transaction_id + user_options.delete :xid + dpid = user_options[:dpid] + user_options[:datapath_id] = dpid if dpid + if user_options.keys.size > 1 + user_options + else + user_options[:user_data] || '' + end + else + '' + end @format = self.class.format.new(open_flow_header: header_options, body: body_options) end - private - - def parse_body_options(options) - if options.respond_to?(:fetch) - options.delete :transaction_id - options.delete :xid - dpid = options[:dpid] - options[:datapath_id] = dpid if dpid - if options.keys.size > 1 - options - else - options[:user_data] || '' - end - else - '' - end - end - end - end - # rubocop:enable MethodLength - # rubocop:enable AbcSize - - def self._define_to_binary - proc do def_delegator :@format, :to_binary_s, :to_binary - end + EOT end + # rubocop:enable MethodLength end end end diff --git a/lib/pio/open_flow10/packet_in.rb b/lib/pio/open_flow10/packet_in.rb index 483ce75a..d586d52f 100644 --- a/lib/pio/open_flow10/packet_in.rb +++ b/lib/pio/open_flow10/packet_in.rb @@ -75,7 +75,7 @@ class IPv4Packet < BinData::Record def self.read(raw_data) ethernet_header = EtherTypeParser.read(raw_data) case ethernet_header.ether_type - when EthernetHeader::EtherType::IPV4 + when EthernetHeader::EtherType::IPV4, EthernetHeader::EtherType::VLAN IPv4Packet.read raw_data when EthernetHeader::EtherType::ARP Pio::Arp.read raw_data diff --git a/lib/pio/version.rb b/lib/pio/version.rb index 432f08d2..8173fb5c 100644 --- a/lib/pio/version.rb +++ b/lib/pio/version.rb @@ -1,5 +1,5 @@ # Base module. module Pio # gem version. - VERSION = '0.20.0'.freeze + VERSION = '0.20.1'.freeze end diff --git a/pio.gemspec b/pio.gemspec index 8f5cf86a..338b059b 100644 --- a/pio.gemspec +++ b/pio.gemspec @@ -31,33 +31,33 @@ Gem::Specification.new do |gem| gem.add_dependency 'bindata', '~> 2.1.0' gem.add_development_dependency 'rake' - gem.add_development_dependency 'bundler', '~> 1.9.4' + gem.add_development_dependency 'bundler', '~> 1.10.3' gem.add_development_dependency 'pry', '~> 0.10.1' # Guard - gem.add_development_dependency 'guard', '~> 2.12.5' + gem.add_development_dependency 'guard', '~> 2.12.6' gem.add_development_dependency 'guard-bundler', '~> 2.1.0' gem.add_development_dependency 'guard-cucumber', '~> 1.6.0' - gem.add_development_dependency 'guard-rspec', '~> 4.5.0' + gem.add_development_dependency 'guard-rspec', '~> 4.5.2' gem.add_development_dependency 'guard-rubocop', '~> 1.2.0' gem.add_development_dependency 'rb-fchange', '~> 0.0.6' - gem.add_development_dependency 'rb-fsevent', '~> 0.9.4' + gem.add_development_dependency 'rb-fsevent', '~> 0.9.5' gem.add_development_dependency 'rb-inotify', '~> 0.9.5' gem.add_development_dependency 'terminal-notifier-guard', '~> 1.6.4' # Docs - gem.add_development_dependency 'inch', '~> 0.5.10' + gem.add_development_dependency 'inch', '~> 0.6.2' gem.add_development_dependency 'relish', '~> 0.7.1' gem.add_development_dependency 'yard', '~> 0.8.7.6' # Test gem.add_development_dependency 'codeclimate-test-reporter', '~> 0.4.7' - gem.add_development_dependency 'coveralls', '~> 0.8.0' + gem.add_development_dependency 'coveralls', '~> 0.8.1' gem.add_development_dependency 'cucumber', '~> 2.0.0' gem.add_development_dependency 'flay', '~> 2.6.1' gem.add_development_dependency 'flog', '~> 4.3.2' - gem.add_development_dependency 'reek', '~> 2.1.0' + gem.add_development_dependency 'reek', '~> 2.2.1' gem.add_development_dependency 'rspec', '~> 3.2.0' gem.add_development_dependency 'rspec-given', '~> 3.7.0' - gem.add_development_dependency 'rubocop', '~> 0.30.0' + gem.add_development_dependency 'rubocop', '~> 0.32.0' end diff --git a/spec/pio/mac_spec.rb b/spec/pio/mac_spec.rb index 0a917c27..cb4c9011 100644 --- a/spec/pio/mac_spec.rb +++ b/spec/pio/mac_spec.rb @@ -142,9 +142,9 @@ context 'with reserved address' do (0x0..0xf).each do |each| octet = format('%02x', each) - reserved_address = "01:80:c2:00:00:#{ octet }" + reserved_address = "01:80:c2:00:00:#{octet}" - context "when #{ reserved_address }" do + context "when #{reserved_address}" do let(:value) { reserved_address } describe '#reserved?' do diff --git a/spec/pio/packet_out_spec.rb b/spec/pio/packet_out_spec.rb index c9bc729a..e4da99b5 100644 --- a/spec/pio/packet_out_spec.rb +++ b/spec/pio/packet_out_spec.rb @@ -86,8 +86,7 @@ When(:binary) { [1, 0, 0, 8, 0, 0, 0, 0].pack('C*') } Then do - result == Failure(Pio::ParseError, - 'Invalid PacketOut message.') + result == Failure(Pio::ParseError, 'Invalid PacketOut message.') end end end