sgsnemu ignoring Request accepted

Submitted by Viktor Tsymbalyuk on Jan. 24, 2018, 1:37 p.m.

Details

Message ID 3d21d66d-53a4-f2dd-1e53-64767b267548@gmail.com
State New
Series "sgsnemu ignoring Request accepted"
Headers show

Commit Message

Viktor Tsymbalyuk Jan. 24, 2018, 1:37 p.m. 
Hello,
I continue to play with sgsnemu, and found that it stopped for some 
reason after "Request accepted".
Does sgsnemu working?
At my mind problem inside `create_pdp_conf` function . After small 
changes, I start to see icmp inside gtp tunnel in trace.
Please check attachments with console output, patch and pcap. May be I 
has wrong understanding.

Patch hide | download patch | download mbox 

diff --git a/sgsnemu/sgsnemu.c b/sgsnemu/sgsnemu.c
index bb55b1c..ffe9293 100644
--- a/sgsnemu/sgsnemu.c
+++ b/sgsnemu/sgsnemu.c
@@ -1400,7 +1400,7 @@  static int create_pdp_conf(struct pdp_t *pdp, void *cbp, int cause)
 		return EOF;	/* Not what we expected */
 	}
 
-	if (in46a_from_eua(&pdp->eua, &addr)) {
+	if (in46a_from_eua(&pdp->eua, &addr)<=0) {
 		printf
 		    ("Received create PDP context response. Cause value: %d\n",
 		     cause);

Comments

Pau Espin Pedrol Jan. 24, 2018, 3:18 p.m. 
Hi Viktor,

indeed, you found a bug I introduced in osmo-ggsn 
2d6a69e69a4b4cb2b8cc63c4810dae44e5a4d8f6, sorry for that, I didn't 
notice it since I was testing it with osmo-sgsn instead of using sgsnemu.

You patch is fine, but I'd argue it would be more correct to use "< 1" 
for the condition since it now returns the amount of addresses parsed 
(or -1 in case of error). And better add whitespace in the comparison.

Can you please submit the patch to gerrit? Add a reference to its 
description:
Fixes: 2d6a69e69a4b4cb2b8cc63c4810dae44e5a4d8f6 ("Add support for IPv4v6 
End User Addresses")

You can add me as reviewer.

Also please note that sgsnemu doesn't yet support IPv4v6 EUAs as opposed 
to osmo-ggsn. I only added support for it (in the commit introducing the 
bug) because modems in osmo-gsm-tester were using that one and I needed 
to support it in osmo-ggsn to have the tests working. As I never used 
sgsnemu before, I only patched it to account for the changes in libgtp 
but without adding the support for the new feature there.

Regards,
Viktor Tsymbalyuk Jan. 25, 2018, 2:52 p.m. 
Hello Pau,

Thank you for detailed answer.
 > Can you please submit the patch to gerrit?
yes, I'll do it. But please give me some time. gerrit - is a new 
challenge for me))))

Thank you.
Pau Espin Pedrol Jan. 25, 2018, 2:56 p.m. 
Hi Viktor,

You can find all the related gerrit information in 
https://osmocom.org/projects/cellular-infrastructure/wiki/Gerrit

Feel free to improve if you see something wrong/missing. If you lack 
wiki edit permissions, ask for them in IRC.
Viktor Tsymbalyuk Jan. 26, 2018, 11:05 a.m. 
Hello Pau,

Thank you for the link. I've done it.

Thank you.
  --
Viktor

On 2018-01-25 16:56, Pau Espin Pedrol wrote:
> Hi Viktor,
>
> You can find all the related gerrit information in 
> https://osmocom.org/projects/cellular-infrastructure/wiki/Gerrit
>
> Feel free to improve if you see something wrong/missing. If you lack 
> wiki edit permissions, ask for them in IRC.
>