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

Relax switch vlan filter #900

Merged
merged 2 commits into from
Sep 15, 2022
Merged

Conversation

G10h4ck
Copy link
Member

@G10h4ck G10h4ck commented Oct 11, 2021

Address recent bug discovered in Quintana Libre with eth1.2 LibreRouter interface being ignored when specific configuration section for it is needed.

Log carefully which interfaces get configured or discarded by babeld and
  batadv protocols
@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2021

Codecov Report

Merging #900 (3bafc4d) into master (58bb697) will decrease coverage by 0.13%.
The diff coverage is 63.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #900      +/-   ##
==========================================
- Coverage   74.54%   74.41%   -0.14%     
==========================================
  Files          43       43              
  Lines        3658     3682      +24     
==========================================
+ Hits         2727     2740      +13     
- Misses        931      942      +11     
Impacted Files Coverage Δ
...ges/lime-system/files/usr/lib/lua/lime/network.lua 75.93% <55.17%> (-2.25%) ⬇️
...oto-babeld/files/usr/lib/lua/lime/proto/babeld.lua 95.89% <100.00%> (ø)
...oto-batadv/files/usr/lib/lua/lime/proto/batadv.lua 96.87% <100.00%> (+0.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58bb697...3bafc4d. Read the comment docs.

@ilario
Copy link
Member

ilario commented Oct 11, 2021

I did not test, but looks like the second %s is missing in the log line "dev_parser found Wifi device".

Neither this I tested: can the CPU port be discovered with something like "swconfig dev switch0 help"?

What will happen with DSA? The CPU port is not there anymore?

Log more verbosely this function as it deals with hardware quirks
@G10h4ck
Copy link
Member Author

G10h4ck commented Oct 18, 2021

I did not test, but looks like the second %s is missing in the log line "dev_parser found Wifi device".

It's not that a %s is missing is that the rawif argument (and is not even defined) doesn't make sense for WiFi devices and extra %s would have made the code crash for nil dereferenciation, thanks for pointing this out. Should be fixed now.

Neither this I tested: can the CPU port be discovered with something like "swconfig dev switch0 help"?

Nope it doesn't give that information

What will happen with DSA? The CPU port is not there anymore?

DSA should in theory report this information, but many devices have not been ported to DSA yet and I haven't got info on how really do that reliably for devices which have already been ported to DSA.

@spiccinini
Copy link
Contributor

Hi! @G10h4ck What do you think about providing an explicit per device file, like the board.json file (or even adding more information to it) ? And maybe do a fallback to the automatic diacovery behaviour if the information is not present?

@G10h4ck
Copy link
Member Author

G10h4ck commented Oct 19, 2021

We have discussed such an option in the past, but there was some kind of agreement that we didn't want to do this on LibreMesh behalf and that should be addressed on OpenWrt side, where it is half done with board.json file, which get manually written when a new device support is added to OpenWrt. In fact Linux kernel have all this information into his code but there is no (or we haven't found) a way to access that information.

@G10h4ck G10h4ck changed the title WIP: Relax switch vlan filter Relax switch vlan filter Nov 17, 2021
@ilario
Copy link
Member

ilario commented Sep 15, 2022

So I understand that we can merge this, right?

@G10h4ck G10h4ck merged commit 3585485 into libremesh:master Sep 15, 2022
@ilario
Copy link
Member

ilario commented Sep 21, 2022

Info just got at Battlemesh from @arinc9:

the CPU ports are the one which have such a file:

/sys/firmware/devicetree/base/ethernet@1e100000/mdio-bus/switch@1f/ports/port@6/ethernet

Ref.
https://lkml.kernel.org/netdev/f7d5d961-8f66-e744-ab59-0077445d5873@arinc9.com/T/

@G10h4ck
Copy link
Member Author

G10h4ck commented Sep 21, 2022

that is a really interesting info! Thanks @ilario ! If you can ask also if it is possible to get more informations about the switch in a "standard" way would be great.
Does this method works also with pre-DSA versions?
we just need to check if it is well supported on all kernel versions used by OWRT and then improve our code!

@arinc9
Copy link

arinc9 commented Sep 24, 2022

that is a really interesting info! Thanks @ilario ! If you can ask also if it is possible to get more informations about the switch in a "standard" way would be great.

We have the network interfaces DSA creates. Other than that, you can check the devicetree bindings for DSA.

Does this method works also with pre-DSA versions? we just need to check if it is well supported on all kernel versions used by OWRT and then improve our code!

No, the binding on the referred link is only for DSA. Keep in mind that not all targets on OpenWrt use DSA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants