From 940f0db7246161689d9c2ce717da6446b3456522 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Tue, 26 Sep 2017 16:48:43 +0000 Subject: [PATCH 1/6] Fix bug printing error in config leader --- src/apps/config/leader.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apps/config/leader.lua b/src/apps/config/leader.lua index 3f27828577..bdca7307c2 100644 --- a/src/apps/config/leader.lua +++ b/src/apps/config/leader.lua @@ -401,7 +401,7 @@ local function path_adder_for_grammar(grammar, path) return function(config, subconfig) local tab = getter(config) for k,_ in pairs(subconfig) do - if tab[k] ~= nil then error('already-existing entry', k) end + if tab[k] ~= nil then error('already-existing entry') end end for k,v in pairs(subconfig) do tab[k] = v end return config From edb15dd3cf5807a503e1576d8398fece1b5446cd Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Tue, 26 Sep 2017 16:48:52 +0000 Subject: [PATCH 2/6] Fix accessing first queue This commit adds a common lwutil.parse_instance function and uses it to access the first queue. The first queue is not guaranteed to be at position 1 in the values array; position 1 could have been filled but later deleted. --- src/apps/lwaftr/lwaftr.lua | 10 +++---- src/apps/lwaftr/lwutil.lua | 18 +++++++++++++ src/program/lwaftr/run/run.lua | 9 +++---- src/program/lwaftr/setup.lua | 38 +++++++-------------------- src/program/snabbvmx/lwaftr/setup.lua | 22 ++-------------- 5 files changed, 37 insertions(+), 60 deletions(-) diff --git a/src/apps/lwaftr/lwaftr.lua b/src/apps/lwaftr/lwaftr.lua index 9fc98d2dd6..98ef360c05 100644 --- a/src/apps/lwaftr/lwaftr.lua +++ b/src/apps/lwaftr/lwaftr.lua @@ -342,15 +342,11 @@ local function select_instance(conf) for k,v in pairs(t2) do ret[k] = v end return ret end - local device = next(conf.softwire_config.instance) + local device, id, queue = lwutil.parse_instance(conf) conf.softwire_config.external_interface = table_merge( - conf.softwire_config.external_interface, - conf.softwire_config.instance[device].queue.values[1].external_interface - ) + conf.softwire_config.external_interface, queue.external_interface) conf.softwire_config.internal_interface = table_merge( - conf.softwire_config.internal_interface, - conf.softwire_config.instance[device].queue.values[1].internal_interface - ) + conf.softwire_config.internal_interface, queue.internal_interface) return conf end diff --git a/src/apps/lwaftr/lwutil.lua b/src/apps/lwaftr/lwutil.lua index 25848bee4f..d6721289ba 100644 --- a/src/apps/lwaftr/lwutil.lua +++ b/src/apps/lwaftr/lwutil.lua @@ -6,6 +6,7 @@ local S = require("syscall") local bit = require("bit") local ffi = require("ffi") local lib = require("core.lib") +local cltable = require("lib.cltable") local band = bit.band local cast = ffi.cast @@ -18,6 +19,23 @@ local ehs = constants.ethernet_header_size local o_ipv4_flags = constants.o_ipv4_flags local ntohs = lib.ntohs +-- Return device PCI address, queue ID, and queue configuration. +function parse_instance(conf) + local device, instance + for k, v in pairs(conf.softwire_config.instance) do + assert(device == nil, "configuration has more than one instance") + device, instance = k, v + end + assert(device ~= nil, "configuration has no instance") + local id, queue + for k, v in cltable.pairs(instance.queue) do + assert(id == nil, "configuration has more than one RSS queue") + id, queue = k.id, v + end + assert(id ~= nil, "configuration has no RSS queues") + return device, id, queue +end + function get_ihl_from_offset(pkt, offset) local ver_and_ihl = pkt.data[offset] return band(ver_and_ihl, 0xf) * 4 diff --git a/src/program/lwaftr/run/run.lua b/src/program/lwaftr/run/run.lua index 171fad853d..881b383be2 100644 --- a/src/program/lwaftr/run/run.lua +++ b/src/program/lwaftr/run/run.lua @@ -132,10 +132,10 @@ end -- Requires a V4V6 splitter if running in on-a-stick mode and VLAN tag values -- are the same for the internal and external interfaces. local function requires_splitter (opts, conf) + local device, id, queue = lwutil.parse_instance(conf) if opts["on-a-stick"] then - local _, instance = next(conf.softwire_config.instance) - local internal_interface = instance.queue.values[1].internal_interface - local external_interface = instance.queue.values[1].external_interface + local internal_interface = queue.internal_interface + local external_interface = queue.external_interface return internal_interface.vlan_tag == external_interface.vlan_tag end return false @@ -161,8 +161,7 @@ function run(args) -- If instance has external-interface.device configure as bump-in-the-wire -- otherwise configure it in on-a-stick mode. - local name, instance = next(lwconfig.softwire_config.instance) - local queue = instance.queue.values[1] + local device, id, queue = lwutil.parse_instance(lwconfig) if queue.external_interface.device then return setup.load_phy(graph, lwconfig, 'inetNic', 'b4sideNic', opts.ring_buffer_size) diff --git a/src/program/lwaftr/setup.lua b/src/program/lwaftr/setup.lua index 90b4277f97..178c64f535 100644 --- a/src/program/lwaftr/setup.lua +++ b/src/program/lwaftr/setup.lua @@ -26,7 +26,6 @@ local ipv4 = require("lib.protocol.ipv4") local ethernet = require("lib.protocol.ethernet") local ipv4_ntop = require("lib.yang.util").ipv4_ntop local binary = require("lib.yang.binary") -local cltable = require("lib.cltable") local S = require("syscall") local engine = require("core.app") local lib = require("core.lib") @@ -60,23 +59,6 @@ local function validate_pci_devices(devices) end end --- Return device PCI address, queue ID, and queue configuration. -local function parse_instance(conf) - local device, instance - for k, v in pairs(conf.softwire_config.instance) do - assert(device == nil, "configuration has more than one instance") - device, instance = k, v - end - assert(device ~= nil, "configuration has no instance") - local id, queue - for k, v in cltable.pairs(instance.queue) do - assert(id == nil, "configuration has more than one RSS queue") - id, queue = k.id, v - end - assert(id ~= nil, "configuration has no RSS queues") - return device, id, queue -end - function lwaftr_app(c, conf) assert(type(conf) == 'table') @@ -101,7 +83,7 @@ function lwaftr_app(c, conf) end switch_names(conf) - local device, id, queue = parse_instance(conf) + local device, id, queue = lwutil.parse_instance(conf) -- Global interfaces local gexternal_interface = conf.softwire_config.external_interface @@ -237,7 +219,7 @@ local function link_sink(c, v4_out, v6_out) end function load_phy(c, conf, v4_nic_name, v6_nic_name, ring_buffer_size) - local v4_pci, id, queue = parse_instance(conf) + local v4_pci, id, queue = lwutil.parse_instance(conf) local v6_pci = queue.external_interface.device local v4_info = pci.device_info(v4_pci) local v6_info = pci.device_info(v6_pci) @@ -268,7 +250,7 @@ function load_phy(c, conf, v4_nic_name, v6_nic_name, ring_buffer_size) end function load_on_a_stick(c, conf, args) - local pciaddr, id, queue = parse_instance(conf) + local pciaddr, id, queue = lwutil.parse_instance(conf) local device = pci.device_info(pciaddr) local driver = require(device.driver).driver validate_pci_devices({pciaddr}) @@ -326,7 +308,7 @@ function load_on_a_stick(c, conf, args) end function load_virt(c, conf, v4_nic_name, v6_nic_name) - local v4_pci, id, queue = parse_instance(conf) + local v4_pci, id, queue = lwutil.parse_instance(conf) local v6_pci = queue.external_device.device lwaftr_app(c, conf, device) @@ -345,7 +327,7 @@ function load_virt(c, conf, v4_nic_name, v6_nic_name) end function load_bench(c, conf, v4_pcap, v6_pcap, v4_sink, v6_sink) - local device, id, queue = parse_instance(conf) + local device, id, queue = lwutil.parse_instance(conf) lwaftr_app(c, conf, device) config.app(c, "capturev4", pcap.PcapReader, v4_pcap) @@ -380,7 +362,7 @@ function load_bench(c, conf, v4_pcap, v6_pcap, v4_sink, v6_sink) end function load_check_on_a_stick (c, conf, inv4_pcap, inv6_pcap, outv4_pcap, outv6_pcap) - local device, id, queue = parse_instance(conf) + local device, id, queue = lwutil.parse_instance(conf) lwaftr_app(c, conf, device) config.app(c, "capturev4", pcap.PcapReader, inv4_pcap) @@ -432,7 +414,7 @@ function load_check_on_a_stick (c, conf, inv4_pcap, inv6_pcap, outv4_pcap, outv6 end function load_check(c, conf, inv4_pcap, inv6_pcap, outv4_pcap, outv6_pcap) - local device, id, queue = parse_instance(conf) + local device, id, queue = lwutil.parse_instance(conf) lwaftr_app(c, conf, device) config.app(c, "capturev4", pcap.PcapReader, inv4_pcap) @@ -470,7 +452,7 @@ function load_check(c, conf, inv4_pcap, inv6_pcap, outv4_pcap, outv6_pcap) end function load_soak_test(c, conf, inv4_pcap, inv6_pcap) - local device, id, queue = parse_instance(conf) + local device, id, queue = lwutil.parse_instance(conf) lwaftr_app(c, conf, device) config.app(c, "capturev4", pcap.PcapReader, inv4_pcap) @@ -512,7 +494,7 @@ function load_soak_test(c, conf, inv4_pcap, inv6_pcap) end function load_soak_test_on_a_stick (c, conf, inv4_pcap, inv6_pcap) - local device, id, queue = parse_instance(conf) + local device, id, queue = lwutil.parse_instance(conf) lwaftr_app(c, conf, device) config.app(c, "capturev4", pcap.PcapReader, inv4_pcap) @@ -692,6 +674,7 @@ local function compute_worker_configs(conf) local make_copy = copier(conf) for device, queues in pairs(conf.softwire_config.instance) do for k, _ in cltable.pairs(queues.queue) do + local worker_id = string.format('%s/%s', device, k.id) local worker_config = make_copy() local instance = worker_config.softwire_config.instance for other_device, queues in pairs(conf.softwire_config.instance) do @@ -705,7 +688,6 @@ local function compute_worker_configs(conf) end end end - local worker_id = string.format('%s/%s', device, k.id) ret[worker_id] = worker_config end end diff --git a/src/program/snabbvmx/lwaftr/setup.lua b/src/program/snabbvmx/lwaftr/setup.lua index d94b3332ec..5623d63dd8 100644 --- a/src/program/snabbvmx/lwaftr/setup.lua +++ b/src/program/snabbvmx/lwaftr/setup.lua @@ -18,7 +18,6 @@ local lwaftr = require("apps.lwaftr.lwaftr") local lwutil = require("apps.lwaftr.lwutil") local constants = require("apps.lwaftr.constants") local nh_fwd = require("apps.lwaftr.nh_fwd") -local cltable = require("lib.cltable") local pci = require("lib.hardware.pci") local raw = require("apps.socket.raw") local tap = require("apps.tap.tap") @@ -45,26 +44,9 @@ local function load_driver (pciaddr) return require(device_info.driver).driver, device_info.rx, device_info.tx end --- Return device PCI address, queue ID, and queue configuration. -local function parse_instance(conf) - local device, instance - for k, v in pairs(conf.softwire_config.instance) do - assert(device == nil, "configuration has more than one instance") - device, instance = k, v - end - assert(device ~= nil, "configuration has no instance") - local id, queue - for k, v in cltable.pairs(instance.queue) do - assert(id == nil, "configuration has more than one RSS queue") - id, queue = k.id, v - end - assert(id ~= nil, "configuration has no RSS queues") - return device, id, queue -end - local function load_virt (c, nic_id, lwconf, interface) -- Validate the lwaftr and split the interfaces into global and instance. - local device, id, queue = parse_instance(lwconf) + local device, id, queue = lwutil.parse_instance(lwconf) local gexternal_interface = lwconf.softwire_config.external_interface local ginternal_interface = lwconf.softwire_config.internal_interface @@ -161,7 +143,7 @@ function lwaftr_app(c, conf, lwconf, sock_path) assert(type(lwconf) == 'table') -- Validate the lwaftr and split the interfaces into global and instance. - local device, id, queue = parse_instance(lwconf) + local device, id, queue = lwutil.parse_instance(lwconf) local gexternal_interface = lwconf.softwire_config.external_interface local ginternal_interface = lwconf.softwire_config.internal_interface From 1cccf7a210474812f8d8403aceab9c9c4555a063 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 27 Sep 2017 10:44:22 +0200 Subject: [PATCH 3/6] Fix sparse cltable value serialization It's possible that cltable values aren't packed, so we need to serialize value indices in addition to values. Bump binary version as well. --- src/lib/yang/binary.lua | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/lib/yang/binary.lua b/src/lib/yang/binary.lua index f43186ddf9..5cb7f23725 100644 --- a/src/lib/yang/binary.lua +++ b/src/lib/yang/binary.lua @@ -14,7 +14,7 @@ local ctable = require('lib.ctable') local cltable = require('lib.cltable') local MAGIC = "yangconf" -local VERSION = 0x00005000 +local VERSION = 0x00006000 local header_t = ffi.typeof([[ struct { @@ -217,7 +217,10 @@ local function data_emitter(production) stream:write_stringref('cltable') emit_keys(data.keys, stream) stream:write_uint32(#data.values) - for i=1,#data.values do emit_value(data.values[i], stream) end + for i, value in ipairs(data.values) do + stream:write_uint32(i) + emit_value(value, stream) + end end else local emit_key = visit1({type='struct', members=production.keys, @@ -410,7 +413,10 @@ local function read_compiled_data(stream, strtab) function readers.cltable() local keys = read1() local values = {} - for i=1,stream:read_uint32() do table.insert(values, read1()) end + for i=1,stream:read_uint32() do + local i = stream:read_uint32() + values[i] = read1() + end return cltable.build(keys, values) end function readers.lltable() From 0e73d944c6e301765e235f201457971236a68ff0 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 27 Sep 2017 09:10:57 +0000 Subject: [PATCH 4/6] Fix insertion into sparse cltables Previously we would do a table.insert, then assume the index was inserted at #values. However the # operator just returns a filled index for which the next index is nil, and thus table.insert could insert into the middle of a table but we don't know afterwards where that insertion was! So, instead use # to find an insertion point, and don't use table.insert. --- src/lib/cltable.lua | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib/cltable.lua b/src/lib/cltable.lua index 885c960e6d..c583e1c4fb 100644 --- a/src/lib/cltable.lua +++ b/src/lib/cltable.lua @@ -28,8 +28,9 @@ function set(cltable, key, value) cltable.values[entry.value] = value if value == nil then cltable.keys:remove_ptr(entry) end elseif value ~= nil then - table.insert(cltable.values, value) - cltable.keys:add(key, #cltable.values) + local idx = #cltable.values + 1 + cltable.values[idx] = value + cltable.keys:add(key, idx) end end From 293838e44d7451b2f0991e465205cb351bde501e Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 27 Sep 2017 09:13:41 +0000 Subject: [PATCH 5/6] Fix serialization of sparse arrays, again --- src/lib/yang/binary.lua | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/lib/yang/binary.lua b/src/lib/yang/binary.lua index 5cb7f23725..b202bd626f 100644 --- a/src/lib/yang/binary.lua +++ b/src/lib/yang/binary.lua @@ -14,7 +14,7 @@ local ctable = require('lib.ctable') local cltable = require('lib.cltable') local MAGIC = "yangconf" -local VERSION = 0x00006000 +local VERSION = 0x00008000 local header_t = ffi.typeof([[ struct { @@ -102,6 +102,8 @@ local function table_size(tab) return size end +local SPARSE_ARRAY_END = 0xffffffff + local function data_emitter(production) local handlers = {} local translators = {} @@ -216,11 +218,11 @@ local function data_emitter(production) return function(data, stream) stream:write_stringref('cltable') emit_keys(data.keys, stream) - stream:write_uint32(#data.values) for i, value in ipairs(data.values) do stream:write_uint32(i) emit_value(value, stream) end + stream:write_uint32(SPARSE_ARRAY_END) end else local emit_key = visit1({type='struct', members=production.keys, @@ -413,8 +415,9 @@ local function read_compiled_data(stream, strtab) function readers.cltable() local keys = read1() local values = {} - for i=1,stream:read_uint32() do + while true do local i = stream:read_uint32() + if i == SPARSE_ARRAY_END then break end values[i] = read1() end return cltable.build(keys, values) From e29810f9083d5116fe03fcebe95882b02438a532 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 27 Sep 2017 09:28:08 +0000 Subject: [PATCH 6/6] Fix yet another sparse cltable serialization problem (!) --- src/lib/yang/binary.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/yang/binary.lua b/src/lib/yang/binary.lua index b202bd626f..8434e46dcd 100644 --- a/src/lib/yang/binary.lua +++ b/src/lib/yang/binary.lua @@ -218,7 +218,7 @@ local function data_emitter(production) return function(data, stream) stream:write_stringref('cltable') emit_keys(data.keys, stream) - for i, value in ipairs(data.values) do + for i, value in pairs(data.values) do stream:write_uint32(i) emit_value(value, stream) end