Adze1502 / mwan

Simple policy routing for multiple WANs in OpenWrt
84 stars 24 forks source link

mwan3_get_weight_member() collects wrong/multiple interface names #9

Closed arfett closed 11 years ago

arfett commented 11 years ago

Created new issue as this wasn't precisely what my problem was in issue #8.

Let me know if this resolves your issue, Adze and if not I'll keep brainstorming.

Cliff's Notes:

  1. mwan3_ifupdown() writes $INTERFACE, $DEVICE and the default route to temp file
  2. mwan3_policy_routes() gets device name and default gateway of existing route and checks the temp file for a line containing both and sends that to mwan3_get_weight_member()
  3. mwan3_rules_iptables() also checks the temp file for a line with the device name and gateway and feeds that interface name to mwan3_get_weight_member()
Adze1502 commented 11 years ago

Sorry for the late reply. I admire your effort, but i'm not sure that i want to go with a temp file. Have not looked at it in detail though.

During writing of mwan3 i had several issues which were temporary fixed by using "state" files. But this was definitely not what i wanted. Until now i always found a suitable fix, as i will this time. So i'm sorry to tell you that i think i won't adopt your suggestion. I will however take a good look on what you did!

Op 11 jul. 2013, om 04:31 heeft arfett notifications@github.com het volgende geschreven:

Created new issue as this wasn't precisely what my problem was in issue #8.

Let me know if this resolves your issue, Adze and if not I'll keep brainstorming.

Cliff's Notes:

  1. mwan3_ifupdown() writes $INTERFACE, $DEVICE and the default route to temp file

mwan3_policy_routes() gets device name and default gateway of existing route and checks the temp file for a line containing both and sends that to mwan3_get_weight_member()

mwan3_rules_iptables() also checks the temp file for interface name and feeds that to mwan3_get_weight_member()

You can merge this Pull Request by running

git pull https://github.com/arfett/mwan master Or view, comment on, or merge it at:

https://github.com/Adze1502/mwan/pull/9

Commit Summary

possible fix for github issue #8 revised fix for github issue #8 code cleanup up fix for issue #8 File Changes

M mwan3/files/etc/hotplug.d/iface/15-mwan3 (18) Patch Links:

https://github.com/Adze1502/mwan/pull/9.patch https://github.com/Adze1502/mwan/pull/9.diff

arfett commented 11 years ago

I'm looking into other options but it may work for at least a temporary fix. In its current state it's broken...

edit: And just like that it came to me while I was half asleep still reading your response. I am testing something now that does not involve a state file.

Adze1502 commented 11 years ago

And briefly looking at your code (correct me if i'm wrong)

eval sed -i '/$DEVICE/d' /tmp/mwan3iface.tmp >/dev/null 2>&1
if [ "$ACTION" == "ifup" ]; then
    echo "$INTERFACE $route_args" >> /tmp/mwan3iface.tmp
fi

This will keep adding a line to mwan3iface.tmp on each ifup event, which is somewhat of a memory leak

I'm looking into other options but it may work for at least a temporary fix. In its current state it's broken...

On Thursday, July 11, 2013, Jeroen Louwes wrote:

Sorry for the late reply. I admire your effort, but i'm not sure that i want to go with a temp file. Have not looked at it in detail though.

During writing of mwan3 i had several issues which were temporary fixed by using "state" files. But this was definitely not what i wanted. Until now i always found a suitable fix, as i will this time. So i'm sorry to tell you that i think i won't adopt your suggestion. I will however take a good look on what you did!

Op 11 jul. 2013, om 04:31 heeft arfett <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>> het volgende geschreven:

Created new issue as this wasn't precisely what my problem was in issue

8.

Let me know if this resolves your issue, Adze and if not I'll keep brainstorming.

Cliff's Notes:

  1. mwan3_ifupdown() writes $INTERFACE, $DEVICE and the default route to temp file

mwan3_policy_routes() gets device name and default gateway of existing route and checks the temp file for a line containing both and sends that to mwan3_get_weight_member()

mwan3_rules_iptables() also checks the temp file for interface name and feeds that to mwan3_get_weight_member()

You can merge this Pull Request by running

git pull https://github.com/arfett/mwan master Or view, comment on, or merge it at:

https://github.com/Adze1502/mwan/pull/9

Commit Summary

possible fix for github issue #8 revised fix for github issue #8 code cleanup up fix for issue #8 File Changes

M mwan3/files/etc/hotplug.d/iface/15-mwan3 (18) Patch Links:

https://github.com/Adze1502/mwan/pull/9.patch https://github.com/Adze1502/mwan/pull/9.diff

— Reply to this email directly or view it on GitHubhttps://github.com/Adze1502/mwan/pull/9#issuecomment-20838734 .

— Reply to this email directly or view it on GitHub.

arfett commented 11 years ago

The sed line deletes the line regardless of ifup/ifdown.

arfett commented 11 years ago

Although I realize now that this would have had a "bug" where the gateway check wouldn't matter if there was an entry with the same device name and different gateway that was still valid. It would require it to check and delete lines with the device and gateway. Although if there was ever a situation involving the gateway changing a lot that would cause it to add lines each time.

This shouldn't matter with what I'm about to push which is what I think you're looking for.

arfett commented 11 years ago

The method I was trying wouldn't have worked without adding a new dependency so I came up with this.

Where the original line tried to 'grep -v "@"' which wasn't showing up in the uci show command it will go ahead and let $iface grab the multiple interface names then check each one in /etc/config/network and break on the first one that doesn't have @

arfett commented 11 years ago

I assume then this worked fine with your IPv6 interface wan6?

Adze1502 commented 11 years ago

I have troubles with my 3 routers.. One of them is broken, and 2 of them are currently bricked as i tried to upgrade to kernel 3.10. So honestly i haven't tested it yet. I figured that it could take days before i get to test your version, so i might as well accept it (as it looks promising) and let the community react.

Thnx


Met vriendelijke groet, Jeroen Louwes

Verstuurd vanaf mijn ENIAC

2013/7/16 arfett notifications@github.com

I assume then this worked fine with your IPv6 interface wan6?

— Reply to this email directly or view it on GitHubhttps://github.com/Adze1502/mwan/pull/9#issuecomment-21076861 .