Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve parseCDR error and add concatenated CDRs update #37

Merged
merged 2 commits into from
Dec 16, 2024
Merged

fix: resolve parseCDR error and add concatenated CDRs update #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ceb86a
fix: resolve parseCDR error and add concatenated CDRs update
TungChiaMing Nov 12, 2024
0ad4e92
fix: CI test on resolve parseCDR error
TungChiaMing Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions cdr/asn/ber_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ func TestMarshal(t *testing.T) {
{"intTest1", 10, "02010a", ""},
{"intTest2", 127, "02017f", ""},
{"intTest3", 128, "02020080", ""},
{"intTest4", -128, "020180", ""},
{"intTest5", -129, "0202ff7f", ""},
Comment on lines -58 to -59
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these two test cases be deleted?

Copy link
Contributor Author

@TungChiaMing TungChiaMing Nov 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I remove 2’s complement conversion so these two test cases will not be passed.

The function parseInt64 in the file cdr/asn/ber_unmarshal.go originally handles the 2’s complement conversion to deal with the negative value.
If the first byte value greater than 127, the function will convert the input to a negative number.

This will eventually cause the parseCDR error because the next value will become negative.
The byte value here is the cdrBytes, which is from the asn.BerMarshalWithParams with input CHF ChargingRecord. (Actually, removing the 2's complement conversion may have issue in the online charging because the value may become negative but I didn't notice it.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing the 2's complement conversion may have issue in the online charging because the value may become negative but I didn't notice it.

This means that the online charging CDR file is not correct if we merge this PR?

Copy link
Contributor Author

@TungChiaMing TungChiaMing Nov 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, but we’d better test the online charging first.

Copy link
Contributor Author

@TungChiaMing TungChiaMing Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @andy89923,

I have tested the online billing functionality and found no issues. During the testing, I observed that online billing uses the Quota variable to manage the remaining traffic quota rather than directly deducting available traffic from the CDR. As a result, there are no concerns about negative cdrBytes values.

Based on this, we can proceed with merging this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @TungChiaMing ,

Thanks for your testing result!
@ianchen0119 FYI!

{"intTest6", 0, "020100", ""},
{"boolTest1", true, "0101ff", ""},
{"boolTest2", false, "010100", ""},
Expand Down Expand Up @@ -249,8 +247,6 @@ func TestUnmarshal(t *testing.T) {
{"intTest1", newInt(10), "02010a", ""},
{"intTest2", newInt(127), "02017f", ""},
{"intTest3", newInt(128), "02020080", ""},
{"intTest4", newInt(-128), "020180", ""},
{"intTest5", newInt(-129), "0202ff7f", ""},
{"intTest6", newInt(0), "020100", ""},
{"boolTest1", newBool(true), "0101ff", ""},
{"boolTest2", newBool(false), "010100", ""},
Expand Down Expand Up @@ -433,7 +429,7 @@ func TestUnmarshal(t *testing.T) {

func TestParseInt64(t *testing.T) {
testCases := [][]byte{}
origInts := []int64{0, 1, 127, 128, 32767, -128, -129, -32768}
origInts := []int64{0, 1, 127, 128, 32767}

for _, origInt := range origInts {
buf := new(bytes.Buffer)
Expand Down
23 changes: 0 additions & 23 deletions cdr/asn/ber_unmarshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,34 +68,11 @@ func parseInt64(bytes []byte) (r int64, e error) {
return r, e
}

minus := false
if uint(bytes[0]) > 127 {
minus = true
}

for i := 0; i < 8-len(bytes); i++ {
extend_byte := byte(0)
if minus {
extend_byte = byte(255)
}
bytes = append([]byte{extend_byte}, bytes...)
}

if minus {
for i := range bytes {
bytes[i] = ^bytes[i]
}
}

for _, b := range bytes {
r <<= 8
r |= int64(b)
}

if minus {
r = -r - 1
}

return r, e
}

Expand Down
7 changes: 4 additions & 3 deletions cdr/cdrFile/cdrFile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,8 @@ func (cdfFile *CDRFile) Decoding(fileName string) {

// fmt.Println("[Decode]cdrfileheader:\n", cdfFile.Hdr)

tail := n + 2
var tail uint32
tail = uint32(n) + 2

for i := 1; i <= int(numberOfCdrsInFile); i++ {
cdrLength := binary.BigEndian.Uint16(data[tail : tail+2])
Expand All @@ -506,10 +507,10 @@ func (cdfFile *CDRFile) Decoding(fileName string) {

cdr := CDR{
Hdr: cdrHeader,
CdrByte: data[tail+5 : tail+5+cdrLength],
CdrByte: data[tail+5 : tail+5+uint32(cdrLength)],
}
cdfFile.CdrList = append(cdfFile.CdrList, cdr)
tail += 5 + cdrLength
tail += 5 + uint32(cdrLength)
}
// fmt.Println("[Decode]cdrfile:\n", cdfFile)
// fmt.Printf("%#v\n", cdfFile)
Expand Down
2 changes: 2 additions & 0 deletions internal/context/ue_context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type ChfUe struct {
RatingChan chan *diam.Message
RatingType map[int32]charging_datatype.RequestSubType
RateSessionId uint32
Records []*cdrType.CHFRecord

// lock
Cdr map[string]*cdrType.CHFRecord
Expand All @@ -58,6 +59,7 @@ func (ue *ChfUe) init() {
config := factory.ChfConfig

ue.Cdr = make(map[string]*cdrType.CHFRecord)
ue.Records = []*cdrType.CHFRecord{}
ue.VolumeLimit = config.Configuration.VolumeLimit
ue.VolumeLimitPDU = config.Configuration.VolumeLimitPDU
ue.QuotaValidityTime = config.Configuration.QuotaValidityTime
Expand Down
3 changes: 2 additions & 1 deletion internal/sbi/processor/cdr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ func dumpCdrFile(ueid string, records []*cdrType.CHFRecord) error {
cdrfile.Hdr.HeaderLength = uint32(54 + cdrfile.Hdr.LengthOfCdrRouteingFilter + cdrfile.Hdr.LengthOfPrivateExtension)
cdrfile.Hdr.NumberOfCdrsInFile = uint32(len(records))
cdrfile.Hdr.FileLength = cdrfile.Hdr.HeaderLength
logger.ChargingdataPostLog.Traceln("cdrfile.Hdr.NumberOfCdrsInFile:", uint32(len(records)))

for _, record := range records {
cdrBytes, err := asn.BerMarshalWithParams(&record, "explicit,choice")
Expand All @@ -274,7 +275,7 @@ func dumpCdrFile(ueid string, records []*cdrType.CHFRecord) error {
}
cdrfile.CdrList = append(cdrfile.CdrList, tmpCdr)

cdrfile.Hdr.FileLength += uint32(cdrHdr.CdrLength) + 5
cdrfile.Hdr.FileLength += uint32(len(cdrBytes)) + 5
}

cdrfile.Encoding("/tmp/" + ueid + ".cdr")
Expand Down
56 changes: 51 additions & 5 deletions internal/sbi/processor/converged_charging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package processor

import (
"context"
"encoding/json"
"math"
"net/http"
"strconv"
Expand All @@ -13,6 +14,8 @@ import (
"golang.org/x/exp/constraints"

"github.com/fiorix/go-diameter/diam/datatype"
"github.com/free5gc/chf/cdr/asn"
"github.com/free5gc/chf/cdr/cdrConvert"
"github.com/free5gc/chf/cdr/cdrType"
"github.com/free5gc/chf/internal/abmf"
"github.com/free5gc/chf/internal/cgf"
Expand Down Expand Up @@ -186,6 +189,7 @@ func (p *Processor) ChargingDataCreate(chargingData models.ChargingDataRequest)
}

ue.Cdr[chargingSessionId] = cdr
ue.Records = append(ue.Records, ue.Cdr[chargingSessionId])

if chargingData.OneTimeEvent {
err = p.CloseCDR(cdr, false)
Expand Down Expand Up @@ -219,7 +223,6 @@ func (p *Processor) ChargingDataCreate(chargingData models.ChargingDataRequest)
func (p *Processor) ChargingDataUpdate(
chargingData models.ChargingDataRequest, chargingSessionId string,
) (*models.ChargingDataResponse, *models.ProblemDetails) {
var records []*cdrType.CHFRecord

self := chf_context.GetSelf()
ueId := chargingData.SubscriberIdentifier
Expand All @@ -239,6 +242,52 @@ func (p *Processor) ChargingDataUpdate(
responseBody, partialRecord := p.BuildConvergedChargingDataUpdateResopone(chargingData)

cdr := ue.Cdr[chargingSessionId]

if len(ue.Records) > 1 {
cdr = ue.Records[len(ue.Records)-1]
}

cdrBytes, errCdrBer := asn.BerMarshalWithParams(&cdr, "explicit,choice")
if errCdrBer != nil {
logger.ChargingdataPostLog.Error(errCdrBer)
problemDetails := &models.ProblemDetails{
Status: http.StatusBadRequest,
Detail: errCdrBer.Error(),
}
return nil, problemDetails
}

var chgDataBytes []byte
var errChgDataBer error
if chargingData.MultipleUnitUsage != nil && len(chargingData.MultipleUnitUsage) != 0 {
cdrMultiUnitUsage := cdrConvert.MultiUnitUsageToCdr(chargingData.MultipleUnitUsage)
chgDataBytes, errChgDataBer = asn.BerMarshalWithParams(&cdrMultiUnitUsage, "explicit,choice")
if errChgDataBer != nil {
logger.ChargingdataPostLog.Error(errChgDataBer)
problemDetails := &models.ProblemDetails{
Status: http.StatusBadRequest,
Detail: errChgDataBer.Error(),
}
return nil, problemDetails
}
}

if len(cdrBytes)+len(chgDataBytes) > math.MaxUint16 {
var newRecord *cdrType.CHFRecord
cdrJson, err := json.Marshal(cdr)
if err != nil {
logger.ChargingdataPostLog.Error(err)
}
err = json.Unmarshal(cdrJson, &newRecord)
if err != nil {
logger.ChargingdataPostLog.Error(err)
}

newRecord.ChargingFunctionRecord.ListOfMultipleUnitUsage = []cdrType.MultipleUnitUsage{}
cdr = newRecord
ue.Records = append(ue.Records, cdr)
}

err := p.UpdateCDR(cdr, chargingData)
if err != nil {
problemDetails := &models.ProblemDetails{
Expand Down Expand Up @@ -270,10 +319,7 @@ func (p *Processor) ChargingDataUpdate(
"CDR Record Sequence Number after Reopen %+v", *cdr.ChargingFunctionRecord.RecordSequenceNumber)
}

for _, cdr := range ue.Cdr {
records = append(records, cdr)
}
err = dumpCdrFile(ueId, records)
err = dumpCdrFile(ueId, ue.Records)
if err != nil {
problemDetails := &models.ProblemDetails{
Status: http.StatusBadRequest,
Expand Down
Loading