From: Ang Way Chuang <wcang@sfc.wide.ad.jp>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, yoshfuji@linux-ipv6.org
Subject: Re: [RFC][PATCH] ipv6 multicast forwarding: Remove threshold checking and some trivial bugs
Date: Tue, 18 Dec 2012 13:19:53 +0800 [thread overview]
Message-ID: <50CFFCF9.3070906@sfc.wide.ad.jp> (raw)
In-Reply-To: <20121217.210333.1952508082741483861.davem@davemloft.net>
Oops, sorry. You're right. I am not very confident with this modification. It may break some multicast
routing daemon. Let's drop this for now.
On 18/12/2012 13:03, David Miller wrote:
> From: Ang Way Chuang <wcang@sfc.wide.ad.jp>
> Date: Tue, 18 Dec 2012 12:57:11 +0800
>
>> This patch fixes trivial bugs for IPv6 multicast forwarding code and remove
>> threshold checking for multicast forwarding cache.
>>
>> 1. Threshold checking in IPv6 multicast forwarding cache (MFC) was not properly implemented.
>> syscall to setsockopt(... MRT6_ADD_MIF,...) doesn't affect the TTL because it is never used.
>> In fact, all MFC will always have ttl of 1 as set by ip6mr_mfc_add. From my limited knowledge of
>> multicast routing, threshold setting on interface is only used by DVMRP which doesn't support
>> IPv6. FreeBSD's struct mif6ctl doesn't have vifc_threshold. This patch removes the ttl cruft
>> within kernel. Userspace ABI for backward compatibility. Can someone knowledgable in multicast
>> routing please verify whether my understanding is correct?
>> 2. Don't allow addition of MFC with non-existent multicast interface index.
>> 3. Don't allow addition of MFC where incoming interface is part of oif list. This does not make
>> sense. Why would we want to send a multicast back to the interface where it originates from.
>> 4. setsockopt(....MRT6_ADD_MIF, ) allows a "physical" interface to be registered as multicast
>> interface multiple times. This doesn't make sense. Don't allow registration duplicate
>> registration of the same "physical" interface.
>>
>> This patch has been tested, albeit minimally using a simple program. Is this patch okay for
>> inclusion? Will sign off if it is okay.
>
> How about we don't mix together a set of bug fixes, with a semantic
> change (the removal of the threshold checking)?
>
> I also don't see what the point is of not signing off on this change
> when you submit it.
>
> If you delay the signoff until after review, you're just causing it to
> take longer to have your changes integrated. It also makes it look
> like you didn't believe fully in your change, so you probably should
> have sent it as an RFC and listed your doubts in the email instead.
>
> Overall I would rate this as an extremely poor patch submission,
> sorry.
> --
> 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
>
prev parent reply other threads:[~2012-12-18 5:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-18 4:57 [RFC][PATCH] ipv6 multicast forwarding: Remove threshold checking and some trivial bugs Ang Way Chuang
2012-12-18 5:03 ` David Miller
2012-12-18 5:19 ` Ang Way Chuang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50CFFCF9.3070906@sfc.wide.ad.jp \
--to=wcang@sfc.wide.ad.jp \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=yoshfuji@linux-ipv6.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).