Pcrf json dbi #1

lynxis · 2022-09-12T12:30:10Z

No description provided.

To allow the include to work even when build without mongo installed. No data types require mongoc.h to be in ogs_mongoc.h.

The only user of ogs-mongoc.h are the tests. Add comment to remove the comment as soon they aren't depend on the ogs-mongoc.h

…ends The current dbi only supports a single database backend. Add an abstraction layer to allow multiple database backends. Introduce a dbi backend struct containing all database functions.

…nal() Nobody is calling ogs_mongoc_final() directly. In preparation of making ogs_dbi_final() more generic

Allow database driver to only implement parts of the database driver. In preprataion of the json interface which will only implement session_data as might only be used by the PCRF.

Add json based database backend. TODO: add example json interface

Add the charging characteristics to the dbi function to allow the backend to decide the profile based on it. The charging characteristics can be filled by the HSS and will be passed by the PCEF towards the PCRF. This is an API dbi breakage.

To make use of the new json database backend, allow the pcrf to parse json backend configuration.

Also pass the 3GPP-Charging-Characteristics to the dbi session_data call

The tests won't be build as they depend on mongodb. To build without mongodb do meson configure -Dmongodb=false

pespin · 2022-09-12T17:28:50Z

lib/dbi/ogs-mongoc.h

@@ -24,8 +24,6 @@
 #ifndef OGS_MONGOC_H
 #define OGS_MONGOC_H

-#include <mongoc.h>


I don't think this will be hapilly accepted upstream. A "mongoc" specific header which doesn't include mongoc.h? This looks weird. Looks like you'd want to generalize the API to remove all the "mongo" references, or import this ogs-mongoc.h file based on whether mongo support is enabled.

pespin · 2022-09-12T17:29:46Z

lib/dbi/ogs-dbi.h

@@ -24,7 +24,9 @@

 #define OGS_DBI_INSIDE

+/* TODO: remove ogs-mongoc.h as soon the test cases don't depend on it */


This commit in itself does nothing, so I'm sure you want to merge this into some other commit

pespin · 2022-09-12T17:40:11Z

lib/dbi/ogs-dbi.h

@@ -25,7 +25,7 @@
 #define OGS_DBI_INSIDE

 /* TODO: remove ogs-mongoc.h as soon the test cases don't depend on it */
-#include "dbi/ogs-mongoc.h"
+#include "dbi/mongo/ogs-mongoc.h"


the meson file seems to indicate you make the mongo files optional, but you are including it here non-conditionally?

pespin · 2022-09-12T17:40:16Z

