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

Sort local routes first so that they're created before default routes #38

Merged
merged 2 commits into from
May 15, 2019
Merged

Sort local routes first so that they're created before default routes #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ebd16cb
Sort local routes first
fhunleth May 15, 2019
ba7aabe
Check return values when setting routes
fhunleth May 15, 2019
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
27 changes: 26 additions & 1 deletion lib/vintage_net/route/calculator.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,36 @@ defmodule VintageNet.Route.Calculator do
{new_table_indices, entries} =
Enum.reduce(infos, {table_indices, []}, &make_entries(&1, &2, prioritization))

sorted_entries = Enum.sort(entries)
sorted_entries = Enum.sort(entries, &sort/2)

{new_table_indices, sorted_entries}
end

# Sort order
#
# 1. Local routes
# 2. Rules
# 3. Default routes
#
# The most important part is that local routes get created before default
# routes. Linux disallows default routes that can't be supported and the
# local routes are needed for that.
defp sort({:local_route, _, _, _, _} = a, {:local_route, _, _, _, _} = b) do
a <= b
end

defp sort({:local_route, _, _, _, _}, _other) do
true
end

defp sort(_other, {:local_route, _, _, _, _}) do
false
end

defp sort(a, b) do
a <= b
end

@doc """
Utility function to trim IP address to its subnet

Expand Down
15 changes: 12 additions & 3 deletions lib/vintage_net/route_manager.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,29 +216,32 @@ defmodule VintageNet.RouteManager do

defp handle_delete({:default_route, ifname, _default_gateway, _metric, table_index}) do
IPRoute.clear_a_route(ifname, table_index)
|> warn_on_error("clear_a_route")
end

defp handle_delete({:local_route, ifname, address, subnet_bits, metric}) do
IPRoute.clear_a_local_route(ifname, address, subnet_bits, metric)
|> warn_on_error("clear_a_local_route")
end

defp handle_delete({:rule, table_index, _address}) do
IPRoute.clear_a_rule(table_index)
|> warn_on_error("clear_a_rule")
end

defp handle_insert({:default_route, ifname, default_gateway, metric, table_index}) do
IPRoute.add_default_route(ifname, default_gateway, metric, table_index)
:ok = IPRoute.add_default_route(ifname, default_gateway, metric, table_index)
end

defp handle_insert({:rule, table_index, address}) do
IPRoute.add_rule(address, table_index)
:ok = IPRoute.add_rule(address, table_index)
end

defp handle_insert({:local_route, ifname, address, subnet_bits, metric}) do
# HACK: Delete automatically created local routes that have a 0 metric
_ = IPRoute.clear_a_local_route(ifname, address, subnet_bits, 0)

IPRoute.add_local_route(ifname, address, subnet_bits, metric)
:ok = IPRoute.add_local_route(ifname, address, subnet_bits, metric)
end

defp update_available_interfaces(routes) do
Expand All @@ -258,4 +261,10 @@ defmodule VintageNet.RouteManager do
for {:local_route, ifname, _address, _subnet_bits, metric} <- routes,
do: {metric, ifname}
end

defp warn_on_error(:ok, _label), do: :ok

defp warn_on_error({:error, reason}, label) do
Logger.warn("route_manager(#{label}): ignoring failure #{inspect(reason)}")
end
end
22 changes: 11 additions & 11 deletions test/vintage_net/route/calculator_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ defmodule VintageNet.Route.CalculatorTest do

assert {%{"eth0" => 100},
[
{:local_route, "eth0", {192, 168, 1, 50}, 24, 10},
{:rule, 100, {192, 168, 1, 50}},
{:default_route, "eth0", {192, 168, 1, 1}, 0, 100},
{:default_route, "eth0", {192, 168, 1, 1}, 10, :main},
{:local_route, "eth0", {192, 168, 1, 50}, 24, 10}
{:default_route, "eth0", {192, 168, 1, 1}, 10, :main}
]} == Calculator.compute(state, interfaces, prioritization)
end

Expand Down Expand Up @@ -65,7 +65,7 @@ defmodule VintageNet.Route.CalculatorTest do
}

assert {%{"eth0" => 100},
[{:rule, 100, {192, 168, 1, 50}}, {:local_route, "eth0", {192, 168, 1, 50}, 24, 50}]} ==
[{:local_route, "eth0", {192, 168, 1, 50}, 24, 50}, {:rule, 100, {192, 168, 1, 50}}]} ==
Calculator.compute(state, interfaces, prioritization)
end

Expand All @@ -90,14 +90,14 @@ defmodule VintageNet.Route.CalculatorTest do

assert {%{"eth0" => 100, "wlan0" => 101},
[
{:local_route, "eth0", {192, 168, 1, 50}, 24, 10},
{:local_route, "wlan0", {192, 168, 1, 60}, 24, 20},
{:rule, 100, {192, 168, 1, 50}},
{:rule, 101, {192, 168, 1, 60}},
{:default_route, "eth0", {192, 168, 1, 1}, 0, 100},
{:default_route, "eth0", {192, 168, 1, 1}, 10, :main},
{:default_route, "wlan0", {192, 168, 1, 1}, 0, 101},
{:default_route, "wlan0", {192, 168, 1, 1}, 20, :main},
{:local_route, "eth0", {192, 168, 1, 50}, 24, 10},
{:local_route, "wlan0", {192, 168, 1, 60}, 24, 20}
{:default_route, "wlan0", {192, 168, 1, 1}, 20, :main}
]} == Calculator.compute(state, interfaces, prioritization)
end

Expand All @@ -122,14 +122,14 @@ defmodule VintageNet.Route.CalculatorTest do

assert {%{"eth0" => 100, "wlan0" => 101},
[
{:local_route, "eth0", {192, 168, 1, 50}, 24, 50},
{:local_route, "wlan0", {192, 168, 1, 60}, 24, 20},
{:rule, 100, {192, 168, 1, 50}},
{:rule, 101, {192, 168, 1, 60}},
{:default_route, "eth0", {192, 168, 1, 1}, 0, 100},
{:default_route, "eth0", {192, 168, 1, 1}, 50, :main},
{:default_route, "wlan0", {192, 168, 1, 1}, 0, 101},
{:default_route, "wlan0", {192, 168, 1, 1}, 20, :main},
{:local_route, "eth0", {192, 168, 1, 50}, 24, 50},
{:local_route, "wlan0", {192, 168, 1, 60}, 24, 20}
{:default_route, "wlan0", {192, 168, 1, 1}, 20, :main}
]} == Calculator.compute(state, interfaces, prioritization)
end

Expand All @@ -152,12 +152,12 @@ defmodule VintageNet.Route.CalculatorTest do

assert {%{"eth0" => 100},
[
{:local_route, "eth0", {192, 168, 1, 50}, 24, 10},
{:rule, 100, {192, 168, 1, 50}},
{:rule, 100, {192, 168, 1, 51}},
{:rule, 100, {192, 168, 1, 52}},
{:default_route, "eth0", {192, 168, 1, 1}, 0, 100},
{:default_route, "eth0", {192, 168, 1, 1}, 10, :main},
{:local_route, "eth0", {192, 168, 1, 50}, 24, 10}
{:default_route, "eth0", {192, 168, 1, 1}, 10, :main}
]} == Calculator.compute(state, interfaces, prioritization)
end
end