12 Jul
2013
12 Jul
'13
7:08 p.m.
Hi all,
Attached patch adds vty command for setting Access control classes (GSM
02.11 Section 4 Access control) in RACH Control Parameters (GSM 04.08
Section 10.5.2.29
RACH Control Parameters).
--
Regards,
Ivan Kluchnikov.
http://fairwaves.ru
12 Jul
12 Jul
8:28 p.m.
Ivan Kluchnikov wrote:
+DEFUN(cfg_bts_rach_ac_classes,
cfg_bts_rach_ac_classes_cmd,
+ "rach access control classes (0|1) (0|1) (0|1) (0|1) (0|1) (0|1) (0|1) (0|1)
(0|1) (0|1) x (0|1) (0|1) (0|1) (0|1) (0|1)",
+ RACH_STR
+ "Access control classes\n"
+ "Access control classes\n"
+ "Access control classes\n"
+ "Access is NOT barred for ACC 0\n"
+ "Access is barred for ACC 0\n"
+ "Access is NOT barred for ACC 1\n"
..
The command looks pretty horrible. Do you think that there could be a
better user interface than a single command with 16 parameters?
I'd suggest something like: rach access control class (0..15) (0|1)
//Peter
8:48 p.m.
Peter Stuge wrote:
The command looks pretty horrible. Do you think that
there could be a
better user interface than a single command with 16 parameters?
I'd suggest something like: rach access control class (0..15) (0|1)
yes, it
makes sense. also it would make sense to actually write the
access classes to vty config, if "show running-configuration" or
"write"
is used. (see config_write_bts_single())
since "0" (not barred) is the default, i would suggest to only write the
barred ("1") classes to config. this way the config is kept slim.
9:07 p.m.
On Fri, Jul 12, 2013 at 10:48 PM, Andreas Eversberg
<andreas(a)eversberg.eu> wrote:
also it would make sense to actually write the access
classes to vty config, if "show running-configuration" or "write" is
used.
(see config_write_bts_single())
Yes, this is a good point. Should be fixed.
since "0" (not barred) is the default, i
would suggest to only write the
barred ("1") classes to config. this way the config is kept slim.
See my e-mail to Peter.
Looks like a matter of taste.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
9:06 p.m.
On Fri, Jul 12, 2013 at 10:28 PM, Peter Stuge <peter(a)stuge.se> wrote:
Ivan Kluchnikov wrote:
I like our way more, as it's a single line vs many-many lines. A
single line doesn't occupy so much screen space and is easier to
manipulate from VTY or over a slow satellite connection (happen
frequently here). Also it gives you the whole picture in a single
glance.
Does it sound like a valid reason?
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
+DEFUN(cfg_bts_rach_ac_classes,
cfg_bts_rach_ac_classes_cmd,
+ "rach access control classes (0|1) (0|1) (0|1) (0|1) (0|1) (0|1) (0|1) (0|1)
(0|1) (0|1) x (0|1) (0|1) (0|1) (0|1) (0|1)",
+ RACH_STR
+ "Access control classes\n"
+ "Access control classes\n"
+ "Access control classes\n"
+ "Access is NOT barred for ACC 0\n"
+ "Access is barred for ACC 0\n"
+ "Access is NOT barred for ACC 1\n"
..
The command looks pretty horrible. Do you think that there could be a
better user interface than a single command with 16 parameters?
I'd suggest something like: rach access control class (0..15) (0|1)
9:27 p.m.
Alexander Chemeris wrote:
No not the least. Compare:
rach access control classes 0 1 1 0 0 1 0 0 1 0 0 0 1 1 0
with:
rach access control class 1 1
rach access control class 2 1
rach access control class 5 0
rach access control class 5 1
rach access control class 8 1
rach access control class 13 1
rach access control class 14 1
I know which I find clearer, and easier to manipulate with a minimum
of unwanted side effects.
"The connectivity to my sites suck" really can't motivate such a bad
user interface. Use mosh and/or train operators to deal with higher
latency connections.
//Peter
PS. Did you notice the error in my 'class' commands?
PPS. Did you notice the error in your 'classes' command?
The command
looks pretty horrible. Do you think that there could be a
better user interface than a single command with 16 parameters?
I'd suggest something like: rach access control class (0..15) (0|1)
I like our way more, as it's a single line vs many-many lines. A
single line doesn't occupy so much screen space and is easier to
manipulate from VTY or over a slow satellite connection (happen
frequently here). Also it gives you the whole picture in a single
glance.
Does it sound like a valid reason?
9:35 p.m.
Peter Stuge wrote:
PS. Did you notice the error in my 'class'
commands?
PPS. Did you notice the error in your 'classes' command?
I hope this made my point clear.
There is another option though; you could allow to specify ranges.
rach access control classes 1-5,8-10,12,15
...it just takes a few sscanf() calls in a loop.
(No 0|1 at all, the given classes are allowed, others are barred.)
//Peter
9:41 p.m.
Peter Stuge wrote:
Peter Stuge wrote:
I think it should still be called "rach access control class" however.
//Peter
PS. Did you notice the error in my
'class' commands?
PPS. Did you notice the error in your 'classes' command?
I hope this made my point clear.
There is another option though; you could allow to specify ranges.
rach access control classes 1-5,8-10,12,15
13 Jul
13 Jul
7:49 a.m.
On Fri, Jul 12, 2013 at 11:35 PM, Peter Stuge <peter(a)stuge.se> wrote:
There is another option though; you could allow to
specify ranges.
rach access control classes 1-5,8-10,12,15
...it just takes a few sscanf() calls in a loop.
(No 0|1 at all, the given classes are allowed, others are barred.)
That would be a nice option. I don't think we have time to implement
parsing of that right now, but if someone makes this, we definitely
vote for this way. Otherwise we'll stick to the way discussed above.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
7:47 a.m.
On Fri, Jul 12, 2013 at 11:27 PM, Peter Stuge <peter(a)stuge.se> wrote:
Compare:
rach access control classes 0 1 1 0 0 1 0 0 1 0 0 0 1 1 0
with:
rach access control class 1 1
rach access control class 2 1
rach access control class 5 0
rach access control class 5 1
rach access control class 8 1
rach access control class 13 1
rach access control class 14 1
I know which I find clearer, and easier to manipulate with a minimum
of unwanted side effects.
PS. Did you notice the error in my 'class' commands?
PPS. Did you notice the error in your 'classes' command?
Ok, now it makes sense.
I think we could even drop the last (0|1) and make it like that:
rach access control class barred 1
rach access control class barred 2
rach access control class barred 5
(default being not barred)
OTOH I like the idea that you could change value without removing a
whole string, so may be make it even more user-friendly:
rach access control class 1 barred
rach access control class 2 barred
rach access control class 5 allowed
The idea is that "1" meaning "barred" is counter-intuitive and may
lead to wrong setting. Even I will forget abut the trick in a couple
of months.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
7:58 a.m.
On Fri, Jul 12, 2013 at 09:08:42PM +0400, Ivan Kluchnikov wrote:
Hi all,
Attached patch adds vty command for setting Access control classes (GSM
02.11 Section 4 Access control) in RACH Control Parameters (GSM 04.08
Section 10.5.2.29
RACH Control Parameters).
Hi,
mutt doesn't allow me to comment on the patch (for inline patches this
always works, for some attachments it does work too.. not for yours).
My comments:
* Peter is right, and use "barred"|"allowed" as well instead of 0,1.
* The 'current' VTY rule is that unless there is a common prefix use
'-' in the command. So it would be 'rach access-control-class" right
now until we have someone else at 'access-control'..
* The code is missing a 'write' implementation.
* You might want to write a VTY test[1] to test if the command is taking
effect on the SIs. You will need to create a subclass for the NITB.
* Please take a look at the kernel coding style
* Great to have this feature, for things like Fusion this is a useful
addition!
* Great you patched pySIM for the ACC support.
thanks
holger
[1] http://cgit.osmocom.org/openbsc/tree/openbsc/tests/vty_test_runner.py
8:50 a.m.
Hi Holger,
On Sat, Jul 13, 2013 at 9:58 AM, Holger Hans Peter Freyther
<holger(a)freyther.de> wrote:
mutt doesn't allow me to comment on the patch (for
inline patches this
always works, for some attachments it does work too.. not for yours).
Probably because it's base64 encoded.
* The 'current' VTY rule is that unless there
is a common prefix use
'-' in the command. So it would be 'rach access-control-class" right
now until we have someone else at 'access-control'..
Huh, is that a new rule?
I see commands like "rach max transmission", "rach tx integer" and so
on which do not use "-".
Thanks for your other comments. We'll look into updating the patch.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
11:46 a.m.
On Sat, Jul 13, 2013 at 10:50:40AM +0400, Alexander Chemeris wrote:
Not exactly, the rule has been there before we used the VTY.. I just
never used cisco stuff before. ;)
* The
'current' VTY rule is that unless there is a common prefix use
'-' in the command. So it would be 'rach access-control-class" right
now until we have someone else at 'access-control'..
Huh, is that a new rule? I see commands like "rach max transmission",
"rach tx integer" and so
on which do not use "-".
Yeah, we have plenty of violations (and I don't think it is worth
fixing). For new ones just try to get it right.
12:11 p.m.
On Sat, Jul 13, 2013 at 1:46 PM, Holger Hans Peter Freyther
<holger(a)freyther.de> wrote:
Ok.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
On Sat, Jul 13, 2013 at 10:50:40AM +0400, Alexander Chemeris wrote:
Not exactly, the rule has been there before we used the VTY.. I just
never used cisco stuff before. ;)
Me either :)
* The
'current' VTY rule is that unless there is a common prefix use
'-' in the command. So it would be 'rach access-control-class" right
now until we have someone else at 'access-control'..
Huh, is that a new rule? I see commands
like "rach max transmission", "rach tx integer" and so
on which do not use "-".
Yeah, we have plenty of violations (and I don't think it is worth
fixing). For new ones just try to get it right.
25 Jul
25 Jul
4:19 p.m.
Hi all,
This is new version of patch for review and merge:
- more user-friendly interface for command:
rach access-control-class 2 barred
rach access-control-class 11 allowed
- added 'write' implementation
2013/7/13 Alexander Chemeris <alexander.chemeris(a)gmail.com>
On Sat, Jul 13, 2013 at 1:46 PM, Holger Hans Peter
Freyther
<holger(a)freyther.de> wrote:
Not exactly, the rule has been there before we used the VTY.. I just
never used cisco stuff before. ;)
Me either :)
Ok.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
--
Regards,
Ivan Kluchnikov.
http://fairwaves.ru
On Sat, Jul 13, 2013 at 10:50:40AM +0400, Alexander Chemeris wrote:
> > * The 'current' VTY rule is that unless there is a common prefix use
> > '-' in the command. So it would be 'rach
access-control-class"
right
now until we have someone else at
'access-control'..
Huh, is that a new rule? I see
commands like "rach max transmission", "rach tx integer" and so
on which do not use "-".
Yeah, we have plenty of violations (and I don't think it is worth
fixing). For new ones just try to get it right.
4:33 p.m.
On Thu, Jul 25, 2013 at 06:19:24PM +0400, Ivan Kluchnikov wrote:
- added 'write' implementation
please write a VTY test for this routine. It is like 20 lines of
python and will make sure that bitrot is detected early (a perfect
way to secure your time investment).
+ if ((bts->si_common.rach_control.t3) != 0)
...
+ if ((bts->si_common.rach_control.t2 & 0xfb) != 0)
...
you want to avoid executing the for loop?
holger
5:21 p.m.
2013/7/25 Holger Hans Peter Freyther <holger(a)freyther.de>
On Thu, Jul 25, 2013 at 06:19:24PM +0400, Ivan
Kluchnikov wrote:
Yes, I agree, tests are very important, I will try to write one for this
command.
- added 'write' implementation
please write a VTY test for this routine. It is like 20 lines of
python and will make sure that bitrot is detected early (a perfect
way to secure your time investment).
+ if ((bts->si_common.rach_control.t3) != 0)
...
+ if ((bts->si_common.rach_control.t2 & 0xfb) != 0)
...
you want to avoid executing the for loop?
Yes, do you think it is redundant part of the code?
holger
--
Regards,
Ivan Kluchnikov.
http://fairwaves.ru
26 Jul
26 Jul
6:27 a.m.
On Thu, Jul 25, 2013 at 07:21:55PM +0400, Ivan Kluchnikov wrote:
2013/7/25 Holger Hans Peter Freyther
<holger(a)freyther.de>
Yes, I agree, tests are very important, I will try to
write one for this
command.
To help you I created the below snippet. You will just need to write
a new test method in there to do config and show configure. E.g. you
can issue all bar commands and verify that it shows up in the write
terminal command, then allow them again and see that it goes away.
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py
index 3aa40f4..a3e29d3 100644
--- a/openbsc/tests/vty_test_runner.py
+++ b/openbsc/tests/vty_test_runner.py
@@ -55,6 +55,19 @@ class TestVTYBase(unittest.TestCase):
self.vty = None
osmoutil.end_proc(self.proc)
+class TestVTYNITB(TestVTYBase):
+
+ def vty_command(self):
+ return ["./src/osmo-nitb/osmo-nitb", "-c",
+ "doc/examples/osmo-nitb/nanobts/openbsc.cfg"]
+
+ def vty_app(self):
+ return (4242, "./src/osmo-nitb/osmo-nitb", "OpenBSC",
"nitb")
+
+ def testShowNetwork(self):
+ res = self.vty.command("show network")
+ print res
+
class TestVTYNAT(TestVTYBase):
@@ -138,6 +151,7 @@ if __name__ == '__main__':
os.chdir(workdir)
print "Running tests for specific VTY commands"
suite = unittest.TestSuite()
+ suite.addTest(unittest.TestLoader().loadTestsFromTestCase(TestVTYNITB))
add_nat_test(suite, workdir)
res = unittest.TextTestRunner(verbosity=verbose_level).run(suite)
sys.exit(len(res.errors) + len(res.failures))
25 Jul
25 Jul
4:55 p.m.
Ivan Kluchnikov wrote:
This is new version of patch for review and merge:
- more user-friendly interface for command:
rach access-control-class 2 barred
rach access-control-class 11 allowed
Yes, much better than before, but perhaps spend the half hour it
takes to write that %d-%d parser using sscanf?
+++ b/openbsc/src/libbsc/bsc_vty.c
..
@@ -2059,6 +2067,51 @@ DEFUN(cfg_bts_rach_ec_allowed,
cfg_bts_rach_ec_allowed_cmd,
return CMD_SUCCESS;
}
+DEFUN(cfg_bts_rach_ac_class, cfg_bts_rach_ac_class_cmd,
+ "rach access-control-class (0|1|2|3|4|5|6|7|8|9|11|12|13|14|15)
(barred|allowed)",
+ RACH_STR
+ "Set access control class\n"
+ "Access control class 0\n"
+ "Access control class 1\n"
+ "Access control class 2\n"
+ "Access control class 3\n"
+ "Access control class 4\n"
+ "Access control class 5\n"
+ "Access control class 6\n"
+ "Access control class 7\n"
+ "Access control class 8\n"
+ "Access control class 9\n"
+ "Access control class 11 for PLMN use\n"
+ "Access control class 12 for security services\n"
+ "Access control class 13 for public utilities (e.g. water/gas
suppliers)\n"
+ "Access control class 14 for emergency services\n"
+ "Access control class 15 for PLMN staff\n"
+ "barred to use access control class\n"
+ "allowed to use access control class\n")
+{
Is that long array of strings correct?
//Peter
5:29 p.m.
2013/7/25 Peter Stuge <peter(a)stuge.se>
I think, yes, because it works.
Do you know other way to do it?
Ivan Kluchnikov wrote:
You are welcome :)
If you implement %d-%d parser, I will change style of this VTY command.
This is new version of patch for review and
merge:
- more user-friendly interface for command:
rach access-control-class 2 barred
rach access-control-class 11 allowed
Yes, much better than before, but perhaps spend the half hour it
takes to write that %d-%d parser using sscanf?
+++ b/openbsc/src/libbsc/bsc_vty.c
..
@@ -2059,6 +2067,51 @@
DEFUN(cfg_bts_rach_ec_allowed,
cfg_bts_rach_ec_allowed_cmd,
return CMD_SUCCESS;
}
+DEFUN(cfg_bts_rach_ac_class, cfg_bts_rach_ac_class_cmd,
+ "rach access-control-class (0|1|2|3|4|5|6|7|8|9|11|12|13|14|15)
(barred|allowed)",
+ RACH_STR
+ "Set access control class\n"
+ "Access control class 0\n"
+ "Access control class 1\n"
+ "Access control class 2\n"
+ "Access control class 3\n"
+ "Access control class 4\n"
+ "Access control class 5\n"
+ "Access control class 6\n"
+ "Access control class 7\n"
+ "Access control class 8\n"
+ "Access control class 9\n"
+ "Access control class 11 for PLMN use\n"
+ "Access control class 12 for security services\n"
+ "Access control class 13 for public utilities (e.g. water/gas
suppliers)\n"
+ "Access control class 14 for
emergency services\n"
+ "Access control class 15 for PLMN staff\n"
+ "barred to use access control class\n"
+ "allowed to use access control class\n")
+{
Is that long array of strings correct?
//Peter
--
Regards,
Ivan Kluchnikov.
http://fairwaves.ru
6:02 p.m.
Ivan Kluchnikov wrote:
Here you go.
//Peter
Ivan
Kluchnikov wrote:
You are welcome :)
If you implement %d-%d parser, I will change style of this VTY command. This is new version of patch for review and
merge:
- more user-friendly interface for command:
rach access-control-class 2 barred
rach access-control-class 11 allowed
Yes, much better than before, but perhaps spend the half hour it
takes to write that %d-%d parser using sscanf?
6:18 p.m.
Peter Stuge wrote:
And there was an off-by-one error for the upper bound of a range.
I think I've caught all the issues in this version of the file. :)
//Peter
if
(',' == *p)
p++;
Here it should actually also throw parse error if there is any other
character than comma or end-of-input:
6:37 p.m.
Yes, the next step is to integrate this code into osmocore VTY code.
2013/7/25 Peter Stuge <peter(a)stuge.se>
Peter Stuge wrote:
And there was an off-by-one error for the upper bound of a range.
I think I've caught all the issues in this version of the file. :)
//Peter
--
Regards,
Ivan Kluchnikov.
http://fairwaves.ru
if
(',' == *p)
p++;
Here it should actually also throw parse error if there is any other
character than comma or end-of-input:
3935
days inactive
3948
days old
24 comments
5 participants
participants (5)
-
Alexander Chemeris -
Andreas Eversberg -
Holger Hans Peter Freyther -
Ivan Kluchnikov -
Peter Stuge