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

[Harald Jenny]

Bart could you do a check with the current state of development to see if there
is a problem?

On Mon, Oct 25, 2010 at 02:10:09PM +0200, Roel van Meer wrote:
> 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
> 

> # 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

> _______________________________________________
> Dev mailing list
> Dev at openswan.org
> http://lists.openswan.org/mailman/listinfo/dev