svn commit: r217592 - head/sys/netinet

Daniel Eischen deischen at freebsd.org
Wed Mar 30 15:42:02 UTC 2011


On Wed, 30 Mar 2011, John Baldwin wrote:

> On Wednesday, March 30, 2011 7:27:36 am Randall Stewart wrote:
>> John:
>>
>> The original complaint on this came from Daniel... I believe he
>> claimed that up until bms's multi-cast work.. you would NOT
>> get a packet sent to you if you did not join the multi-cast group.
>
> Not necessarily. :(  See below..
>
>> I will also comment in-line below...
>>
>> On Mar 29, 2011, at 2:01 PM, John Baldwin wrote:
>>
>>> On Wednesday, January 19, 2011 2:07:16 pm Randall Stewart wrote:
>>>> Author: rrs
>>>> Date: Wed Jan 19 19:07:16 2011
>>>> New Revision: 217592
>>>> URL: http://svn.freebsd.org/changeset/base/217592
>>>>
>>>> Log:
>>>>  Fix a bug where Multicast packets sent from a
>>>>  udp endpoint may end up echoing back to the sender
>>>>  even with OUT joining the multi-cast group.
>>>>
>>>>  Reviewed by:	gnn, bms, bz?
>>>>  Obtained from:	deischen (with help from)
>>>>
>>>> Modified:
>>>>  head/sys/netinet/udp_usrreq.c
>>>>
>>>> Modified: head/sys/netinet/udp_usrreq.c
>>>>
>>> ==============================================================================
>>>> --- head/sys/netinet/udp_usrreq.c	Wed Jan 19 18:20:11 2011	(r217591)
>>>> +++ head/sys/netinet/udp_usrreq.c	Wed Jan 19 19:07:16 2011	(r217592)
>>>> @@ -479,11 +479,13 @@ udp_input(struct mbuf *m, int off)
>>>> 			 * and source-specific multicast. [RFC3678]
>>>> 			 */
>>>> 			imo = inp->inp_moptions;
>>>> -			if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) &&
>>>> -			    imo != NULL) {
>>>> +			if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr))) {
>>>> 				struct sockaddr_in	 group;
>>>> 				int			 blocked;
>>>> -
>>>> +				if(imo == NULL) {
>>>> +					INP_RUNLOCK(inp);
>>>> +					continue;
>>>> +				}
>>>> 				bzero(&group, sizeof(struct sockaddr_in));
>>>> 				group.sin_len = sizeof(struct sockaddr_in);
>>>> 				group.sin_family = AF_INET;
>>>
>>> So it turns out that this is a feature, not a bug, and is how multicast has
>>> always worked.  Specifically, if you bind a UDP socket with a wildcard
>>> address, it should receive all traffic for the bound port, unicast or
>>> multicast.  When you join a group, you have switched the socket into a mode
>>> where it now has a whitelist of acceptable multicast groups, but if a socket
>>> has no joined groups, it should receive all multicast traffic, not none.  This
>>> change breaks that.
>>>
>>> I did not find this behavior intuitive at first, but it does seem to be
>>> required.  Note the description of IP_ADD_MEMBERSHIP from RFC 3678 for
>>> example:
>>
>>
>> I agree getting a packet that is coming to your port without joining the
>> multi-cast group is not intuitive to me...
>>
>>>
>>> 3.  Overview of APIs
>>>
>>>   There are a number of different APIs described in this document that
>>>   are appropriate for a number of different application types and IP
>>>   versions.  Before providing detailed descriptions, this section
>>>   provides a "taxonomy" with a brief description of each.
>>>
>>>   There are two categories of source-filter APIs, both of which are
>>>   designed to allow multicast receiver applications to designate the
>>>   unicast address(es) of sender(s) along with the multicast group
>>>   (destination address) to receive.
>>>
>>>      o  Basic (Delta-based): Some applications desire the simplicity of
>>>         a delta-based API in which each function call specifies a
>>>         single source address which should be added to or removed from
>>>         the existing filter for a given multicast group address on
>>>         which to listen.  Such applications typically fall into either
>>>         of two categories:
>>>
>>>         +  Any-Source Multicast: By default, all sources are accepted.
>>>            Individual sources may be turned off and back on as needed
>>>            over time.  This is also known as "exclude" mode, since the
>>>            source filter contains a list of excluded sources.
>>>
>>>         +  Source-Specific Multicast: Only sources in a given list are
>>>            allowed.  The list may change over time.  This is also known
>>>            as "include" mode, since the source filter contains a list
>>>            of included sources.
>>>
>>>            This API would be used, for example, by "single-source"
>>>            applications such as audio/video broadcasting.  It would
>>>            also be used for logical multi-source sessions where each
>>>            source independently allocates its own Source-Specific
>>>            Multicast group address.
>>
>>
>> Not the above document is talking about a receiver that as joined the
>> multicast group (or is joining it and wants some filtering)... I don't
>> see how that applies to a UDP socket that has NOT joined the M-cast group..
>>
>>>
>>>
>>> .....
>>>
>>> 4.1.1.  IPv4 Any-Source Multicast API
>>>
>>>   The following socket options are defined in <netinet/in.h> for
>>>   applications in the Any-Source Multicast category:
>>>
>>>   Socket option             Argument type
>>>   IP_ADD_MEMBERSHIP         struct ip_mreq
>>>   IP_BLOCK_SOURCE           struct ip_mreq_source
>>>   IP_UNBLOCK_SOURCE         struct ip_mreq_source
>>>   IP_DROP_MEMBERSHIP        struct ip_mreq
>>>
>>>   IP_ADD_MEMBERSHIP and IP_DROP_MEMBERSHIP are already implemented on
>>>   most operating systems, and are used to join and leave an any-source
>>>   group.
>>>
>>>   IP_BLOCK_SOURCE can be used to block data from a given source to a
>>>   given group (e.g., if the user "mutes" that source), and
>>>   IP_UNBLOCK_SOURCE can be used to undo this (e.g., if the user then
>>>   "unmutes" the source).
>>>
>>> As to why the packets loop back to the receiver, I believe that is a separate
>>> issue on the output side, not the receive side.
>>
>>
>>
>> But that is what the commit fixes...
>
> Except you change receive, not transmit.  I think the error in Daniel's case
> is on the transmit side, not receive.  I would be happy to know what the true
> "official" behavior is for a socket that binds to INADDR_ANY but does not join
> any groups.
>
> As far as prior to BMS's changes: in the older version of udp_input() we only
> checked the membership list if inp->inp_moptions was != NULL.  If it was NULL,
> we would send all multicast packets to a given socket for which the address
> fields matched (addr, port).
>
> BMS preserved this behavior and your patch changes

But he changed the behavior on output.  Pre-BMS and post-BMS
behave differently and not like Solaris 10 or VxWorks.  Haven't
tried Linux.

> it.  UDP sockets start off with inp_moptions == NULL, so if you never do any
> multicast-related setsockopt() you will receive all matching multicast packets.
> However, once you do any multicast-related setsockopt() (IP_MULTICAST_LOOP,
> IP_ADD_MEMBERSHIP, etc.) then inp_moptions is allocated and is non-NULL.
> At that point it only accepts packets that match, except that even then we
> used a sysctl which defaulted to off (!) to see if we should check the list of
> memberships (net.inet.udp.strict_mcast_mship).  This options structure was never
> free'd, however, so you could get the truly bizarre behavior of:
>
> - bind a new socket, it will not receive all matching multicast traffic
> - use IP_ADD_MEMBERSHIP to add a group, it will now receive only matching multicast
>  traffic for the group
> - use IP_DROP_MEMBERSHIP to remove the group, it will now receive no multicast
>  traffic
>
> The different behavior in states 1 and 3 I find confusing and odd.  By default
> all sockets just always received all matching multicast traffic though. :)

Not if a multicast group was not joined.  The pre-BMS changes
did not loop back multicast packets in ip_output.c.

> However, this change is not restoring "old" behavior, it is a change in behavior
> compared to the pre-BMS changes.

Agreed, to preserve pre-BMS behavior, the change should be
made on output.

>> and as to the above, it again is
>> talking about Multicast members.. AFAIKT... I am actually at the IETF
>> so if you would like I can gladly go talk to the authors of this RFC
>> (if they are here) and get their opinion on this.
>>
>> One other thing.. note this is NOT a standard but a informational RFC. Informational
>> RFC are guidelines and NOT mandatory at all.. there will never be a MUST/SHOULD etc
>> within them.
>
> Well, for lack of anything else I was looking to that.  I would really prefer
> the behavior in your change as I find it far more intuitive.  I wasn't able to
> find anything else in Stevens or elsewhere that seemed to indicate what the
> proper behavior was beyond this.

I also agree :-)

-- 
DE


More information about the svn-src-all mailing list