search

uw-ictd / colte

Community LTE Project
MIT License
59 stars 27 forks source link

change LTE subnet addresses written to smf.conf and upf.conf #94

Closed spencersevilla closed 3 years ago

spencersevilla commented 3 years ago

The default values for subnet in the open5gs conf files pertaining to the LTE subnet (smf.conf and upf.conf) use the first valid address in the subnet (e.g. 10.45.0.1/16), whereas colteconf writes the subnet notation directly (e.g. 10.45.0.0/16). I am not sure if this breaks anything or not, but consistency is good.

matt9j commented 3 years ago

Is one form more correct than the other? I would actually prefer to not take this change if it works otherwise, since it makes it more clear to trace the value through the config files, and clear that this is indeed a subnet specification rather than an address. systemd-networkd is much more particular about wanting a subnet specification ending in 0, so the entry in colte/config.yaml needs to actually use the 10.45.0.0/16 style notation if it's going to be propagated through to the ogstun config directly.

spencersevilla commented 3 years ago

sorry, ill try to be more clear. i agree about systemd-networkd (which is why our value in colte/config.yml needs to be e.g. 10.45.0.0/16). This is a change to the code that takes that value and writes it into smf.conf and upf.conf, because they seem to want 10.45.0.1/16 but are getting 10.45.0.0/16. Does that make sense?

matt9j commented 3 years ago

It does! All I'm trying to say is that I think the use of 10.45.0.1/16 in the example conf files is arbitrary. If open5gs works with 10.45.0.0/16 I would have a slight preference for keeping our scripts simpler. Right now by adding this we do more magic work and add an edge case. I'm happy to merge though too if you would prefer it this way.

spencersevilla commented 3 years ago

Ah, I understand you now. Good point, let's wait on merging until I can confirm this is actually a problem.

matt9j commented 3 years ago

Any update if this is an issue in practice?

spencersevilla commented 3 years ago

hmmm. i'm torn. on one hand, it appears to be fine for both smf and upf, but I still don't think this is best practice. I dug through the code to understand what was going on, and when it reads/parses this value, it does two things. first it stores the actual address listed in addr as belonging to the "gateway", and then it does some calculations to create the "subnet" and stores that separately (used for things like UE address allocation etc). the code works fine for both smf and upf because even though they're storing a wrong value under "gateway", that value is never used. i'm worried that if code down the line ever decides to look at this value to do something related to its own ip address, itll be a bug.

matt9j commented 3 years ago

Okay, that seems reasonable. We can merge if that's the case. It's unfortunate that the word subnet is used in the open5gs config, without a clear distinction between the gateway addr and the subnet pool range. Assuming the gateway is always on 1 seems brittle, although I know putting it there is the usual common practice : )