Re: [net-next RFC] net: ipv4: Send IGMP messages from highest scoped address

2015-08-18 Thread David Miller
From: Andrew Lunn and...@lunn.ch
Date: Tue, 18 Aug 2015 15:36:41 +0200

 We currently take the first address from the interface which is scope
 link or higher.
 
 Historically, the global scope address would of been used, but my
 previous fix, which stopped it taking a global scope address from a
 different interface altogether under some conditions, changed this
 behaviour.
 
 The first address from the interface, then broke one of my use
 case. The querier is only in one of the subnets on this interface, and
 using an address from the global scope address range. It then drops
 the membership reports when they are sent from the first address on
 the interface. This is why i want to restore the previous behaviour,
 take the global scope address from this interface.
 
 The patch works for me and is restoring previous behaviour, but is
 that sufficient to make it correct?

Preferring link-scope addresses make so much more sense for me.

The querier is on the local network, and he can do things like the
validity check on the subnet of the source address to try and avoid
forged IGMP responses.

So if anything I'd be advising you to change the code to prefer
link local addresses on the interface and keep avoiding global
addresses, as it is the only correct source address selection
scheme I can think of for IGMPs.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [net-next RFC] net: ipv4: Send IGMP messages from highest scoped address

2015-08-18 Thread Andrew Lunn
On Tue, Aug 18, 2015 at 12:01:10PM -0700, David Miller wrote:
 From: Andrew Lunn and...@lunn.ch
 Date: Tue, 18 Aug 2015 15:36:41 +0200
 
  We currently take the first address from the interface which is scope
  link or higher.
  
  Historically, the global scope address would of been used, but my
  previous fix, which stopped it taking a global scope address from a
  different interface altogether under some conditions, changed this
  behaviour.
  
  The first address from the interface, then broke one of my use
  case. The querier is only in one of the subnets on this interface, and
  using an address from the global scope address range. It then drops
  the membership reports when they are sent from the first address on
  the interface. This is why i want to restore the previous behaviour,
  take the global scope address from this interface.
  
  The patch works for me and is restoring previous behaviour, but is
  that sufficient to make it correct?
 
 Preferring link-scope addresses make so much more sense for me.
 
 The querier is on the local network, and he can do things like the
 validity check on the subnet of the source address to try and avoid
 forged IGMP responses.
 
 So if anything I'd be advising you to change the code to prefer
 link local addresses on the interface and keep avoiding global
 addresses, as it is the only correct source address selection
 scheme I can think of for IGMPs.

Hi David

O.K, thanks for the discussion, and this patch can be ignored.

 Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [net-next RFC] net: ipv4: Send IGMP messages from highest scoped address

2015-08-18 Thread Andrew Lunn
On Mon, Aug 17, 2015 at 09:31:58PM -0700, David Miller wrote:
 From: Andrew Lunn and...@lunn.ch
 Date: Mon, 17 Aug 2015 18:09:24 +0200
 
  This is RFC because i personally don't know if this is the best fix.
  The patch restores previous behavior, while still keeping the bug fix.
  
  It is not obvious what is the correct source address for an IGMP
  message when an interface has multiple addresses. IGMP messages are
  sent either spontaneously, or as a result of a query. It could be
  argued that when replying to a query, an address take from the same
  subnet as the querier should be used. Doing this adds complexity for a
  corner case which does not seem to effect people. In the spontaneous
  case, there is no such hint, so an address has to be picked some other
  way. Taking the highest scope address seems reasonable, and works for
  me.
 
 Yes, unfortunately almost no guidance is given in this area in the
 various RFCs.
 
 Except that one recommended way to avoid forged IGMPs is to reject
 any IGMP that has a source address no on that interface's subnet.
 
 And if people actually do that, then the link scope address is the
 best address to use and that is what we do now if I understand things
 correctly.

Hi David

We currently take the first address from the interface which is scope
link or higher.

Historically, the global scope address would of been used, but my
previous fix, which stopped it taking a global scope address from a
different interface altogether under some conditions, changed this
behaviour.

The first address from the interface, then broke one of my use
case. The querier is only in one of the subnets on this interface, and
using an address from the global scope address range. It then drops
the membership reports when they are sent from the first address on
the interface. This is why i want to restore the previous behaviour,
take the global scope address from this interface.

The patch works for me and is restoring previous behaviour, but is
that sufficient to make it correct?

 Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [net-next RFC] net: ipv4: Send IGMP messages from highest scoped address

2015-08-17 Thread David Miller
From: Andrew Lunn and...@lunn.ch
Date: Mon, 17 Aug 2015 18:09:24 +0200

 This is RFC because i personally don't know if this is the best fix.
 The patch restores previous behavior, while still keeping the bug fix.
 
 It is not obvious what is the correct source address for an IGMP
 message when an interface has multiple addresses. IGMP messages are
 sent either spontaneously, or as a result of a query. It could be
 argued that when replying to a query, an address take from the same
 subnet as the querier should be used. Doing this adds complexity for a
 corner case which does not seem to effect people. In the spontaneous
 case, there is no such hint, so an address has to be picked some other
 way. Taking the highest scope address seems reasonable, and works for
 me.

