Add VLAN related apps #863
Conversation
plajjan
force-pushed
the
vlanmux
branch
3 times, most recently
from
22407cb to
088c468
Compare
|
|
||
| ## VlanMux | ||
|
|
||
| Despite the name, VlanMux can act both as a multiplexer, i.e. receive packes |
|
@dpino I believe I have addressed all of your concerns now :) |
|
@plajjan Cool, thanks. LGTM. |
It's not used since the app is configured entirely based on the connected links.
|
@eugeneia mmkay, I was thinking that would be a separate PR but if you want to see it in the same I'll see if I can put something together :) |
Use the standard VLAN module instead of the one previously shipped with lwAFTR.
|
@eugeneia I actually found way fewer references than I thought so I've updated them. PTAL. |
|
LGTM. |
|
Its was rightfully pointed out in #865 that I should probably not be the next hop for this, but @wingo or @kbara instead. Unless there is some argument against merging this I would just go ahead and merge it, and look more closely if I should be the next hop next time, OK? Alternatively you could assign yourselves and take over the wheel. |
|
I'm of two minds on that. I'd like @wingo or I to be the next hop on changes within the lwaftr-related directories, but I also like modifications that involve code spilling out from the lwaftr into Snabb as a whole to go through you/someone outside Igalia. |
|
So maybe the best thing here is that we split this. First one PR to add apps.vlan.vlan that @eugeneia can accept. Then a second one to change lwaftr to use apps.vlan.vlan. @kbara sounds good? Where should I open that second PR? Here? igalia/snabb? That first change need to land on whatever I'm submitting the second PR for... |
|
This change set LGTM as is, and I'm happy with it being merged as-is. My comment was with regards to @eugeneia 's musings on who should review such changes. Pragmatically, I think they should be LGTM'd by him and (either Wingo or I), which this now has been, regardless of whether the first or second set is officially the assignee. |
This code is based on the work of Andy Wingo as part of the lwAFTR but I have expanded the simple Tagger/Untagger with a Mux program that can mux / demux packets based on VLAN tag. See the README for more info.
I tried to put a few useful functions as functions that can potentially be called from other applications, like if someone want to extract TCI or whatever themselves.
Unfortunately I think there's a HOLB issue in there. It's a tad tricky checking output when there are so many potential outputs. I think this works well enough for most use cases though.