fix(mme): Fix network address in create bearer packet filter #12038
Conversation
|
Thanks for opening a PR! 💯
Howto
More infoPlease take a moment to read through the Magma project's
If this is your first Magma PR, also consider reading
|
|
Oops! Looks like you failed the Howto
♻️ Updated: ✅ The check is passing the Python Format Check after the last commit. |
|
We do have some unit test coverage for the parse function. Is it worth adding the previously failing test case here? https://sourcegraph.com/github.com/magma/magma/-/blob/lte/gateway/c/core/oai/test/spgw_task/test_spgw_service_impl.cpp?L27 or maybe it is the fill function that is in question here? I do not know, so I will defer to @LKreutzer who added the unit tests for this :) |
There was a problem hiding this comment.
LGTM. @themarwhal we will add some test cases (on Monday as I am travelling)!
@LKreutzer thanks! sounds great :) Safe travels! |
ssanadhya
added
backport-v1.7
Agree with @themarwhal , it will be good to add test coverage for the fill function here. I did not modify the parsing function since it is working correctly as a parser in this case. The logic has to be vetted in the fillIpv4 function. Thanks for taking this up @LKreutzer |
Signed-off-by: Shruti Sanadhya <ssanadhya@fb.com>
Signed-off-by: Shruti Sanadhya <ssanadhya@fb.com>
Signed-off-by: Shruti Sanadhya <ssanadhya@fb.com>
Signed-off-by: Shruti Sanadhya <ssanadhya@fb.com>
ssanadhya
force-pushed
the
maxbearer_twopdn_fix
branch
from
3a119a9
to
60b3f5c
Compare
* fix(mme): Fix network address in create bearer packet filter Signed-off-by: Shruti Sanadhya <ssanadhya@fb.com>
…2038) * fix(mme): Fix network address in create bearer packet filter Signed-off-by: Shruti Sanadhya <ssanadhya@fb.com>
Signed-off-by: Shruti Sanadhya ssanadhya@fb.com
Summary
The test case test_attach_detach_maxbearers_twopdns.py started failing after the changes in b4fc690 .
The new
parseIpv4Networklogic behaves differently fromfolly::IPAddress::tryCreateNetworkwhen the input address is of the form "192.168.129.42/16". Specifically, the output is:Since the default downlink flow rules in
s1ap_utils.pyinclude rules for "192.168.129.42/32" and "192.168.129.42/16", the above logic , in addition with the overlapping port number range (as highlighted in #11969) , was causing an overlap on flow rules. This was causing the number of created rules to be 28, while the expected count was 33 ( = 1 default + 8 bearers * 4 rules_per_bearer).This can be seen by dumping all the DL flows from OVS for
test_attach_detach_maxbearers_twopdns.pyon:2d30661 , which shows 33 downlink flows
b4fc690, which shows 28 downlink flows
The changes in #11969 circumvent the issue by expanding the port number range so that the flow rules do not overlap, but the IP address match is still incorrect.
This change:
fillIpv4to treat the IP address returned fromparseipv4networkas a string and apply the mask when copying that address into a packet filter.s1ap_utils.pyto print the number of DL flows created vs expectedTest Plan
test_attach_detach_maxbearers_twopdns.pyBefore the change, MME Debug log reports:
After the change
make integ_test