Discussion:
/proc/net/igmp group address encoding
Pekka Savola
2008-09-24 10:31:32 UTC
Permalink
Hi,

I've spotted some irritating IGMPv3/v2 compat problems on RHEL4, and
I'm working on trying to see if these are due to bugs already fixed in
latest kernels. In the process, I started wondering about
/proc/net/igmp format (on 2.6.26.5-39.fc9.i686):

[***@lapps ~]$ more /proc/net/igmp
Idx Device : Count Querier Group Users Timer Reporter
1 lo : 0 V3
010000E0 1 0:00000000 0
2 eth0 : 2 V3
010000E0 1 0:00000000 0
3 wmaster0 : 3 V3
010000E0 1 0:00000000 0
4 wlan0 : 3 V3
010000E0 1 0:00000000 0

(The header and the fields seem to be a little off in the output..)

[***@lapps ~]$ netstat -gn | grep eth0
eth0 1 224.0.0.1
eth0 1 ff02::1

It appears "010000E0" is 224.0.0.1 encoded in in hex but
right-to-left. Other groups are similarly encoded. /proc/net/igmp6
is encoded in a human-friendly format.

Is this intentional? Should this be changed, e.g. to flow
left-to-right and use the dotted quad IP address format?
--
Pekka Savola "You each name yourselves king, yet the
Netcore Oy kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Pekka Savola
2008-09-24 11:20:48 UTC
Permalink
Post by Pekka Savola
Idx Device : Count Querier Group Users Timer Reporter
1 lo : 0 V3
010000E0 1 0:00000000 0
2 eth0 : 2 V3
010000E0 1 0:00000000 0
3 wmaster0 : 3 V3
010000E0 1 0:00000000 0
4 wlan0 : 3 V3
010000E0 1 0:00000000 0
(The header and the fields seem to be a little off in the output..)
eth0 1 224.0.0.1
eth0 1 ff02::1
Another observation here: the "Count" field appears to correspond to
'netstat -gn | wc -l' output. But this includes both IPv4 and IPv6
groups. Is this intentional? Shouldn't this only count the v4
entries, just like /proc/net/igmp6 only includes v6 entries?
--
Pekka Savola "You each name yourselves king, yet the
Netcore Oy kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Stevens
2008-09-24 17:52:03 UTC
Permalink
Post by Pekka Savola
Idx Device : Count Querier Group Users Timer Reporter
1 lo : 0 V3
010000E0 1 0:00000000 0
2 eth0 : 2 V3
010000E0 1 0:00000000 0
3 wmaster0 : 3 V3
010000E0 1 0:00000000 0
4 wlan0 : 3 V3
010000E0 1 0:00000000 0
(The header and the fields seem to be a little off in the output..)
eth0 1 224.0.0.1
eth0 1 ff02::1
Another observation here: the "Count" field appears to correspond to
'netstat -gn | wc -l' output. But this includes both IPv4 and IPv6
groups. Is this intentional? Shouldn't this only count the v4
entries, just like /proc/net/igmp6 only includes v6 entries?
I'm guessing this is Alan Cox's code, but it's at least 8 years
old, so I'm not sure original intent is easy to establish. :-)

I agree it'd be a good idea to separate the counts. Are you
planning to do a patch?

+-DLS

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Pekka Savola
2008-09-24 20:12:17 UTC
Permalink
Post by David Stevens
Post by Pekka Savola
Idx Device : Count Querier Group Users Timer Reporter
1 lo : 0 V3
010000E0 1 0:00000000 0
2 eth0 : 2 V3
010000E0 1 0:00000000 0
3 wmaster0 : 3 V3
010000E0 1 0:00000000 0
4 wlan0 : 3 V3
010000E0 1 0:00000000 0
(The header and the fields seem to be a little off in the output..)
eth0 1 224.0.0.1
eth0 1 ff02::1
Another observation here: the "Count" field appears to correspond to
'netstat -gn | wc -l' output. But this includes both IPv4 and IPv6
groups. Is this intentional? Shouldn't this only count the v4
entries, just like /proc/net/igmp6 only includes v6 entries?
I'm guessing this is Alan Cox's code, but it's at least 8 years
old, so I'm not sure original intent is easy to establish. :-)
I agree it'd be a good idea to separate the counts. Are you
planning to do a patch?
I'm less than familiar with the code in question and not best placed
to work on this, so if you (or someone else) would be
interested/willing to do this, great. If nothing happens, I may need
to reconsider my options..
--
Pekka Savola "You each name yourselves king, yet the
Netcore Oy kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Stevens
2008-09-24 20:49:34 UTC
Permalink
Post by Pekka Savola
I'm less than familiar with the code in question and not best placed
to work on this, so if you (or someone else) would be
interested/willing to do this, great. If nothing happens, I may need
to reconsider my options..
Sure, I'll have a look, though probably not immediately.

+-DLS

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Rami Rosen
2008-09-25 06:44:45 UTC
Permalink
Hi,

