Merge to upstream Snabb

[wingo]

Following snabbco#722 (comment), it would be great to merge to upstream Snabb. We'd maintain the lwaftr part of Snabb, probably with an Igalia lwaftr-dev branch and a lwaftr-next branch or something. However Snabb is undergoing a developmental transformation to a more distributed development model, and part of this is making a sort of layered division between (in descending order of vettedness) the core of snabb, the core libraries, the core apps, then more peripheral programs and their apps, libraries and so on. This is going to be an interesting social and technical challenge but we'll figure it out as we go along.

One shape that we're going to need is to isolate our changes to be only in src/programs/lwaftr (It sure would be nice to rename that directory to lwaftr, wouldn't it? If we have to call it src/programs/snabb_lwaftr that's fine too of course.) If there are core changes we need, we should separate them out and prepare them for landing prior to the lwaftr itself landing.

As a checklist, here we go:

• Identify a list of upstream changes that we have but are not essential to merge -- i.e. we can make the change somehow in src/program/lwaftr. Move those core changes into the lwaftr directory in a series of pull requests.
• Identify a list of upstream changes that we need. Prepare pull requests for all of these changes.
• Rebase our history in such a way that all of our changes are in src/program/lwaftr.
• Create a lwaftr integration branch that merges in all pending upstream changes and our lwaftr branch

Once our lwaftr integration branch is merged, we can start to migrate the "non-essential" core changes to Snabb core, but in such a way that involves moves and renames within the repository instead of imports. That might mean podhashmap too; we'll see how snabbco#722 (comment) goes.

This will take some time :) @lukego / @eugeneia thoughts are welcome.

[lukego]

Thought experiment: If you would simply PR this whole branch to SnabbCo:master then what would be the problem(s) with merging it?

I am browsing the changes with git diff --stat v2016.01..igalia/lwaftr_rabutan and to me the code looks mostly very contained within isolated directories e.g. src/apps/lwaftr and src/program/snabb_lwaftr. There are a small number of changes to common files (e.g. Makefile, core/packet.lua, lib/checksum.lua) that would need to be reviewed to see if they make sense for master but I think we're only talking about a couple of hundred lines of code there?

So hypothetically if you would PR the whole branch then it seems like we would wrangle about:

• Details of those relatively few changes to core code.
• File system layout / directory structure.

... anything else?

Famous last words but this doesn't seem so hard to me...

[wingo]

I guess I was worried about src/apps/lwaftr as well. It's true that there's not a whole lot of core changes to work on but I dunno, getting things into Snabb upstream lately has taken many times the amount of effort that I initially thought it would, for many reasons. I'm happy to PR but to avoid review problems I'm happy to separate out core changes too :)

[eugeneia]

Can we get a comparison of pros and cons for this move? Quoting my own view on this matter:

I have to voice strong discomfort here since I need to review every changed file to weed out forgotten debug statements etc. Going through a PRs “changed files” and categorizing them between “do care about” and “do not care about” based on directory prefix is not something I would like to do. Generally I also like the parallel branches model way better than the parallel module prefixes that I think are being proposed here: would it not be an annoyance for the “lwaftr subtree” to be forcefully updated every month? Either we ignore compatibility and lwaftr has to accept “master's” pace or we ensure no breakage and “lwaftr” glues master to the floor.

Intuitively, I would merge everything from the lwaftr branch that is not in program/lwaftr, e.g. converge to a shared snabb “core”. I am realizing this is the current model of two distinct repositories. This is the perspective the quote is coming from, e.g. it focuses two problems:

1. Conflicting development pace of lwaftr and upstream
2. Ambiguity which changes are reviewed by whom

So the new model would be that lwaftr becomes part of upstream snabb. I imagine the upcoming wingo-next branch would then be the place where lwaftr related changes land? I like the idea of having multiple “products” in upstream. Implicitly we did this with Snabb NFV previously, and lwaftr would be in the same place. I think the problems above could be resolved in the new more decentralized upstream as follows:

1. Conflicting development pace of lwaftr and upstream: this could be solved by “fanning out” in the forks and converging through upstream PRs
2. Ambiguity which changes are reviewed by whom: maybe a set of new PR title tags ([snabbnfv]/[lwaftr]) to label PRs that affect one of the programs specifically

Generally, I think with the questions of “how” eventually solved this is a very good move. The current state with upstream containing Snabb NFV but not lwaftr is a bit dishonest with regard to the decentralized Git workflow.

[lukego]

getting things into Snabb upstream lately has taken many times the amount of effort that I initially thought it would, for many reasons

The reality is that we are working things out and you are taking the brunt of that. This is very valuable if we all arrive at a good common understanding that we can take forward before we all burn out on the whole discussion. I am sure we are close to that point now. Let us try to wrap this up, get lwaftr merged onto master, and celebrate that we made the whole damn process work :).

[lukego]

@eugeneia I think that lwaftr and master can develop at independent paces. For example in the future the Igalians may prefer to merge master and PR back the result once every three months (say). For assigning PRs that are lwaftr changes to the right team we have several mechanisms: "assign" the PR to somebody, ask for the PR to be sent to the SnabbCo:lwaftr mirror branch, or ask for the PR to be sent to the Igalia:lwaftr repository directly (and not seen on the main repo). I'd imagine that we could start simple and adapt according to demand e.g. if the SnabbCo:master repo has too many PRs to keep up with we could suggest sending them directly to program repos in the future. (Like when the Linux Kernel Mailing List spins off a side list.)

[lukego]

@wingo I'm sure we could strike a bargain on apps/lwaftr :-). I think the main requests would be for the code to be reasonably consistent with the other code in apps and for contributions to be welcome (including for features that maybe aren't interesting to the program/lwaftr application.) If that sounds onerous then the directory could be moved into program/lwaftr/ where you can do more as you please.

Could probably also make program/lwaftr a symlink to snabbswitch/lwaftr is that would sweeten the deal :-)

[lukego]

Just a note: based on the watchers page I am the only non-Igalia-employee who is following the development on this repository. When engaging with upstream it is probably reasonable to assume that nobody else is aware of any of the development that has been going on here.

This should become more visible in the future when people see this development being mirrored on a SnabbCo/lwaftr branch and being merged onto master.

(Github "watch" seems extremely chatty and I suspect that most people have trouble keeping up with all the messages received from the SnabbCo repo and would not have bandwidth to watch other repos too.)

[wingo]

Done in snabbco#802! Thanks especially to upstream for helping. Looking forward to figuring out our new workflow :)

[eugeneia]

👍