diff mbox

[RFC] ipv6: always join solicited-node address

Message ID 1381685064-24805-1-git-send-email-bjorn@mork.no
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Bjørn Mork Oct. 13, 2013, 5:24 p.m. UTC
RFC 4861 section 7.2.1 "Interface Initialization" says:

   When a multicast-capable interface becomes enabled, the node MUST
   join the all-nodes multicast address on that interface, as well as
   the solicited-node multicast address corresponding to each of the IP
   addresses assigned to the interface.

The current dependency on IFF_NOARP seems unwarranted. We need to
listen on the solicited-node address whether or not we intend to
initiate Neigbour Discovery ourselves.

This fixes a bug where Linux fails to respond to received Neigbour
Solicitations on multicast capable links when IFF_NOARP is set.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
I am not at all sure about this... Comments are appreciated.

The observed problem is a MBIM mobile broadband modem sending NS
to the host.  MBIM is a point-to-point USB protocol which does not
have any L2 headers at all.  It can only transport IPv4 or IPv6
packets.  So for IPv4 there is no question at all:  ARP just
cannot be transported. The driver emulates an ethernet interface,
setting IFF_NOARP to make sure the upper layers doesn't attempt
to resolve the neighbours non-existing L2 addresses.

But then there is this modem which sends IPv6 Neigbour
Solicitations to the host over the MBIM transport. The link
layer addresses are meaningless, but it seems the modem still
expects an answer.  Which we will not currently provide, because
the NS is addressed to a solicited-node address we don't listen
to.

So this patch seems like a quick-fix to that problem.  But it does
change the semantics of IFF_NOARP, making us reply to NS even if
this flag is set.  Which probably is wrong?


Bjørn

 net/ipv6/addrconf.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Hannes Frederic Sowa Oct. 14, 2013, 12:55 a.m. UTC | #1
On Sun, Oct 13, 2013 at 07:24:24PM +0200, Bjørn Mork wrote:
> RFC 4861 section 7.2.1 "Interface Initialization" says:
> 
>    When a multicast-capable interface becomes enabled, the node MUST
>    join the all-nodes multicast address on that interface, as well as
>    the solicited-node multicast address corresponding to each of the IP
>    addresses assigned to the interface.
> 
> The current dependency on IFF_NOARP seems unwarranted. We need to
> listen on the solicited-node address whether or not we intend to
> initiate Neigbour Discovery ourselves.
> 
> This fixes a bug where Linux fails to respond to received Neigbour
> Solicitations on multicast capable links when IFF_NOARP is set.
> 
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> I am not at all sure about this... Comments are appreciated.
> 
> The observed problem is a MBIM mobile broadband modem sending NS
> to the host.  MBIM is a point-to-point USB protocol which does not
> have any L2 headers at all.  It can only transport IPv4 or IPv6
> packets.  So for IPv4 there is no question at all:  ARP just
> cannot be transported. The driver emulates an ethernet interface,
> setting IFF_NOARP to make sure the upper layers doesn't attempt
> to resolve the neighbours non-existing L2 addresses.
> 
> But then there is this modem which sends IPv6 Neigbour
> Solicitations to the host over the MBIM transport. The link
> layer addresses are meaningless, but it seems the modem still
> expects an answer.  Which we will not currently provide, because
> the NS is addressed to a solicited-node address we don't listen
> to.
> 
> So this patch seems like a quick-fix to that problem.  But it does
> change the semantics of IFF_NOARP, making us reply to NS even if
> this flag is set.  Which probably is wrong?

IFF_NOARP seems to be a bit messed up in ipv6. Your patch seems fine to
me but I would add protection to the ndisc_rcv and sending routines to
do nothing if IFF_NOARP is set for that interface.

So it would be possible that you could resolve this issue by just issuing
an "ip link set arp on dev <interface>" and won't have hassle with racing
interface initialization.

Is this a specific bug of the modem you are using or are all devices
powered by this driver like this?

Greetings,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjørn Mork Oct. 14, 2013, 7:09 a.m. UTC | #2
Hannes Frederic Sowa <hannes@stressinduktion.org> writes:

> IFF_NOARP seems to be a bit messed up in ipv6. Your patch seems fine to
> me but I would add protection to the ndisc_rcv and sending routines to
> do nothing if IFF_NOARP is set for that interface.

That's what I thought.  So for my problem, this will not really change
much.

> So it would be possible that you could resolve this issue by just issuing
> an "ip link set arp on dev <interface>" and won't have hassle with racing
> interface initialization.

Yes, but that would also make the IP layer try to resolve IP to link
layer addressess both for IPv4 and IPv6, which just won't work. At least
not for IPv4, where there just is no way to transport an ARP to the
modem.  And I assume it may fail for IPv6 too on any sane device.

> Is this a specific bug of the modem you are using or are all devices
> powered by this driver like this?

Unfortunately I have no IPv6 enabled SIM myself, so I have no
information about other devices.  This report was based on user
feedback.

I assume the bug is specific to this firmware implementation, probably
extending to all similar devices from the same vendor.  But it could be
more common than that.  The fact that the bug is there indicates that
this works just fine in Windows.