- I had noticed a long ago that the IP addresses under
/proc/net/igmp are in Hexa format. I am preparing a patch to fix it
and it will be send very soon.

On this occasion -
Regarding "count" column of /proc/net/igmp :
This is simply a display the mc_count field of the device (number of
installed mcasts on the net_device instance).
Whenever you join an igmp group, you should specify on which
device you wish to join this igmp group. As a result, the counter of
the corresponding device (mc_count of the corresponding net_device) is
incremented.
See; dev_mc_add() in core/dev_mcast.c and
__dev_addr_add() in core/dev.c

As far as I know, there is no difference in current implementation in
this regards when you join an igmp group in IPv4 or in IPv6;
the same dev_mc_add() is called by
igmp6_group_added() (see net/ipv6/mcast.c).

So indeed , the count (mc_count of netdevice) shows both IPv4 and IPv6 igmp
groups, but in current implementation it is not immediate
to change it. Currently, net_device struct does not have specific
fields for ipv4 or ipv6. One way of preparing a patch
is to change net_device to have mc_count4 and mc_count6;
I am really willing to prepare such a patch, but I am not sure such a
change in net_device is a proper and a justified
change to do.

Any ideas?



Regards,
Rami Rosen



I am preparing a patch which
Post by David Stevens
Post by Pekka Savola
I'm less than familiar with the code in question and not best placed
to work on this, so if you (or someone else) would be
interested/willing to do this, great. If nothing happens, I may need
to reconsider my options..
Sure, I'll have a look, though probably not immediately.
+-DLS
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Stevens
2008-09-25 07:35:40 UTC
Permalink
Post by Rami Rosen
Hi,
- I had noticed a long ago that the IP addresses under
/proc/net/igmp are in Hexa format. I am preparing a patch to fix it
and it will be send very soon.
I don't think we can do that, since I believe netstat expects
to see this the way it is. Maybe if we change netstat to support
both and wait a year for all distros to get it... :-)
Post by Rami Rosen
So indeed , the count (mc_count of netdevice) shows both IPv4 and IPv6 igmp
groups, but in current implementation it is not immediate
to change it. Currently, net_device struct does not have specific
fields for ipv4 or ipv6. One way of preparing a patch
is to change net_device to have mc_count4 and mc_count6;
I am really willing to prepare such a patch, but I am not sure such a
change in net_device is a proper and a justified
change to do.
These belong with mc_list in inet6_dev and in_device. Really,
I'd just add "mc_count" as a field right next to mc_list and inc/dec
it when calling the group join/leave functions-- use the list lock
to protect it. The structure it's in distinguishes the protocol,
and the names are otherwise common.

+-DLS

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Rami Rosen
2008-09-25 07:59:13 UTC
Permalink
Hi,
since I believe netstat expects to see this the way it is.
Thanks for drawing my attention to this; due to this , displaying in
quad format should not be implemented.

I am working on a patch for the mc_counter according to your
suggestion putting it in inet6_dev and in_device).

Thanks,

Regards,
Rami Rosen
Post by Rami Rosen
Hi,
- I had noticed a long ago that the IP addresses under
/proc/net/igmp are in Hexa format. I am preparing a patch to fix it
and it will be send very soon.
I don't think we can do that, since I believe netstat expects
to see this the way it is. Maybe if we change netstat to support
both and wait a year for all distros to get it... :-)
Post by Rami Rosen
So indeed , the count (mc_count of netdevice) shows both IPv4 and IPv6
igmp
Post by Rami Rosen
groups, but in current implementation it is not immediate
to change it. Currently, net_device struct does not have specific
fields for ipv4 or ipv6. One way of preparing a patch
is to change net_device to have mc_count4 and mc_count6;
I am really willing to prepare such a patch, but I am not sure such a
change in net_device is a proper and a justified
change to do.
These belong with mc_list in inet6_dev and in_device. Really,
I'd just add "mc_count" as a field right next to mc_list and inc/dec
it when calling the group join/leave functions-- use the list lock
to protect it. The structure it's in distinguishes the protocol,
and the names are otherwise common.
+-DLS
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller
2008-10-06 21:59:02 UTC
Permalink
From: "Rami Rosen" <***@gmail.com>
Date: Thu, 25 Sep 2008 10:59:13 +0300
Post by Rami Rosen
since I believe netstat expects to see this the way it is.
Thanks for drawing my attention to this; due to this , displaying in
quad format should not be implemented.
Indeed, netstat even seems to transparently handle the fact that the
value is kept in big-endian (rather than being converted using
be32_to_cpu()) as it is printed by the kernel, regardless of cpu
endianness.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Pekka Savola
2008-09-25 06:40:37 UTC
Permalink
Post by Pekka Savola
I've spotted some irritating IGMPv3/v2 compat problems on RHEL4, and
I'm working on trying to see if these are due to bugs already fixed
in latest kernels.
OK, I've done some tests.

