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

SRv6 support #141

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

SRv6 support #141

wants to merge 5 commits into from

Conversation

dypet
Copy link
Contributor

@dypet dypet commented Dec 3, 2024

Overview:

These changes add support for SRv6 in sonic-vpp platform. Supported mysid behaviors: uN, uA, DT6, DT4. SRv6 policy and steering also supported.

Changes:

Created SwitchStateBaseSRv6.cpp to handle SRv6 SAI calls, and added the relevant functions to SaiVppXlate.c to translate into VPP API calls. Calls to SwitchStateBaseSRv6.cpp functions added to SwitchStateBase.cpp and SwitchStateBaseRoute.cpp for when SRv6 objects are received.

if (isLoopback == false && is_ip_nbr_active() == true)
{
attr.id = SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID;
CHECK_STATUS(get(SAI_OBJECT_TYPE_ROUTE_ENTRY, serializedObjectId, 1, &attr));
Copy link
Contributor

Choose a reason for hiding this comment

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

srv6 route can't be over loopback interface so it is safe to move it to the if clause, where it is already getting route_entry object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

nh_oid = attr.value.oid;

attr.id = SAI_NEXT_HOP_ATTR_TYPE;
CHECK_STATUS(get(SAI_OBJECT_TYPE_NEXT_HOP, nh_oid, 1, &attr));
Copy link
Contributor

Choose a reason for hiding this comment

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

nexthop can be null and we don't want it to error out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


using namespace saivpp;

sai_status_t TunnelManager::fill_next_hop(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it is an implementation of TunnelManager, maybe rename it to TunnelManagerSRv6? You can even create a new class if there is no commonality with existing code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return status;
}

sai_status_t TunnelManager::add_sidlist(
Copy link
Contributor

Choose a reason for hiding this comment

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

add_sidlist and create_sidlist (same for del_sidlist and remove_sidlist) are too similar. It is easy to confuse between them. Maybe create_sidlist_internal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

std::string hwif_name;

attr.id = SAI_NEXT_HOP_ATTR_TYPE;
CHECK_STATUS(m_switch_db->get(SAI_OBJECT_TYPE_NEXT_HOP, next_hop_oid, 1, &attr));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change to get_linked_object from SaiObjectDB. You can follow

auto tunnel_obj = tunnel_nh_obj->get_linked_object(SAI_OBJECT_TYPE_TUNNEL, SAI_NEXT_HOP_ATTR_TUNNEL_ID);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return SAI_STATUS_FAILURE;
}
attr.id = SAI_NEXT_HOP_ATTR_IP;
if(m_switch_db->get(SAI_OBJECT_TYPE_NEXT_HOP, next_hop_oid, 1, &attr) == SAI_STATUS_SUCCESS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use get_mandatory_attr from SaiObject

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated


S (mp);

W (ret);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use WR. W can return earlier if there is unsolicited event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


S (mp);

W (ret);
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to WR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -507,6 +528,25 @@ static bool vpp_to_vl_api_ip_addr(vl_api_address_t *vpp_addr, vpp_ip_addr_t *ipa
return ret;
}

static bool vpp_to_vl_api_ip6_address(vl_api_ip6_address_t *vpp_addr, vpp_ip_addr_t *ipaddr)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is already vpp_to_vl_api_ip_addr. The only difference it supports IPv4 address. But there is too much overlap between them. You can call that function then check if it is ipv6. Too many specialized functions add maintenance cost

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added due to different VPP API datatype; vl_api_ip6_address_t compared to vl_api_address_t. Can use vpp_to_vl_api_ip_addr but may require an additional copy

@@ -3197,3 +3296,252 @@ static int vpp_bfd_udp_enable_multihop ()

return ret;
}

static u8 translate_behavior(u32 behavior)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the name is a little bit too generic. Maybe translate_sr_behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@mssonicbld
Copy link

/azp run

Copy link

Pull request contains merge conflicts.

@dypet dypet requested a review from yue-fred-gao January 2, 2025 15:19
@mssonicbld
Copy link

/azp run

Copy link

No pipelines are associated with this pull request.

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.

3 participants