Yes, I realize that I am in ugly-hack-to-workaround-firmware-issues land
again... But it sure would be nice to have some way for a driver to
indicate that L2 neighbour tables are meaningless, but that any incoming
requests should still be answered.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Frederic Sowa Oct. 14, 2013, 3:56 p.m. UTC | #3
On Mon, Oct 14, 2013 at 09:09:59AM +0200, Bjørn Mork wrote:
> Yes, but that would also make the IP layer try to resolve IP to link
> layer addressess both for IPv4 and IPv6, which just won't work. At least
> not for IPv4, where there just is no way to transport an ARP to the
> modem.  And I assume it may fail for IPv6 too on any sane device.

I don't think that clearing the IFF_NOARP flag would kill connectivity
for either IPv4 or IPv6. It may compromise security for IPv6 though
(no idea how the telco network behind the modem looks like).

> > Is this a specific bug of the modem you are using or are all devices
> > powered by this driver like this?
> 
> Unfortunately I have no IPv6 enabled SIM myself, so I have no
> information about other devices.  This report was based on user
> feedback.
> 
> I assume the bug is specific to this firmware implementation, probably
> extending to all similar devices from the same vendor.  But it could be
> more common than that.  The fact that the bug is there indicates that
> this works just fine in Windows.
> 
> Yes, I realize that I am in ugly-hack-to-workaround-firmware-issues land
> again... But it sure would be nice to have some way for a driver to
> indicate that L2 neighbour tables are meaningless, but that any incoming
> requests should still be answered.

L2 neighbour tables are resolved on demand and won't be queried for the
link you are talking about (at least for IPv6, but I assume IPv4, too).

A new flag should have clear semantics then:

* split IFF_NOARP to IFF_NOARP and IFF_NONDISC
* split IFF_NOARP to IFF_NOLLRESOLV_RESPONSE and IFF_NOLLRESOLV_MODIFY
  (each one flag which is applicable for both IPv4 and IPv6)

I tend to lean towards the last alternative but still wonder if this is
just overhead for this one buggy device.

Greetings,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjørn Mork Oct. 31, 2013, 3 p.m. UTC | #4
Hannes Frederic Sowa <hannes@stressinduktion.org> writes:

> On Mon, Oct 14, 2013 at 09:09:59AM +0200, Bjørn Mork wrote:
>> Yes, but that would also make the IP layer try to resolve IP to link
>> layer addressess both for IPv4 and IPv6, which just won't work. At least
>> not for IPv4, where there just is no way to transport an ARP to the
>> modem.  And I assume it may fail for IPv6 too on any sane device.
>
> I don't think that clearing the IFF_NOARP flag would kill connectivity
> for either IPv4 or IPv6.

It does, because the neigbours cannot be resolved.

> It may compromise security for IPv6 though
> (no idea how the telco network behind the modem looks like).
>
>> > Is this a specific bug of the modem you are using or are all devices
>> > powered by this driver like this?
>> 
>> Unfortunately I have no IPv6 enabled SIM myself, so I have no
>> information about other devices.  This report was based on user
>> feedback.
>> 
>> I assume the bug is specific to this firmware implementation, probably
>> extending to all similar devices from the same vendor.  But it could be
>> more common than that.  The fact that the bug is there indicates that
>> this works just fine in Windows.
>> 
>> Yes, I realize that I am in ugly-hack-to-workaround-firmware-issues land
>> again... But it sure would be nice to have some way for a driver to
>> indicate that L2 neighbour tables are meaningless, but that any incoming
>> requests should still be answered.
>
> L2 neighbour tables are resolved on demand and won't be queried for the
> link you are talking about (at least for IPv6, but I assume IPv4, too).
>
> A new flag should have clear semantics then:
>
> * split IFF_NOARP to IFF_NOARP and IFF_NONDISC
> * split IFF_NOARP to IFF_NOLLRESOLV_RESPONSE and IFF_NOLLRESOLV_MODIFY
>   (each one flag which is applicable for both IPv4 and IPv6)
>
> I tend to lean towards the last alternative but still wonder if this is
> just overhead for this one buggy device.

Thanks a lot for your review and thoughts on this.  I do agree 100% with
your last comment here, and ended up with an in-driver workaround
instead.

It's not beautiful, but at least it doesn't add a lot of core code for
this very special purpose.



Bjørn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cd3fb30..aa2df3b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1658,7 +1658,7 @@  void addrconf_join_solict(struct net_device *dev, const struct in6_addr *addr)
 {
 	struct in6_addr maddr;
 
-	if (dev->flags&(IFF_LOOPBACK|IFF_NOARP))
+	if (!(dev->flags & IFF_MULTICAST))
 		return;
 
 	addrconf_addr_solict_mult(addr, &maddr);
@@ -1669,7 +1669,7 @@  void addrconf_leave_solict(struct inet6_dev *idev, const struct in6_addr *addr)
 {
 	struct in6_addr maddr;
 
-	if (idev->dev->flags&(IFF_LOOPBACK|IFF_NOARP))
+	if (!(idev->dev->flags & IFF_MULTICAST))
 		return;
 
 	addrconf_addr_solict_mult(addr, &maddr);