Hey Luka,
On 02/22/2012 04:23 PM, Luka Perkov wrote:
* use tabs instead of spaces
* avoid trailing wihite spaces
Totally agree. Sorry for not noticing.
On Wed, Feb 22, 2012 at 02:23:45PM +0100, Kaspar Schleiser wrote:
Currently "1.0" and "1.2" are being recognized.
Default is 1.2, so for upgraders nothing changes, missing option is no problem.
Users of 1.0 servers can set "cwmp_version" to "1.0" and start rocking
oldschool.
We should use 1.0 as default one because it's the one most commonly
used.
Ok. Didn't want to change default behaviour.
Specs say, use the newest one you support, but be able to read the
version out of the server's response...
Do you have an idea of where the actual differences between the
standards are? I just noticed that 1.2 adds some rpc calls like for
module support. If the client doesn't behave differently when not asked
to, maybe always using 1.2 is ok.
I had this implemented just for the expected version urn sent by the
server... If we can handle both versions there, maybe we don't even need
configuration.
I looked at the documentation:
http://www.broadband-forum.org/cwmp.php
http://www.broadband-forum.org/cwmp/cwmp-1-0.xsd
http://www.broadband-forum.org/cwmp/cwmp-1-1.xsd
http://www.broadband-forum.org/cwmp/cwmp-1-2.xsd
http://www.broadband-forum.org/cwmp/cwmp-1-3.xsd
And there are versions 1.0, 1.1, 1.2 and surprisingly no 1.3 version.
I'm thinking why not pass the config string to the xml.c? That way user
can write say 1.9 or whatever and it will get passed to the ACS. By the
default we will put "1.0" and that should not be a issue then. If the
user abuses this communication with ACS might not work, but it will be
his problem then. What do you think about this?
I thought that the version would set a specific but well defined
behaviour of the client. Passing the string would require a strcmp on
all checks which seems inefficient to me.
Compare
if (acs.version == V1_2)
with
if(strcmp(acs.version, "1.2") == 0)
For the urn string, for version "1.0" we have to send "...1-0". I don't
think this should be user configurable. Either we speak 1.0 which
implies sending "1-0" (and maybe other things), or we speak 1.2 which
implies "1-2". If a user wants to get that deep, he can easily hack the
code...
Also, I would rename "cwmp_version" to only "version".
Ok.
I'm still not well and I can not do much work. If you do not resend the
patch I'll rework this one and merge it.
I'll rework it. Hope you get well soon!
+ enum cwmp_version cwmp_version;
What do you think that here we use:
char *version;
and place there what user has written in config file?
See above. I had it implemented with char*, but then all the checks
seemed ugly. Your call! I don't really mind.
+ if (cwmp_version == V1_2) {
...
+ }
Can you explain why you have put this only in v1.2? This is not v1.2
specific.
Well, I just blindly assumed that this was an 1.0 vs 1.2 issue without
consulting the specs. ;) I'll gladly remove it from the version support
patch.
I just checked, the specs say that *if* the server sends cwmp:ID, this
id *must* be used in any response to that request. I'll look into making
the client behave like that, maybe we don't need special care for nbbs.
If you have trouble with your ACS because of this we should introduce a
new config option or better yet exclude this code with #ifdef's. And add
configuration option for your specific ACS in configure script:
/configure --enable-acs=nbbs5
Take a look how it is done for HDM, grep for ACS_HDM. You define it at
compile time using:
/configure --enable-acs=hdm
For example we have this in src/http/http.c:
94 # ifdef ACS_HDM
95 status = zstream_http_configure(http_c.stream, ZSTREAM_HTTP_COOKIES,
1);
96 # elif ACS_MULTI
97 status = zstream_http_configure(http_c.stream, ZSTREAM_HTTP_COOKIES,
3);
98 # endif
I hoped to not need any (non-generic) server specific code, but I'll
integrate it this way next time.
Kaspar
|