osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-iuh/+/30088 )
Change subject: asn1tostruct: fix defines getting redefined
......................................................................
asn1tostruct: fix defines getting redefined
Patch asn1tostruct.py, so it does not write the same define twice for
two fields in a struct, for example:
#define ENHANCEDRELOCATIONCOMPLETEREQUESTIES_RANAP_EXTENDEDRNC_ID_PRESENT (1 << 0)
#define ENHANCEDRELOCATIONCOMPLETEREQUESTIES_RANAP_EXTENDEDRNC_ID_PRESENT (1 << 1)
#define ENHANCEDRELOCATIONCOMPLETEREQUESTIES_RANAP_RAB_SETUPLIST_ENHANCEDRELOCCOMPLETEREQ_PRESENT (1 << 2)
typedef struct RANAP_EnhancedRelocationCompleteRequestIEs_s {
uint16_t presenceMask;
RANAP_IuSignallingConnectionIdentifier_t oldIuSigConId;
RANAP_IuSignallingConnectionIdentifier_t iuSigConId;
RANAP_GlobalRNC_ID_t relocation_SourceRNC_ID;
RANAP_ExtendedRNC_ID_t relocation_SourceExtendedRNC_ID; ///< Optional field
RANAP_GlobalRNC_ID_t relocation_TargetRNC_ID;
RANAP_ExtendedRNC_ID_t relocation_TargetExtendedRNC_ID; ///< Optional field
RANAP_RAB_SetupList_EnhancedRelocCompleteReq_t raB_SetupList_EnhancedRelocCompleteReq; ///< Optional field
} RANAP_EnhancedRelocationCompleteRequestIEs_t;
The problem is that the type is used and it may not be unique inside a
struct. Change the code to use the name of the field if the type is not
unique. Keep using the type otherwise so existing code doesn't need to
be modified a lot to fix this.
Fix for:
../include/osmocom/ranap/ranap_ies_defs.h:514: warning: "RANAP_ENHANCEDRELOCATIONINFORMATIONREQUESTIES_RANAP_IUSIGNALLINGCONNECTIONIDENTIFIER_PRESENT" redefined
Change-Id: I2ecae6789899952d1dc5691ab76907abeaa71c12
---
M asn1/utils/asn1tostruct.py
1 file changed, 24 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/88/30088/1
diff --git a/asn1/utils/asn1tostruct.py b/asn1/utils/asn1tostruct.py
index 220517d..988f7f3 100755
--- a/asn1/utils/asn1tostruct.py
+++ b/asn1/utils/asn1tostruct.py
@@ -79,6 +79,29 @@
print("-p [pfx] Prefix all types with given prefix")
print("-h Print this help and return")
+def getUniqueIENameForDefine(ies, ie):
+ """ Usually the type of the IE is used for defines. However a struct may
+ use the same type multiple times, in that case we use the actual name
+ of the field. """
+ unique = True
+
+ for ie_other in ies:
+ if ie_other == ie:
+ continue
+ if ie_other[2] == ie[2]:
+ unique = False
+ assert ie[0] != ie_other[0], "failed to find a unique name for" \
+ f" IE {ie}. Found another entry in ies {ie_other}" \
+ " that has the same ie[0] value."
+
+ if unique:
+ ret = ie[2]
+ else:
+ ret = ie[0]
+
+ ret = re.sub('-', '_', ret.upper())
+ return ret
+
try:
opts, args = getopt.getopt(sys.argv[1:], "df:ho:p:", ["debug", "file", "help", "outdir", "prefix"])
except getopt.GetoptError as err:
@@ -167,7 +190,7 @@
# Presence mask
for ie in iesDefs[key]["ies"]:
- ieupperunderscore = re.sub('-', '_', ie[2].upper())
+ ieupperunderscore = getUniqueIENameForDefine(iesDefs[key]["ies"], ie)
if ie[3] == "optional" or ie[3] == "conditional":
f.write("#define {0:<{pad}} {1}\n".format("%s_%s%s_PRESENT" % (keyupperunderscore, prefix, ieupperunderscore), "(1 << %d)" % shift,
pad=iesDefs[key]["length"] + len(keyupperunderscore) + 9))
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/30088
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I2ecae6789899952d1dc5691ab76907abeaa71c12
Gerrit-Change-Number: 30088
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newchange
osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-iuh/+/30080 )
Change subject: move-asn1-header-files: error for wrong asn1c
......................................................................
move-asn1-header-files: error for wrong asn1c
Add a helpful error message if the user doesn't have the expected asn1c
version installed. Link to a new wiki page that explains which branch
one needs to build.
Without this patch, it just complains that HNBAP_Criticality.h can't be
moved.
Change-Id: I11980bf868055bccc05c5338ea330b70022cd874
---
M move-asn1-header-files.sh
1 file changed, 17 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/80/30080/1
diff --git a/move-asn1-header-files.sh b/move-asn1-header-files.sh
index ec94e24..a0e57da 100755
--- a/move-asn1-header-files.sh
+++ b/move-asn1-header-files.sh
@@ -12,6 +12,22 @@
include_subdir="$1"
shift
+# Abort if any of the header files are missing
+for i in "$@"; do
+ if ! [ -e "$i" ]; then
+ echo
+ echo "ERROR: file '$i' has not been generated by asn1c!"
+ echo
+ echo "Are you using the Osmocom asn1c fork with the" \
+ "ASN1C_PREFIX feature?"
+ echo
+ echo "More information:"
+ echo "https://osmocom.org/projects/osmohnbgw/wiki/Generate_sources_from_ASN1_files"
+ echo
+ exit 1
+ fi
+done
+
include_dir="$base_dir/include/$include_subdir"
mkdir -p "$include_dir"
echo "$PWD/*.h --> $include_dir"
@@ -27,6 +43,7 @@
}
move_headers() {
+ echo "pwd: $PWD"
echo mv $@ "$include_dir/"
mv $@ "$include_dir/"
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/30080
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I11980bf868055bccc05c5338ea330b70022cd874
Gerrit-Change-Number: 30080
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: neels, laforge, fixeria, pespin.
lynxis lazus has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29788 )
Change subject: Add BTS setup ramping to prevent BSC overloading
......................................................................
Patch Set 16:
(7 comments)
File include/osmocom/bsc/bts_setup_ramp.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/5881e245_9af48487
PS3, Line 31: BTS_SETUP_RAMP_INIT, /** initial state */
> I am surprised it took so many iterations, but still does not match what Neels actually suggested. […]
One is just wrong. doxygen doesn't accept a single one. Or maybe you're using a different version or config of doxygen.
It's either /*! (java doc) or /**< but not /*< according
https://www.doxygen.nl/manual/docblocks.html#specialblock
I've tested it with doxygen using the config from libosmocore.
I've switched to /*!, but honestly we should have a linter yelling here as well.
File src/osmo-bsc/bsc_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/275d9f81_3c967533
PS4, Line 3100: bts-setup-ramping-step-interval
> I am sorry, but no. This should be addressed before merging.
As pespin proposed I followed the example of access-control-class-ramping and be consistent with access-control-class-ramping. I don't want to do more review rounds with different taste and style between different reviewers.
A proper solution for this would be a vty style guide including good and bad examples.
File src/osmo-bsc/bts_setup_ramp.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/4f4a3b8e_c9a0262f
PS4, Line 98: && net->bts_setup_ramp.step_size > 0
> Ack
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/a7d62928_386b6fd1
PS4, Line 101: net->bts_setup_ramp.active
> Ack
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/225ab2d6_e9c3bcaa
PS4, Line 201: INIT_LLIST_HEAD(&bts->bts_setup_ramp.list);
> The Linux kernel llist API has always worked in the way that you cannot llist_del() a llist_entry th […]
Maybe I missed the point. But I don't see any issue with the current implementation nor do I understand why it needs to be an explicit comment in here.
File src/osmo-bsc/bts_setup_ramp.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/7d943aaf_9af8c4e2
PS15, Line 51: if (llist_empty(&bts->bts_setup_ramp.list))
> Why don't you check the bts->bts_setup_ramp.state against BTS_SETUP_RAMP_WAIT here? […]
good point. only possible after patchset14
https://gerrit.osmocom.org/c/osmo-bsc/+/29788/comment/4a93f6bd_846517e9
PS15, Line 74: net->bts_setup_ramp.count >= net->bts_setup_ramp.step_size)
> Ack
Done
--
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: 16
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 11 Nov 2022 14:53:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-MessageType: comment
Attention is currently required from: fixeria, pespin.
msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/30041 )
Change subject: Add realtime scheduling and set priority in service file
......................................................................
Patch Set 2:
(1 comment)
File contrib/systemd/osmo-mgw.service:
https://gerrit.osmocom.org/c/osmo-mgw/+/30041/comment/880b4dc5_4a7113d0
PS2, Line 13: 1 (lowest priority
> Is it ok for osmo-mgw to have the "lowest priority"?
Why not? That's the conclusion in https://osmocom.org/issues/5687 so far.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/30041
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib1705a164b04b876f129a17c4e8353b9ddcc538e
Gerrit-Change-Number: 30041
Gerrit-PatchSet: 2
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 11 Nov 2022 14:42:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment