* [PATCH] net: Prefer non link-local source addresses @ 2011-09-01 20:20 Jeff Harris 2011-09-01 21:56 ` David Lamparter 2011-09-01 22:14 ` Julian Anastasov 0 siblings, 2 replies; 5+ messages in thread From: Jeff Harris @ 2011-09-01 20:20 UTC (permalink / raw) To: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy <kabe Cc: Jeff Harris Section 2.6.1 of RFC 3927 specifies that if link-local and routable addresses are available on an interface, a routable address is preferred. Update the IPv4 source address selection algorithm to use a 169.254.x.x address only if another matching address is not found. Tested combinations of configured IP addresses with and without link-local to verify a link-local address was chosen only if no routable address was present. Signed-off-by: Jeff Harris <jeff_harris@kentrox.com> --- net/ipv4/devinet.c | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index bc19bd0..70ddf37 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -965,6 +965,8 @@ out: __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope) { __be32 addr = 0; + __be32 lladdr = 0; + __be32 firstaddr = 0; struct in_device *in_dev; struct net *net = dev_net(dev); @@ -977,15 +979,27 @@ __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope) if (ifa->ifa_scope > scope) continue; if (!dst || inet_ifa_match(dst, ifa)) { + if (ipv4_is_linklocal_169(ifa->ifa_address)) { + lladdr = ifa->ifa_local; + continue; + } addr = ifa->ifa_local; break; } - if (!addr) - addr = ifa->ifa_local; + if (!firstaddr) + firstaddr = ifa->ifa_local; } endfor_ifa(in_dev); if (addr) goto out_unlock; + if (lladdr) { + addr = lladdr; + goto out_unlock; + } + if (firstaddr) { + addr = firstaddr; + goto out_unlock; + } no_in_dev: /* Not loopback addresses on loopback should be preferred -- 1.7.0.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] net: Prefer non link-local source addresses 2011-09-01 20:20 [PATCH] net: Prefer non link-local source addresses Jeff Harris @ 2011-09-01 21:56 ` David Lamparter 2011-09-01 22:14 ` Julian Anastasov 1 sibling, 0 replies; 5+ messages in thread From: David Lamparter @ 2011-09-01 21:56 UTC (permalink / raw) To: Jeff Harris Cc: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, linux-kernel On Thu, Sep 01, 2011 at 04:20:54PM -0400, Jeff Harris wrote: > Section 2.6.1 of RFC 3927 specifies that if link-local and routable addresses > are available on an interface, a routable address is preferred. Update the > IPv4 source address selection algorithm to use a 169.254.x.x address only if > another matching address is not found. > + if (ipv4_is_linklocal_169(ifa->ifa_address)) { > + lladdr = ifa->ifa_local; > + continue; > + } IP addresses have a scope field: # ip -4 a l eth0 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP qlen 1000 inet 10.0.0.1/24 brd 10.0.0.255 scope link eth0 inet 172.22.80.50/25 brd 172.22.80.127 scope global eth0 would it not make sense to use to use that field instead? -David ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: Prefer non link-local source addresses 2011-09-01 20:20 [PATCH] net: Prefer non link-local source addresses Jeff Harris 2011-09-01 21:56 ` David Lamparter @ 2011-09-01 22:14 ` Julian Anastasov 2011-09-06 15:12 ` Harris, Jeff 1 sibling, 1 reply; 5+ messages in thread From: Julian Anastasov @ 2011-09-01 22:14 UTC (permalink / raw) To: Jeff Harris Cc: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, linux-kernel Hello, On Thu, 1 Sep 2011, Jeff Harris wrote: > Section 2.6.1 of RFC 3927 specifies that if link-local and routable addresses > are available on an interface, a routable address is preferred. Update the > IPv4 source address selection algorithm to use a 169.254.x.x address only if > another matching address is not found. > > Tested combinations of configured IP addresses with and without link-local to > verify a link-local address was chosen only if no routable address was > present. As David Lamparter already said, isn't the scope value suitable for this purpose? Eg. ip addr add 169.254.5.5/16 brd + dev eth0 scope link iproute2 already has function default_scope() in ip/ipaddress.c that assigns scope if it is not specified while adding address. May be we can add RT_SCOPE_LINK for 169.254 there? Another such place is inet_set_ifa() in net/ipv4/devinet.c where we can assign scope, so that ifconfig works too. I see also that net/ipv6/addrconf.c (sit_add_v4_addrs) avoids link-local addresses. What I mean is that the scope can be checked at many places and it is a mechanism that already works. As result, we will not complicate inet_select_addr. > Signed-off-by: Jeff Harris <jeff_harris@kentrox.com> > --- > net/ipv4/devinet.c | 18 ++++++++++++++++-- > 1 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c > index bc19bd0..70ddf37 100644 > --- a/net/ipv4/devinet.c > +++ b/net/ipv4/devinet.c > @@ -965,6 +965,8 @@ out: > __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope) > { > __be32 addr = 0; > + __be32 lladdr = 0; > + __be32 firstaddr = 0; > struct in_device *in_dev; > struct net *net = dev_net(dev); > > @@ -977,15 +979,27 @@ __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope) > if (ifa->ifa_scope > scope) > continue; > if (!dst || inet_ifa_match(dst, ifa)) { > + if (ipv4_is_linklocal_169(ifa->ifa_address)) { > + lladdr = ifa->ifa_local; > + continue; > + } > addr = ifa->ifa_local; > break; > } > - if (!addr) > - addr = ifa->ifa_local; > + if (!firstaddr) > + firstaddr = ifa->ifa_local; > } endfor_ifa(in_dev); > > if (addr) > goto out_unlock; > + if (lladdr) { > + addr = lladdr; > + goto out_unlock; > + } > + if (firstaddr) { > + addr = firstaddr; > + goto out_unlock; > + } > no_in_dev: > > /* Not loopback addresses on loopback should be preferred > -- > 1.7.0.5 Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] net: Prefer non link-local source addresses 2011-09-01 22:14 ` Julian Anastasov @ 2011-09-06 15:12 ` Harris, Jeff 2011-09-06 19:48 ` Julian Anastasov 0 siblings, 1 reply; 5+ messages in thread From: Harris, Jeff @ 2011-09-06 15:12 UTC (permalink / raw) To: Julian Anastasov Cc: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev@vger.kernel.org, linux-kernel@vger.kernel.org In this case, the address scope values are being set properly. The link-local address has link scope and the routable address has global scope. When the inet_select_addr function is called, the dst address is 0 and the scope is 253 (link scope). So, in this case, both address could match. It just happens that the link local address is first in the list for the device. This condition looks to be arising from the use of interface routes on our device (e.g. ip route add default dev eth0). The routes are being installed with link scope. Forcing a scope of global causes a scope of 0 in inet_select_addr which then selects the routable address always. I have not found any definite documentation on whether local or global should be used for the route, but the default behavior of the 'ip' command is to use link scope on these routes and global on routes with a gateway address. Also, I have only been able to test against 2.6.33 which we use on our embedded device. It is not easy to update to a more recent version. The patch, though, applied cleanly to the latest stable version. Jeff -----Original Message----- From: Julian Anastasov [mailto:ja@ssi.bg] Sent: Thursday, September 01, 2011 6:15 PM To: Harris, Jeff Cc: David S. Miller; Alexey Kuznetsov; James Morris; Hideaki YOSHIFUJI; Patrick McHardy; netdev@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] net: Prefer non link-local source addresses Hello, On Thu, 1 Sep 2011, Jeff Harris wrote: > Section 2.6.1 of RFC 3927 specifies that if link-local and routable addresses > are available on an interface, a routable address is preferred. Update the > IPv4 source address selection algorithm to use a 169.254.x.x address only if > another matching address is not found. > > Tested combinations of configured IP addresses with and without link-local to > verify a link-local address was chosen only if no routable address was > present. As David Lamparter already said, isn't the scope value suitable for this purpose? Eg. ip addr add 169.254.5.5/16 brd + dev eth0 scope link iproute2 already has function default_scope() in ip/ipaddress.c that assigns scope if it is not specified while adding address. May be we can add RT_SCOPE_LINK for 169.254 there? Another such place is inet_set_ifa() in net/ipv4/devinet.c where we can assign scope, so that ifconfig works too. I see also that net/ipv6/addrconf.c (sit_add_v4_addrs) avoids link-local addresses. What I mean is that the scope can be checked at many places and it is a mechanism that already works. As result, we will not complicate inet_select_addr. > Signed-off-by: Jeff Harris <jeff_harris@kentrox.com> > --- > net/ipv4/devinet.c | 18 ++++++++++++++++-- > 1 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c > index bc19bd0..70ddf37 100644 > --- a/net/ipv4/devinet.c > +++ b/net/ipv4/devinet.c > @@ -965,6 +965,8 @@ out: > __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope) > { > __be32 addr = 0; > + __be32 lladdr = 0; > + __be32 firstaddr = 0; > struct in_device *in_dev; > struct net *net = dev_net(dev); > > @@ -977,15 +979,27 @@ __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope) > if (ifa->ifa_scope > scope) > continue; > if (!dst || inet_ifa_match(dst, ifa)) { > + if (ipv4_is_linklocal_169(ifa->ifa_address)) { > + lladdr = ifa->ifa_local; > + continue; > + } > addr = ifa->ifa_local; > break; > } > - if (!addr) > - addr = ifa->ifa_local; > + if (!firstaddr) > + firstaddr = ifa->ifa_local; > } endfor_ifa(in_dev); > > if (addr) > goto out_unlock; > + if (lladdr) { > + addr = lladdr; > + goto out_unlock; > + } > + if (firstaddr) { > + addr = firstaddr; > + goto out_unlock; > + } > no_in_dev: > > /* Not loopback addresses on loopback should be preferred > -- > 1.7.0.5 Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] net: Prefer non link-local source addresses 2011-09-06 15:12 ` Harris, Jeff @ 2011-09-06 19:48 ` Julian Anastasov 0 siblings, 0 replies; 5+ messages in thread From: Julian Anastasov @ 2011-09-06 19:48 UTC (permalink / raw) To: Harris, Jeff Cc: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Hello, On Tue, 6 Sep 2011, Harris, Jeff wrote: > In this case, the address scope values are being set properly. The link-local address has link scope and the routable address has global scope. When the inet_select_addr function is called, the dst address is 0 and the scope is 253 (link scope). So, in this case, both address could match. It just happens that the link local address is first in the list for the device. Yes, all primary addresses are sorted in decreasing scope, link before global. For non-gatewayed routes (nh_gw=0) inet_select_addr always selects the first address with scope <= the route's scope. > This condition looks to be arising from the use of interface routes on our device (e.g. ip route add default dev eth0). The routes are being installed with link scope. Forcing a scope of global causes a scope of 0 in inet_select_addr which then selects the routable address always. I have not found any definite documentation on whether local or global should be used for the route, but the default behavior of the 'ip' command is to use link scope on these routes and global on routes with a gateway address. It is not good idea to have scope link for default route if you plan to contact the world because link routes can select link addresses for the target network. That is what you see as a problem. May be ip route should use scope global as default value for route 0.0.0.0/0. You have 2 choices: 1. Preferred: Use global scope for the default route: ip route add default scope global dev eth0 or ip route add default nexthop dev eth0 (not recommended) Global routes will not select link addresses. 2. Specify prefsrc for the route ip route add default dev eth0 src <Global_IP> but you risk to cause problems for MSG_DONTROUTE users if scope remains 'link' for target addresses that are not really onlink. > Also, I have only been able to test against 2.6.33 which we use on our embedded device. It is not easy to update to a more recent version. The patch, though, applied cleanly to the latest stable version. > > Jeff > > -----Original Message----- > From: Julian Anastasov [mailto:ja@ssi.bg] > Sent: Thursday, September 01, 2011 6:15 PM > To: Harris, Jeff > Cc: David S. Miller; Alexey Kuznetsov; James Morris; Hideaki YOSHIFUJI; Patrick McHardy; netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] net: Prefer non link-local source addresses > > > Hello, > > On Thu, 1 Sep 2011, Jeff Harris wrote: > > > Section 2.6.1 of RFC 3927 specifies that if link-local and routable addresses > > are available on an interface, a routable address is preferred. Update the > > IPv4 source address selection algorithm to use a 169.254.x.x address only if > > another matching address is not found. > > > > Tested combinations of configured IP addresses with and without link-local to > > verify a link-local address was chosen only if no routable address was > > present. > > As David Lamparter already said, isn't the scope value > suitable for this purpose? Eg. > ip addr add 169.254.5.5/16 brd + dev eth0 scope link > > iproute2 already has function default_scope() in > ip/ipaddress.c that assigns scope if it is not specified > while adding address. May be we can add RT_SCOPE_LINK for > 169.254 there? > > Another such place is inet_set_ifa() in > net/ipv4/devinet.c where we can assign scope, so that > ifconfig works too. > > I see also that net/ipv6/addrconf.c (sit_add_v4_addrs) > avoids link-local addresses. What I mean is that the scope > can be checked at many places and it is a mechanism that > already works. > > As result, we will not complicate inet_select_addr. Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-09-06 19:49 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-01 20:20 [PATCH] net: Prefer non link-local source addresses Jeff Harris 2011-09-01 21:56 ` David Lamparter 2011-09-01 22:14 ` Julian Anastasov 2011-09-06 15:12 ` Harris, Jeff 2011-09-06 19:48 ` Julian Anastasov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox