Add transceiver's frequency support for BF devices

juraul1 · Feb 3, 2022 · 1ea9ef6 · 1ea9ef6
1 parent cfb950d
commit 1ea9ef6
Show file tree

Hide file tree

Showing 16 changed files with 10 additions and 145 deletions.
diff --git a/stratum/hal/lib/barefoot/bf_chassis_manager.cc b/stratum/hal/lib/barefoot/bf_chassis_manager.cc
@@ -24,9 +24,6 @@
 #include "stratum/lib/constants.h"
 #include "stratum/lib/macros.h"
 #include "stratum/lib/utils.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_phal.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_switch_configurator.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_wrapper.h"
 
 namespace stratum {
 namespace hal {
@@ -439,16 +436,6 @@ ::util::Status BfChassisManager::PushChassisConfig(
     }
   }
 
-  for (const auto& singleton_port : config.singleton_ports()) {
-    uint32 port_id = singleton_port.id();
-    uint64 node_id = singleton_port.node();
-    const auto& config_params = singleton_port.config_params();
-    if(config_params.frequency() != 0) {
-          RETURN_IF_ERROR(phal_interface_->PushChassisConfig(config));
-    }
-    LOG(INFO) << "PushChassisConfig from bf_chassis_manager successful.";
-  }
-
   if (config.has_vendor_config() &&
       config.vendor_config().has_tofino_config()) {
     // Handle port shaping.
@@ -1143,9 +1130,7 @@ ::util::Status BfChassisManager::ReplayChassisConfig(uint64 node_id) {
               << sdk_port_id << ").";
     }
     if (config.frequency) {
-      LOG(INFO) << "ReplayChassisConfig - no SetSfpFrequency.";
       config_new->frequency = *config.frequency;
-      LOG(INFO) << "ReplayChassisConfig - config_new->frequency.";
     }
     if (config.admin_state == ADMIN_STATE_ENABLED) {
       RETURN_IF_ERROR(bf_sde_interface_->EnablePort(device, sdk_port_id));

diff --git a/stratum/hal/lib/barefoot/bf_chassis_manager.h b/stratum/hal/lib/barefoot/bf_chassis_manager.h
@@ -19,9 +19,6 @@
 #include "stratum/hal/lib/common/utils.h"
 #include "stratum/hal/lib/common/writer_interface.h"
 #include "stratum/lib/channel/channel.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_phal.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_switch_configurator.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_wrapper.h"
 
 namespace stratum {
 namespace hal {

diff --git a/stratum/hal/lib/barefoot/bfrt_switch.cc b/stratum/hal/lib/barefoot/bfrt_switch.cc
@@ -204,7 +204,6 @@ ::util::Status BfrtSwitch::RetrieveValue(uint64 node_id,
       case DataRequest::Request::kAutonegStatus:
       case DataRequest::Request::kFrontPanelPortInfo:
       case DataRequest::Request::kLoopbackStatus:
-//      case DataRequest::Request::kSfpFrequency:
       case DataRequest::Request::kSdnPortId: {
         auto port_data = bf_chassis_manager_->GetPortData(req);
         if (!port_data.ok()) {
@@ -247,52 +246,9 @@ ::util::Status BfrtSwitch::RetrieveValue(uint64 node_id,
 ::util::Status BfrtSwitch::SetValue(uint64 node_id, const SetRequest& request,
                                     std::vector<::util::Status>* details) {
   LOG(INFO) << "BfrtSwitch::SetValue is not implemented yet, but changes will "
-            << "be performed when ChassisConfig is pushed again."
+            << "be performed when ChassisConfig is pushed again. "
             << request.ShortDebugString() << ".";
-//  for (const auto& req : request.requests()) {
-//    ::util::Status status = ::util::OkStatus();
-//    switch (req.request_case()) {
-//      case SetRequest::Request::RequestCase::kPort:
-//        switch (req.port().value_case()) {
-//          case SetRequest::Request::Port::ValueCase::kHealthIndicator:
-//            LOG(ERROR) << "Request " << req.port().ShortDebugString()
-//                       << " through SetValue() is ignored. Modify the "
-//                          "ChassisConfig instead!";
-//            break;
-//          case SetRequest::Request::Port::ValueCase::kLoopbackStatus:
-//          case SetRequest::Request::Port::ValueCase::kSfpFrequency: {
-//            LOG(INFO) << "SetRequest::Request::Port::ValueCase::kSfpFrequency:";
-//            //absl::WriterMutexLock l(&chassis_lock);
-//            status.Update(phal_interface_->SetSfpFrequencyPhal(req.port().port_id(), req.port().frequency()));
-//            //status.Update(onlp::OnlpPhal::SetSfpFrequencyOnlp(
-//            //    req.port().port_id(),
-//            //    req.port().frequency()));
-//            break;
-//          }
-//          default:
-//            status = MAKE_ERROR(ERR_INTERNAL) << "Not supported yet!";
-//        }
-//        break;
-//      case SetRequest::Request::RequestCase::kOpticalNetworkInterface:
-//        switch (req.optical_network_interface().value_case()) {
-//          case SetRequest::Request::OpticalNetworkInterface::ValueCase::
-//              kOpticalTransceiverInfo: {
-//            status.Update(phal_interface_->SetOpticalTransceiverInfo(
-//                req.optical_network_interface().module(),
-//                req.optical_network_interface().network_interface(),
-//                req.optical_network_interface().optical_transceiver_info()));
-//            break;
-//          }
-//          default:
-//            status = MAKE_ERROR(ERR_INTERNAL) << "Not supported yet!";
-//        }
-//        break;
-//      default:
-//        status = MAKE_ERROR(ERR_INTERNAL)
-//                 << req.ShortDebugString() << " Not supported yet!";
-//    }
-//    if (details) details->push_back(status);
-//  }
+
   return ::util::OkStatus();
 }
 

diff --git a/stratum/hal/lib/bcm/bcm_switch.cc b/stratum/hal/lib/bcm/bcm_switch.cc
@@ -19,9 +19,6 @@
 #include "stratum/glue/status/status_macros.h"
 #include "stratum/lib/constants.h"
 #include "stratum/lib/macros.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_phal.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_switch_configurator.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_wrapper.h"
 
 namespace stratum {
 namespace hal {
@@ -481,16 +478,6 @@ ::util::Status BcmSwitch::SetValue(uint64 node_id, const SetRequest& request,
                 req.port().loopback_status().state()));
             break;
           }
-          case SetRequest::Request::Port::ValueCase::kSfpFrequency: {
-            absl::WriterMutexLock l(&chassis_lock);
-            // or put the function in phal.cc calling onlp_phal.cc
-            //status.Update(phal_interface->PushChassisConfig(config))
-            status.Update(phal_interface_->SetSfpFrequencyOnlp(req.port().port_id(), req.port().frequency()));
-            //status.Update(onlp::OnlpPhal::SetSfpFrequencyOnlp(
-            //    req.port().port_id(),
-            //    req.port().frequency()));
-            break;
-          }
           default:
             status = MAKE_ERROR(ERR_INTERNAL) << "Not supported yet!";
         }

diff --git a/stratum/hal/lib/bcm/bcm_switch.h b/stratum/hal/lib/bcm/bcm_switch.h
@@ -17,9 +17,6 @@
 #include "stratum/hal/lib/bcm/bcm_node.h"
 #include "stratum/hal/lib/common/phal_interface.h"
 #include "stratum/hal/lib/common/switch_interface.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_phal.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_switch_configurator.h"
-//#include "stratum/hal/lib/phal/onlp/onlp_wrapper.h"
 
 namespace stratum {
 namespace hal {

diff --git a/stratum/hal/lib/common/common.proto b/stratum/hal/lib/common/common.proto
@@ -1328,6 +1328,7 @@ message SdnPortId {
   uint32 port_id = 1;
 }
 
+//Wrapper around the Frequency of the tunable transceiver.
 message SfpFrequency {
   uint64 frequency = 1;
 }

diff --git a/stratum/hal/lib/common/gnmi_events.h b/stratum/hal/lib/common/gnmi_events.h
@@ -206,7 +206,7 @@ class PortLoopbackStateChangedEvent
   LoopbackState new_state_;
 };
 
-// A Port's frequency Has Changed event.
+// A Port's SFP frequency Has Changed event.
 class PortSfpFrequencyChangedEvent
     : public PerPortGnmiEvent<PortSfpFrequencyChangedEvent> {
  public:

diff --git a/stratum/hal/lib/common/openconfig_converter.cc b/stratum/hal/lib/common/openconfig_converter.cc
@@ -350,12 +350,6 @@ SingletonPortToInterfaces(const SingletonPort& in) {
         IsLoopbackStateEnabled(in.config_params().loopback_mode()));
   }
 
-//  // SingletonPort.config_params.frequency
-//  // -> /interfaces/interface/config/sfp-frequency
-//  if (in.config_params().frequency() != 0) {
-//    interface->mutable_frequency()->set_value(
-//        in.config_params().frequency());
-//  }
   // FIXME(Yi Tseng): Should we use other field to store interface channel?
   interface->add_physical_channel()->set_value(in.channel());
 
@@ -767,11 +761,6 @@ ::util::StatusOr<SingletonPort> InterfaceToSingletonPort(
     config_params->set_loopback_mode(lpbk_mode);
   }
 
-//  if (interface.has_frequency()) {
-//    uint64 freq = interface.frequency().value();
-//    config_params->set_frequency(freq);
-//  }
-
   // FIXME(Yi Tseng): Should we use other field to store interface channel?
   if (interface.physical_channel_size() > 0) {
     to.set_channel(interface.physical_channel(0).value());

diff --git a/stratum/hal/lib/common/phal_interface.h b/stratum/hal/lib/common/phal_interface.h
@@ -126,10 +126,6 @@ class PhalInterface {
       int module, int network_interface,
       const OpticalTransceiverInfo& optical_netif_info) = 0;
 
-//  virtual ::util::Status SetSfpFrequencyPhal(
-//      uint32 port_number, uint32 frequency) = 0;
-//
-
   // Set the color/state of a frontpanel port LED, corresponding to the physical
   // port specified by (slot, port, channel). The caller assumes each physical
   // port has one frontpanel port LED, i.e., if a transceiver has 4 channels we

diff --git a/stratum/hal/lib/common/yang_parse_tree_paths.cc b/stratum/hal/lib/common/yang_parse_tree_paths.cc
@@ -1513,8 +1513,6 @@ void SetUpInterfacesInterfaceConfigSfpFrequency(uint64 node_id,
     for (auto& singleton_port : *new_config->mutable_singleton_ports()) {
       if (singleton_port.node() == node_id && singleton_port.id() == port_id) {
         singleton_port.mutable_config_params()->set_frequency(uint_frequency);
-        // const auto& config_params = singleton_port.config_params();
-        // config_params.set_frequency(uint_frequency);
         break;
       }
     }

diff --git a/stratum/hal/lib/phal/onlp/onlp_phal.cc b/stratum/hal/lib/phal/onlp/onlp_phal.cc
@@ -66,16 +66,14 @@ ::util::Status OnlpPhal::PushChassisConfig(const ChassisConfig& config) {
   absl::WriterMutexLock l(&config_lock_);
 
   // TODO(unknown): Process Chassis Config here
-  for (const auto& singleton_port : config.singleton_ports()) { // or const SingletonPort&?
+  for (const auto& singleton_port : config.singleton_ports()) {
     uint32 port_id = singleton_port.id();
     uint32 port_number = singleton_port.port();
-    OnlpOid sfp_oid_ = ONLP_SFP_ID_CREATE(port_number); // Is it correct? Or port_id? (sfp_id)
+    OnlpOid sfp_oid_ = ONLP_SFP_ID_CREATE(port_number);
     const auto& config_params = singleton_port.config_params();
     if(config_params.frequency() != 0) {
       RETURN_IF_ERROR(onlp_interface_->SetSfpFrequency(sfp_oid_, port_number, config_params.frequency()));
-      LOG(INFO) << "SetSfpFrequency from onlp_phal.cc successful.";
     }
-    //config->frequency = config_params.frequency();
   }
   return ::util::OkStatus();
 }
@@ -102,17 +100,6 @@ ::util::Status OnlpPhal::RegisterOnlpEventCallback(
   return onlp_event_handler_->RegisterEventCallback(callback);
 }
 
-::util::Status OnlpPhal::SetSfpFrequencyOnlp(uint32 port_number, uint32 frequency) {
-  absl::WriterMutexLock l(&config_lock_);
-
-  OnlpOid sfp_oid_ = ONLP_SFP_ID_CREATE(port_number);
-  if(frequency != 0) {
-    RETURN_IF_ERROR(onlp_interface_->SetSfpFrequency(sfp_oid_, port_number, frequency));
-    LOG(INFO) << "SetSfpFrequency from onlp done.";
-  }
-  return ::util::OkStatus();
-}
-
 }  // namespace onlp
 }  // namespace phal
 }  // namespace hal

diff --git a/stratum/hal/lib/phal/onlp/onlp_phal.h b/stratum/hal/lib/phal/onlp/onlp_phal.h
@@ -38,8 +38,6 @@ class OnlpPhal final : public OnlpPhalInterface {
   ::util::Status Shutdown() override LOCKS_EXCLUDED(config_lock_);
   ::util::Status RegisterOnlpEventCallback(OnlpEventCallback* callback)
       EXCLUSIVE_LOCKS_REQUIRED(config_lock_) override;
-  ::util::Status SetSfpFrequencyOnlp(uint32 port_number, uint32 frequency)
-      LOCKS_EXCLUDED(config_lock_);
 
   // Creates the singleton instance. Expected to be called once to initialize
   // the instance.

diff --git a/stratum/hal/lib/phal/onlp/onlp_switch_configurator.cc b/stratum/hal/lib/phal/onlp/onlp_switch_configurator.cc
@@ -8,9 +8,6 @@
 #include <utility>
 #include <vector>
 
-#include<iostream>
-using namespace std;
-
 #include "stratum/glue/gtl/map_util.h"
 #include "stratum/hal/lib/common/common.pb.h"
 #include "stratum/hal/lib/phal/onlp/onlp_fan_datasource.h"
@@ -56,7 +53,6 @@ ::util::Status OnlpSwitchConfigurator::CreateDefaultConfig(
     PhalCardConfig::Port port;
     port.set_port(ONLP_OID_ID_GET(oid));
     // See if we've got an sfp type and set the physical port type
-    cout << "onlp_switch_configurator --- oid: " << oid << ".\n";
     switch (sfp_info.GetSfpType()) {
       case SFP_TYPE_SFP28:
       case SFP_TYPE_SFP:

diff --git a/stratum/hal/lib/phal/onlp/onlp_wrapper.cc b/stratum/hal/lib/phal/onlp/onlp_wrapper.cc
@@ -193,12 +193,12 @@ ::util::Status OnlpWrapper::SetSfpFrequency(OnlpOid oid, int port_number, int fr
       return ::util::OkStatus();
   }
 
-  //Check if the symbol is present
-//  TODO
+  // TODO: Check if the symbol is present
 //  CHECK_RETURN_IF_FALSE(onlp_functions_.set_sfp_frequency(port_number, frequency) != nullptr)
 //      << "Symbol set_sfp_frequency does not exist.";
 
-  // Set the new frequency. If a failure occurs the function will still return TRUE but a LOG message will apprear with the error.
+  // Set the new frequency. If a failure occurs the function will still return TRUE
+  // but a LOG message will appear with the error.
   CHECK_RETURN_IF_FALSE(
        ONLP_SUCCESS(onlp_functions_.set_sfp_frequency(port_number, frequency)))
        << "Failed to set SFP frequency.";

diff --git a/stratum/hal/lib/phal/phal.cc b/stratum/hal/lib/phal/phal.cc
@@ -129,7 +129,7 @@ ::util::Status Phal::PushChassisConfig(const ChassisConfig& config) {
   for (const auto& phal_interface : phal_interfaces_) {
     RETURN_IF_ERROR(phal_interface->PushChassisConfig(config));
   }
-  LOG(INFO) << "PushChassiConfig from phal.cc successful.";
+
   return ::util::OkStatus();
 }
 
@@ -213,23 +213,6 @@ ::util::Status Phal::SetPortLedState(int slot, int port, int channel,
   return ::util::OkStatus();
 }
 
-//::util::Status Phal::SetSfpFrequencyPhal(uint32 port_number, uint32 frequency) {
-//  absl::WriterMutexLock l(&config_lock_);
-//
-//  //::util::Status status = ::util::OkStatus();
-//  //::util::Status status = onlp::OnlpPhal::SetSfpFrequencyOnlp(port_number, frequency);
-//  //APPEND_STATUS_IF_ERROR(status, onlp::OnlpPhal::SetSfpFrequencyOnlp(port_number, frequency));
-//  //APPEND_STATUS_IF_ERROR(status, onlp_phal->SetSfpFrequencyOnlp(port_number, frequency));
-////  if (status.ok()) {
-////      LOG(INFO) << "SetSfpFrequencyOnlp successful.";
-////  }
-//  if(frequency != 0) {
-//    RETURN_IF_ERROR(onlp_phal_->SetSfpFrequencyOnlp(port_number, port_number, frequency));
-//  }
-//  LOG(INFO) << "SetSfpFrequencyOnlp successful.";
-//  return ::util::OkStatus();
-//}
-
 }  // namespace phal
 }  // namespace hal
 }  // namespace stratum
diff --git a/stratum/hal/lib/phal/phal.h b/stratum/hal/lib/phal/phal.h
@@ -53,11 +53,6 @@ class Phal : public PhalInterface {
                                  LedColor color, LedState state) override
       LOCKS_EXCLUDED(config_lock_);
 
-////JR
-//  ::util::Status SetSfpFrequencyPhal(uint32 port_number, uint32 frequency)
-//      LOCKS_EXCLUDED(config_lock_);
-
-
   // Creates the singleton instance. Expected to be called once to initialize
   // the instance.
   static Phal* CreateSingleton() LOCKS_EXCLUDED(config_lock_, init_lock_);