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

sonic-sairedis: Add support to sonic-sairedis for gearbox phys #624

Merged
merged 4 commits into from
Jun 26, 2020
Merged
Show file tree
Hide file tree
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
8 changes: 4 additions & 4 deletions lib/src/context_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
{
"guid" : 1,
"name" : "syncd1",
"dbAsic" : "ASIC_DB2",
"dbCounters" : "COUNTERS_DB2",
"dbFlex": "FLEX_COUNTER_DB2",
"dbState" : "STATE_DB2",
"dbAsic" : "GB_ASIC_DB",
"dbCounters" : "GB_COUNTERS_DB",
"dbFlex": "GB_FLEX_COUNTER_DB",
"dbState" : "STATE_DB",
"switches": [
{
"index" : 0,
Expand Down
24 changes: 24 additions & 0 deletions lib/src/sai_redis_switch.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,29 @@
#include "sai_redis.h"

sai_status_t redis_switch_mdio_read(
_In_ sai_object_id_t switch_id,
_In_ uint32_t device_addr,
_In_ uint32_t start_reg_addr,
_In_ uint32_t number_of_registers,
_Out_ uint32_t *reg_val)
{
SWSS_LOG_ENTER();

return SAI_STATUS_NOT_IMPLEMENTED;
}

sai_status_t redis_switch_mdio_write(
_In_ sai_object_id_t switch_id,
_In_ uint32_t device_addr,
_In_ uint32_t start_reg_addr,
_In_ uint32_t number_of_registers,
_In_ const uint32_t *reg_val)
{
SWSS_LOG_ENTER();

return SAI_STATUS_NOT_IMPLEMENTED;
}

REDIS_GENERIC_QUAD(SWITCH,switch);
REDIS_GENERIC_STATS(SWITCH,switch);

Expand Down
32 changes: 29 additions & 3 deletions syncd/SaiSwitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,19 @@ SaiSwitch::SaiSwitch(

helperInternalOids();

helperCheckLaneMap();
if (getSwitchType() == SAI_SWITCH_TYPE_NPU)
{
helperCheckLaneMap();
}

helperLoadColdVids();

helperPopulateWarmBootVids();

saiGetMacAddress(m_default_mac_address);
if (getSwitchType() == SAI_SWITCH_TYPE_NPU)
{
saiGetMacAddress(m_default_mac_address);
}

if (warmBoot)
{
Expand Down Expand Up @@ -131,6 +137,26 @@ void SaiSwitch::getDefaultMacAddress(
memcpy(mac, m_default_mac_address, sizeof(sai_mac_t));
}

sai_switch_type_t SaiSwitch::getSwitchType() const
{
SWSS_LOG_ENTER();

sai_attribute_t attr;

attr.id = SAI_SWITCH_ATTR_TYPE;

sai_status_t status = m_vendorSai->get(SAI_OBJECT_TYPE_SWITCH, m_switch_rid, 1, &attr);

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_THROW("failed to get switch type");
}

SWSS_LOG_ERROR("switch type: '%s'", (attr.value.s32 == SAI_SWITCH_TYPE_NPU ? "SAI_SWITCH_TYPE_NPU" : "SAI_SWITCH_TYPE_PHY"));

return (sai_switch_type_t) attr.value.s32;
}

#define MAX_HARDWARE_INFO_LENGTH 0x1000

std::string SaiSwitch::saiGetHardwareInfo() const
Expand Down Expand Up @@ -885,7 +911,7 @@ void SaiSwitch::helperPopulateWarmBootVids()
{
sai_object_id_t vid = m_translator->translateRidToVid(rid, m_switch_vid);

m_warmBootDiscoveredVids.insert(vid);
m_warmBootDiscoveredVids.insert(vid);
}
}

Expand Down
1 change: 1 addition & 0 deletions syncd/SaiSwitch.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ namespace syncd
sai_object_id_t getVid() const;
sai_object_id_t getRid() const;

sai_switch_type_t getSwitchType() const;
std::string getHardwareInfo() const;

std::unordered_map<sai_object_id_t, sai_object_id_t> getVidToRidMap() const;
Expand Down
7 changes: 7 additions & 0 deletions syncd/scripts/physyncd_start.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env bash
#
# Script to start syncd using supervisord
#

exec "/usr/bin/physyncd_startup.py"

48 changes: 48 additions & 0 deletions syncd/scripts/physyncd_startup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#!/usr/bin/python

'''
Copyright 2019 Broadcom. The term "Broadcom" refers to Broadcom Inc.
and/or its subsidiaries.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
'''
import os
import json
import subprocess


def physyncd_enable(gearbox_config):
i = 1
for phy in gearbox_config['phys']:
subprocess.Popen(["/bin/bash", "-c", "/bin/bash -c {}".format('"exec /usr/bin/syncd -p /etc/sai.d/pai.profile -x /usr/share/sonic/hwsku/context_config.json -g {}"'.format(i))], close_fds=True)
i += 1

def main():

# Only privileged users can execute this command
if os.geteuid() != 0:
sys.exit("Root privileges required for this operation")

""" Loads json file """
try:
with open('/usr/share/sonic/hwsku/gearbox_config.json') as file_object:
sydlogan marked this conversation as resolved.
Show resolved Hide resolved
gearbox_config=json.load(file_object)
except:
sys.exit("No external PHY / gearbox supported on this platform, existing physycd application")

physyncd_enable(gearbox_config)


if __name__== "__main__":
main()

File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ if SAITHRIFT
vssyncd_LDADD += -lrpcserver -lthrift
endif

TESTS = aspellcheck.pl conflictnames.pl swsslogentercheck.sh brcm.pl mlnx.pl
TESTS = aspellcheck.pl conflictnames.pl swsslogentercheck.sh BCM56850.pl MLNX2700.pl
104 changes: 104 additions & 0 deletions vslib/inc/SwitchBCM81724.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*

Copyright 2019 Broadcom. The term Broadcom refers to Broadcom Inc. and/or
its subsidiaries.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

*/

#pragma once

#include "SwitchStateBase.h"

namespace saivs
{
class SwitchBCM81724:
public SwitchStateBase
{
public:

SwitchBCM81724(
_In_ sai_object_id_t switch_id,
_In_ std::shared_ptr<RealObjectIdManager> manager,
_In_ std::shared_ptr<SwitchConfig> config);

SwitchBCM81724(
_In_ sai_object_id_t switch_id,
_In_ std::shared_ptr<RealObjectIdManager> manager,
_In_ std::shared_ptr<SwitchConfig> config,
_In_ std::shared_ptr<WarmBootState> warmBootState);

virtual ~SwitchBCM81724();

protected:

virtual sai_status_t create_port_dependencies(_In_ sai_object_id_t port_id) override;

virtual sai_status_t create_qos_queues_per_port(_In_ sai_object_id_t port_id) override;

virtual sai_status_t create_qos_queues() override;

virtual sai_status_t set_switch_mac_address() override;

virtual sai_status_t create_default_vlan() override;

virtual sai_status_t create_default_1q_bridge() override;

virtual sai_status_t create_default_virtual_router() override;

virtual sai_status_t create_default_stp_instance() override;

virtual sai_status_t create_default_trap_group() override;

virtual sai_status_t create_ingress_priority_groups_per_port(
_In_ sai_object_id_t port_id) override;

virtual sai_status_t create_ingress_priority_groups() override;

virtual sai_status_t create_vlan_members() override;

virtual sai_status_t create_bridge_ports() override;

virtual sai_status_t set_acl_entry_min_prio() override;

virtual sai_status_t set_acl_capabilities() override;

virtual sai_status_t set_maximum_number_of_childs_per_scheduler_group() override;

virtual sai_status_t set_number_of_ecmp_groups() override;

virtual sai_status_t create_cpu_port();

virtual sai_status_t create_ports();

protected : // refresh

virtual sai_status_t refresh_port_list(
_In_ const sai_attr_metadata_t *meta) override;

virtual sai_status_t refresh_bridge_port_list(
_In_ const sai_attr_metadata_t *meta,
_In_ sai_object_id_t bridge_id) override;

virtual sai_status_t refresh_vlan_member_list(
_In_ const sai_attr_metadata_t *meta,
_In_ sai_object_id_t vlan_id) override;

protected:

virtual sai_status_t refresh_read_only( _In_ const sai_attr_metadata_t *meta, _In_ sai_object_id_t object_id) override;
virtual sai_status_t set_switch_default_attributes();
virtual sai_status_t initialize_default_objects() override;
};
}
12 changes: 12 additions & 0 deletions vslib/inc/SwitchConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
#include <string>
#include <memory>

extern "C" {
#include "sai.h" // for sai_switch_type_t
}

namespace saivs
{
typedef enum _sai_vs_switch_type_t
Expand All @@ -14,6 +18,8 @@ namespace saivs

SAI_VS_SWITCH_TYPE_BCM56850,

SAI_VS_SWITCH_TYPE_BCM81724,

SAI_VS_SWITCH_TYPE_MLNX2700,

} sai_vs_switch_type_t;
Expand All @@ -38,6 +44,10 @@ namespace saivs

public:

static bool parseSaiSwitchType(
_In_ const char* saiSwitchTypeStr,
_Out_ sai_switch_type_t& saiSwitchType);

static bool parseSwitchType(
_In_ const char* switchTypeStr,
_Out_ sai_vs_switch_type_t& switchType);
Expand All @@ -51,6 +61,8 @@ namespace saivs

public:

sai_switch_type_t m_saiSwitchType;

sai_vs_switch_type_t m_switchType;

sai_vs_boot_type_t m_bootType;
Expand Down
5 changes: 5 additions & 0 deletions vslib/inc/saivs.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ extern "C" {
}

#define SAI_KEY_VS_SWITCH_TYPE "SAI_VS_SWITCH_TYPE"
#define SAI_KEY_VS_SAI_SWITCH_TYPE "SAI_VS_SAI_SWITCH_TYPE"

#define SAI_VALUE_SAI_SWITCH_TYPE_NPU "SAI_SWITCH_TYPE_NPU"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why you need this ? if i understand correctly this is attribute on switch_create

#define SAI_VALUE_SAI_SWITCH_TYPE_PHY "SAI_SWITCH_TYPE_PHY"

/**
* @def SAI_KEY_VS_INTERFACE_LANE_MAP_FILE
Expand All @@ -31,6 +35,7 @@ extern "C" {
#define SAI_KEY_VS_HOSTIF_USE_TAP_DEVICE "SAI_VS_HOSTIF_USE_TAP_DEVICE"

#define SAI_VALUE_VS_SWITCH_TYPE_BCM56850 "SAI_VS_SWITCH_TYPE_BCM56850"
#define SAI_VALUE_VS_SWITCH_TYPE_BCM81724 "SAI_VS_SWITCH_TYPE_BCM81724"
#define SAI_VALUE_VS_SWITCH_TYPE_MLNX2700 "SAI_VS_SWITCH_TYPE_MLNX2700"

/*
Expand Down
1 change: 1 addition & 0 deletions vslib/src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ libSaiVS_a_SOURCES = \
SelectableFd.cpp \
SwitchState.cpp \
SwitchBCM56850.cpp \
SwitchBCM81724.cpp \
SwitchMLNX2700.cpp

libsaivs_la_SOURCES = \
Expand Down
19 changes: 18 additions & 1 deletion vslib/src/Sai.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,19 @@ sai_status_t Sai::initialize(
return SAI_STATUS_FAILURE;
}

auto sai_switch_type = service_method_table->profile_get_value(0, SAI_KEY_VS_SAI_SWITCH_TYPE);
sai_switch_type_t saiSwitchType;

if (sai_switch_type == NULL)
{
SWSS_LOG_NOTICE("failed to obtain service method table value: %s", SAI_KEY_VS_SAI_SWITCH_TYPE);
saiSwitchType = SAI_SWITCH_TYPE_NPU;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why you need this, i though this is switch_create attribute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need, at this early state, to know if this is NPU, which supports the starting of fdb agent thread, or is PHY, which does not. We experienced failures if we start this thread for phy, and it is not desireable anyway. Introducing the switch type in the ini file as in this example, is how we determine type.

SAI_WARM_BOOT_READ_FILE=./sai_warmboot.bin
SAI_WARM_BOOT_WRITE_FILE=./sai_warmboot.bin
SAI_VS_SWITCH_TYPE=SAI_VS_SWITCH_TYPE_BCM81724
SAI_VS_SAI_SWITCH_TYPE=SAI_SWITCH_TYPE_PHY
SAI_VS_HOSTIF_USE_TAP_DEVICE=false
SAI_VS_USE_BCMSIM_LINK_MON=true
SAI_VS_INTERFACE_LANE_MAP_FILE=BCM81724/lanemapphy.ini

Another option would be to hard code a mapping of part number to type somewhere, but that hardly seems like a good solution (might as well put it here, where the switch resource is defined).

The field is considered optional (default is NPU) since that is the dominant switch type, and for backwards compatibility. Effectively, at this point, making it a boolean "isPhy" but if you consider that downstream there might be a third or fourth class of "switch", better to call it out explicitly.

}
else if (!SwitchConfig::parseSaiSwitchType(sai_switch_type, saiSwitchType))
{
return SAI_STATUS_FAILURE;
}

auto *laneMapFile = service_method_table->profile_get_value(0, SAI_KEY_VS_INTERFACE_LANE_MAP_FILE);

m_laneMapContainer = LaneMapFileParser::parseLaneMapFile(laneMapFile);
Expand Down Expand Up @@ -128,6 +141,7 @@ sai_status_t Sai::initialize(

auto sc = std::make_shared<SwitchConfig>();

sc->m_saiSwitchType = saiSwitchType;
sc->m_switchType = switchType;
sc->m_bootType = bootType;
sc->m_switchIndex = 0;
Expand Down Expand Up @@ -161,7 +175,10 @@ sai_status_t Sai::initialize(

startUnittestThread();

startFdbAgingThread();
if (saiSwitchType == SAI_SWITCH_TYPE_NPU)
{
startFdbAgingThread();
}

m_apiInitialized = true;

Expand Down
Loading