| Message ID | 1461070287-13469-4-git-send-email-erezsh@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, 19 Apr 2016, Erez Shitrit wrote: > +/* > +* There are 4 types of join states: > +* FullMember, NonMember, SendOnlyNonMember, SendOnlyFullMember. > +*/ > +enum { > + FULLMEMBER_JOIN, > + NONMEMBER_JOIN, > + SENDONLY_NONMEBER_JOIN, > + SENDONLY_FULLMEMBER_JOIN, > + NUM_JOIN_MEMBERSHIP_TYPES = 4 It will be 4 because there are 4 definitions before this. No need to assign a value. The value will be wrong if more definitions are added later. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > +* There are 4 types of join states: > > +* FullMember, NonMember, SendOnlyNonMember, SendOnlyFullMember. > > +*/ > > +enum { > > + FULLMEMBER_JOIN, > > + NONMEMBER_JOIN, > > + SENDONLY_NONMEBER_JOIN, > > + SENDONLY_FULLMEMBER_JOIN, > > + NUM_JOIN_MEMBERSHIP_TYPES = 4 These seem better suited as flags -- fullmember (y/n), sendonly (y/n) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 26, 2016 at 08:57:08PM +0000, Hefty, Sean wrote: > > > +* There are 4 types of join states: > > > +* FullMember, NonMember, SendOnlyNonMember, SendOnlyFullMember. > > > +*/ > > > +enum { > > > + FULLMEMBER_JOIN, > > > + NONMEMBER_JOIN, > > > + SENDONLY_NONMEBER_JOIN, > > > + SENDONLY_FULLMEMBER_JOIN, > > > + NUM_JOIN_MEMBERSHIP_TYPES = 4 > > These seem better suited as flags -- fullmember (y/n), sendonly (y/n) Thanks, excellent point. > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/27/2016 02:42 AM, Leon Romanovsky wrote: > On Tue, Apr 26, 2016 at 08:57:08PM +0000, Hefty, Sean wrote: >>>> +* There are 4 types of join states: >>>> +* FullMember, NonMember, SendOnlyNonMember, SendOnlyFullMember. >>>> +*/ >>>> +enum { >>>> + FULLMEMBER_JOIN, >>>> + NONMEMBER_JOIN, >>>> + SENDONLY_NONMEBER_JOIN, >>>> + SENDONLY_FULLMEMBER_JOIN, >>>> + NUM_JOIN_MEMBERSHIP_TYPES = 4 >> >> These seem better suited as flags -- fullmember (y/n), sendonly (y/n) > > Thanks, excellent point. This doesn't seem to make any sense to me. Without going back and re-reading this part of the spec, as I recall, there is: UnJoined SendOnly Join Full Join You can never have a SendOnly_FullMember join. Once you are FullMember, you are no longer SendOnly. Is a NonMember (assuming here that NonMember is referring to what the CA is listed as according to the SM) even allowed to join, either as SendOnly or FullMember? I would have thought if the SM listed that CA as a NonMember, that any joins would be flat rejected and NonMember join states wouldn't make any sense.
> This doesn't seem to make any sense to me. Without going back and > re-reading this part of the spec, as I recall, there is: > > UnJoined > SendOnly Join > Full Join The 1.2.1 version of the spec has: JoinState - 4 bits Join/Leave Status requested by the port. See discussion below. bit 0: FullMember: Include/delete this endport from the multicast group as a member sender and receiver. bit 1: NonMember: Include/delete this endport from the multicast group as a non-member sender and receiver. bit 2: SendOnlyNonMember: Include/delete this endport from the multicast group as a non-member sender only. bit 3: Reserved • FullMember: Group messages are routed both to and from the port. The port is considered a member for purposes of group creation and deletion, i.e.: if no member ports with FullMember=1 remain, the group may be deleted; otherwise it may not. • NonMember: Group messages are routed both to and from the port. The port is not considered a member for purposes of group creation/deletion. • SendOnlyNonMember: Group messages are only routed from the port; none are routed to the port. The port is not considered a member for purposes of group creation/deletion. Any combination of the MCMemberRecord:JoinState bits may be 1. When multiple bits are 1, the qualities of the port's type of membership are the union of the qualities specified by the bits that are 1. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/29/2016 11:50 AM, Hefty, Sean wrote: >> This doesn't seem to make any sense to me. Without going back and >> re-reading this part of the spec, as I recall, there is: >> >> UnJoined >> SendOnly Join >> Full Join > > The 1.2.1 version of the spec has: > > JoinState - 4 bits > > Join/Leave Status requested by the port. See discussion below. > bit 0: FullMember: Include/delete this endport from the multicast group as a > member sender and receiver. > bit 1: NonMember: Include/delete this endport from the multicast group as > a non-member sender and receiver. > bit 2: SendOnlyNonMember: Include/delete this endport from the multicast > group as a non-member sender only. > bit 3: Reserved > > • FullMember: Group messages are routed both to and from the port. > The port is considered a member for purposes of group creation and > deletion, i.e.: if no member ports with FullMember=1 remain, the > group may be deleted; otherwise it may not. > • NonMember: Group messages are routed both to and from the port. > The port is not considered a member for purposes of group creation/deletion. > • SendOnlyNonMember: Group messages are only routed from the > port; none are routed to the port. The port is not considered > a member for purposes of group creation/deletion. > > Any combination of the MCMemberRecord:JoinState bits may be 1. When > multiple bits are 1, the qualities of the port's type of membership are the > union of the qualities specified by the bits that are 1. > OK, that makes more sense. Thanks for the refresher.
On 4/28/2016 10:34 PM, Doug Ledford wrote: > On 04/27/2016 02:42 AM, Leon Romanovsky wrote: >> On Tue, Apr 26, 2016 at 08:57:08PM +0000, Hefty, Sean wrote: >>>>> +* There are 4 types of join states: >>>>> +* FullMember, NonMember, SendOnlyNonMember, SendOnlyFullMember. >>>>> +*/ >>>>> +enum { >>>>> + FULLMEMBER_JOIN, >>>>> + NONMEMBER_JOIN, >>>>> + SENDONLY_NONMEBER_JOIN, >>>>> + SENDONLY_FULLMEMBER_JOIN, >>>>> + NUM_JOIN_MEMBERSHIP_TYPES = 4 >>> >>> These seem better suited as flags -- fullmember (y/n), sendonly (y/n) >> >> Thanks, excellent point. > > This doesn't seem to make any sense to me. Without going back and > re-reading this part of the spec, as I recall, there is: > > UnJoined > SendOnly Join > Full Join > > You can never have a SendOnly_FullMember join. Once you are FullMember, > you are no longer SendOnly. > > Is a NonMember (assuming here that NonMember is referring to what the CA > is listed as according to the SM) even allowed to join, either as > SendOnly or FullMember? I would have thought if the SM listed that CA > as a NonMember, that any joins would be flat rejected and NonMember join > states wouldn't make any sense. There is now a new SendOnlyFullMember option added by IBTA MgtWG post IBA 1.3. -- Hal -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 29, 2016 at 12:27:30PM -0400, Hal Rosenstock wrote: > > You can never have a SendOnly_FullMember join. Once you are FullMember, > > you are no longer SendOnly. > > > > Is a NonMember (assuming here that NonMember is referring to what the CA > > is listed as according to the SM) even allowed to join, either as > > SendOnly or FullMember? I would have thought if the SM listed that CA > > as a NonMember, that any joins would be flat rejected and NonMember join > > states wouldn't make any sense. > > There is now a new SendOnlyFullMember option added by IBTA MgtWG post > IBA 1.3. As I understand it, this is done because the SendOnly Non-Member essentially has useless reference counting semantics. FullMember allows the group to be auto-created and continue to exist even if there is only one SendOnly member. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/29/2016 1:12 PM, Jason Gunthorpe wrote: > On Fri, Apr 29, 2016 at 12:27:30PM -0400, Hal Rosenstock wrote: >>> You can never have a SendOnly_FullMember join. Once you are FullMember, >>> you are no longer SendOnly. >>> >>> Is a NonMember (assuming here that NonMember is referring to what the CA >>> is listed as according to the SM) even allowed to join, either as >>> SendOnly or FullMember? I would have thought if the SM listed that CA >>> as a NonMember, that any joins would be flat rejected and NonMember join >>> states wouldn't make any sense. >> >> There is now a new SendOnlyFullMember option added by IBTA MgtWG post >> IBA 1.3. > > As I understand it, this is done because the SendOnly Non-Member > essentially has useless reference counting semantics. FullMember > allows the group to be auto-created and continue to exist even if > there is only one SendOnly member. My understanding of this option is similar to the above. -- Hal > Jason > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 29 Apr 2016, Hal Rosenstock wrote: > On 4/29/2016 1:12 PM, Jason Gunthorpe wrote: > > On Fri, Apr 29, 2016 at 12:27:30PM -0400, Hal Rosenstock wrote: > >>> You can never have a SendOnly_FullMember join. Once you are FullMember, > >>> you are no longer SendOnly. > >>> > >>> Is a NonMember (assuming here that NonMember is referring to what the CA > >>> is listed as according to the SM) even allowed to join, either as > >>> SendOnly or FullMember? I would have thought if the SM listed that CA > >>> as a NonMember, that any joins would be flat rejected and NonMember join > >>> states wouldn't make any sense. > >> > >> There is now a new SendOnlyFullMember option added by IBTA MgtWG post > >> IBA 1.3. > > > > As I understand it, this is done because the SendOnly Non-Member > > essentially has useless reference counting semantics. FullMember > > allows the group to be auto-created and continue to exist even if > > there is only one SendOnly member. > > My understanding of this option is similar to the above. The continued existence of the multicast group is important for Ethernet gateways etc on the IB fabric. The Ethernet gateways will ignore join requests from the Ethernet side if there is no IB multicast group. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 26, 2016 at 11:57 PM, Hefty, Sean <sean.hefty@intel.com> wrote: >> > +* There are 4 types of join states: >> > +* FullMember, NonMember, SendOnlyNonMember, SendOnlyFullMember. >> > +*/ >> > +enum { >> > + FULLMEMBER_JOIN, >> > + NONMEMBER_JOIN, >> > + SENDONLY_NONMEBER_JOIN, >> > + SENDONLY_FULLMEMBER_JOIN, >> > + NUM_JOIN_MEMBERSHIP_TYPES = 4 > > These seem better suited as flags -- fullmember (y/n), sendonly (y/n) The ib_core tracks over the number of members for each join types, in order not to double join from specific port to specific join type. It keeps the numbers of members in special array, entry for each join type. So, the meaning of the enum is to define the size of the that array and not to define a bit mask. > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> The ib_core tracks over the number of members for each join types, in > order not to double join from specific port to specific join type. > It keeps the numbers of members in special array, entry for each join type. > So, the meaning of the enum is to define the size of the that array > and not to define a bit mask. They still seem better as flags and are defined as such in the spec. The number of bits can define the size of an array. Internal implementation convenience should not drive the API.
diff --git a/drivers/infiniband/core/multicast.c b/drivers/infiniband/core/multicast.c index 250937c..31d9d41 100644 --- a/drivers/infiniband/core/multicast.c +++ b/drivers/infiniband/core/multicast.c @@ -93,6 +93,18 @@ enum { struct mcast_member; +/* +* There are 4 types of join states: +* FullMember, NonMember, SendOnlyNonMember, SendOnlyFullMember. +*/ +enum { + FULLMEMBER_JOIN, + NONMEMBER_JOIN, + SENDONLY_NONMEBER_JOIN, + SENDONLY_FULLMEMBER_JOIN, + NUM_JOIN_MEMBERSHIP_TYPES = 4 +}; + struct mcast_group { struct ib_sa_mcmember_rec rec; struct rb_node node; @@ -102,7 +114,7 @@ struct mcast_group { struct list_head pending_list; struct list_head active_list; struct mcast_member *last_join; - int members[3]; + int members[NUM_JOIN_MEMBERSHIP_TYPES]; atomic_t refcount; enum mcast_group_state state; struct ib_sa_query *query; @@ -220,8 +232,9 @@ static void queue_join(struct mcast_member *member) } /* - * A multicast group has three types of members: full member, non member, and - * send only member. We need to keep track of the number of members of each + * A multicast group has four types of members: full member, non member, + * sendonly non member and sendonly full member. + * We need to keep track of the number of members of each * type based on their join state. Adjust the number of members the belong to * the specified join states. */ @@ -229,7 +242,7 @@ static void adjust_membership(struct mcast_group *group, u8 join_state, int inc) { int i; - for (i = 0; i < 3; i++, join_state >>= 1) + for (i = 0; i < NUM_JOIN_MEMBERSHIP_TYPES; i++, join_state >>= 1) if (join_state & 0x1) group->members[i] += inc; } @@ -245,7 +258,7 @@ static u8 get_leave_state(struct mcast_group *group) u8 leave_state = 0; int i; - for (i = 0; i < 3; i++) + for (i = 0; i < NUM_JOIN_MEMBERSHIP_TYPES; i++) if (!group->members[i]) leave_state |= (0x1 << i);