-
Notifications
You must be signed in to change notification settings - Fork 297
[draft] Create new 'snabb' module to define the core API #465
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
Conversation
The idea is to explicitly define our core API in one place. Then
users will not required a lot of individual modules (core.packet,
core.link, etc) but instead access everything through a unified and
well-defined module (snabb).
This draft API would make code look more like this:
local config = snabb.config()
config:add_app(...)
config:add_link(...)
snabb.engine.configure(config)
and
local packet = snabb.packet()
packet:shiftleft(42)
print("packet length after shortening: " .. packet:length())
Notably you would not have to require() any modules anymore. (You
could require("snabb") if you want to but we could probably just make
it global.)
This MIGHT also make it easier to create new versions of the API with
semantic versioning (http://semver.org/) and even to allow apps to say
which API version they depend on so that we could keep compatibility
with older versions for as long as we want.
|
This would be a big change. What do we think (a) of the idea of a unified |
|
I don't feel qualified to comment on the concrete API, but the approach with the |
|
Thanks @sleinen! You are the target user for this change in many ways i.e. the idea is to create a friendly and stable interface for people to write their first app. I mean "stable" in the sense that we would only make backwards-compatible changes to the So the user module might actually start with: local snabb = require("snabb1") -- get warning/error if deprecated/removedThe implementation cost seems minimal thus far in that the I am experimenting with how the Getting Started apps would look in this scheme. I will push progress on those experiments onto this branch. |
This is an experiment in programming style. The programs are written
realistically but have not been tested. The idea is to see if this is
the way we want people's first programs to look.
Compared with the master branch the intended differences are:
- Use the new Snabb API from this branch.
- Use require("snabb1") to explicitly depend on API version 1.
- Use the "apps have a private sandbox" model in sprayer.lua.
- Use the 'link:transmit(packet)' object-oriented syntax variant.
- Make program/ module files simply run instead of loading first.
- Added engine:run({duration = 'toidle'}) option (run until idle).
The overall effect should be shorter code that looks more like plain
Lua scripts.
The use of 'link:transmit(packet)' syntax feels right to me in this
example. It is also a retreat from the position that I took on
snabb-devel where I advocated for 'l:link_transmit(packet)' (after
having previously advocated for link.transmit(l, p)).
|
In commit b44f510 I wrote an experimental new version of the Getting Started sprayer app/program. For comparison the version that works on master is in #439. This is a style experiment: if I could write this code using any of the programming styles currently under discussion on snabb-devel then how would I want it to look? This code is an attempt to make the code simple and more like plain scripts rather than parts of a big framework. (The code does not actually run yet.) See the commit message and code for full details. Question: Is this the coding style that we want to adopt and explain to new users? What is good and what is bad? What details do I miss? |
Use short method names like transmit() instead of link_transmit(). This is consistent with spray.lua but inconsistent with snabb.lua. Question is: which is better?
The previous commit consistently used short method names:
link:transmit(packet)
this new one switches to long ones:
link:link_transmit(packet)
This is a style experiment: redundancy vs ambiguity.
|
Commit 40bc269 uses short method names and commit eb36db5 uses long ones. The trade off seems to be between redundancy when the variable name conveys type information: verses ambiguity when the name does not clearly tell you what the type is: Just at the time I am writing this I prefer the long names. That frees you up from having to think about whether you are conveying type information in your variable names. You could call a config object |
|
I am not sure I like the |
Good question. Suppose we say yes. Could we then support multiple apps each choosing which API to use? That is, some apps already using the latest and greatest, and others using older APIs. This would allow us to introduce a new and incompatible API with a new module name (e.g. That would mean, for example, that Alex could pull from master into his VPN branch and get radically new features, but decide for himself when to adopt them. The goal that is developing in my mind is to support rapid improvement on the master branch (API revisions, app sandboxes, parallel execution, etc) and also for application developers to feel that they can safely pull from master and have the chance to adopt new features on their own schedule. That is unlike the situation now where pulling a feature like the straightline API means you have to immediately update all of your code before you can run again. |
|
Question: is that the right goal? and what is the best way to achieve it? |
|
@eugeneia I suppose the reason for the If we simply made all of the APIs globally accessible then that would be simple too, but then we would be writing function calls like |
|
Speaking of the Makefile... If we supported multiple API versions then a very nice test case would be to run the latest Snabb Switch core with the older apps based on the previous API. Then we would have full test coverage on the original API instead of gradually less as fewer modules are using it. How could we do that? I mean, run This thread becomes long :-). That is good: we can use this PR mostly for discussing ideas. I will resubmit the code on a new PR once we know how it should look. |
|
What if the I think we should eliminate patterns that will effectively will be the same in almost every file, e.g. a common header. I feel the same way about our |
|
@eugeneia If we have a global I am totally on board with avoiding boilerplate. I am not sure that is what we are talking about here though. The The whole scheme could be naive of course. For example we may end up with a mess of modules based on different versions calling each other and passing incompatible objects. I am not sure what is the best precedent for a scheme like this. |
|
Here is one more idea: If we would go ahead with the design to sandbox each app in a private Lua VM then we could also have one API version valid in each VM. That way each app could depend on a specific API version and we would not have the issue of calling modules with different APIs (each app can only load one API). |
|
This can also be that I am drifting into fantasy land a bit. Concretely the problem to solve is to be able to redefine core APIs and deprecate rather than immediately remove older ones. Could be a simple idea to have a single |
|
I just meant that an app would use the latest and greatest by default or lock to a specific version using the
This is a good point. To be honest to me the multi-api-version scenario sounds a bit wonky to me. I feel like keeping radical API revisions in a long lived feature branch until we are happy with it might be the simpler and more failure proof approach. How about divide and conquer? Split this train of thought into (a) a branch with the API we want, (b) considerations on how/when to integrate it. But let us not get one in the way of the other. |
The scenario that I want to support is that a long-lived branch supporting a specific application (for example Alex's VPN device) can safely pull from master. The expected effect is to automatically get fixes and optimizations but that API changes do not cause breakage and can be adopted separately. That is, to stop doing what we have been doing recently, where pulling |
I am coming around to this way of thinking at the moment. :-)
Would two weeks be "long lived"? :-) I would love to land a new API in the June release so that we can bring new people into the project with a warm and fuzzy getting started experience. |
|
The next step on this branch for me is to flesh out the proposed API more and make it actually work and then we can see whether it gets a thumbs up or thumbs down. |
|
I personally feel like right now is still a sensible moment to make radical changes, but we should ask Alex and the people who actually feel the pain. |
|
The code currently on this branch is actually both radically new and backwards compatible. The documentation would tell people to use the Isolating apps in sandboxes might be done only when the app class is given as a string so that "classic" apps can continue to share a Lua VM and operate exactly as they always did. (People might want to wait for an excellent implementation of parallelism before bothering to update their apps to the new scheme to take advantage of it.) |
That is actually a really excellent point. It is a perfectly valid idea to stick with the API that we have today, that we want to support for some time to come, and that by now have good documentation for. Then the new features (APIv2, sandboxes, parallelism) could develop at their own pace on their own branches and be merged onto master when it makes sense for everybody. (Or not at all in the case of ideas that lead to dead ends.) There is more than enough to get on with this month just in terms of shaving off rough edges and fixing open bugs. |
|
Closing because the goal of making an experiment and getting feedback is accomplished. |
874e48b adds counter support in core.app, core.packet and core.link. Cache counter functions lookup locally for better performance.
Move lwaftr/nexthop to snabbvmx/nexthop
The idea is to explicitly define our core API in one place. Then users will not required a lot of individual modules (core.packet, core.link, etc) but instead access everything through a unified and well-defined module (snabb).
This draft API would make code look more like this:
and
Notably you would not have to require() any modules anymore. (You could require("snabb") if you want to but we could probably just make it global.)
This MIGHT also make it easier to create new versions of the API with semantic versioning (http://semver.org/) and even to allow apps to say which API version they depend on so that we could keep compatibility with older versions for as long as we want.