[Openswan dev] [PATCH] Incorrect automatic route via ipsec0

[Roel van Meer]

Bart Trojanowski writes:

> I think the proposed patch defeats the spirit of the code it modifies.
>  I don't quite understand why the metric bumping code is triggering at
> all.

The metric bumping code isn't triggered in my case. That's not what the 
problem is.

While reading over my replies: which version of the patch are you referring
to, by the way? On September 30 I posted a patch named
openswan-2.6.29-startklips-route.patch, which, worked fixed my problem by 
extending the metric bumping code. On October 20 I posted a similar patch, 
which modified the way ip addresses were added to the virtual device.

Perhaps I should have split the patch in two, since it effectively does two 
things:

1. It changes the ip assignment of the virtual interface so the ip 
address(es) of the physical device get added with a /32 netmask. This 
ensures that at startup there will be no network routes added over the 
virtual interface. This is the most important part of the patch.

2. Since there are no longer any network routes added over the virtual 
interface, the workaround for ubuntu is no longer necessary, so it is 
removed.

> The 'phys_otheraddr' variable should only be set by the
> getinterfaceinfo() function, if the physical interface is a
> point-to-point device (like ppp for example).  Is that the case for
> you?

No, I have a broadcast device (eth1). But it seems phys_otheraddr gets set 
anyhow. If I put the getinterfaceinfo() function of 2.6.31 in a separate 
shell script (see attachment) and I run it with eth1 as its argument, I get

---/---
root at polariseer:~# sh getinterfaceinfo.sh
addr=87.253.148.33
type=broadcast
otheraddr=87.253.148.127
mask=255.255.255.128
mtu=1500
---/---

For a device with these parameters:
---/---
root at polariseer:~# ip addr show dev eth1
3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
    link/ether d8:d3:85:63:6d:5d brd ff:ff:ff:ff:ff:ff
    inet 87.253.148.33/25 brd 87.253.148.127 scope global eth1
---/---

or
---/---
root at polariseer:~# ifconfig eth1
eth1      Link encap:Ethernet  HWaddr d8:d3:85:63:6d:5d  
          inet addr:87.253.148.33  Bcast:87.253.148.127  Mask:255.255.255.128
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:14944544 errors:0 dropped:0 overruns:0 frame:0
          TX packets:20790862 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:1856936430 (1.7 GiB)  TX bytes:3474306888 (3.2 GiB)
          Memory:fbee0000-fbf00000 
---/---

This should be easily reproducible for you. It's clear there's a problem 
with the awk code: the fifth field is assigned to the "other" variable, but 
"other" is not cleared if it is a broadcast device.

> If the physical device is a regular ethernet device -- as I think
> yours is based on the eth1 name -- then the code you modified should
> never execute since the 'maxmetric=$()' line will run:
>     ip route show $phys_otheraddr/$phys_mask
> ... and regardless of what the mask is, as long as otheraddr is an
> empty string, then ip will halt with an error printed to stderr.

It seems phys_otheraddr is not empty.
 
> So, I am confused why the change you proposed makes a difference.
> Maybe I falsely expect ip to fail when given bad input parameters.

> Can you help me out?  Could you revert your _startklips to the
> published code, add a "set -x" at the top of the script, and run ipsec
> setup --start ?

Well, this host is in production now, so I'll have to to see when I can 
schedule some downtime. But I can run parts of _startklips separately.


Note: in order to avoid adding confusion to an already long and confusing 
thread: I think your comments are based on a version of a patch I submitted 
but which has been replaced by a different version. In that light most of it 
is no longer relevant, since the new patch removes the metric bumping code 
altogether.

However, I think you found an issue in _startklips where otheraddr gets set 
when it should not be. Luckily the preview of _startklips, where the 
use of ifconfig has been replaced by use of ip, that Paul sent to the list 
no longer has that issue. So probably it has already been fixed.

Regards,

roel

-------------- next part --------------
# get info about a given interface
# getinterfaceinfo eth0 prefix_
getinterfaceinfo() {
        ifconfig $1 \
         | awk '$1 == "inet" && $2 ~ /^addr:/ && $NF ~ /^Mask:/ {
            gsub(/:/, " ", $0)
            print "'$2'addr=" $3
            other = $5
            if ($4 == "Bcast")
                print "'$2'type=broadcast"
            else if ($4 == "P-t-P")
                print "'$2'type=pointopoint"
            else if (NF == 5) {
                print "'$2'type="
                other = ""
            } else
                print "'$2'type=unknown"
            print "'$2'otheraddr=" other
            print "'$2'mask=" $NF
        }
                $NF ~ /^Metric:/ && $(NF-1) ~ /^MTU:/ {
                        sub(/^MTU:/, "", $(NF-1))
                        print "'$2'mtu=" $(NF-1)
                }
                '
}
getinterfaceinfo eth1