modemfwd: make the carrier firmware check more robust
In the firmware manifest, a given carrier firmware might be used for
multiple carrier UUIDs (e.g. a carrier and its MVNO).
In this case, when asked for the flashed carrier firmware version and UUID,
the l850gl-helper might return an arbitrary UUID (which is hardcoded on
their side) for this firmware rather than current real one.
Detect this corner case to avoid endlessly re-flashing the carrier
firmware by comparing the carrier firmware file for the flashed UUID
with the file for the detected UUID (rather than comparing just the
UUIDs).
BUG=b:171367064
TEST=unit-tests: added SkipCarrierWithTwoUuidSameFirmware test case,
fails before the change and pass with it.
TEST=manual testing: put 2 different UUID in the
firmware_manifest and make the l850gl-helper returns the wrong one (not
matching the network one) and verify that we are not doing useless
updates.
Change-Id: I6aab9535f0c7dee43844c4477138e850853e3aec
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2507711
Tested-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Commit-Queue: Vincent Palatin <vpalatin@chromium.org>
(cherry picked from commit a6d2f6a8e1590ca1fd5ac0b914160dec10c70a49)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2581749
Tested-by: Henry Sun <henrysun@google.com>
Reviewed-by: Henry Sun <henrysun@google.com>
Commit-Queue: Henry Sun <henrysun@google.com>
diff --git a/modemfwd/firmware_directory.cc b/modemfwd/firmware_directory.cc
index 4c9066a..113802e 100644
--- a/modemfwd/firmware_directory.cc
+++ b/modemfwd/firmware_directory.cc
@@ -87,6 +87,35 @@
return result;
}
+ // modemfwd::IsUsingSameFirmware overrides.
+ bool IsUsingSameFirmware(const std::string& device_id,
+ const std::string& carrier_a,
+ const std::string& carrier_b) override {
+ // easy case: identical carrier UUID
+ if (carrier_a == carrier_b)
+ return true;
+
+ DeviceType type{device_id, variant_};
+ auto device_it = index_.find(type);
+ // no firmware for this device
+ if (device_it == index_.end())
+ return true;
+
+ const DeviceFirmwareCache& cache = device_it->second;
+ auto main_a = cache.main_firmware.find(carrier_a);
+ auto main_b = cache.main_firmware.find(carrier_b);
+ auto cust_a = cache.carrier_firmware.find(carrier_a);
+ auto cust_b = cache.carrier_firmware.find(carrier_b);
+ // one or several firmwares are missing
+ if (main_a == cache.main_firmware.end() ||
+ main_b == cache.main_firmware.end() ||
+ cust_a == cache.carrier_firmware.end() ||
+ cust_b == cache.carrier_firmware.end())
+ return main_a == main_b && cust_a == cust_b;
+ // same firmware if they are pointing to the 2 same files.
+ return main_a->second == main_b->second && cust_a->second == cust_b->second;
+ }
+
private:
bool FindFirmwareForCarrier(
const DeviceFirmwareCache::CarrierIndex& carrier_index,
diff --git a/modemfwd/firmware_directory.h b/modemfwd/firmware_directory.h
index 650af1e..3a9334a 100644
--- a/modemfwd/firmware_directory.h
+++ b/modemfwd/firmware_directory.h
@@ -34,6 +34,13 @@
// that supports the carrier |carrier_id|, such as a generic one.
virtual Files FindFirmware(const std::string& device_id,
std::string* carrier_id) = 0;
+
+ // Determine whether two potentially different carrier ID |carrier_a| and
+ // |carrier_b| are using the same base and carrier firmwares.
+ // e.g. a carrier and MVNO networks.
+ virtual bool IsUsingSameFirmware(const std::string& device_id,
+ const std::string& carrier_a,
+ const std::string& carrier_b) = 0;
};
std::unique_ptr<FirmwareDirectory> CreateFirmwareDirectory(
diff --git a/modemfwd/firmware_directory_stub.cc b/modemfwd/firmware_directory_stub.cc
index a883405..ee18032 100644
--- a/modemfwd/firmware_directory_stub.cc
+++ b/modemfwd/firmware_directory_stub.cc
@@ -85,4 +85,25 @@
return false;
}
+bool FirmwareDirectoryStub::IsUsingSameFirmware(const std::string& device_id,
+ const std::string& carrier_a,
+ const std::string& carrier_b) {
+ // easy case: identical carrier UUID
+ if (carrier_a == carrier_b)
+ return true;
+
+ FirmwareFileInfo info_a;
+ FirmwareFileInfo info_b;
+ bool has_a =
+ GetValue(carrier_fw_info_, std::make_pair(device_id, carrier_a), &info_a);
+ bool has_b =
+ GetValue(carrier_fw_info_, std::make_pair(device_id, carrier_b), &info_b);
+ // one or several firmwares are missing
+ if (!has_a || !has_b)
+ return false;
+
+ // same firmware if they are pointing to the 2 same files.
+ return info_a.firmware_path == info_b.firmware_path;
+}
+
} // namespace modemfwd
diff --git a/modemfwd/firmware_directory_stub.h b/modemfwd/firmware_directory_stub.h
index 2f245d2..2a6c249 100644
--- a/modemfwd/firmware_directory_stub.h
+++ b/modemfwd/firmware_directory_stub.h
@@ -30,6 +30,10 @@
// modemfwd::FirmwareDirectory overrides.
FirmwareDirectory::Files FindFirmware(const std::string& device_id,
std::string* carrier_id) override;
+ // modemfwd::IsUsingSameFirmware overrides.
+ bool IsUsingSameFirmware(const std::string& device_id,
+ const std::string& carrier_a,
+ const std::string& carrier_b) override;
private:
using CarrierFirmwareMap =
diff --git a/modemfwd/modem_flasher.cc b/modemfwd/modem_flasher.cc
index 6bb252a..5c3b257 100644
--- a/modemfwd/modem_flasher.cc
+++ b/modemfwd/modem_flasher.cc
@@ -164,7 +164,9 @@
ELOG(INFO) << "No carrier firmware is currently installed";
}
- if (!has_carrier_fw || carrier_fw_id != current_carrier ||
+ if (!has_carrier_fw ||
+ !firmware_directory_->IsUsingSameFirmware(device_id, carrier_fw_id,
+ current_carrier) ||
carrier_fw_version != file_info.version) {
FirmwareFile firmware_file;
if (!firmware_file.PrepareFrom(file_info))
diff --git a/modemfwd/modem_flasher_test.cc b/modemfwd/modem_flasher_test.cc
index 36b1357..baa7cfa 100644
--- a/modemfwd/modem_flasher_test.cc
+++ b/modemfwd/modem_flasher_test.cc
@@ -34,6 +34,7 @@
constexpr char kMainFirmware2Version[] = "versionB";
constexpr char kCarrier1[] = "uuid_1";
+constexpr char kCarrier1Mvno[] = "uuid_1_1";
constexpr char kCarrier1Firmware1Path[] = "carrier_1_fw_1.fls";
constexpr char kCarrier1Firmware1Version[] = "v1.00";
constexpr char kCarrier1Firmware2Path[] = "carrier_1_fw_2.fls";
@@ -698,4 +699,23 @@
modem_flasher_->TryFlash(modem.get());
}
+TEST_F(ModemFlasherTest, SkipCarrierWithTwoUuidSameFirmware) {
+ base::FilePath current_firmware(kCarrier1Firmware1Path);
+ AddCarrierFirmwareFile(kDeviceId1, kCarrier1, current_firmware,
+ kCarrier1Firmware2Version);
+ AddCarrierFirmwareFile(kDeviceId1, kCarrier1Mvno, current_firmware,
+ kCarrier1Firmware2Version);
+
+ auto modem = GetDefaultModem();
+ EXPECT_CALL(*modem, GetDeviceId()).Times(AtLeast(1));
+ EXPECT_CALL(*modem, GetCarrierFirmwareVersion()).Times(AtLeast(1));
+ // The modem will say that the currently flashed firmware has the carrier UUID
+ // KCarrier1Mvno while the current carrier UUID is always returned as
+ // kCarrier1.
+ SetCarrierFirmwareInfo(modem.get(), kCarrier1Mvno, kCarrier1Firmware2Version);
+ EXPECT_CALL(*modem, FlashMainFirmware(_, _)).Times(0);
+ EXPECT_CALL(*modem, FlashCarrierFirmware(_, _)).Times(0);
+ modem_flasher_->TryFlash(modem.get());
+}
+
} // namespace modemfwd
