[ofa-general] Re: [PATCH] opensm/include/iba/ib_types.h: Definition of Congestion Control MADs

Sasha Khapyorsky sashak at voltaire.com
Mon Jun 30 02:00:14 PDT 2008


Hi Yevgeny,

On 09:53 Mon 30 Jun     , Yevgeny Kliteynik wrote:
> @@ -7126,6 +7250,19 @@ typedef struct _ib_mad_notice_attr	// Total Size calc  Accumulated
>  			uint8_t details[54];
>  		} PACK_SUFFIX raw_data;
> 
> +		struct _ntc_0 {			// CC_Key violation [54 bytes]
> +			ib_net16_t source_lid;	// Source LID from offending packet LRH
> +			uint8_t method;		// Method, from common MAD header
> +			uint8_t resv0;
> +			ib_net16_t attr_id;	// Attribute ID, from common MAD header
> +			ib_net16_t resv1;
> +			ib_net32_t attr_mod;	// Attribute Modif, from common MAD header
> +			ib_net32_t qp;		// 8b pad, 24b dest QP from BTH

Looked at the spec (1.2.1). Should not here be:

	ib_net16_t attr_id;
	ib_net32_t attr_mode;
	ib_net32_t qp;

?

> +			ib_net64_t cc_key;	// CC key of the offending packet
> +			ib_gid_t source_gid;	// GID from GRH of the offending packet
> +			uint8_t padding[14];	// Padding - ignored on read

And
	uint8_t padding[24];
	
here?

> +		} PACK_SUFFIX ntc_0;
> +
>  		struct _ntc_64_67 {
>  			uint8_t res[6];
>  			ib_gid_t gid;	// the Node or Multicast Group that came in/out

[snip...]

> +/****s* IBA Base: Types/ib_cong_info_t
> +* NAME
> +*	ib_cong_info_t
> +*
> +* DESCRIPTION
> +*	IBA defined CongestionInfo attribute (A10.4.3.3)
> +*
> +* SYNOPSIS
> +*/
> +#include <complib/cl_packon.h>
> +typedef struct _ib_cong_info {
> +	uint8_t cong_info;
> +	uint8_t resv;

It is probably better to make it

	ib_net16_t cong_info;

, so we will not need to rework later. Bits values we can define in
network byte order.

> +	uint8_t ctrl_table_cap;
> +} PACK_SUFFIX ib_cong_info_t;
> +#include <complib/cl_packoff.h>
> +/*
> +* FIELDS
> +*	cong_info
> +*		Congestion control capabilities of the node.
> +*
> +*	ctrl_table_cap
> +*		Number of 64 entry blocks in the CongestionControlTable.
> +*
> +* SEE ALSO
> +*	ib_cc_mad_t
> +*********/

[snip...]

> +/****s* IBA Base: Types/ib_cong_log_event_sw_t
> +* NAME
> +*	ib_cong_log_event_sw_t
> +*
> +* DESCRIPTION
> +*	IBA defined CongestionLogEvent (SW) entry (A10.4.3.5)
> +*
> +* SYNOPSIS
> +*/
> +#include <complib/cl_packon.h>
> +typedef struct _ib_cong_log_event_sw {
> +	ib_net16_t slid;
> +	ib_net16_t dlid;
> +	uint8_t resv0_sl;
> +	uint8_t resv1;
> +	ib_net16_t resv2;
> +	ib_net32_t time_stamp;
> +} PACK_SUFFIX ib_cong_log_event_sw_t;
> +#include <complib/cl_packoff.h>
> +/*
> +* FIELDS
> +*	slid
> +*		Source LID of congestion event.
> +*
> +*	slid
        ^^^^

typo - 'dlid'.

> +*		Destination LID of congestion event.
> +*
> +*	sl
> +*		bits [3:0] SL of congestion event.
> +*		bits [7:4] reserved.
> +*
> +*	time_stamp
> +*		Timestamp of congestion event.
> +*
> +* SEE ALSO
> +*	ib_cc_mad_t, ib_cong_log_t
> +*********/
> +
> +/****s* IBA Base: Types/ib_cong_log_event_ca_t
> +* NAME
> +*	ib_cong_log_event_ca_t
> +*
> +* DESCRIPTION
> +*	IBA defined CongestionLogEvent (CA) entry (A10.4.3.5)
> +*
> +* SYNOPSIS
> +*/
> +#include <complib/cl_packon.h>
> +typedef struct _ib_cong_log_event_ca {
> +	ib_net32_t resv0_local_qp;
> +	ib_net32_t remote_qp_sl_service_type;
> +	ib_net16_t remote_lid;
> +	ib_net16_t resv1;
> +	ib_net32_t time_stamp;
> +} PACK_SUFFIX ib_cong_log_event_ca_t;
> +#include <complib/cl_packoff.h>

Does it implement table 514?

> +/*
> +* FIELDS
> +*	resv0_local_qp
> +*		bits [23:0] local QP that reached CN threshold.
> +*		bits [31:24] reserved.
> +*
> +*	remote_qp_sl_service_type
> +*		bits [23:0] remote QP that is connected to local QP.
> +*		bits [27:24] SL of the local QP.
> +*		bits [31:28] Service Type of the local QP.
> +*
> +*	remote_lid
> +*		LID of the remote port that is connected to local QP.
> +*
> +*	time_stamp
> +*		Timestamp when threshold reached.
> +*
> +* SEE ALSO
> +*	ib_cc_mad_t, ib_cong_log_t
> +*********/
> +
> +/****s* IBA Base: Types/ib_cong_log_t
> +* NAME
> +*	ib_cong_log_t
> +*
> +* DESCRIPTION
> +*	IBA defined CongestionLog attribute (A10.4.3.5)
> +*
> +* SYNOPSIS
> +*/
> +#include <complib/cl_packon.h>
> +typedef struct _ib_cong_log {
> +	uint8_t log_type;
> +	union _log_details
> +	{
> +		struct _log_sw {
> +			uint8_t cong_flags;
> +			ib_net16_t event_counter;
> +			ib_net32_t time_stamp;
> +			uint8_t port_map[32];
> +			ib_cong_log_event_sw_t entry_list[15];
> +		} PACK_SUFFIX log_sw;
> +
> +		struct _log_ca {
> +			uint8_t cong_flags;
> +			ib_net16_t event_counter;
> +			ib_net16_t event_map;
> +			ib_net16_t resv;
> +			ib_net32_t time_stamp;

In the spec I see:

	ib_net16_t event_map;
	ib_net16_t time_stamp;

> +			ib_cong_log_event_ca_t log_event[13];
> +		} PACK_SUFFIX log_ca;
> +
> +	} log_details;
> +} PACK_SUFFIX ib_cong_log_t;
> +#include <complib/cl_packoff.h>
> +/*
> +* FIELDS
> +*
> +*	log_{sw,ca}.log_type
> +*		Log type: 0x1 is for Switch, 0x2 is for CA
> +*
> +*	log_{sw,ca}.cong_flags
> +*		Congestion Flags.
> +*
> +*	log_{sw,ca}.event_counter
> +*		Number of events since log last sent.
> +*
> +*	log_{sw,ca}.time_stamp
> +*		Timestamp when log sent.
> +*
> +*	log_sw.port_map
> +*		If a bit set to 1, then the corresponding port
> +*		has marked packets with a FECN.
> +*		bits 0 and 255 - reserved
> +*		bits [254..1] - ports [254..1].
> +*
> +*	log_sw.entry_list
> +*		Array of 13 most recent congestion log events.
> +*
> +*	log_ca.event_map
> +*		array 16 bits, one for each SL.
> +*
> +*	log_ca.log_event
> +*		Array of 13 most recent congestion log events.
> +*
> +* SEE ALSO
> +*	ib_cc_mad_t, ib_cong_log_event_sw_t, ib_cong_log_event_ca_t
> +*********/
> +
> +/****s* IBA Base: Types/ib_sw_cong_setting_t
> +* NAME
> +*	ib_sw_cong_setting_t
> +*
> +* DESCRIPTION
> +*	IBA defined SwitchCongestionSetting attribute (A10.4.3.6)
> +*
> +* SYNOPSIS
> +*/
> +#include <complib/cl_packon.h>
> +typedef struct _ib_sw_cong_setting {
> +	ib_net32_t control_map;
> +	uint8_t victim_mask[32];
> +	uint8_t credit_mask[32];
> +	uint8_t threshold;
> +	uint8_t packet_size;
> +	uint8_t cs_threshold;
> +	uint8_t resv0;

I don't see this resv0 byte in the spec.

> +	ib_net16_t cs_return_delay;
> +	ib_net16_t marking_rate;
> +} PACK_SUFFIX ib_sw_cong_setting_t;
> +#include <complib/cl_packoff.h>
> +/*
> +* FIELDS
> +*
> +*	control_map
> +*		Indicates which components of this attribute are valid
> +*
> +*	victim_mask
> +*		If the bit set to 1, then the port corresponding to
> +*		that bit shall mark packets that encounter congestion
> +*		with a FECN, whether they are the source or victim
> +*		of congestion. (See A10.2.1.1.1)
> +*		  bit 0: port 0 (enhanced port 0 only)
> +*		  bits [254..1]: ports [254..1]
> +*		  bit 255: reserved
> +*
> +*	credit_mask
> +*		If the bit set to 1, then the port corresponding
> +*		to that bit shall apply Credit Starvation.
> +*		  bit 0: port 0 (enhanced port 0 only)
> +*		  bits [254..1]: ports [254..1]
> +*		  bit 255: reserved
> +*
> +*	threshold
> +*		bits [3..0] Indicates how agressive cong. marking should be
> +*		bits [7..4] Reserved
> +*
> +*	packet_size
> +*		Any packet less than this size won't be marked with FECN
> +*
> +*	cs_threshold
> +*		bits [3..0] How agressive Credit Starvation should be
> +*		bits [7..4] Reserved
> +*
> +*	cs_return_delay
> +*		Value that controls credit return rate.
> +*
> +*	marking_rate
> +*		The value that provides the mean number of packets
> +*		between marking eligible packets with FECN.
> +*
> +* SEE ALSO
> +*	ib_cc_mad_t
> +*********/
> +
> +/****s* IBA Base: Types/ib_sw_port_cong_setting_element_t
> +* NAME
> +*	ib_sw_port_cong_setting_element_t
> +*
> +* DESCRIPTION
> +*	IBA defined SwitchPortCongestionSettingElement (A10.4.3.7)
> +*
> +* SYNOPSIS
> +*/
> +#include <complib/cl_packon.h>
> +typedef struct _ib_sw_port_cong_setting_element {
> +	uint8_t valid_ctrl_type_res_threshold;
> +	uint8_t packet_size;
> +	ib_net16_t cong_param;

packet_size and cong_param is a same 16-bit field in the spec.

> +} PACK_SUFFIX ib_sw_port_cong_setting_element_t;
> +#include <complib/cl_packoff.h>
> +/*
> +* FIELDS
> +*
> +*	valid_ctrl_type_res_threshold
> +*		bit 0: "Valid"
> +*			when set to 1, indicates this switch
> +*			port congestion setting element is valid.
> +*		bit 1: "Control Type"
> +*			Indicates which type of attribute is being set:
> +*			0b = Congestion Control parameters are being set.
> +*			1b = Credit Starvation parameters are being set.
> +*		bits [3..2]: reserved
> +*		bits [7..4]: "Threshold"
> +*			When Control Type is 0, contains the congestion
> +*			threshold value (Threshold) for this port.
> +*			When Control Type is 1, contains the credit
> +*			starvation threshold (CS_Threshold) value for
> +*			this port.
> +*
> +*	packet_size
> +*		When Control Type is 0, this field contains the minimum
> +*		size of packets that may be marked with a FECN.
> +*		When Control Type is 1, this field is reserved.
> +*
> +*	cong_parm
> +*		When Control Type is 0, this field contains the port
> +*		marking_rate.
> +*		When Control Type is 1, this field is reserved.
> +*
> +* SEE ALSO
> +*	ib_cc_mad_t, ib_sw_port_cong_setting_t
> +*********/
> +
> +/****d* IBA Base: Types/ib_sw_port_cong_setting_block_t
> +* NAME
> +*	ib_sw_port_cong_setting_block_t
> +*
> +* DESCRIPTION
> +*	Defines the SwitchPortCongestionSetting Block (A10.4.3.7).
> +*
> +* SOURCE
> +*/
> +typedef ib_sw_port_cong_setting_element_t ib_sw_port_cong_setting_block_t[32];

In the spec I see that it should be 64 24-bit elements here.

Are we using the same spec?

Sasha



More information about the general mailing list