Change in libosmo-abis[master]: add ipa ping/pong keepalive for OML/RSL links between bts and bsc

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

osmith gerrit-no-reply at lists.osmocom.org
Tue Jan 21 14:57:15 UTC 2020


osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/14743 )

Change subject: add ipa ping/pong keepalive for OML/RSL links between bts and bsc
......................................................................


Patch Set 8: Code-Review-2

(10 comments)

After assigning #4070 to myself, Harald pointed me at this WIP patch. I've fixed up the cosmetics and added a no-VTY command as suggested in fixeria's review.

Set to -2, because it's segfaulting for me when testing with real hardware. I'll look into it more tomorrow.

https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c 
File src/e1_input_vty.c:

https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c@175 
PS7, Line 175: Enable
> If there is a way to enable this feature, shouldn't there be a way to disable it?
Line below says "0 disables keepalive". But this is not consistent with "e1_line <0-255> keepalive <1-300> ..." (zero is not allowed!), which has "no e1_line <0-255> keepalive". (Also there is no !interval code below that actually disables it).

I've changed it to match "e1_line <1-255> keepalive".


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c@193 
PS7, Line 193: 	if (!line->driver->has_keepalive) {
This check is redundant, due to the check above only the "ipa" driver is allowed. It would not make sense to disable has_keepalive for ipa. Removed.


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c@195 
PS7, Line 195: line->driver->name, VTY_NEWLINE);
> Cosmetic: alignment.
Done


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c@282 
PS7, Line 282: 					line->ipa_kap->interval, line->ipa_kap->wait_for_resp,
> Cosmetic: alignment.
Done


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c 
File src/input/ipaccess.c:

https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@65 
PS7, Line 65: int
> bool?
Changing to bool makes sense to me. However, this patch only moves the struct around, changing from int -> bool is a separate change. So I've added a follow-up patch.


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@65 
PS7, Line 65:    
> Please use tabs.
Done


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@103 
PS7, Line 103:  {
> coding style
Done


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@105 
PS7, Line 105: send
> Cosmetic (not a merge blocker): send() without flags equals to write().
Done


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@109 
PS7, Line 109:  {
> coding style
Done


https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@120 
PS7, Line 120: 	ka_fsm_for_ts(il, bfd) = NULL;
It segfaults here when testing with hardware, I'm looking into it.



-- 
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/14743
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I30e3bd601e55355aaf738ee2f2c44c1ec2c46c6a
Gerrit-Change-Number: 14743
Gerrit-PatchSet: 8
Gerrit-Owner: Hoernchen <ewild at sysmocom.de>
Gerrit-Reviewer: Hoernchen <ewild at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-CC: fixeria <axilirator at gmail.com>
Gerrit-CC: laforge <laforge at osmocom.org>
Gerrit-Comment-Date: Tue, 21 Jan 2020 14:57:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: fixeria <axilirator at gmail.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200121/2ea4ce5d/attachment.htm>


More information about the gerrit-log mailing list