wmnsk / go-pfcp

PFCP (Packet Forwarding Control Protocol) in pure Go.
MIT License
124 stars 54 forks source link

[Question] About always calling ParseMultiIEs(i.Payload) in helper function #120

Closed tim-ywliu closed 11 months ago

tim-ywliu commented 1 year ago

I found many helper functions will ParseMultiIEs(i.Payload) directly even it was already parsed. Fox example:

func (i *IE) CreatePDR() ([]*IE, error) {
    if i.Type != CreatePDR {
        return nil, &InvalidTypeError{Type: i.Type}
    }

    return ParseMultiIEs(i.Payload)
}

I think it can enhance performance to add ChildIEs length checking to determine if this IE was parsed.

func (i *IE) CreatePDR() ([]*IE, error) {
    if i.Type != CreatePDR {
        return nil, &InvalidTypeError{Type: i.Type}
    }
    if len(i.ChildIEs) > 0 {
        return i.ChildIEs
    }
    return ParseMultiIEs(i.Payload)
}

Or something I missed?

wmnsk commented 1 year ago

sounds legit, but let me have a closer look into codes, as I don't actually remember what was the idea behind calling ParseMultiIEs here TBH :sweat_smile:

wmnsk commented 1 year ago

I agree, in most real use-cases the child IEs are already parsed it seems, and your idea can improve performance. Can you work on it? Otherwise I'll do it in my spare time (with not-so-high priority).

wmnsk commented 11 months ago

Clarified how to handle grouped IEs efficiently in README. I will leave these <IE-Name>() methods as they are for consistency/for specific purpose. What you proposed is implemented as ValueAsGrouped.

https://github.com/wmnsk/go-pfcp#retrieving-values-from-ies

tim-ywliu commented 11 months ago

Thanks!