Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: add jhash.h, copied from linux/jhash.h
......................................................................
add jhash.h, copied from linux/jhash.h
Allow using arbitrary length data as hashtable key:
Copy jhash.h implementation from the linux kernel.
Apply osmo_ prefix to all global symbols.
Add jhash_test to ensure the code import works as intended.
First application will be a hashtable keyed by umts_cell_id in
osmo-hnbgw.git.
Related: SYS#6773
Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
---
M include/osmocom/core/Makefile.am
A include/osmocom/core/jhash.h
M tests/Makefile.am
A tests/jhash/jhash_test.c
A tests/jhash/jhash_test.ok
M tests/testsuite.at
6 files changed, 284 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/13/36913/5
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
Gerrit-Change-Number: 36913
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email )
Change subject: add jhash.h, copied from linux/jhash.h
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS3:
> @pespin do you still have the exact source of the __get_unaligned_cpu32() snippet? I want to add a l […]
nevermind, found it
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
Gerrit-Change-Number: 36913
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 May 2024 14:28:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: add jhash.h, copied from linux/jhash.h
......................................................................
add jhash.h, copied from linux/jhash.h
Allow using arbitrary length data as hashtable key:
Copy jhash.h implementation from the linux kernel.
Apply osmo_ prefix to all global symbols.
Add jhash_test to ensure the code import works as intended.
First application will be a hashtable keyed by umts_cell_id in
osmo-hnbgw.git.
Related: SYS#6773
Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
---
M include/osmocom/core/Makefile.am
A include/osmocom/core/jhash.h
M tests/Makefile.am
A tests/jhash/jhash_test.c
A tests/jhash/jhash_test.ok
M tests/testsuite.at
6 files changed, 281 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/13/36913/4
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
Gerrit-Change-Number: 36913
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email )
Change subject: add jhash.h, copied from linux/jhash.h
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
@pespin do you still have the exact source of the __get_unaligned_cpu32() snippet? I want to add a license identifier for it...
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
Gerrit-Change-Number: 36913
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 May 2024 14:16:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: add jhash.h, copied from linux/jhash.h
......................................................................
add jhash.h, copied from linux/jhash.h
Allow using arbitrary length data as hashtable key:
Copy jhash.h implementation from the linux kernel.
Apply osmo_ prefix to all global symbols.
Add jhash_test to ensure the code import works as intended.
First application will be a hashtable keyed by umts_cell_id in
osmo-hnbgw.git.
Related: SYS#6773
Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
---
M include/osmocom/core/Makefile.am
A include/osmocom/core/jhash.h
M tests/Makefile.am
A tests/jhash/jhash_test.c
A tests/jhash/jhash_test.ok
M tests/testsuite.at
6 files changed, 280 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/13/36913/3
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
Gerrit-Change-Number: 36913
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email )
Change subject: add jhash.h, copied from linux/jhash.h
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> I'm not sure either. […]
i think it is fine to leave it inline, because we don't always inlcude jhash.h -- only in the .c file that actually does the hashing:
our linuxlist.h and msgb.h are all over the place, so that code gets recompiled over and over. They are very complex, so we always need all of the .h file.
in comparison, jhash.h is only needed in the .c file that implements the my_foo_hash(cont struct my_foo *val) function. IOW it is easy to contain the recompilations, not like in linuxlist and msgb with an entire API that is "always" needed.
agree?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
Gerrit-Change-Number: 36913
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 May 2024 14:12:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email )
Change subject: add jhash.h, copied from linux/jhash.h
......................................................................
Patch Set 2:
(1 comment)
File include/osmocom/core/jhash.h:
https://gerrit.osmocom.org/c/libosmocore/+/36913/comment/7a707006_fe4bf5b8
PS1, Line 100: c += osmo_load32le(k + 8);
> ... i couldn't find it =) ... […]
ah but load32le() is about always reading little endian, no matter what CPU,
and get_unaligned_cpu32 is about always reading simply the CPU byte order, right?
So osmo_get_unaligned_cpu32() is a legitimate addition and is not a dup of osmo_load32le(), correct? i guess then i'll indeed add the osmo_get_unaligned_cpu32()...
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
Gerrit-Change-Number: 36913
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 May 2024 13:52:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email )
Change subject: add jhash.h, copied from linux/jhash.h
......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2:
> I'm not saying we should do it one way or another, just raising the topic here.
I'm not sure either. We have large parts of msgb, the entire linuxlist and hashtable in static inline, so i guessed that's kind of the way to go.
(maybe it compiles to more efficient code when inline??)
File include/osmocom/core/jhash.h:
https://gerrit.osmocom.org/c/libosmocore/+/36913/comment/074769d2_190c232e
PS1, Line 100: c += osmo_load32le(k + 8);
> Why don't you simply copy the following code in this same file instead of changing it? […]
... i couldn't find it =) ...
I guess we should *replace* our osmo_load32le with the above for LE machines?
current (generated) osmo_load32le is implemented with a loop:
```
static inline uint32_t osmo_load32le_ext(const void *p, uint8_t n)
{
uint8_t i;
uint32_t r = 0;
const uint8_t *q = (uint8_t *)p;
OSMO_ASSERT(n <= sizeof(r));
for(i = 0; i < n; r |= ((uint32_t)q[i] << (8 * i)), i++);
return r;
}
```
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa
Gerrit-Change-Number: 36913
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 May 2024 13:48:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment