Skip to content

[vnetorch]: Set default VxLAN encap TTL value #980

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.

Sign up for GitHub

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

Merged
liat-grozovik merged 1 commit into from
Jul 22, 2019
Merged

[vnetorch]: Set default VxLAN encap TTL value #980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions orchagent/vnetorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,9 +465,10 @@ VnetBridgeInfo VNetBitmapObject::getBridgeInfoByVni(uint32_t vni, string tunnelN
vector<sai_attribute_t> bpt_attrs;
auto* vxlan_orch = gDirectory.get<VxlanTunnelOrch*>();
auto *tunnel = vxlan_orch->getVxlanTunnel(tunnelName);

if (!tunnel->isActive())
{
tunnel->createTunnel(MAP_T::BRIDGE_TO_VNI, MAP_T::VNI_TO_BRIDGE);
tunnel->createTunnel(MAP_T::BRIDGE_TO_VNI, MAP_T::VNI_TO_BRIDGE, VXLAN_ENCAP_TTL);
}

attr.id = SAI_BRIDGE_PORT_ATTR_TYPE;
Expand Down Expand Up @@ -1448,7 +1449,7 @@ bool VNetOrch::addOperation(const Request& request)

VNetVrfObject *vrf_obj = dynamic_cast<VNetVrfObject*>(obj.get());
if (!vxlan_orch->createVxlanTunnelMap(tunnel, TUNNEL_MAP_T_VIRTUAL_ROUTER, vni,
vrf_obj->getEncapMapId(), vrf_obj->getDecapMapId()))
vrf_obj->getEncapMapId(), vrf_obj->getDecapMapId(), VXLAN_ENCAP_TTL))
{
SWSS_LOG_ERROR("VNET '%s', tunnel '%s', map create failed",
vnet_name.c_str(), tunnel.c_str());
Expand Down
1 change: 1 addition & 0 deletions orchagent/vnetorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#define VNET_BITMAP_SIZE 32
#define VNET_TUNNEL_SIZE 512
#define VNET_NEIGHBOR_MAX 0xffff
#define VXLAN_ENCAP_TTL 128

extern sai_object_id_t gVirtualRouterId;

Expand Down
24 changes: 18 additions & 6 deletions orchagent/vxlanorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ create_tunnel(
sai_object_id_t tunnel_encap_id,
sai_object_id_t tunnel_decap_id,
sai_ip_address_t *src_ip,
sai_object_id_t underlay_rif)
sai_object_id_t underlay_rif,
sai_uint8_t encap_ttl=0)
Copy link
Contributor

@stcheng stcheng Jul 19, 2019

Choose a reason for hiding this comment

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

no need to specify the default value here, as already specified in the header file

Copy link
Collaborator Author

@volodymyrsamotiy volodymyrsamotiy Jul 19, 2019

Choose a reason for hiding this comment

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

This is static helper function to create tunnel in SAI, so we need default value here (in header file default value specified for "create tunnel" public APIs)

{
sai_attribute_t attr;
std::vector<sai_attribute_t> tunnel_attrs;
Expand Down Expand Up @@ -264,6 +265,17 @@ create_tunnel(
tunnel_attrs.push_back(attr);
}

if (encap_ttl != 0)
Copy link
Contributor

@stcheng stcheng Jul 19, 2019

Choose a reason for hiding this comment

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

since you're not exposing any places for user to change the encap_ttl value, why do you need to expose the capability of setting it via different functions while providing all the functions with default value? could you directly set the TTL mode to pipe model and the value to the default value?

Copy link
Collaborator

@prsunny prsunny Jul 19, 2019

Choose a reason for hiding this comment

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

@stcheng , the VxlanTunnel is created for different scenarios. For the VNET use-case, we would want the Tunnel to have default 128 TTL. So I think it is better to pass this as parameter.

{
attr.id = SAI_TUNNEL_ATTR_ENCAP_TTL_MODE;
attr.value.s32 = SAI_TUNNEL_TTL_MODE_PIPE_MODEL;
tunnel_attrs.push_back(attr);

attr.id = SAI_TUNNEL_ATTR_ENCAP_TTL_VAL;
attr.value.u8 = encap_ttl;
tunnel_attrs.push_back(attr);
}

sai_object_id_t tunnel_id;
sai_status_t status = sai_tunnel_api->create_tunnel(
&tunnel_id,
Expand Down Expand Up @@ -358,7 +370,7 @@ remove_tunnel_termination(sai_object_id_t term_table_id)
}
}

