[ofa-general] Re: [PATCH 1/4] multicast multiplexing - definitions for new data types, and infrastructure functions
Sasha Khapyorsky
sashak at voltaire.com
Sun Jul 19 08:14:23 PDT 2009
Hi Slava,
On 13:36 Sun 28 Jun , Slava Strebkov wrote:
> [PATCH 1/4] Patch implements multicast multiplexing as proposed in the
> thread entitled "IPv6 and IPoIB scalability issue".
> This first patch contains definitions for new data types
> and infrastructure functions.
> Signed-off-by: Slava Strebkov <slavas at voltaire.com>
It looks for me that this patch defines random non-used yet structures
and functions from the follow stuff, it makes it really hard to review
without seeing what purpose of this.
I would suggest to prepare this patch series in functional order - one
thing per patch. For example using fleximap for mgid resolution can be
completely isolated, and mgid to mlid compression algorithm (which still
be discussable) of course too.
I will comment some technical things over patches.
>
> ---
> opensm/include/opensm/osm_multicast.h | 94 +++++++++++++++++++++++++++++++--
> opensm/opensm/osm_multicast.c | 82 ++++++++++++++++++++++++++++-
> 2 files changed, 170 insertions(+), 6 deletions(-)
>
> diff --git a/opensm/include/opensm/osm_multicast.h b/opensm/include/opensm/osm_multicast.h
> index a871306..02b63bd 100644
> --- a/opensm/include/opensm/osm_multicast.h
> +++ b/opensm/include/opensm/osm_multicast.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2004-2008 Voltaire, Inc. All rights reserved.
> + * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
> * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
> * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
> *
> @@ -45,6 +45,7 @@
>
> #include <iba/ib_types.h>
> #include <complib/cl_qmap.h>
> +#include <complib/cl_fleximap.h>
> #include <complib/cl_qlist.h>
> #include <complib/cl_spinlock.h>
> #include <opensm/osm_base.h>
> @@ -93,7 +94,7 @@ BEGIN_C_DECLS
> *
> * SYNOPSIS
> */
> -typedef struct osm_mcast_mgr_ctxt {
> + typedef struct osm_mcast_mgr_ctxt {
In please don't mix formatting and functional changes in one patch. In
particular this change looks like indent bug :)
> cl_list_item_t list_item;
> ib_net16_t mlid;
> } osm_mcast_mgr_ctxt_t;
> @@ -107,6 +108,89 @@ typedef struct osm_mcast_mgr_ctxt {
> * SEE ALSO
> *********/
>
> +/****s* OpenSM: Multicast Group Holder/osm_mgrp_holder_t
> +* NAME
> +* osm_mgrp_holder_t
> +*
> +* DESCRIPTION
> +* Holder for mgroups.
> +*
> +* The osm_mgrp_t object should be treated as opaque and should
> +* be manipulated only through the provided functions.
> +*
> +* SYNOPSIS
> +*/
> +
> +typedef struct osm_mgrp_holder {
> + cl_fmap_t mgrp_map;
> + cl_qmap_t mgrp_port_map;
> + ib_gid_t common_mgid;
> + osm_mtree_node_t *p_root;
> + boolean_t to_be_deleted;
> + uint32_t last_tree_id;
> + uint32_t last_change_id;
> + ib_net16_t mlid;
> +} osm_mgrp_holder_t;
> +
> +/*
> +* FIELDS
> +* mgrp_map
> +* Map for mgroups. Must be first element!!
> +*
> +* mgrp_port_map
> +* Map of all ports joined same mlid
> +*
> +* common_mgid
> +* mgid of mgroup, ANDed with bitmask.
> +* mgid of each mgroup in mgrp_map, ANDed with bitmask,
> +* see osm_mgrp_holder_prepare_common_mgid
Bad formatted comment.
> +*
> +* p_root
> +* Pointer to the root "tree node" in the single spanning tree
> +* for this multicast group holder. The nodes of the tree represent
> +* switches. Member ports are not represented in the tree.
> +*
> +* to_be_deleted
> +* Since holders are deleted when there are no mgroups in.
> +*
> +* last_change_id
> +* a counter for the number of changes applied to the group in this holder.
80 characters per line please.
> +* This counter shuold be incremented on any modification
> +* to the group: joining or leaving of ports.
> +*
> +* last_tree_id
> +* the last change id used for building the current tree.
> +*
> +* mlid
> +* mlid of current group holder
> +*/
> + /****s* OpenSM: Multicast group Port /osm_mgrp_port _t
> +* NAME
> +* osm_mgrp_port _t
> +*
> +* DESCRIPTION
> +* Holder for pointers to mgroups and port guid.
> +*
> +*
> +* SYNOPSIS
> +*/
> +typedef struct _osm_mgrp_port {
> + cl_map_item_t guid_item;
> + cl_qlist_t mgroups;
> + ib_net64_t port_guid;
> +} osm_mgrp_port_t;
> +/*
> +* FIELDS
> +* guid_item
> +* Map for ports. Must be first element
> +*
> +* mgroups
> +* Map for mgroups opened by this port.
> +*
> +* portguid
> +* guid of port representing current structure
> +*/
> +
> /****s* OpenSM: Multicast Group/osm_mgrp_t
> * NAME
> * osm_mgrp_t
> @@ -355,7 +439,7 @@ static inline ib_net16_t osm_mgrp_get_mlid(IN const osm_mgrp_t * const p_mgrp)
> *
> * SYNOPSIS
> */
> -osm_mcm_port_t *osm_mgrp_add_port(osm_subn_t *subn, osm_log_t *log,
> +osm_mcm_port_t *osm_mgrp_add_port(osm_subn_t * subn, osm_log_t * log,
> IN osm_mgrp_t * const p_mgrp,
> IN const ib_gid_t * const p_port_gid,
> IN const uint8_t join_state,
> @@ -452,8 +536,8 @@ osm_mgrp_delete_port(IN osm_subn_t * const p_subn,
> * SEE ALSO
> *********/
>
> -int osm_mgrp_remove_port(osm_subn_t *subn, osm_log_t *log, osm_mgrp_t *mgrp,
> - osm_mcm_port_t *mcm, uint8_t join_state);
> +int osm_mgrp_remove_port(osm_subn_t * subn, osm_log_t * log, osm_mgrp_t * mgrp,
> + osm_mcm_port_t * mcm, uint8_t join_state);
>
> /****f* OpenSM: Multicast Group/osm_mgrp_apply_func
> * NAME
> diff --git a/opensm/opensm/osm_multicast.c b/opensm/opensm/osm_multicast.c
> index d2733c4..ae0a818 100644
> --- a/opensm/opensm/osm_multicast.c
> +++ b/opensm/opensm/osm_multicast.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2004-2008 Voltaire, Inc. All rights reserved.
> + * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
> * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
> * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
> *
> @@ -48,6 +48,7 @@
> #include <opensm/osm_mcm_port.h>
> #include <opensm/osm_mtree.h>
> #include <opensm/osm_inform.h>
> +#include <arpa/inet.h>
>
> /**********************************************************************
> **********************************************************************/
> @@ -298,3 +299,82 @@ void osm_mgrp_apply_func(const osm_mgrp_t * p_mgrp, osm_mgrp_func_t p_func,
> if (p_mtn)
> mgrp_apply_func_sub(p_mgrp, p_mtn, p_func, context);
> }
> +
> +/**********************************************************************
> + **********************************************************************/
> +#define PREFIX_MASK_IP CL_HTON64(0xff10ffff0000ffffULL)
> +#define PREFIX_SIGNATURE_IPV4 CL_HTON64(0xff10401b00000000ULL)
> +#define INTERFACE_ID_IPV4 CL_HTON64(0x0000000fffffffffULL)
> +#define PREFIX_SIGNATURE_IPV6 CL_HTON64(0xff10601b00000000ULL)
> +#define INTERFACE_ID_ALL_NODES CL_HTON64(0x00000000ffffffffULL)
> +#define INTERFACE_ID_ALL_HOSTS CL_HTON64(0x0000000000000001ULL)
> +#define INTERFACE_ID_ALL_GATEWAYS CL_HTON64(0x000000000000004dULL)
> +#define INTERFACE_ID_ALL_ROUTERS CL_HTON64(0x0000000000000002ULL)
> +#define PREFIX_PKEY_MASK_OFF CL_HTON64(0xffffffff0000ffffULL)
> +#define PREFIX_MASK CL_HTON64(0xff10ffff0000ffffULL)
> +#define PREFIX_SIGNATURE CL_HTON64(0xff10601b00000000ULL)
> +#define INT_ID_MASK CL_HTON64(0xfffffff1ff000000ULL)
> +#define INT_ID_SIGNATURE CL_HTON64(0x00000001ff000000ULL)
Please use spaces for offsetting (and tabs for indentation).
Sasha
More information about the general
mailing list