From dd6f8c31703c9b0562866f8616b2d26f9e2f51e2 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Tue, 11 Jul 2023 14:12:04 -0700 Subject: [PATCH] skf cleanup 2 (#976) * when a device is found to be deleted, remove all pairs * use the router_device helper * always send skf adds through the filter (#979) * usort from the start --- src/apis/router_console_ws_worker.erl | 22 +++++---------- src/device/router_device.erl | 12 +++++++-- src/device/router_device_worker.erl | 39 ++++++++------------------- 3 files changed, 27 insertions(+), 46 deletions(-) diff --git a/src/apis/router_console_ws_worker.erl b/src/apis/router_console_ws_worker.erl index 0a49518c9..49a33f663 100644 --- a/src/apis/router_console_ws_worker.erl +++ b/src/apis/router_console_ws_worker.erl @@ -401,12 +401,8 @@ update_device_record(DB, CF, DeviceID) -> case router_device_cache:get(DeviceID) of {ok, Device} -> catch router_ics_eui_worker:remove([DeviceID]), - case router_device:devaddr_int_nwk_key(Device) of - {ok, {DevAddrInt, NwkSKey}} -> - catch router_ics_skf_worker:update([{remove, DevAddrInt, NwkSKey, 0}]); - {error, _} -> - ok - end; + Removes = router_device:make_skf_removes(Device), + catch router_ics_skf_worker:update(Removes); _ -> ok end, @@ -477,25 +473,19 @@ update_device_record(DB, CF, DeviceID) -> case router_device:devaddr_int_nwk_key(Device) of {error, _} -> ok; - {ok, {DevAddrInt, NwkSKey}} -> + {ok, {_DevAddrInt, _NwkSKey}} -> case {OldIsActive, IsActive} of %% end state is active, multi-buy has changed. {_, true} when OldMultiBuy =/= NewMultiBuy -> - catch router_ics_skf_worker:update([ - {add, DevAddrInt, NwkSKey, NewMultiBuy} - ]), + catch router_ics_skf_worker:add_device_ids([DeviceID]), lager:debug("device active, multi-buy changed, sent SKF add"); %% inactive -> active. {false, true} -> - catch router_ics_skf_worker:update([ - {add, DevAddrInt, NwkSKey, NewMultiBuy} - ]), + catch router_ics_skf_worker:add_device_ids([DeviceID]), lager:debug("device un-paused, sent SKF add"); %% active -> inactive. {true, false} -> - catch router_ics_skf_worker:update([ - {remove, DevAddrInt, NwkSKey, OldMultiBuy} - ]), + catch router_ics_skf_worker:remove_device_ids([DeviceID]), lager:debug("device paused, sent SKF remove"); %% active state has not changed, multi-buy remains the same _ -> diff --git a/src/device/router_device.erl b/src/device/router_device.erl index e5e85725d..74b804076 100644 --- a/src/device/router_device.erl +++ b/src/device/router_device.erl @@ -50,7 +50,7 @@ credentials_to_evict/1, limit_credentials/1, devaddr_int_nwk_key/1, - make_skf_removes/1, make_skf_removes/2 + make_skf_removes/1, make_skf_removes/2, make_skf_removes/3 ]). %% ------------------------------------------------------------------ @@ -551,12 +551,20 @@ make_skf_removes(Device) -> DevAddrs :: list(binary()) ) -> [{remove, non_neg_integer(), binary(), non_neg_integer()}]. make_skf_removes(NwkKeys, DevAddrs) -> + make_skf_removes(NwkKeys, DevAddrs, 0). + +-spec make_skf_removes( + NwkKeys :: list({binary() | undefined, binary() | undefined}), + DevAddrs :: list(binary()), + MultiBuy :: non_neg_integer() +) -> [{remove, non_neg_integer(), binary(), non_neg_integer()}]. +make_skf_removes(NwkKeys, DevAddrs, MultiBuy) -> DevAddrToInt = fun(D) -> <> = lorawan_utils:reverse(D), Int end, - [{remove, DevAddrToInt(D), NSK, 0} || {NSK, _} <- NwkKeys, D <- DevAddrs]. + lists:usort([{remove, DevAddrToInt(D), NSK, MultiBuy} || {NSK, _} <- NwkKeys, D <- DevAddrs]). %% ------------------------------------------------------------------ %% RocksDB Device Functions diff --git a/src/device/router_device_worker.erl b/src/device/router_device_worker.erl index 009992c3c..9a5539f34 100644 --- a/src/device/router_device_worker.erl +++ b/src/device/router_device_worker.erl @@ -366,13 +366,8 @@ handle_cast( case router_console_api:get_device(DeviceID) of {error, not_found} -> catch router_ics_eui_worker:remove([DeviceID]), - ok = - case router_device:devaddr_int_nwk_key(Device0) of - {ok, {DevAddrInt, NwkSKey}} -> - catch router_ics_skf_worker:update([{remove, DevAddrInt, NwkSKey, 0}]); - _ -> - ok - end, + Removes = router_device:make_skf_removes(Device0), + catch router_ics_skf_worker:update(Removes), %% Important to remove details about the device _before_ the device. ok = router_device:delete(DB, CF, DeviceID), ok = router_device_cache:delete(DeviceID), @@ -440,25 +435,19 @@ handle_cast( case router_device:devaddr_int_nwk_key(Device1) of {error, _} -> ok; - {ok, {DevAddrInt, NwkSKey}} -> + {ok, {_DevAddrInt, _NwkSKey}} -> case {OldIsActive, IsActive} of %% end state is active, multi-buy has changed. {_, true} when OldMultiBuy =/= NewMultiBuy -> - catch router_ics_skf_worker:update([ - {add, DevAddrInt, NwkSKey, NewMultiBuy} - ]), + catch router_ics_skf_worker:add_device_ids([DeviceID]), lager:debug("device active, multi-buy changed, sent SKF add"); %% inactive -> active. {false, true} -> - catch router_ics_skf_worker:update([ - {add, DevAddrInt, NwkSKey, NewMultiBuy} - ]), + catch router_ics_skf_worker:add_device_ids([DeviceID]), lager:debug("device un-paused, sent SKF add"); %% active -> inactive. {true, false} -> - catch router_ics_skf_worker:update([ - {remove, DevAddrInt, NwkSKey, OldMultiBuy} - ]), + catch router_ics_skf_worker:remove_device_ids([DeviceID]), lager:debug("device paused, sent SKF remove"); %% active state has not changed, multi-buy remains the same _ -> @@ -797,17 +786,11 @@ handle_cast( MultiBuy = maps:get(multi_buy, router_device:metadata(D1), 0), ToAdd = [{add, DevAddr0, UsedNwkSKey, MultiBuy}], - DevAddrToInt = fun(D) -> - <> = lorawan_utils:reverse(D), - Int - end, - - %% We have to usort just in case DevAddr assigned is the same - ToRemove0 = lists:usort([ - {remove, DevAddrToInt(DevAddr), NwkSKey, MultiBuy} - || {NwkSKey, _} <- Keys, DevAddr <- router_device:devaddrs(Device0) - ]), - + ToRemove0 = router_device:make_skf_removes( + Keys, + router_device:devaddrs(Device0), + MultiBuy + ), %% Making sure that the pair that was added is not getting removed (just in case DevAddr assigned is the same) ToRemove1 = ToRemove0 -- [{remove, DevAddr0, UsedNwkSKey, MultiBuy}], ok = router_ics_skf_worker:update(ToAdd ++ ToRemove1),