[PATCH] NAT: reload BSCs config dynamically

Holger Freyther holger at freyther.de
Tue Apr 5 21:08:58 UTC 2016


> On 04 Apr 2016, at 16:06, msuraev at sysmocom.de wrote:
> 
> 
> @@ -1321,8 +1321,8 @@ static int ipaccess_bsc_read_cb(struct osmo_fd *bfd)
> 			     bsc->cfg ? bsc->cfg->nr : -1);
> 		else
> 			LOGP(DNAT, LOGL_ERROR,
> -			     "Stream error on BSC Nr: %d. Failed to parse ip access message: %d\n",
> -			     bsc->cfg ? bsc->cfg->nr : -1, ret);
> +			     "Stream error on BSC Nr: %d. Failed to parse ip access message: %d (%s)\n",
> +			     bsc->cfg ? bsc->cfg->nr : -1, ret, strerror(-ret));

good! but separate patch. And have a look at where ret is coming from that you need to invert it.



> 
> 	bsc_replace_string(_nat, &_nat->include_file, argv[0]);
> +
> +	llist_for_each_entry_safe(con1, con2, &_nat->bsc_connections,
> +				  list_entry) {
> +		if (con1->cfg)
> +			if (con1->cfg->token_updated || con1->cfg->remove)
> +				bsc_close_connection(con1);
> +	}

In terms of style the following is preferred as it is reducing the level of indentation and makes you see what the code is about (instead of shifting it to the right)

		if (!con1->cfg)
			continue;
		if (!con->cfg->token_updated && !con1->cfg->remove)
			continue;

		bsc_close_connection(con1);


resulting assembly should not be any different and the ultimate action is seen.


> +
> +	llist_for_each_entry_safe(cf1, cf2, &_nat->bsc_configs, entry) {
> +		if (cf1->remove) {
> +			bsc_config_free(cf1);
> +			_nat->num_bsc--;

that one is a "nasty" topic

1.) this should be within  bsc_config_free
2.) 

bsc_config_alloc(..) {
	conf->nr = nat->num_bsc;

and the code assumes that conf->nr is unique so if we do num_bsc-- and then num_bsc++ again we might re-use an existing id and break the assumption..

What I propose (and in a separate patch)

* Remove the requirement to assign BSCs in order. I thought I had lifted it a long time ago (but I did not)

* Change bsc_config_alloc to include the nr to be assigned

* change cfg_bsc_mcd to first search the given BSC and then create one..



holger


More information about the OpenBSC mailing list