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.

Sign up for GitHub

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

Jump to bottom

Local node routing table separation #1568

Closed
wants to merge 3 commits into from
Closed

Local node routing table separation #1568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
909363f
gluon-mesh-batman-adv: fix source MAC for the IPv6 local-node address
T-X Nov 15, 2018
8c6a4d8
gluon-mesh-batman-adv: unify IPv4 and IPv6 local-node route behavior
T-X Nov 15, 2018
86802ac
gluon-mesh-batman-adv: Add IPv4 subnet to br-client
T-X Nov 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions package/gluon-client-bridge/luasrc/lib/gluon/upgrade/310-gluon-client-bridge-local-node
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ uci:section('network', 'device', 'local_node_dev', {
local ip4, ip6

if next_node.ip4 then
local plen = site.prefix4():match('/%d+$')
ip4 = next_node.ip4 .. plen
ip4 = next_node.ip4 .. '/32'
Copy link
Contributor

Choose a reason for hiding this comment

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

@christf Please test this change with the babeld setup.

end

if next_node.ip6 then
Expand Down
27 changes: 25 additions & 2 deletions ...ge/gluon-mesh-batman-adv/luasrc/lib/gluon/upgrade/320-gluon-mesh-batman-adv-client-bridge
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ local site = require 'gluon.site'
local sysconfig = require 'gluon.sysconfig'
local util = require 'gluon.util'
local uci = require('simple-uci').cursor()
local next_node = site.next_node({})


uci:section('network', 'interface', 'client', {
Expand All @@ -24,13 +25,35 @@ uci:section('network', 'interface', 'client', {

uci:delete('network', 'client_lan')

uci:section('network', 'route', 'br_client_route', {
interface = 'client',
target = site.prefix4(),
})
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I wonder if such a change will lead to incorrect source address selection in some cases. I'm not sure if source address selection for hosts with multiple addresses is as well-specified for IPv4 as it is for IPv6 (at least, I don't know the relevant RFCs), but I'm thinking about the following:

When the Gluon node wants to contact a local client, it will now select this route at first (before source address selection), with interface client - which doesn't have any IPv4 address. Now, how is the source address selected now? What tells Linux to use the address of local-node and not, for example, the br-wan address?

The issue might be solved by adding a "source" attribute with the next-node address to this route definition, but I haven't fully thought this through yet.

For IPv6, a similar issue cannot occur, as br-client should always have an address, which would be selected (at least when gluon-radvd is included, which is the default now).

Copy link
Member

@neocturne neocturne May 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@T-X

I also wonder if this route could lead to packets with the local-node source IPv4 address leaving through br-client, with the br-client MAC address, or if packets would go through another round of routing after choosing the source address, and then end up in routing table 2. If the former is the case, I don't think we should merge the IPv4 part of these changes...

Would adding a source address attribute with the local-node IPv4 address to this route be a problem for alt-esc?


uci:section('network', 'rule', 'local_node_rule', {
src = next_node.ip4 .. '/32',
lookup = 2,
})
uci:set('network', 'local_node_rule', 'in', 'loopback')
Copy link
Member

@neocturne neocturne Nov 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Please merge this set into the section above (same for local_node_rule6 below).

I assume this was not done because in is a keyword in Lua. The correct syntax for this (and arbitrary Lua values as table keys) is

{
  ...
  ['in'] = 'loopback',
}

Copy link
Member

Choose a reason for hiding this comment

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

... and is the in setting even necessary at all?


uci:delete('network', 'local_node_route')
uci:section('network', 'route', 'local_node_route', {
interface = 'local_node',
target = site.prefix4(),
table = 2,
})

uci:section('network', 'rule6', 'local_node_rule6', {
src = next_node.ip6 .. '/128',
lookup = 2,
})
uci:set('network', 'local_node_rule6', 'in', 'loopback')

uci:delete('network', 'local_node_route6')
uci:section('network', 'route6', 'local_node_route6', {
interface = 'client',
interface = 'local_node',
target = site.prefix6(),
gateway = '::',
table = 2,
})

uci:save('network')
Expand Down