1) in a RHEL4 2.6.8 kernel, the logic of timeouting seeing a V2
querier earlier seems to be broken -- even if there are no IGMPv2
reports or queries on the LAN for a long time, the system (as seen
by /proc/net/igmp) doesn't go back to V3. I tested that this works
in 2.6.26.5 so this is OK.

2) only force_igmp_version=[12] is supported. It might be useful to
support "force_igmp_version=3" as well, where the system will not
fall back to IGMPv1 or IGMPv2 compat mode even if it thinks it sees
or has seen an IGMPv1/v2 query.

This behaviour would be useful for example in scenarios where you
know that the router is IGMPv3/MLDv2 capable, and you want to
ignore queries sent by other routers, snooping switches or rogue
hosts, or membership reports by other hosts (but Linux currently
doesn't seem to support group-specific IGMP downgrade so this is
not a problem right now) [see e.g. RFC4604 for more on various
other things, e.g. warning errors, that could be added].

What I refer to something like the follows (not tested, whitespace
probably broken due to cut'n'pasting), or some similar changes in
the logic where IGMP_Vx_SEEN is used (similar with MLD_V1_SEEN):

--- igmp.c.orig 2008-09-25 09:33:00.000000000 +0300
+++ igmp.c 2008-09-25 09:35:20.000000000 +0300
@@ -2,9 +2,13 @@
(IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) == 1 || \
IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) == 1 || \
((in_dev)->mr_v1_seen && \
+ IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) != 3 && \
+ IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) != 3 && \
time_before(jiffies, (in_dev)->mr_v1_seen)))
#define IGMP_V2_SEEN(in_dev) \
(IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) == 2 || \
IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) == 2 || \
((in_dev)->mr_v2_seen && \
+ IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) != 3 && \
+ IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) != 3 && \
time_before(jiffies, (in_dev)->mr_v2_seen)))
--
Pekka Savola "You each name yourselves king, yet the
Netcore Oy kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Stevens
2008-09-25 07:10:20 UTC
Permalink
Post by Pekka Savola
2) only force_igmp_version=[12] is supported. It might be useful to
support "force_igmp_version=3" as well, where the system will not
fall back to IGMPv1 or IGMPv2 compat mode even if it thinks it sees
or has seen an IGMPv1/v2 query.
IGMPv3 defines the interaction with IGMPv2-- there isn't any
choice about it, which is why v3 isn't in the force set. Forcing
earlier versions may be a requirement for switches, which is why
the knob is there, but forcing later versions, which all require
full knowledge of prior versions, doesn't make sense to me.
Answering a v2 query with a v3 response wouldn't work, since
obviously v2 queriers don't know v3 packets, and ignoring them is
a technical violation of RFC 3376. Even 4604 doesn't suggest
these queries be ignored.
Logging an error, as suggested in RFC 4604, is ok, but it
really is only a performance issue. Without the filter information,
the group memberships may be bigger than they need to be, but
the filters will still be applied on the receiver. Ignoring queries
or answering them with v3 are both broken, at least for the v2
querier.

So, I don't think forcing IGMPv3 makes much sense. If you
really cared about it, you could always add an iptables entry to
drop them, right?

+-DLS

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Pekka Savola
2008-09-25 09:18:02 UTC
Permalink
Post by David Stevens
Post by Pekka Savola
2) only force_igmp_version=[12] is supported. It might be useful to
support "force_igmp_version=3" as well, where the system will not
fall back to IGMPv1 or IGMPv2 compat mode even if it thinks it sees
or has seen an IGMPv1/v2 query.
...
Post by David Stevens
Logging an error, as suggested in RFC 4604, is ok, but it
really is only a performance issue. Without the filter information,
the group memberships may be bigger than they need to be, but
the filters will still be applied on the receiver. Ignoring queries
or answering them with v3 are both broken, at least for the v2
querier.
Note that it's not just a performance issue if the host would want to
report an (S,G) for an SSM group. This requires IGMPv3/MLDv2. Then
if it falls back to IGMPv2, it can only report (*,G), and it can no
longer get the SSM transmission. If a malicious host would send an
IGMPv2 query on an otherwise IGMPv3-capable network segment, that
packet would be a trivial DoS.

What I'm saying is that the compatibility mode is required in a
scenario where you don't know which IGMP versions the legitimate nodes
in the LAN support, but it also a DoS vector. But in a managed
network where you know the IGMP versions supported, forcing it may
make sense. This is why when you use sysctl toggles to change default
behaviour, you're supposed to know what you're doing.
Post by David Stevens
So, I don't think forcing IGMPv3 makes much sense. If you
really cared about it, you could always add an iptables entry to
drop them, right?
It's a bit tricky to figure out the correct rules to use if you want
to drop all IGMPv1 and IGMPv2 traffic.
--
Pekka Savola "You each name yourselves king, yet the
Netcore Oy kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...