Attention is currently required from: fixeria, pespin. lynxis lazus has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29788 ) Change subject: Add BTS ramping to prevent mass configuration of BTS at the same time ...................................................................... Patch Set 2: (18 comments) File include/osmocom/bsc/nm_bts_ramp.h: https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/ad12c6c4_54bb19b4 PS1, Line 36: bool bts_ramp_active(struct gsm_network *net); Ack https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/b4e342dd_b2dc3d9f PS1, Line 40: /*! Do you know why? I would expect to see this documentation in the header and not in the file. Doxygen in the file only for internal functions which aren't defined in the header. https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/66a2b7b6_63fb4885 PS1, Line 43: * \param max_bts allow how many bts to provision Ack https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/c89f4abe_ac21f0d4 PS1, Line 65: * \brief bts_ramp_unblock_bts Done File src/osmo-bsc/bsc_vty.c: https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/a8acbf69_d129d671 PS1, Line 1284: "allow-bts-configuration <0-65535>", copy paste from drop bts connection. reduced it to 8 bit https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/22538ed9_86518798 PS1, Line 1284: 65535 Done https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/6ccb7d11_217052dc PS1, Line 1284: "allow-bts-configuration <0-65535>", Done https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/ac973332_d6299085 PS1, Line 2659: vty_out(vty, " bts ramp limit %d within %d seconds%s", I've changed it as fixeria suggested. https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/f3bc032f_146ed753 PS1, Line 3087: "bts ramp limit <0-999> within <0-999> seconds", I like to have this in one line as this feature doesn't have a complex configuration. https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/9aa22a11_f447663e PS1, Line 3087: bts ramp limit Done File src/osmo-bsc/nm_bts_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/733ba432_5f2feeff PS1, Line 92: struct gsm_bts *bts, Done File src/osmo-bsc/nm_bts_ramp.c: https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/6bc83e15_4a865ef0 PS1, Line 63: if (llist_entry_empty(&bts->bts_ramp.list)) I don't like to have this llist set to itself. (INIT_LLIST_HEAD) We don't have a function to set it to POISON. https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/1cebe6b7_16ce86c7 PS1, Line 86: net->bts_ramp.count >= net->bts_ramp.max_bts) Fixed. Shouldn't we have a linter now which creates comments in the gerrit review? (Sorry I'm asking here, I would like to have a linter which shows me warnings. None of us should take care of indention while reviewing. I'll check some user space linters later.) https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/ea734258_89ec0a00 PS1, Line 113: void bts_ramp_deactivate(struct gsm_network *net) Ack https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/897f0c67_88db968e PS1, Line 121: bool bts_ramp_active(struct gsm_network *net) will rename it to enabled. https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/f918b2db_c1397fc7 PS1, Line 143: OSMO_ASSERT(llist_entry_empty(&bts->bts_ramp.list)); Removed the assert. A warning would be okay, but I don't see a reason to assert here. It's not a big problem if the BTS just move forward. https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/bc81dc45_f4528db2 PS1, Line 172: if (!llist_entry_empty(&bts->bts_ramp.list)) it does. File src/osmo-bsc/nm_bts_sm_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/f09779c7_2dc90b98 PS1, Line 162: case NM_EV_RAMP_GO: Yes. I forgot to drop it again. It's dead code. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/29788 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id56dde6d58f3d0d20352f6c306598d2cccc6345d Gerrit-Change-Number: 29788 Gerrit-PatchSet: 2 Gerrit-Owner: lynxis lazus &lt;lynxis(a)fe80.eu&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-CC: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Attention: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Comment-Date: Wed, 19 Oct 2022 03:12:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt; Comment-In-Reply-To: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-MessageType: comment