Yes, unfortunately almost no guidance is given in this area in the
various RFCs.

Except that one recommended way to avoid forged IGMPs is to reject
any IGMP that has a source address no on that interface's subnet.

And if people actually do that, then the link scope address is the
best address to use and that is what we do now if I understand things
correctly.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[net-next RFC] net: ipv4: Send IGMP messages from highest scoped address

2015-08-17 Thread Andrew Lunn
Patch 6a21165480a0 (net: ipv4: route: Fix sending IGMP messages with
link address) changed the way the source address of an IGMP message
was determined. Before that patch, a global scope addresses would be
used from another interface, if there was no global scope address on
the outgoing interface. That patch fixes this so a source address from
the outgoing interface was picked. However, in complex configurations,
it is not picking the best address, for example:

7: eth2: BROADCAST,MULTICAST,UP,LOWER_UP mtu 1500 qdisc pfifo_fast state 
UNKNOWN group default qlen 1000
link/ether 02:00:00:20:12:01 brd ff:ff:ff:ff:ff:ff
inet 10.81.0.42/24 brd 10.81.0.255 scope link eth2
   valid_lft forever preferred_lft forever
inet 10.81.100.42/24 brd 10.81.100.255 scope link eth2:2
   valid_lft forever preferred_lft forever
inet 10.81.200.42/24 brd 10.81.200.255 scope global eth2:1
   valid_lft forever preferred_lft forever
inet6 fe80::ff:fe20:1201/64 scope link
   valid_lft forever preferred_lft forever

The first address is used, which has scope link. Before the previous
patch, the global scope address would of been used.

This patch adds a new function to find the highest scope address on an
interface, and this is then used for IGMP messages in the routing
code.

Signed-off-by: Andrew Lunn and...@lunn.ch
---

This is RFC because i personally don't know if this is the best fix.
The patch restores previous behavior, while still keeping the bug fix.

It is not obvious what is the correct source address for an IGMP
message when an interface has multiple addresses. IGMP messages are
sent either spontaneously, or as a result of a query. It could be
argued that when replying to a query, an address take from the same
subnet as the querier should be used. Doing this adds complexity for a
corner case which does not seem to effect people. In the spontaneous
case, there is no such hint, so an address has to be picked some other
way. Taking the highest scope address seems reasonable, and works for
me.

Andrew


include/linux/inetdevice.h |  1 +
 net/ipv4/devinet.c | 33 +
 net/ipv4/route.c   | 10 +++---
 3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index a4328cea376a..351f6feb92bb 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -167,6 +167,7 @@ int devinet_ioctl(struct net *net, unsigned int cmd, void 
__user *);
 void devinet_init(void);
 struct in_device *inetdev_by_index(struct net *, int);
 __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
+__be32 inet_select_highest_scope(const struct net_device *dev);
 __be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
 __be32 local, int scope);
 struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix,
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 2d9cb1748f81..6419356f2893 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1238,6 +1238,39 @@ out_unlock:
 }
 EXPORT_SYMBOL(inet_select_addr);
 
+__be32 inet_select_highest_scope(const struct net_device *dev)
+{
+   __be32 addr = 0;
+   struct in_device *in_dev;
+   struct net *net = dev_net(dev);
+   int best_scope = RT_SCOPE_NOWHERE;
+
+   rcu_read_lock();
+   in_dev = __in_dev_get_rcu(dev);
+   if (!in_dev)
+   goto no_in_dev;
+
+   for_ifa(in_dev) {
+   if (ifa-ifa_scope  best_scope)
+   continue;
+   addr = ifa-ifa_local;
+   best_scope = ifa-ifa_scope;
+   } endfor_ifa(in_dev);
+
+   if (addr)
+   goto out_unlock;
+no_in_dev:
+   /* Not loopback addresses on loopback should be preferred in
+* this case.
+*/
+   addr = inet_select_addr_lo(net, dev, RT_SCOPE_UNIVERSE);
+
+out_unlock:
+   rcu_read_unlock();
+   return addr;
+}
+EXPORT_SYMBOL(inet_select_highest_scope);
+
 static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
  __be32 local, int scope)
 {
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 2c89d294b669..955c24f221ef 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2116,11 +2116,15 @@ struct rtable *__ip_route_output_key(struct net *net, 
struct flowi4 *fl4)
goto out;
}
if (ipv4_is_local_multicast(fl4-daddr) ||
-   ipv4_is_lbcast(fl4-daddr) ||
-   fl4-flowi4_proto == IPPROTO_IGMP) {
+   ipv4_is_lbcast(fl4-daddr)) {
if (!fl4-saddr)
fl4-saddr = inet_select_addr(dev_out, 0,
- RT_SCOPE_LINK);
+ RT_SCOPE_HOST);
+   goto