From 350dc9f41a19ee5b506d135dc9a60a1f51800ab7 Mon Sep 17 00:00:00 2001 From: Roy Luo Date: Wed, 20 Dec 2023 21:34:37 +0000 Subject: [PATCH 1/2] usb: implement heuristics to flag data compliance warnings Support flagging enum failure and flaky connection in device mode, flagging enum failure and missing data lines in host mode. No warning would be flagged until 5 secs after the data session starts to give ample time for the connection to stabilize, a timer is added to support it. Bug: 296119135 Test: manually trigger the warnings Change-Id: I25f08657e328913946add192b5ecb9ee50c3a1a8 (cherry picked from commit 42020dc4585442bd7ca88f183ba29a18834af197) --- usb/usb/UsbDataSessionMonitor.cpp | 125 ++++++++++++++++++++++++++---- usb/usb/UsbDataSessionMonitor.h | 3 + 2 files changed, 113 insertions(+), 15 deletions(-) diff --git a/usb/usb/UsbDataSessionMonitor.cpp b/usb/usb/UsbDataSessionMonitor.cpp index 77defb30..a2ccbafb 100644 --- a/usb/usb/UsbDataSessionMonitor.cpp +++ b/usb/usb/UsbDataSessionMonitor.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -48,6 +49,14 @@ namespace usb { #define UEVENT_MSG_LEN 2048 #define USB_STATE_MAX_LEN 20 #define DATA_ROLE_MAX_LEN 10 +#define WARNING_SURFACE_DELAY_SEC 5 +#define ENUM_FAIL_DEFAULT_COUNT_THRESHOLD 3 +#define DEVICE_FLAKY_CONNECTION_CONFIGURED_COUNT_THRESHOLD 5 +/* + * Typically a smooth and successful enumeration in device mode would go through 5 states at + * maximum: not attached -> default -> default -> addressed -> configured + */ +#define DEVICE_STATE_TRANSITION_PER_ENUMERATION 5 constexpr char kUdcConfigfsPath[] = "/config/usb_gadget/g1/UDC"; constexpr char kNotAttachedState[] = "not attached\n"; @@ -115,6 +124,15 @@ UsbDataSessionMonitor::UsbDataSessionMonitor( if (addEpollFd(epollFd, ueventFd)) abort(); + unique_fd timerFd(timerfd_create(CLOCK_BOOTTIME, TFD_NONBLOCK)); + if (timerFd.get() == -1) { + ALOGE("create timerFd failed"); + abort(); + } + + if (addEpollFd(epollFd, timerFd)) + abort(); + if (addEpollFile(epollFd.get(), dataRolePath, mDataRoleFd) != 0) { ALOGE("monitor data role failed"); abort(); @@ -139,6 +157,7 @@ UsbDataSessionMonitor::UsbDataSessionMonitor( mEpollFd = std::move(epollFd); mUeventFd = std::move(ueventFd); + mTimerFd = std::move(timerFd); mUpdatePortStatusCb = updatePortStatusCb; if (ReadFileToString(kUdcConfigfsPath, &udc) && !udc.empty()) @@ -223,13 +242,60 @@ void UsbDataSessionMonitor::notifyComplianceWarning() { void UsbDataSessionMonitor::evaluateComplianceWarning() { std::set newWarningSet; + int elapsedTimeSec; - // TODO: add heuristics and update newWarningSet - if (mDataRole == PortDataRole::DEVICE && mUdcBind) { - } else if (mDataRole == PortDataRole::HOST) { + elapsedTimeSec = + std::chrono::duration_cast(boot_clock::now() - mDataSessionStart) + .count(); + + if (elapsedTimeSec >= WARNING_SURFACE_DELAY_SEC) { + if (mDataRole == PortDataRole::DEVICE && mUdcBind) { + int stateCount = mDeviceState.states.size(); + int configuredCount = std::count(mDeviceState.states.begin(), + mDeviceState.states.end(), kConfiguredState); + int defaultCount = + std::count(mDeviceState.states.begin(), mDeviceState.states.end(), kDefaultState); + + if (configuredCount == 0 && defaultCount > ENUM_FAIL_DEFAULT_COUNT_THRESHOLD) + newWarningSet.insert(ComplianceWarning::ENUMERATION_FAIL); + + if (configuredCount > DEVICE_FLAKY_CONNECTION_CONFIGURED_COUNT_THRESHOLD && + stateCount > configuredCount * DEVICE_STATE_TRANSITION_PER_ENUMERATION) { + ALOGI("Detected flaky connection: state count %d, configured count %d", + stateCount, configuredCount); + newWarningSet.insert(ComplianceWarning::FLAKY_CONNECTION); + } + } else if (mDataRole == PortDataRole::HOST) { + int host1StateCount = mHost1State.states.size(); + int host1ConfiguredCount = + std::count(mHost1State.states.begin(), mHost1State.states.end(), kConfiguredState); + int host1DefaultCount = + std::count(mHost1State.states.begin(), mHost1State.states.end(), kDefaultState); + int host2StateCount = mHost2State.states.size(); + int host2ConfiguredCount = + std::count(mHost2State.states.begin(), mHost2State.states.end(), kConfiguredState); + int host2DefaultCount = + std::count(mHost2State.states.begin(), mHost2State.states.end(), kDefaultState); + + if (host1ConfiguredCount == 0 && host2ConfiguredCount == 0 && + (host1DefaultCount > ENUM_FAIL_DEFAULT_COUNT_THRESHOLD || + host2DefaultCount > ENUM_FAIL_DEFAULT_COUNT_THRESHOLD)) + newWarningSet.insert(ComplianceWarning::ENUMERATION_FAIL); + + if (host1StateCount == 1 && mHost1State.states.front() == kNotAttachedState && + host2StateCount == 1 && mHost2State.states.front() == kNotAttachedState) + newWarningSet.insert(ComplianceWarning::MISSING_DATA_LINES); + } } if (newWarningSet != mWarningSet) { + std::string newWarningString; + + for (auto e : newWarningSet) { + newWarningString += toString(e) + " "; + } + ALOGI("Usb data compliance warning changed to: %s", newWarningString.c_str()); + mWarningSet = newWarningSet; notifyComplianceWarning(); } @@ -259,6 +325,26 @@ void UsbDataSessionMonitor::handleDeviceStateEvent(struct usbDeviceState *device evaluateComplianceWarning(); } +void UsbDataSessionMonitor::setupNewSession() { + mWarningSet.clear(); + mDataSessionStart = boot_clock::now(); + + if (mDataRole == PortDataRole::DEVICE) { + clearDeviceStateEvents(&mDeviceState); + } else if (mDataRole == PortDataRole::HOST) { + clearDeviceStateEvents(&mHost1State); + clearDeviceStateEvents(&mHost2State); + } + + if (mDataRole != PortDataRole::NONE) { + struct itimerspec delay = itimerspec(); + delay.it_value.tv_sec = WARNING_SURFACE_DELAY_SEC; + int ret = timerfd_settime(mTimerFd.get(), 0, &delay, NULL); + if (ret < 0) + ALOGE("timerfd_settime failed err:%d", errno); + } +} + void UsbDataSessionMonitor::handleDataRoleEvent() { int n; PortDataRole newDataRole; @@ -283,17 +369,8 @@ void UsbDataSessionMonitor::handleDataRoleEvent() { reportUsbDataSessionMetrics(); } - // Set up for the new data session - mWarningSet.clear(); mDataRole = newDataRole; - mDataSessionStart = boot_clock::now(); - - if (newDataRole == PortDataRole::DEVICE) { - clearDeviceStateEvents(&mDeviceState); - } else if (newDataRole == PortDataRole::HOST) { - clearDeviceStateEvents(&mHost1State); - clearDeviceStateEvents(&mHost2State); - } + setupNewSession(); } } @@ -328,8 +405,7 @@ void UsbDataSessionMonitor::updateUdcBindStatus(const std::string &devname) { } else if (!mUdcBind && newUdcBind) { // Gadget soft pullup: reset and start accounting for a new data session. - clearDeviceStateEvents(&mDeviceState); - mDataSessionStart = boot_clock::now(); + setupNewSession(); } } @@ -383,6 +459,23 @@ void UsbDataSessionMonitor::handleUevent() { } } +void UsbDataSessionMonitor::handleTimerEvent() { + int byteRead; + uint64_t numExpiration; + + byteRead = read(mTimerFd.get(), &numExpiration, sizeof(numExpiration)); + + if (byteRead != sizeof(numExpiration)) { + ALOGE("incorrect read size"); + } + + if (numExpiration != 1) { + ALOGE("incorrect expiration count"); + } + + evaluateComplianceWarning(); +} + void *UsbDataSessionMonitor::monitorThread(void *param) { UsbDataSessionMonitor *monitor = (UsbDataSessionMonitor *)param; struct epoll_event events[64]; @@ -400,6 +493,8 @@ void *UsbDataSessionMonitor::monitorThread(void *param) { for (int n = 0; n < nevents; ++n) { if (events[n].data.fd == monitor->mUeventFd.get()) { monitor->handleUevent(); + } else if (events[n].data.fd == monitor->mTimerFd.get()) { + monitor->handleTimerEvent(); } else if (events[n].data.fd == monitor->mDataRoleFd.get()) { monitor->handleDataRoleEvent(); } else if (events[n].data.fd == monitor->mDeviceState.fd.get()) { diff --git a/usb/usb/UsbDataSessionMonitor.h b/usb/usb/UsbDataSessionMonitor.h index 596f378f..aced8a48 100644 --- a/usb/usb/UsbDataSessionMonitor.h +++ b/usb/usb/UsbDataSessionMonitor.h @@ -75,9 +75,11 @@ class UsbDataSessionMonitor { static void *monitorThread(void *param); void handleUevent(); + void handleTimerEvent(); void handleDataRoleEvent(); void handleDeviceStateEvent(struct usbDeviceState *deviceState); void clearDeviceStateEvents(struct usbDeviceState *deviceState); + void setupNewSession(); void reportUsbDataSessionMetrics(); void evaluateComplianceWarning(); void notifyComplianceWarning(); @@ -86,6 +88,7 @@ class UsbDataSessionMonitor { pthread_t mMonitor; unique_fd mEpollFd; unique_fd mUeventFd; + unique_fd mTimerFd; unique_fd mDataRoleFd; struct usbDeviceState mDeviceState; struct usbDeviceState mHost1State; From 1376a71a65706013d62dbff9b8f254c3e65088e1 Mon Sep 17 00:00:00 2001 From: Roy Luo Date: Tue, 26 Dec 2023 20:45:34 +0000 Subject: [PATCH 2/2] usb: adjust heuristics for flaky connection warning The state count requirement is very specific to the case where the signal integrity is the culprit of flaky connection. However, there could be other cases such as bad receptacles causing data pins to disconnect randomly. Remove the state count requirement to cover more cases. Bug: 296119135 Test: manually trigger the warnings Change-Id: Ic2ae376ad6062d9930614381503f44e4a5ac760f (cherry picked from commit 5e14ba01be9acc31d3df0f506b4287eea0bf9583) --- usb/usb/UsbDataSessionMonitor.cpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/usb/usb/UsbDataSessionMonitor.cpp b/usb/usb/UsbDataSessionMonitor.cpp index a2ccbafb..bae261d7 100644 --- a/usb/usb/UsbDataSessionMonitor.cpp +++ b/usb/usb/UsbDataSessionMonitor.cpp @@ -52,11 +52,6 @@ namespace usb { #define WARNING_SURFACE_DELAY_SEC 5 #define ENUM_FAIL_DEFAULT_COUNT_THRESHOLD 3 #define DEVICE_FLAKY_CONNECTION_CONFIGURED_COUNT_THRESHOLD 5 -/* - * Typically a smooth and successful enumeration in device mode would go through 5 states at - * maximum: not attached -> default -> default -> addressed -> configured - */ -#define DEVICE_STATE_TRANSITION_PER_ENUMERATION 5 constexpr char kUdcConfigfsPath[] = "/config/usb_gadget/g1/UDC"; constexpr char kNotAttachedState[] = "not attached\n"; @@ -250,7 +245,6 @@ void UsbDataSessionMonitor::evaluateComplianceWarning() { if (elapsedTimeSec >= WARNING_SURFACE_DELAY_SEC) { if (mDataRole == PortDataRole::DEVICE && mUdcBind) { - int stateCount = mDeviceState.states.size(); int configuredCount = std::count(mDeviceState.states.begin(), mDeviceState.states.end(), kConfiguredState); int defaultCount = @@ -259,12 +253,8 @@ void UsbDataSessionMonitor::evaluateComplianceWarning() { if (configuredCount == 0 && defaultCount > ENUM_FAIL_DEFAULT_COUNT_THRESHOLD) newWarningSet.insert(ComplianceWarning::ENUMERATION_FAIL); - if (configuredCount > DEVICE_FLAKY_CONNECTION_CONFIGURED_COUNT_THRESHOLD && - stateCount > configuredCount * DEVICE_STATE_TRANSITION_PER_ENUMERATION) { - ALOGI("Detected flaky connection: state count %d, configured count %d", - stateCount, configuredCount); + if (configuredCount > DEVICE_FLAKY_CONNECTION_CONFIGURED_COUNT_THRESHOLD) newWarningSet.insert(ComplianceWarning::FLAKY_CONNECTION); - } } else if (mDataRole == PortDataRole::HOST) { int host1StateCount = mHost1State.states.size(); int host1ConfiguredCount =