[PATCH 0/2] Multicast: add mc_count to in_device and to inet6_dev

[PATCH 0/2] Multicast: add mc_count to in_device and to inet6_dev

[Rami Rosen]

Hi,
    - This patch set adds mc_count to  struct in_device (IPv4) and to
struct inet6_dev (IPv6) and updates increment/decrement/initialization
of this field in IPv4 and in IPv6.

- Also printing the /proc entry (/proc/net/igmp) is adjusted to use
the new mc_count.

 - Thanks to David Stevens for his feedback.

Regards,
Rami Rosen

On Thu, Sep 25, 2008 at 10:35 AM, David Stevens <dlstevens@us.ibm.com> wrote:
> netdev-owner@vger.kernel.org wrote on 09/24/2008 11:44:45 PM:
>
>> 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... :-)
>
>
>> 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
>
>

Re: [PATCH 0/2] Multicast: add mc_count to in_device and to inet6_dev

[David Miller]

From: "Rami Rosen" <roszenrami@gmail.com>
Date: Wed, 1 Oct 2008 09:57:42 +0300

> Hi,
>     - This patch set adds mc_count to  struct in_device (IPv4) and to
> struct inet6_dev (IPv6) and updates increment/decrement/initialization
> of this field in IPv4 and in IPv6.
> 
> - Also printing the /proc entry (/proc/net/igmp) is adjusted to use
> the new mc_count.
> 
>  - Thanks to David Stevens for his feedback.

Why even add the mc_count to the ipv6 side if it isn't
even going to be used?  I see nothing actually reading
the ipv6 version of the value.

If you're going to do that eventually in some later
set of changes, add it then.

Re: [PATCH 0/2] Multicast: add mc_count to in_device and to inet6_dev

[Rami Rosen]

Hi,
  - When sending the patch, I was aware that the mc_count is currently
not used (printed) in IPv6;
I added it for the sake of symmetry with IPv4 and for future use.
- Adding printing of mc_count in IPv6 is of course trivial; it was not
added not because of laziness;  I know less about IPv6 user space
tools (which read "/proc/net/igmp6") ; I was afraid that adding
printing of this mc_count field in IPv6 might break parsing of IPv6
user space tools (like netstat, etc).
So maybe this addition should be left for somebody who knows more
about IPv6 user space tools ?

Regards,
Rami Rosen


On Wed, Oct 1, 2008 at 12:08 PM, David Miller <davem@davemloft.net> wrote:
> From: "Rami Rosen" <roszenrami@gmail.com>
> Date: Wed, 1 Oct 2008 09:57:42 +0300
>
>> Hi,
>>     - This patch set adds mc_count to  struct in_device (IPv4) and to
>> struct inet6_dev (IPv6) and updates increment/decrement/initialization
>> of this field in IPv4 and in IPv6.
>>
>> - Also printing the /proc entry (/proc/net/igmp) is adjusted to use
>> the new mc_count.
>>
>>  - Thanks to David Stevens for his feedback.
>
> Why even add the mc_count to the ipv6 side if it isn't
> even going to be used?  I see nothing actually reading
> the ipv6 version of the value.
>
> If you're going to do that eventually in some later
> set of changes, add it then.
>

Re: [PATCH 0/2] Multicast: add mc_count to in_device and to inet6_dev

[David Miller]

From: "Rami Rosen" <roszenrami@gmail.com>
Date: Thu, 2 Oct 2008 08:24:33 +0300

>   - When sending the patch, I was aware that the mc_count is currently
> not used (printed) in IPv6;
> I added it for the sake of symmetry with IPv4 and for future use.

Unused struct members take up space in every instance of that
datastructure, so we don't add things like this until we have
real uses.

> - Adding printing of mc_count in IPv6 is of course trivial; it was not
> added not because of laziness;  I know less about IPv6 user space
> tools (which read "/proc/net/igmp6") ; I was afraid that adding
> printing of this mc_count field in IPv6 might break parsing of IPv6
> user space tools (like netstat, etc).
> So maybe this addition should be left for somebody who knows more
> about IPv6 user space tools ?

Of course modifying the output of this procfs file will break
things.  This is yet another reason to not add the ipv6 side
mc_count until we actually come up with a real use.