Attention is currently required from: pespin.

fixeria has posted comments on this change by pespin. ( 
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37923?usp=email )

Change subject: erab_fsm: Abort and reply call with error if unable to 
establish PFCP sess
......................................................................


Patch Set 1:

(2 comments)

File src/erab_fsm.erl:

https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37923/comment/b294973f_390ac223?usp=email
 :
PS1, Line 176: next_state, ?FUNCTION_NAME, S, %% loop transition to enable 
state_timeout
> Yes, that sounded weird to me at the time when you said so. […]
Ok, I see you have also changed the loop transition to `keep_state` in the 
`pfcp_peer` (Ic17c1c7210ab4ae0ddc840a5ffd13b7cdda243c9). I gave it a try and 
indeed it's working as expected: the peer re-tries association.

This is weird, because I spent quite some time debugging and figuring out why 
state timeout does not work, e.g. here: 
https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37265. Maybe it was a bug of 
older Erlang/OTP version, or this problem occurs only under some conditions.

I would keep it as-is for now, until we know more about this problem. Strictly 
speaking, changing from `next_state` to `keep_state` is not needed to get 
things done in this patch. You're just doing this by the way since you're 
touching the related code path, but this is clearly not a must in this case.


https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37923/comment/763ea469_5e49a624?usp=email
 :
PS1, Line 179: {stop_and_reply,
(cosmetic) Also, please move the case body to the next line for consistency 
with the existing code (like done above).

```
{error, Reason} ->
    {stop_and_reply, ...
```



--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37923?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: If23881205b995e76647289d5e6c1c85d02f93b47
Gerrit-Change-Number: 37923
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Mon, 26 Aug 2024 04:21:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <[email protected]>
Comment-In-Reply-To: fixeria <[email protected]>

Reply via email to