Change in osmo-bsc[master]: power_control: implement BCCH carrier power reduction operation

neels gerrit-no-reply at
Fri Jul 2 15:52:07 UTC 2021

neels has posted comments on this change. ( )

Change subject: power_control: implement BCCH carrier power reduction operation

Patch Set 3:

Commit Message: 
PS3, Line 14: However, in the recent 3GPP TS 45.008 there is a feature called
(could be nice to mention the spec version nr) 
PS3, Line 30: A value greater than zero would make osmo-bts reduce the power
what do you mean, "would"? 
File doc/manuals/chapters/power_control.adoc: 
PS3, Line 334: a BTS shall maintain discontinuous Downlink transmission at full power in order to
(are you sure that you don't mean "continuous") 
PS3, Line 337: version 13.0.0 (2015-11) - "BCCH carrier power reduction operation".
"in version" 
PS3, Line 348: and the simulation results can be found in 3GPP TR 45.926.
PS3, Line 352: At the moment of writing this manual, the only BTS model that can instructed to
(the usual expression is "at the time of writing")

"that can be instructed" 
PS3, Line 354: can be added on request
sounds like it doesn't belong in this manual?

if you need to say this, maybe rather say "may be added in the future", otherwise it sounds like osmocom is offering a free service of implementing things on request 
PS3, Line 358: `show bts` command in the VTY interface.
maybe "in OsmoBSC's VTY interface" 
PS3, Line 363: There is currently no logic in osmo-bsc for automatic activation and deactivation
(technically "osmo-bsc" is the binary and here it should be OsmoBSC ...) 
PS3, Line 407: 3652121201381481804
maybe rather make it "1" ? otherwise it looks like a meaningful important number 
File src/osmo-bsc/bsc_ctrl_commands.c: 
PS3, Line 555: 	const int red = atoi(value);
atoi() is not sufficient to verify proper input. The value could be *anything*, supplied by an external user. atoi() would return some int value on error, e.g. if the user supplies value = "potatoe" or "1.234".

A proper input value checking is found at e.g.

	errno = 0;
	_mnc = strtol(mnc_str, &endptr, 10);
	if (errno)
		return -errno;
	else if (*endptr)
		return -EINVAL;
	if (_mnc < 0 || _mnc > 999)
		return -ERANGE;

(slightly tweaked the example here to return -errno) 
PS3, Line 584: 	const int red = atoi(cmd->value);
(here atoi() is fine because the verify function should have verified the value already) 
File src/osmo-bsc/bts_osmobts.c: 
PS3, Line 61: 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
(usually no spaces in type cast) 
PS3, Line 65: 	dh->chan_nr = RSL_CHAN_BCCH;
in the commit log you're saying this is abusing the message?
maybe some comment here would be nice to indicate that it is an osmocom specific coding? 
File tests/osmo-bsc.vty: 
PS3, Line 49:   c0-power-reduction             BCCH carrier power reduction operation
i would recommend also adding tests for the parameters in the way done below, e.g. 'bts 0 c0-power-reduction ?' to verify that the argument doc strings are matched up correctly

To view, visit
To unsubscribe, or for help writing mail filters, visit

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I047fce33d4d3e4c569dd006ba17858467a2f4783
Gerrit-Change-Number: 24777
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <vyanitskiy at>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at>
Gerrit-Reviewer: neels <nhofmeyr at>
Gerrit-CC: pespin <pespin at>
Gerrit-Comment-Date: Fri, 02 Jul 2021 15:52:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <>

More information about the gerrit-log mailing list