Out "ip route" output parsing did not handle the "ip route" output properly in the case we were trying to find out which interface is used to get to a host when that host lives outside our subnet.
This patch fixes this. Note this only fixes part of #577193, but as the other part is really unrelated I'm splitting the fix into 2 patches. --- network.py | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/network.py b/network.py index a3fb8ee..f992d18 100644 --- a/network.py +++ b/network.py @@ -786,12 +786,13 @@ class Network: return ""
routeInfo = route.split() - if routeInfo[0] != host or len(routeInfo) < 5: + if routeInfo[0] != host or len(routeInfo) < 5 or \ + "dev" not in routeInfo or routeInfo.index("dev") > 3: log.error('Unexpected "ip route get to %s" reply: %s' % (host, routeInfo)) return ""
- nic = routeInfo[2] + nic = routeInfo[routeInfo.index("dev") + 1]
if nic not in self.netdevices.keys(): log.error('Unknown network interface: %s' % nic)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Tue, 30 Mar 2010, Hans de Goede wrote:
Out "ip route" output parsing did not handle the "ip route" output properly in the case we were trying to find out which interface is used to get to a host when that host lives outside our subnet.
This patch fixes this. Note this only fixes part of #577193, but as the other part is really unrelated I'm splitting the fix into 2 patches.
network.py | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/network.py b/network.py index a3fb8ee..f992d18 100644 --- a/network.py +++ b/network.py @@ -786,12 +786,13 @@ class Network: return ""
routeInfo = route.split()
if routeInfo[0] != host or len(routeInfo) < 5:
if routeInfo[0] != host or len(routeInfo) < 5 or \
"dev" not in routeInfo or routeInfo.index("dev") > 3:
Sure we want: "dev" not in routeInfo
In this test?
log.error('Unexpected "ip route get to %s" reply: %s' % (host, routeInfo)) return ""
nic = routeInfo[2]
nic = routeInfo[routeInfo.index("dev") + 1]
Is this ok given the test above? If "dev" not in routeInfo would mean this would become:
nic = routeInfo[-1 + 1] nic = routeInfo[0]
I don't know what the possible /sbin/ip output could look like. On my system, I get this:
dcantrel@mitre ~$ ip route get to 172.31.1.11 172.31.1.11 dev eth0 src 172.31.1.13 cache mtu 1500 advmss 1460 hoplimit 64
if nic not in self.netdevices.keys(): log.error('Unknown network interface: %s' % nic)
- -- David Cantrell dcantrell@redhat.com Red Hat / Honolulu, HI
Hi,
On 03/31/2010 04:54 PM, David Cantrell wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Tue, 30 Mar 2010, Hans de Goede wrote:
Out "ip route" output parsing did not handle the "ip route" output properly in the case we were trying to find out which interface is used to get to a host when that host lives outside our subnet.
This patch fixes this. Note this only fixes part of #577193, but as the other part is really unrelated I'm splitting the fix into 2 patches.
network.py | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/network.py b/network.py index a3fb8ee..f992d18 100644 --- a/network.py +++ b/network.py @@ -786,12 +786,13 @@ class Network: return ""
routeInfo = route.split()
- if routeInfo[0] != host or len(routeInfo) < 5:
- if routeInfo[0] != host or len(routeInfo) < 5 or \
- "dev" not in routeInfo or routeInfo.index("dev") > 3:
Sure we want: "dev" not in routeInfo
In this test?
Yes, the entire test block reads:
if routeInfo[0] != host or len(routeInfo) < 5 or \ "dev" not in routeInfo or routeInfo.index("dev") > 3: log.error('Unexpected "ip route get to %s" reply: %s' % (host, routeInfo)) return ""
We are doing sanity checking on the output here, and if the output does *not* contain "dev" it is not sane
log.error('Unexpected "ip route get to %s" reply: %s' % (host, routeInfo)) return ""
- nic = routeInfo[2]
- nic = routeInfo[routeInfo.index("dev") + 1]
Is this ok given the test above? If "dev" not in routeInfo would mean this would become:
nic = routeInfo[-1 + 1] nic = routeInfo[0]
If "dev" is not in the routeInfo, the above test will cause us to return "" (and log an error).
I don't know what the possible /sbin/ip output could look like. On my system, I get this:
dcantrel@mitre ~$ ip route get to 172.31.1.11 172.31.1.11 dev eth0 src 172.31.1.13 cache mtu 1500 advmss 1460 hoplimit 64
Yes, which works correct with both the old and the new code, but when the host is outside the current subnet the output looks something like this:
dcantrel@mitre ~$ ip route get to 111.222.111.222 111.222.111.222 via 172.31.1.1 dev eth0 src 172.31.1.13 cache mtu 1500 advmss 1460 hoplimit 64
Note that eth0 is now in a different position, but still after "dev", this is what this patch tries to handle (and unless I'm mistaken it should handle).
Regards,
Hans
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Wed, 31 Mar 2010, Hans de Goede wrote:
Hi,
On 03/31/2010 04:54 PM, David Cantrell wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Tue, 30 Mar 2010, Hans de Goede wrote:
Out "ip route" output parsing did not handle the "ip route" output properly in the case we were trying to find out which interface is used to get to a host when that host lives outside our subnet.
This patch fixes this. Note this only fixes part of #577193, but as the other part is really unrelated I'm splitting the fix into 2 patches.
network.py | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/network.py b/network.py index a3fb8ee..f992d18 100644 --- a/network.py +++ b/network.py @@ -786,12 +786,13 @@ class Network: return ""
routeInfo = route.split()
- if routeInfo[0] != host or len(routeInfo) < 5:
- if routeInfo[0] != host or len(routeInfo) < 5 or \
- "dev" not in routeInfo or routeInfo.index("dev") > 3:
Sure we want: "dev" not in routeInfo
In this test?
Yes, the entire test block reads:
if routeInfo[0] != host or len(routeInfo) < 5 or \ "dev" not in routeInfo or routeInfo.index("dev") > 3: log.error('Unexpected "ip route get to %s" reply: %s' % (host, routeInfo)) return ""
We are doing sanity checking on the output here, and if the output does *not* contain "dev" it is not sane
Whoops. Right. Geez, no more patch reading at 5:00 AM.
log.error('Unexpected "ip route get to %s" reply: %s' % (host, routeInfo)) return ""
- nic = routeInfo[2]
- nic = routeInfo[routeInfo.index("dev") + 1]
Is this ok given the test above? If "dev" not in routeInfo would mean this would become:
nic = routeInfo[-1 + 1] nic = routeInfo[0]
If "dev" is not in the routeInfo, the above test will cause us to return "" (and log an error).
I don't know what the possible /sbin/ip output could look like. On my system, I get this:
dcantrel@mitre ~$ ip route get to 172.31.1.11 172.31.1.11 dev eth0 src 172.31.1.13 cache mtu 1500 advmss 1460 hoplimit 64
Yes, which works correct with both the old and the new code, but when the host is outside the current subnet the output looks something like this:
dcantrel@mitre ~$ ip route get to 111.222.111.222 111.222.111.222 via 172.31.1.1 dev eth0 src 172.31.1.13 cache mtu 1500 advmss 1460 hoplimit 64
Note that eth0 is now in a different position, but still after "dev", this is what this patch tries to handle (and unless I'm mistaken it should handle).
OK, looks fine now given that I misread in the first go.
Ack.
- -- David Cantrell dcantrell@redhat.com Red Hat / Honolulu, HI
On 03/31/2010 05:19 PM, Hans de Goede wrote:
On 03/31/2010 04:54 PM, David Cantrell wrote:
On Tue, 30 Mar 2010, Hans de Goede wrote:
Out "ip route" output parsing did not handle the "ip route" output properly in the case we were trying to find out which interface is used to get to a host when that host lives outside our subnet.
This patch fixes this. Note this only fixes part of #577193, but as the other part is really unrelated I'm splitting the fix into 2 patches.
network.py | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/network.py b/network.py index a3fb8ee..f992d18 100644 --- a/network.py +++ b/network.py @@ -786,12 +786,13 @@ class Network: return ""
routeInfo = route.split()
- if routeInfo[0] != host or len(routeInfo) < 5:
- if routeInfo[0] != host or len(routeInfo) < 5 or \
We're only interested in the first part for paranoid sanity checking if ip really searched for our given target argument and for the device name next to "dev", so a valid len could be <5 but >= 3. I hope ip output won't shorten in the future, if so checking for <5 would be too strict, <3 would be the absolut minimum, e.g. the following contains all we need: 172.31.1.11 dev eth0
- "dev" not in routeInfo or routeInfo.index("dev") > 3:
What if ip all of a sudden changes to the following output and "dev" will be beyond index 3?: 111.222.111.222 via 172.31.1.1 foo bar dev eth0 src 172.31.1.13
We could still get all our required info, if we didn't check for dev's index >3. I guess dev's index must just be less than len(routeInfo)-1 such that there would be one argument after "dev" to not access the array out of bounds later on.
BTW, what happens with the second line of ip's output? I'm not familiar enough with python and its split. Does it also split at newline and the parts of the second line are also in routeInfo? If so, then that would be uncool and we'd rather crop anything but the first line before splitting and comparing indexes.
log.error('Unexpected "ip route get to %s" reply: %s' % (host, routeInfo)) return ""
- nic = routeInfo[2]
- nic = routeInfo[routeInfo.index("dev") + 1]
dcantrel@mitre ~$ ip route get to 172.31.1.11 172.31.1.11 dev eth0 src 172.31.1.13 cache mtu 1500 advmss 1460 hoplimit 64
Yes, which works correct with both the old and the new code, but when the host is outside the current subnet the output looks something like this:
dcantrel@mitre ~$ ip route get to 111.222.111.222 111.222.111.222 via 172.31.1.1 dev eth0 src 172.31.1.13 cache mtu 1500 advmss 1460 hoplimit 64
Note that eth0 is now in a different position, but still after "dev", this is what this patch tries to handle (and unless I'm mistaken it should handle).
Steffen
Linux on System z Development
IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Hi,
On 04/01/2010 03:16 AM, Steffen Maier wrote:
On 03/31/2010 05:19 PM, Hans de Goede wrote:
On 03/31/2010 04:54 PM, David Cantrell wrote:
On Tue, 30 Mar 2010, Hans de Goede wrote:
Out "ip route" output parsing did not handle the "ip route" output properly in the case we were trying to find out which interface is used to get to a host when that host lives outside our subnet.
This patch fixes this. Note this only fixes part of #577193, but as the other part is really unrelated I'm splitting the fix into 2 patches.
network.py | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/network.py b/network.py index a3fb8ee..f992d18 100644 --- a/network.py +++ b/network.py @@ -786,12 +786,13 @@ class Network: return ""
routeInfo = route.split()
- if routeInfo[0] != host or len(routeInfo)< 5:
- if routeInfo[0] != host or len(routeInfo)< 5 or \
We're only interested in the first part for paranoid sanity checking if ip really searched for our given target argument and for the device name next to "dev", so a valid len could be<5 but>= 3. I hope ip output won't shorten in the future, if so checking for<5 would be too strict, <3 would be the absolut minimum, e.g. the following contains all we need: 172.31.1.11 dev eth0
I'm assuming that ip route's info is pretty consistent, our old code already had the len < 5 sanity check. If "ip route"'s output turns out to not be consistent we will have to stop using it and come up with another way to get the info we need.
- "dev" not in routeInfo or routeInfo.index("dev")> 3:
What if ip all of a sudden changes to the following output and "dev" will be beyond index 3?: 111.222.111.222 via 172.31.1.1 foo bar dev eth0 src 172.31.1.13
Again "ip route"'s output has not changed for as long as we've been using it, the problem is that it behaves differently when asking for information about a route to a host outside one's own subnet. This is what this patch addresses.
BTW, what happens with the second line of ip's output? I'm not familiar enough with python and its split. Does it also split at newline and the parts of the second line are also in routeInfo? If so, then that would be uncool and we'd rather crop anything but the first line before splitting and comparing indexes.
AFAIK the second line also gets split and added to the routeInfo array.
Regards,
Hans
Fair enough. Ack.
On 04/01/2010 08:29 AM, Hans de Goede wrote:
On 04/01/2010 03:16 AM, Steffen Maier wrote:
On 03/31/2010 05:19 PM, Hans de Goede wrote:
On 03/31/2010 04:54 PM, David Cantrell wrote:
On Tue, 30 Mar 2010, Hans de Goede wrote:
Out "ip route" output parsing did not handle the "ip route" output properly in the case we were trying to find out which interface is used to get to a host when that host lives outside our subnet.
This patch fixes this. Note this only fixes part of #577193, but as the other part is really unrelated I'm splitting the fix into 2 patches.
network.py | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/network.py b/network.py index a3fb8ee..f992d18 100644 --- a/network.py +++ b/network.py @@ -786,12 +786,13 @@ class Network: return ""
routeInfo = route.split()
- if routeInfo[0] != host or len(routeInfo)< 5:
- if routeInfo[0] != host or len(routeInfo)< 5 or \
We're only interested in the first part for paranoid sanity checking if ip really searched for our given target argument and for the device name next to "dev", so a valid len could be<5 but>= 3. I hope ip output won't shorten in the future, if so checking for<5 would be too strict, <3 would be the absolut minimum, e.g. the following contains all we need: 172.31.1.11 dev eth0
I'm assuming that ip route's info is pretty consistent, our old code already had the len < 5 sanity check. If "ip route"'s output turns out to not be consistent we will have to stop using it and come up with another way to get the info we need.
- "dev" not in routeInfo or routeInfo.index("dev")> 3:
What if ip all of a sudden changes to the following output and "dev" will be beyond index 3?: 111.222.111.222 via 172.31.1.1 foo bar dev eth0 src 172.31.1.13
Again "ip route"'s output has not changed for as long as we've been using it, the problem is that it behaves differently when asking for information about a route to a host outside one's own subnet. This is what this patch addresses.
BTW, what happens with the second line of ip's output? I'm not familiar enough with python and its split. Does it also split at newline and the parts of the second line are also in routeInfo? If so, then that would be uncool and we'd rather crop anything but the first line before splitting and comparing indexes.
AFAIK the second line also gets split and added to the routeInfo array.
Steffen
Linux on System z Development
IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
anaconda-devel@lists.stg.fedoraproject.org