lib/dbi/meson.build

 '''.split())

-libmongoc_dep = dependency('libmongoc-1.0')
+libmongoc_dep = dependency('libmongoc-1.0', required: true)


why is this marked as "required:true" then check if found? Sounds like marking required would make everything fail if not found?

pespin

Please merge this commit with all the previous one, otherwise it's a big set of incomplete steps and it's difficult to figure out what are you doing other than guessing.

pespin

This patch can probably the first since it can be already sent as a separate PR and merge straight away.

pespin · 2022-09-12T17:49:45Z

lib/dbi/dbi.c


 int __ogs_dbi_domain;

 static ogs_dbi_t *dbi_interfaces[] = {
 #ifdef WITH_MONGOC
    &ogs_dbi_mongo_interface,
 #endif
+    &ogs_dbi_json_interface,


so how is one or the other selected for use? I don't recall seeing any explanation anywhere.

pespin · 2022-09-12T17:50:55Z

lib/dbi/dbi-private.h

@@ -34,7 +34,7 @@ struct ogs_dbi_s {
    void (*final)(void);
    /* session */
    int (*session_data)(char *supi, ogs_s_nssai_t *s_nssai, char *dnn,
-	ogs_session_data_t *data);
+            int32_t charging_char, ogs_session_data_t *data);


since it's more like a bitmask, maybe makes more sense to use uint32_t?

pespin · 2022-09-12T17:52:20Z

lib/dbi/session.h

@@ -28,8 +28,10 @@
 extern "C" {
 #endif

+#define OGS_DBI_NO_CHARGING_CHAR -1


so the peer cannot send an all 0xff value meaning something different?
Or it's because ChargingChracteristics is 16bit?

pespin · 2022-09-12T17:56:09Z

configs/pcrf-no-mongodb/pcrf-default.json

@@ -0,0 +1,17 @@
+[


TBH it looks a bit weird that being all the config files yaml this is done in JSON. I forsee this will be NACKed upon review. If I were you I'd move this to yaml, drop cJSON and even incorporate this as a node in pcrf.cfg

pespin · 2022-09-12T17:57:54Z

src/pcrf/pcrf-context.c

@@ -267,7 +267,7 @@ int pcrf_context_parse_config(void)
 }

 int pcrf_db_qos_data(
-        char *imsi_bcd, char *apn, ogs_session_data_t *session_data)
+        char *imsi_bcd, char *apn, int32_t charging_char, ogs_session_data_t *session_data)


If ChargingChar is 16bit, did you think about passing a "uint16_t *" ptr and pass it NULL if none is to be used?

I think having a int32 is simpler than passing a pointer.

pespin · 2022-09-12T18:02:40Z

lib/dbi/mongo/dummy.c

+/* This file will be used when build without mongodb support to support the old api */
+#include "ogs-dbi.h"
+
+int ogs_dbi_init(const char *db_uri)


why is this needed? This commint looks like needs to be meregd with some of the previous commits doing stuff with the API.

Do you know if open5gs cares about so library versions and compatibility?
The dummy functions are needed when building without mongdb. dbi is still a shared library.

github-actions · 2023-06-07T22:32:09Z

As there has been no recent activity on this PR, it has been marked as stale. It will be automatically closed if no further action is taken.

lynxis added 12 commits September 11, 2022 19:29

[dbi]: ogs_mongoc.h: remove include <mongoc.h>

c72f41f

To allow the include to work even when build without mongo installed. No data types require mongoc.h to be in ogs_mongoc.h.

[dbi]: ogs-dbi.h: add comment to the include of ogs-mongoc.h

8b3abcc

The only user of ogs-mongoc.h are the tests. Add comment to remove the comment as soon they aren't depend on the ogs-mongoc.h

[dbi]: move all mongo related files into subdirectory

68efeac

[dbi]: introduce an abstraction layer to allow multiple database back…

9c66348

…ends The current dbi only supports a single database backend. Add an abstraction layer to allow multiple database backends. Introduce a dbi backend struct containing all database functions.

[dbi]: mongo: merge all parts from ogs_dbi_final() into ogs_mongoc_fi…

f0c97bf

…nal() Nobody is calling ogs_mongoc_final() directly. In preparation of making ogs_dbi_final() more generic

[dbi]: make ogs_dbi_final depend on the selected database driver

0e0de17

[dbi]: make all db functions optional

c54fb59

Allow database driver to only implement parts of the database driver. In preprataion of the json interface which will only implement session_data as might only be used by the PCRF.

[dbi] add database backend json

0d6ef1e

Add json based database backend. TODO: add example json interface

[app/pcrf]: parse json database configuration

1a6ceca

To make use of the new json database backend, allow the pcrf to parse json backend configuration.

[pcrf]: Gx: parse 3GPP Charging Characteristic

8960372

Also pass the 3GPP-Charging-Characteristics to the dbi session_data call

[dbi]: allow to build without mongodb

1151c6a

The tests won't be build as they depend on mongodb. To build without mongodb do meson configure -Dmongodb=false

pespin reviewed Sep 12, 2022

View reviewed changes

pespin approved these changes Sep 12, 2022

View reviewed changes

pespin reviewed Sep 12, 2022

View reviewed changes

github-actions bot added the Stale label Jun 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pcrf json dbi #1

Pcrf json dbi #1

lynxis commented Sep 12, 2022

pespin Sep 12, 2022

pespin Sep 12, 2022

pespin Sep 12, 2022

pespin Sep 12, 2022

pespin left a comment

pespin left a comment

pespin Sep 12, 2022

pespin Sep 12, 2022

pespin Sep 12, 2022

pespin Sep 12, 2022

pespin Sep 12, 2022

lynxis Sep 20, 2022

pespin Sep 12, 2022

lynxis Sep 20, 2022

github-actions bot commented Jun 7, 2023

		@@ -24,7 +24,9 @@

		#define OGS_DBI_INSIDE

		/* TODO: remove ogs-mongoc.h as soon the test cases don't depend on it */

Pcrf json dbi #1

Are you sure you want to change the base?

Pcrf json dbi #1

Conversation

lynxis commented Sep 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pespin left a comment

Choose a reason for hiding this comment

pespin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 7, 2023