bool VxlanTunnel::createTunnel(MAP_T encap, MAP_T decap)
bool VxlanTunnel::createTunnel(MAP_T encap, MAP_T decap, uint8_t encap_ttl)
{
try
{
Expand All @@ -378,7 +390,7 @@ bool VxlanTunnel::createTunnel(MAP_T encap, MAP_T decap)
ip = &ips;
}

ids_.tunnel_id = create_tunnel(ids_.tunnel_encap_id, ids_.tunnel_decap_id, ip, gUnderlayIfId);
ids_.tunnel_id = create_tunnel(ids_.tunnel_encap_id, ids_.tunnel_decap_id, ip, gUnderlayIfId, encap_ttl);

ip = nullptr;
if (!dst_ip_.isZero())
Expand Down Expand Up @@ -562,7 +574,7 @@ VxlanTunnelOrch::removeNextHopTunnel(string tunnelName, IpAddress& ipAddr, MacAd
}

bool VxlanTunnelOrch::createVxlanTunnelMap(string tunnelName, tunnel_map_type_t map, uint32_t vni,
sai_object_id_t encap, sai_object_id_t decap)
sai_object_id_t encap, sai_object_id_t decap, uint8_t encap_ttl)
{
SWSS_LOG_ENTER();

Expand All @@ -578,11 +590,11 @@ bool VxlanTunnelOrch::createVxlanTunnelMap(string tunnelName, tunnel_map_type_t
{
if (map == TUNNEL_MAP_T_VIRTUAL_ROUTER)
{
tunnel_obj->createTunnel(MAP_T::VRID_TO_VNI, MAP_T::VNI_TO_VRID);
tunnel_obj->createTunnel(MAP_T::VRID_TO_VNI, MAP_T::VNI_TO_VRID, encap_ttl);
}
else if (map == TUNNEL_MAP_T_BRIDGE)
{
tunnel_obj->createTunnel(MAP_T::BRIDGE_TO_VNI, MAP_T::VNI_TO_BRIDGE);
tunnel_obj->createTunnel(MAP_T::BRIDGE_TO_VNI, MAP_T::VNI_TO_BRIDGE, encap_ttl);
}
}

Expand Down
4 changes: 2 additions & 2 deletions orchagent/vxlanorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class VxlanTunnel
return active_;
}

bool createTunnel(MAP_T encap, MAP_T decap);
bool createTunnel(MAP_T encap, MAP_T decap, uint8_t encap_ttl=0);
Copy link
Contributor

@stcheng stcheng Jul 19, 2019

Choose a reason for hiding this comment

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

why do you set the default argument with value 0 instead of VXLAN_ENCAP_TTL?

Copy link
Collaborator Author

@volodymyrsamotiy volodymyrsamotiy Jul 19, 2019

Choose a reason for hiding this comment

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

As Prince mentioned in another comment, VxLAN tunnel is created for different scenarios and for the VNET use-case we have default TTL set to 128 but for other use-cases all should work as before.

sai_object_id_t addEncapMapperEntry(sai_object_id_t obj, uint32_t vni);
sai_object_id_t addDecapMapperEntry(sai_object_id_t obj, uint32_t vni);

Expand Down Expand Up @@ -172,7 +172,7 @@ class VxlanTunnelOrch : public Orch2
}

bool createVxlanTunnelMap(string tunnelName, tunnel_map_type_t mapType, uint32_t vni,
sai_object_id_t encap, sai_object_id_t decap);
sai_object_id_t encap, sai_object_id_t decap, uint8_t encap_ttl=0);

bool removeVxlanTunnelMap(string tunnelName, uint32_t vni);

Expand Down