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

make it possible to use routing algo BATMAN-V of batman-adv #104

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

Conversation

genofire
Copy link
Member

No description provided.

@genofire genofire requested review from oliver, a user and jortgies May 22, 2022 09:19
@genofire genofire force-pushed the feat/batman-v branch 2 times, most recently from 1dce41a to 736c288 Compare May 22, 2022 09:42
@oliver
Copy link
Contributor

oliver commented May 22, 2022

I cannot say whether the change is correct, because I'm not familiar with the code. But I think we first need to decide whether we actually want to use BATMAN_V instead of BATMAN_IV. And my impression is that BATMAN_V is still experimental, and that it is not as well supported in Gluon as BATMAN_IV is. Therefore I would use BATMAN_IV.

Using BATMAN_IV would mean that we would have to do another "breaking change" when we want to switch to BATMAN_V; but:

  • a) my impression is that BATMAN_IV is still well-supported by Gluon, and we can use that for quite a long time still
  • b) we (in the infrastructure team, and also in the entire FFHB user community) are getting more familiar with doing these breaking changes already :-) so a later switch to BATMAN_V might not be that much effort
  • c) we are really few people working on Freifunk Bremen infrastructure, so I would like to do this Batman v15 upgrade with as little effort as possible. If BATMAN_V brings up any new problems now, the entire switch might be delayed, and then the Breminale happens, and after Breminale all of us will be tired, and then the entire Batman v15 switch might not happen at all, again. My opinion is therefore: let's get this switch over, and let's not add any further complications.

@ghost
Copy link

ghost commented May 22, 2022

I agree with @oliver in all points

@genofire
Copy link
Member Author

Okay, should we put the code already inside the repo (with BATMAN_IV activated) ?

{% if batman_routing_algo == 'BATMAN_V' %}
# let batman handle the multicast
# https://gluon.readthedocs.io/en/latest/package/gluon-mesh-batman-adv.html
post-up bash -c 'echo 2 > /sys/class/net/{{ batman_dummy_interface }}/brport/multicast_router'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@genofire wofür ist das?

{% if batman_routing_algo is defined and batman_routing_algo == 'BATMAN_V' %}
# let batman handle the multicast
# https://gluon.readthedocs.io/en/latest/package/gluon-mesh-batman-adv.html
post-up bash -c 'echo 2 > /sys/class/net/br-{{ site_code }}/brport/multicast_router'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@genofire wofür ist das?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

siehe Kommentar / Link

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, hab ich gluon docs doch falsch verstanden ...
gerade denk

e ich, ich sollte es beim batman interface und nicht deren bridge interface.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Und: ich hab die Doku jetzt so verstanden, dass diese Option unabhängig von BATMAN_V ist (also die ist anscheinend für BATMAN_IV und BATMAN_V sinnvoll). @genofire: kannst du nicht einen eigenen Pull Request machen, wo du nur den Commit für diese multicast_router-Option reinpackst?

# https://gluon.readthedocs.io/en/latest/package/gluon-mesh-batman-adv.html
post-up bash -c 'echo 2 > /sys/class/net/br-{{ site_code }}/brport/multicast_router'
{% endif %}
post-up bash -c 'echo 2048 > /sys/class/net/br-{{ site_code }}/bridge/hash_max'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@genofire wofür ist das?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die Zeile hab ich nur "gefixed", um die ansible variable - aber ja, der commentar ist dabei unglücklich durch die Änderung von brport/multicast_router verrutscht.

# increase multicast table

@ghost ghost force-pushed the master branch from 5675af2 to e81449e Compare June 11, 2022 08:07
@ghost ghost force-pushed the feat/batman-v branch from 736c288 to 38428cf Compare March 20, 2023 17:50
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.

2 participants