Skip to content

Commit d995501

Browse files
Santhosh Nagarajfacebook-github-bot
authored andcommitted
Fix crash in BcmPort check
Summary: On wedge agent conveyor there was a crash in one of the test suite - https://www.internalfb.com/intern/everpaste/?color=1&handle=GLxFjgSoGqUGjwEFAI4-Hc_BAiAPbr0LAAAz Seems to be a possible race condition where the port settings is not programmed yet. This fix is making sure pointer dereferences are properly guarded with null checks. ``` AddressSanitizer:DEADLYSIGNAL ================================================================= ==3490145==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000002c8 (pc 0x000047311002 bp 0x7f7b06dfab70 sp 0x7f7b06dfab00 T146) ==3490145==The signal is caused by a READ memory access. ==3490145==Hint: address points to the zero page. V0630 04:59:37.361566 3490381 BcmPort.cpp:2950] set pause setting for port 23, Tx/Rx=>False/False V0630 04:59:37.361660 3490381 BcmPort.cpp:2898] Skip same PFC setting for port 23, Tx/Rx =False/False V0630 04:59:37.361681 3490381 BcmPort.cpp:2026] Updated Port flowlet config for eth3/9/1 V0630 04:59:37.361698 3490381 BcmPort.cpp:773] Updating Port flowlet config for eth3/9/1 V0630 04:59:37.361744 3490381 BcmPort.cpp:677] Flowlet port quality on port: 23programmed: false SCARINESS: 10 (null-deref) V0630 04:59:37.362781 3490381 BcmPort.cpp:391] Reinitializing stats for eth3/9/1 V0630 04:59:37.367948 3490381 BcmPort.cpp:1198] BCM_PORT_PHY_CONTROL_PRBS_TX_ENABLE is already disable for port 23 V0630 04:59:37.368042 3490381 BcmPort.cpp:1198] BCM_PORT_PHY_CONTROL_PRBS_RX_ENABLE is already disable for port 23 V0630 04:59:37.368143 3490381 BcmPort.cpp:763] Saving port settings for eth3/9/1 #0 0x000047311002 in facebook::fboss::BcmPort::isPortPgConfigured() const (/tmp/setup_bcm_bin-6.5.26+0x47311002) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #1 0x0000474f09f7 in facebook::fboss::BcmBstStatsMgr::updateStats() (/tmp/setup_bcm_bin-6.5.26+0x474f09f7) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #2 0x0000474c6982 in facebook::fboss::BcmSwitch::updateGlobalStats() (/tmp/setup_bcm_bin-6.5.26+0x474c6982) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #3 0x00004741b2bd in facebook::fboss::BcmSwitch::updateStatsImpl() (/tmp/setup_bcm_bin-6.5.26+0x4741b2bd) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #4 0x000016b168d3 in facebook::fboss::HwSwitch::updateStats() (/tmp/setup_bcm_bin-6.5.26+0x16b168d3) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #5 0x00001aa8a76a in facebook::fboss::SwSwitch::updateStats() (/tmp/setup_bcm_bin-6.5.26+0x1aa8a76a) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #6 0x0000190b8337 in folly::FunctionScheduler::runOneFunction(std::unique_lock<std::mutex>&, std::chrono::time_point<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 1000000000l>>>) (/tmp/setup_bcm_bin-6.5.26+0x190b8337) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #7 0x0000190b7c13 in folly::FunctionScheduler::run() (/tmp/setup_bcm_bin-6.5.26+0x190b7c13) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #8 0x7f7b8c8df3b4 in execute_native_thread_routine (/usr/local/fbcode/platform010-compat/lib/libstdc++.so.6+0xdf3b4) (BuildId: bfd470bfcd746bb985706407f7f2f21d0d0f0394) #9 0x0000167d2826 in asan_thread_start(void*) (/tmp/setup_bcm_bin-6.5.26+0x167d2826) (BuildId: 34b07063ef04d2be8754ee4cc6c9fc619138995b) #10 0x7f7b8cc97988 in start_thread (/usr/local/fbcode/platform010-compat/lib/libc.so.6+0x97988) (BuildId: 8412691973fd914199b3e0331455960169d22837) #11 0x7f7b8cd2851b in __GI___clone3 (/usr/local/fbcode/platform010-compat/lib/libc.so.6+0x12851b) (BuildId: 8412691973fd914199b3e0331455960169d22837) ``` Reviewed By: maxwindiff, zechengh09 Differential Revision: D77550860 fbshipit-source-id: 56a69893626d5b5b4fd7635899936600b2589933
1 parent f410e2e commit d995501

File tree

1 file changed

+11
-3
lines changed

1 file changed

+11
-3
lines changed

fboss/agent/hw/bcm/BcmPort.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3070,9 +3070,17 @@ cfg::PortProfileID BcmPort::getCurrentProfile() const {
30703070
}
30713071

30723072
bool BcmPort::isPortPgConfigured() const {
3073-
return (
3074-
hw_->getPlatform()->getAsic()->isSupported(HwAsic::Feature::PFC) &&
3075-
(*programmedSettings_.rlock())->getPortPgConfigs());
3073+
if (!hw_->getPlatform()->getAsic()->isSupported(HwAsic::Feature::PFC)) {
3074+
return false;
3075+
}
3076+
3077+
auto settings = programmedSettings_.rlock();
3078+
if (*settings == nullptr) {
3079+
// Port settings haven't been programmed yet
3080+
return false;
3081+
}
3082+
3083+
return static_cast<bool>((*settings)->getPortPgConfigs());
30763084
}
30773085

30783086
PortPgConfigs BcmPort::getCurrentProgrammedPgSettings() const {

0 commit comments

Comments
 (0)