Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a RIF attribute to specify if the corresponding My MAC entry should not be created.. #2021

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions inc/sairouterinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,19 @@ typedef enum _sai_router_interface_attr_t
*/
SAI_ROUTER_INTERFACE_ATTR_SELECTIVE_COUNTER_LIST,

/**
* @brief Attribute used to specify external My MAC entry that will
* be used in place of any implicit entry created during RIF processing
* for this {port, vlan, MAC address}
*
* @type sai_object_id_t
* @flags CREATE_ONLY
* @objects SAI_OBJECT_TYPE_MY_MAC
* @allownull true
* @default SAI_NULL_OBJECT_ID
*/
SAI_ROUTER_INTERFACE_ATTR_MY_MAC,

Copy link
Contributor

Choose a reason for hiding this comment

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

@erohsik  

  1. If SAI_ROUTER_INTERFACE_ATTR_MY_MAC is not null, does it mean that the device must not create a termination entry with MAC == SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS?
  2. What is the meaning of binding a RIF to MY_MAC entry? Does it mean that the MY_MAC entry takes effect only for packets that ingress on this RIF? I am trying to see how it relates to SAI_MY_MAC_ATTR_PORT/VLAN_ID
  3. And lastly, why not achieve this by having a SAI_ROUTER_INTERFACE_ATTR_NO_TERM_MAC of type boolean to achieve the goal listed in the heading.

Copy link
Contributor Author

@erohsik erohsik Nov 25, 2024

Choose a reason for hiding this comment

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

  1. Yes, the device will skip creating a termination entry with the RIF SMAC
    2, 3. A bool attribute was in the original change, but during the PR review, a suggestion was mad to add the corresponding MY_MAC entry so that the MY_MAC entry does not get deleted inadvertently while the RIF is still relying on it for L3 forwarding determination.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. I prefer your original model over the suggestion. I think all NOS would anyway maintain this reference counting;  we can avoid adding unnecessary state in the SAI implementation.

 Further, the suggested model gives a impression that the pointed to MyMAC entry should only be used by the RIFs that point to this MyMAC entry and vice versa. Today, one RIF can be terminated by more than one MyMAC entries. One MyMAC entry (if we skip SAI_MY_MAC_ATTR_PORT/VLAN_ID) can terminate multiple RIFs.

@JaiOCP @itaibaz Can you please give your feedback on the above. 

Today, SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS is create-and-set, however SAI_ROUTER_INTERFACE_ATTR_MY_MAC is create-only. Any reasons?

/**
* @brief End of attributes
*/
Expand Down
Loading