Skip to content
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

Common I/O, take 2 #1068

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Common I/O, take 2 #1068

wants to merge 14 commits into from

Conversation

eugeneia
Copy link
Member

Second iteration on my quest to find a shared abstraction for I/O apps. This supersedes #1043, and splits the responsibilities between two apps: IOControl and IO. Again these make use of a new configure callback to act as configuration “macros”. The convention of drivers providing a driver variable is extended to cover a “control driver”, and apps.io.io contains the glue code that is needed to support existing (virtual) drivers with minimal changes. In theory (not fully tested) this PR already supports the Intel10G and Solarflare drivers, emulation I/O, RawSocket, Tap, and VhostUser.

The IOControl and IO macro apps implement the following interface:

config.app(c, "ctrl", IOControl,
           {pciaddr="01:00.0",
            queues = {{id="a", macaddr="10:10:10:10:10:10", vlan=42, buckets=2},
                      {id="b", macaddr="20:20:20:20:20:20", vlan=43}}})

All keys except queues, id, buckets, queue, and bucket are driver dependent.

To add support for a PCI driver module it must be registered in lib.hardware.pci. To add support for a virtual driver module it must be registered in the virtual_module table below.

The driver module must expose a driver variable that contains the app that
implements the queue driver. If a control app is required for queue setup,
the driver must expose a control variable that contains the respective
app, which must accept the configuration argument passed to IOControl.

Note that the buckets and bucket properties must default to 1.

Finally, a configuration formula for the driver must be selected. See how
the formula table is populated.

FURTHER USAGE EXAMPLES:

config.app(c, "ctrl", IOControl,
           {virtual="tap",
            queues = {{id="a", ifname="foo"}}})

config.app(c, "tap_a", IO, {virtual="tap", queue="a"})

config.app(c, "ctrl", IOControl,
           {virtual="emu",
            queues = {{id="a", macaddr="10:10:10:10:10:10", buckets=2}}})

config.app(c, "emu_a", IO, {virtual="emu", queue="a", bucket=1})

This PR is marked WIP because it lacks documentation, and further testing. One known caveat is that the new emulation layer apps.io.emu is most likely more of a bottleneck than virtual_ether_mux.

Cc @lukego

@lukego
Copy link
Member

lukego commented Nov 11, 2016

Great to be making this really happen! I am really liking the uniform interface towards so many different I/O drivers (each PCI driver, tap, raw socket, emulated). This is definitely what I want as a user.

The implementation is a bit sophisticated for my caveman tastes :-). It feels like a lot of cognitive load to take on new concepts like formula and virtual table, to have a Lua driver interface between modules in addition to the app interface, and to use fancy mechanisms like weak tables.

Could we solve this problem in a stupider and more low-brow way?

Just dreaming for a moment... one thought in the back of my head is that the IO app is basically a macro. It's a placeholder where a more specific app (or apps) should be substituted at configuration time. This seems like a substitution that could be made in some simple local way along the lines of sed s/IOControl/Intel82599/.

For context here is the approach I am taking on the Mellanox driver. The controller app initializes the NIC with the necessary queues and then creates shm frames containing all of the necessary information for a queue handler to attach and perform IO. Snippet:

      -- Create shared memory objects containing all of the
      -- information needed to access the send and receive queues.
      --
      -- Snabb processes will use this information to take ownership
      -- of the queue to send and receive packets.
      local basepath = "/pci/"..pciaddress.."/"..queuename
      local sendpath = basepath.."/send"
      local recvpath = basepath.."/recv"
      local u64 = function (x) return ffi.cast("uint64_t", x) end
      shm.create_frame(sendpath,
                       {lock     = {counter},
                        sqn      = {counter, sqn},
                        wq       = {counter, u64(swq)},
                        wqsize   = {counter, sendq_size},
                        cqn      = {counter, send_cq.cqn},
                        cqe      = {counter, u64(send_cq.cqe)},
                        doorbell = {counter, u64(wq_doorbell)},
                        uar_page = {counter, uar},
                        rlkey    = {counter, rlkey}})
      shm.create_frame(recvpath,
                       {lock     = {counter},
                        rqn      = {counter, rqn},
                        wq       = {counter, u64(rwq)},
                        wqsize   = {counter, recvq_size},
                        cqn      = {counter, recv_cq.cqn},
                        cqe      = {counter, u64(recv_cq.cqe)},
                        doorbell = {counter, u64(wq_doorbell)},
                        uar_page = {counter, uar},
                        rlkey    = {counter, rlkey}})

So in this case there is no need for the IO abstraction to share any state between apps via the Lua heap (queues table) because the Mellanox driver is responsible for doing that behind-the-scenes.

On reflection I suppose that this approach would have more impact on the existing drivers. For example if RawSocket needed to be split into a separate controller and IO handler then it would need to grow some code for sharing any necessary information between the two (e.g. for the IO app to discover the interface name that the IOControl was setup with so that it attaches to the right place).

Hard problem!

@lukego
Copy link
Member

lukego commented Nov 11, 2016

One use case that I find interesting: Can the IO framework provide a simple and uniform way for users to specify how to do IO on the command line?

For example supposed we had a tcpdump program in Snabb and we wanted to be able to capture on multiple processes and RSS buckets:

snabb tcpdump tap=tap0
snabb tcpdump raw=eth0
snabb tcpdump pci=01:00.0
snabb tcpdump pci=01:00.0,rss=4      # four worker processes
snabb tcpdump synth=64,100,250,1500  # synthetic traffic mix

Can we cook up a good syntax for this? And a straightforward mapping onto a collection of IO and IOControl apps running in different processes?

Could be that solving this problem will lead us to the optimal interface for the IO apps. I am not sure.

@lukego
Copy link
Member

lukego commented Nov 11, 2016

... Just continuing the same train of thought. On the one hand we could have a syntax for specifying a whole tree of processes:

snabb tcpdump pci=01:00.0,rss=4

and on the other hand we could specify each process separately:

snabb tcpdump pci=01:00.0,rss=4
snabb tcpdump pci=01:00.0,rssbucket=0
snabb tcpdump pci=01:00.0,rssbucket=1
snabb tcpdump pci=01:00.0,rsbuckets=2
snabb tcpdump pci=01:00.0,rssbucket=3

and it may even make sense to support both styles. Snabb could internally operate as a collection of discrete processes, like in the second example, but Snabb programs may want to provide a simple command-line syntax that automatically spawns the processes just to save the user extra typing, like in the first example.

EDIT: Could also make sense to add CPU core assignments to this example since that is also an aspect that we have suggested making the user assign explicitly.

@lukego
Copy link
Member

lukego commented Nov 11, 2016

Just continuing to continue...

Could also be that command-line syntax is the past and we should start looking to YANG based configuration files now. I wonder if that would be sensible or overkill for applications like packetblaster and snabbmark? WDYT @wingo?

@lukego
Copy link
Member

lukego commented Nov 11, 2016

(The reason for this digression into command-line and application configuration is that I see this as the primary user of the IO and IOControl abstractions. The main purpose of this IO abstraction is to make it easy for users to say what I/O interfaces they want to use and how they want that divided up between cores. Just now my concern is that we are taking a step backwards from @petebristow's RSS interface on the intel_mp driver in terms of user friendliness and it would be nice to minimize that.)

@wingo
Copy link
Contributor

wingo commented Nov 11, 2016

Regarding YANG and snabbmark -- I don't know :) I don't see it for snabbmark, but I could be missing something. How would you see that working? Command-line stuff is nice for developers I think; YANG is nice internally for parsing, serialization, validation (somewhat), compilation, and binary loading of configuration data from users; but if the interface is from a developer perhaps it is overkill? I don't know.

I must also say that I am at my limit for hackpower over the next 3/4 weeks and so I am going to be ignoring more things than usual, even meritorious hacks like this one :)

@eugeneia
Copy link
Member Author

eugeneia commented Nov 11, 2016

@lukego You’re right, what a complicated mess. I simplified and came up with this imho sane approach:

  • there is a single IO macro app that configures an arbitrary number of queues
  • its configuration looks like this:
{type=<type>, device=<device>, queues={<name>=<qconf>, ...}
  • <name> will be the name of the queue app
  • each driver has to implement this API in any way it wants (lib.hardware.pci forwards to the correct device driver for <device>, for Tap <device> would be the interface name
  • we can implement any command line syntax we want, it just has to compile to the above API. E.g. compile_io_arg(<string>, <name_pattern>) ⇒ <arg>

@eugeneia
Copy link
Member Author

The GitHub diff view is broken right now...

@eugeneia
Copy link
Member Author

Actually its not, my mistake. I reverted the leftovers and now its looking good.

@lukego
Copy link
Member

lukego commented Nov 17, 2016

Wow! I really like this new configure() mechanism.

Looks to me like the user-facing API for creating apps is exactly the same but behind the scenes each app class now has the possibility to customize how it is instantiated. So when you write:

config.app(c, name, myclass, args)

this can call myclass.configure(c, name, args) which can make an arbitrary update of the app network e.g. add zero, one, or many apps. This way an app like io can be a "macro" that expands by considering context (e.g. PCI device type) and then substituting one or more appropriate apps (e.g. Intel82599, ConnectX4, Emu, etc.)

This seems like a tasteful choice of mechanism. The alternatives would be to make the user explicitly call the configure function, which would not be consistent with current usage of the config api, or to call configure() from inside the engine where the mechanics might be more complicated.

Question: Could we put all of the smarts directly into IO:configure() and skip the whole business of having a framework for registering PCI drivers and making each PCI driver implement a separate configure() method of its own? This could make the code simpler by reducing indirection and also potentially make it easier to spot differences/incompatibilities in how the IO is mapped onto the various drivers.

(Could also be worth spinning out the configure() method on its own branch and PR'ing that for wider feedback. Could label as [sketch] to avoid that branch accidentally being merged instead of this one.)

@wingo
Copy link
Contributor

wingo commented Nov 17, 2016

I also like the app:configure mechanism. Very tasteful!

@wingo
Copy link
Contributor

wingo commented Nov 17, 2016

I assume that app:configure is side-effect-free besides its effect on the app graph? If so that works well with my work on multi-process configuration.

@eugeneia
Copy link
Member Author

@wingo In concept yes, the idea is to allow the configure method to call config.app and config.link on its argument. Of course, bad configure callbacks could do bad things.

@eugeneia
Copy link
Member Author

@lukego I refactored all the framework bits to be centralized in apps.io.common, I agree that its less intrusive that way.

dpino pushed a commit to dpino/snabb that referenced this pull request Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants