Skip to content

Optimized Forwarding Header#2297

Open
JaiOCP wants to merge 1 commit into
opencomputeproject:masterfrom
JaiOCP:ofh
Open

Optimized Forwarding Header#2297
JaiOCP wants to merge 1 commit into
opencomputeproject:masterfrom
JaiOCP:ofh

Conversation

@JaiOCP

@JaiOCP JaiOCP commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

New PR is opened.
Old one where most of the review comments are there is closed
#2285

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>
Comment thread inc/saihash.h
SAI_NATIVE_HASH_FIELD_RDMA_BTH_DEST_QP = 0x00000023,

/** Native hash field RDMA packet BTH destination queue pair */
SAI_NATIVE_HASH_FIELD_OFH_FLOW_LABEL = 0x00000024,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the comments cut and paste error.

Comment thread inc/saiacl.h
* @flags CREATE_ONLY
* @default false
*/
SAI_ACL_TABLE_ATTR_FIELD_ACL_OFH_TYPE = SAI_ACL_TABLE_ATTR_FIELD_START + 0x167,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add match for version field

@BYGX-wcr

Copy link
Copy Markdown
Contributor

It will be good to draw an abstract SAI pipeline to illustrate the forwarding model of OFH in the documentation.

Comment thread inc/saiacl.h
* @default false
*/
SAI_ACL_TABLE_ATTR_FIELD_OFH_COS = SAI_ACL_TABLE_ATTR_FIELD_START + 0x168,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add TTL decrement for action as well as match on TTL

Comment thread inc/saiacl.h
* @brief End of Rule Actions
*/
SAI_ACL_ENTRY_ATTR_ACTION_END = SAI_ACL_ENTRY_ATTR_ACTION_TAM_OBJECT,
SAI_ACL_ENTRY_ATTR_ACTION_END = SAI_ACL_ENTRY_ATTR_ACTION_SET_OFH_COS,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SAI_ACL_ACTION_TYPE_SET_ECN = 0x00000017,

Set ECN also supported for normal IP packet.

Comment thread inc/saiswitch.h
/**
* @brief OFH header max size
*
* @type sai_int8_t

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be sai_uint8_t ?

Comment thread inc/saitypes.h

SAI_OFH_ADDR_FAMILY_AFH,

SAI_OFH_ADDR_FAMILY_UFH,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add doxygen comments

Comment thread inc/saitypes.h

/** @validonly meta->attrvaluetype == SAI_ATTR_VALUE_TYPE_UINT64_RANGE_LIST */
sai_u64_range_list_t u64rangelist;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the corresponding sai_ofh_addr_and_mask_t to SAI_ATTR_VALUE_TYPE_OFH_ADDR_AND_MASK type be added here?

Comment thread inc/saiofh.h
/**
* @brief Set OFH attribute value(s).
*
* @param[in] ofh_id TAM id

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct TAM to OFH

Comment thread inc/saiofhroute.h
* @param[out] object_statuses List of status for every object. Caller needs to
* allocate the buffer
*
* @return #SAI_STATUS_SUCCESS on success when all objects are removed or

@rdasari-upscaleai rdasari-upscaleai Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appear to be cut and paste error from route remove entry.
sairoute.h appear to have same cut and paste error.

Comment thread inc/saiofhroute.h
* @param[out] object_statuses List of status for every object. Caller needs to
* allocate the buffer
*
* @return #SAI_STATUS_SUCCESS on success when all objects are removed or

@rdasari-upscaleai rdasari-upscaleai Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appear to be cut and paste error from route remove entry.
sairoute.h appear to have same cut and paste error.

Comment thread inc/saiofh.h
*
* @type sai_uint8_t
* @flags CREATE_AND_SET
* @default 6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while other *_WIDTH fields are default to '0', This seems to be special.

@hzheng5

hzheng5 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Hi @JaiOCP , thanks for the OFH contribution. It aligns well with UEC Scale-Up goals.

A few points before merge:
• Naming: Standardize to SAI_OFH_ATTR_* for OFH objects and SAI_ROUTING_ATTR_OFH_* for forwarding behavior.
• Capability Query: Add sai_query_ofh_capability() to expose max size, supported address families, and hash limits before config.
• Counters: Define SAI_PORT_STAT_OFH_* counters to enable silicon-level verification.
• OFH Size: Clarify units for @default 6 (bytes vs DW) and add validation range.

LGTM overall.

Comment thread inc/saidebugcounter.h
/** OFH VLAN drop */
SAI_IN_DROP_REASON_OFH_VLAN_DROP,

/** OFH VLAN to virtual router id drop */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Please explain this drop reason either as comments in the header file or in the markdown file to be created.
  2. Which of the below drops are accounted under this drop reason?
  • SAI_ROUTER_INTERFACE_ATTR_ADMIN_OFH_STATE is false
  • SAI_VIRTUAL_ROUTER_ATTR_ADMIN_OFH_STATE is false

Comment thread inc/saidebugcounter.h
SAI_IN_DROP_REASON_OFH_DISCARD,

/** OFH VLAN drop */
SAI_IN_DROP_REASON_OFH_VLAN_DROP,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, SAI_IN_DROP_REASON_OFH_VLAN_DROP is meant to account the drops when RIF of type SAI_ROUTER_INTERFACE_TYPE_VLAN is not created for the given ingress port + packet.VID. Ideally, this would be accounted under SAI_IN_DROP_REASON_INGRESS_VLAN_FILTER. We do not have IPv4/IPv6 specific VLAN drops. I am trying to understand as to why we need OFH specific VLAN drops.

Comment thread inc/saiofh.h
* @type sai_ofh_sub_type_t
* @flags CREATE_ONLY
* @default SAI_OFH_SUB_TYPE_NONE
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I presume this is to identify UFH1/2, AFH1/2 and is not valid for ESUN. Please add appropriate @validonly

Comment thread inc/saiofh.h
SAI_OFH_ATTR_SUB_TYPE,

/**
* @brief OFH Version number

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brief Version/Revision number

Comment thread inc/saiofh.h
* @type sai_ofh_ver_t
* @flags CREATE_AND_SET
* @default SAI_OFH_VER_1
* @validonly SAI_OFH_ATTR_TYPE == SAI_OFH_TYPE_AFH or SAI_OFH_ATTR_TYPE == SAI_OFH_TYPE_ESUN

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version is valid for UHF also. If the idea is to add UHF as a separate PR, then we could remove SAI_OFH_TYPE_UFH from sai_ofh_type_t.

Comment thread inc/saiqosmap.h
SAI_QOS_MAP_TYPE_QUEUE_TO_VC = 0x00000011,

/** QOS Map to set OFH COS to Traffic class */
SAI_QOS_MAP_TYPE_OFH_COS_TO_TC = 0x00000012,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to extend sai_qos_map_params_t to include a new field: sai_uint8_t ofh_cos.
Note that increases the size of the struct and needs handling in sairedis serialization/deserialization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants