[devel] [PATCH 1 of 1] dtm: set IPV6_MULTICAST_IF setsock option on snd/rcv sockets to use specific interface[#933]
osaf/services/infrastructure/dtms/dtm/dtm_node_sockets.c | 60 ++-
1 files changed, 54 insertions(+), 6 deletions(-)
dtm: set IPV6_MULTICAST_IF setsock option on snd/rcv sockets to use specific
interface[#933] Currently the dtm discovery udp receive/sender sockets
are
NOT bind to a device name which holds dtmd.conf configured IP address.
The requirement is snd/rcv sockets should use specific interface , to
achive
that :
1) Excludes setting SO_BINDTODEVICE for sender/receiver sockets was
done.
2) multicast/bcast udp sender/receiver sockets join a multicast group
on a specific interface/device name
3) multicast/bcast udp sender/receiver sockets IPV6_MULTICAST_IF set tp
specific interface/device name (Note : This may be redundant will be
cleaned-up in final patch)
4) The patch only addresses the multicast IPv6 udp traffic.
5) If there is any requirement even for the Unicast traffic ( Opensaf
Application traffic also) to be routed through the specific interface
which
was configured as DTM_NODE_IP (dtmd.conf), we need to raise a new
#Ticket for that. To support that we need should be demonizes as root
user
to set SO_BINDTODEVICE and all other Opensaf service still runs as non-
root user as it was.
diff --git a/osaf/services/infrastructure/dtms/dtm/dtm_node_sockets.c
b/osaf/services/infrastructure/dtms/dtm/dtm_node_sockets.c
--- a/osaf/services/infrastructure/dtms/dtm/dtm_node_sockets.c
+++ b/osaf/services/infrastructure/dtms/dtm/dtm_node_sockets.c
@@ -329,6 +329,13 @@ static uint32_t dgram_set_mcast_ttl(DTM_
TRACE_LEAVE2("rc :%d", NCSCC_RC_FAILURE);
return NCSCC_RC_FAILURE;
}
+ unsigned int ifindex;
+ ifindex = if_nametoindex(dtms_cb->ifname);
+ if (setsockopt(dtms_cb->dgram_sock_sndr, IPPROTO_IPV6,
IPV6_MULTICAST_IF,&ifindex, sizeof(ifindex)) < 0) {
+ LOG_ER("DTM :setsockopt(IPV6_MULTICAST_IF) failed err
:%s ifname :%d", strerror(errno),if_nametoindex(dtms_cb->ifname));
+ TRACE_LEAVE2("rc :%d", NCSCC_RC_FAILURE);
+ return NCSCC_RC_FAILURE;
+ }
} else if (mcast_sender_addr->ai_family == AF_INET) { /* v4 specific
*/
/* The v4 mcast TTL socket option requires that the value be */
/* passed in an unsigned char */
@@ -711,7 +718,6 @@ uint32_t dtm_stream_nonblocking_listener
return NCSCC_RC_FAILURE;
}
-
if (set_nonblocking(dtms_cb->stream_sock, true) != NCSCC_RC_SUCCESS) {
LOG_ER("DTM : set_nonblocking() failed");
dtm_sockdesc_close(dtms_cb->stream_sock);
@@ -1053,6 +1059,26 @@ uint32_t dtm_dgram_bcast_listener(DTM_IN
TRACE_LEAVE2("rc :%d", NCSCC_RC_FAILURE);
return NCSCC_RC_FAILURE;
}
+ if (p->ai_family == AF_INET6) {
+ struct sockaddr_in6 *ipv6 = (struct sockaddr_in6
*)p->ai_addr;
+ ipv6->sin6_scope_id = if_nametoindex(dtms_cb->ifname);
+ if (setsockopt(dtms_cb->dgram_sock_rcvr, IPPROTO_IPV6,
IPV6_MULTICAST_IF, &ifindex, sizeof(ifindex)) < 0) {
+ LOG_ER("DTM :setsockopt(IPV6_MULTICAST_IF)
failed err :%s", strerror(errno));
+ TRACE_LEAVE2("rc :%d", NCSCC_RC_FAILURE);
+ return NCSCC_RC_FAILURE;
+ }
+
+ struct ipv6_mreq maddr;
+ struct sockaddr_in6 *ipv6_mr = (struct sockaddr_in6
*)p->ai_addr;
+ maddr.ipv6mr_multiaddr = ipv6_mr->sin6_addr;
+ maddr.ipv6mr_interface =
if_nametoindex(dtms_cb->ifname);
+ if (setsockopt(dtms_cb->dgram_sock_rcvr, IPPROTO_IPV6,
IPV6_ADD_MEMBERSHIP, &maddr,
+ sizeof(maddr)) < 0) {
+ LOG_ER("DTM :setsockopt(IPV6_ADD_MEMBERSHIP)
failed err :%s", strerror(errno));
+ TRACE_LEAVE2("rc :%d", NCSCC_RC_FAILURE);
+ return NCSCC_RC_FAILURE;
+ }
+ }
if (bind(dtms_cb->dgram_sock_rcvr, p->ai_addr, p->ai_addrlen)
== -1) {
LOG_ER("DTM:Socket bind failed err :%s",
strerror(errno));
close(dtms_cb->dgram_sock_rcvr);
@@ -1153,13 +1179,35 @@ uint32_t dtm_dgram_bcast_sender(DTM_INTE
if (dtms_cb->i_addr_family == DTM_IP_ADDR_TYPE_IPV6) {
struct sockaddr_in6 *bcast_sender_addr_in6 = (struct
sockaddr_in6 *)&bcast_dest_storage;
int yes = 1;
- setsockopt(dtms_cb->dgram_sock_sndr, SOL_IPV6,
IPV6_MULTICA
[devel] [PATCH 0 of 1] Review Request for dtm: set IPV6_MULTICAST_IF setsock option on snd/rcv sockets to use specific interface[#933]
Summary:dtm: set IPV6_MULTICAST_IF setsock option on snd/rcv sockets to use specific interface[#933] Review request for Trac Ticket(s): #933 Peer Reviewer(s): Neel Pull request to: <> Affected branch(es): 4.4 & default Development branch: default Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesy Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - <> changeset a36b8a6699b91b0176394460ff290203d048de02 Author: A V Mahesh Date: Thu, 24 Jul 2014 11:04:38 +0530 dtm: set IPV6_MULTICAST_IF setsock option on snd/rcv sockets to use specific interface[#933] dtm: set IPV6_MULTICAST_IF setsock option on snd/rcv sockets to use specific interface[#933] Currently the dtm discovery udp receive/sender sockets are NOT bind to a device name which holds dtmd.conf configured IP address. The requirement is snd/rcv sockets should use specific interface , to achive that : 1) Excludes setting SO_BINDTODEVICE for sender/receiver sockets was done. 2) multicast/bcast udp sender/receiver sockets join a multicast group on a specific interface/device name 3) multicast/bcast udp sender/receiver sockets IPV6_MULTICAST_IF set tp specific interface/device name (Note : This may be redundant will be cleaned-up in final patch) 4) The patch only addresses the multicast IPv6 udp traffic. 5) If there is any requirement even for the Unicast traffic ( Opensaf Application traffic also) to be routed through the specific interface which was configured as DTM_NODE_IP (dtmd.conf), we need to raise a new #Ticket for that. To support that we need should be demonizes as root user to set SO_BINDTODEVICE and all other Opensaf service still runs as non- root user as it was. Complete diffstat: -- osaf/services/infrastructure/dtms/dtm/dtm_node_sockets.c | 60 ++-- 1 files changed, 54 insertions(+), 6 deletions(-) Testing Commands: - use `netstat --inet6 -i` to see dtm discovery udp receive/sender traffic is using specific interface. Testing, Expected Results: -- the dtm discovery udp receive/sender traffic should use specific interface. Conditions of Submission: - <> Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfi
Re: [devel] [PATCH 1 of 1] IMM: Dumpimg resource utilization data for IMMsv [#35]
Hi AndersBj,
Publish the new patch with the below comments.
Thanks,
Neel.
On Wednesday 23 July 2014 08:34 PM, Anders Bjornerstedt wrote:
> Hi Neel.
>
> There is some valuable work done in this patch and most of it is ok.
> But in its current state I have to give it a NACK.
>
> It needs some adjustments and extension before it can be pushed.
>
> There are two main problems:
>
> 1) The PBE is the main OI for the object involved and we actually want
> to be able to reach the PBE
> (not just the IMMND) to also probe the PBE for resource measurements.
> Only the empty OK reply
> on PBE resources needs to be implemented in #35, but the problem now
> is that this patch shortcircuits the admop
> inside the IMMND so that it never reaches the PBE. Instead the
> admin-op should reach the PBE and
> the PBE should just reply OK for now. In the future the PBE would scan
> the list of params for values
> that where owned by the PBE and set those vales in a reply.
>
> 2) The control flow fetches the IMMND params at the requesting ndoe
> and it does so on the invocation.
> It should normally wait for the reply (from the PBE) and only fetch
> the (local) IMMND params immediately
> if there is no PBE (it is disabled).
>
> The good news is that the added new method
> ImmModel::resourceDisplay(...) should be ok as is.
> But it would usually be invoked later (still only at the IMMND where
> the om-client resides).
>
> This is how I would like to see the data flow (assuming the PBE is
> enabled):
>
> A) The admin-op is invoked in the normal way. It reaches the PBE. The
> PBE recognizes
> the new resource admin-op and just replies OK. Later we add support
> for PBE params
> which will be added by the PBE.
>
> B) The reply is received normally (the PBE could be remote so the
> reply could be from remote),
> and caught in immnd_evt_pbe_admop_rsp(). Inside if(reqConn) { ... }
> there should be a check if this
> is the new special admin-op. (The same long condition that you
> currently have in proc_admop().
> If this case matches, then invoke ImmModel::resourceDisplay(...) to
> fetch the parameters that you have implemented.
> Note that here we could potentiually be appending IMMND parameters to
> an existing list of PBE parameters.
>
> For the case where there is no PBE, then the call should bounce sort
> of like it does now, by returning ERR_REPAIR_PENDING
> which is caught in proc_admop() which invokes
> ImmModl::resourceDisplay(). But this would be the rarer 0PBE case.
>
>
> Some additional detailed review comments below.
>
> reddy.neelaka...@oracle.com wrote:
>> osaf/services/saf/immsv/immnd/ImmModel.cc | 279
>> -
>> osaf/services/saf/immsv/immnd/ImmModel.hh | 10 +-
>> osaf/services/saf/immsv/immnd/immnd_evt.c | 43 -
>> osaf/services/saf/immsv/immnd/immnd_init.h |6 +-
>> osaf/tools/safimm/immadm/imm_admin.c | 13 +-
>> 5 files changed, 333 insertions(+), 18 deletions(-)
>>
>>
>> The enhancement dumps the IMMsv resource utilization data using IMM
>> Admin Operation.
>> The IMM AdminOperation must be directed to immsv object
>> opensafImm=opensafImm,safApp=safImmService.
>>
>> Presently two types of operationNames for dumping resource
>> utilization is supported:
>>
>> display -- returns the terse output for the requested resource. The
>> number of requested resource will be returned.
> Clarify to: The allocation count of the resource will be returned,
> "number" can be misread as some kind of id.
>
>>The supported resource are implementers, adminowners, ccbhandles
>> and searchhandels.
> For ccbs its not really ccbHandles but just ccbs. I suggest the
> resource name: 'ccbs'..
> Remeber that one ccb-handle can produce a chain of ccb's and even
> after a ccb-handle is closed,
> the ccb(s) may linger in the system and will be included in the count.
> It is the number of "lingering" ccbs that is printed.
> Also even when there are zero ccbs lingering in the server, there may
> be valid ccb-handles at clients.
> Note: It is the correct count that is printed. It is just the
> explanation of what is counted that is not quite correct here.
> Similarly for "search-handles" I suggest: 'searches'.
>> Eg:
>>
>> immadm -O display -p resource:SA_STRING_T:implementers
>> opensafImm=opensafImm,safApp=safImmService
>>
>> The output is the number of implementers present:
>> Name Type Value(s)
>>
>> count SA_INT64_T 9 (0x9)
>>
>> displayverbose -- returns the verbose output for the requested
>> resource. The verbose output is supported for the resources
>> implementers and adminowners. If the number of resource is greater
>> than 127 then the verbose output is displayed to syslog.
>>
>> Eg:
>> immadm -O displayverbose -p resource:SA_STRING_T:implementers
>> opensafImm=opensafImm,safApp=
Re: [devel] [PATCH 0 of 4] Review Request for imm: add support for long DN [#886]
Hi zoran, Forgot to export SA_ENABLE_EXTENDED_NAMES=1. after exporting new immomtest and new immoitest are working fine. Its good to add SA_ENABLE_EXTENDED_NAMES=1 to immnd.conf Ack for patch 4. Thanks, Neel. On Wednesday 23 July 2014 08:19 PM, Zoran Milinkovic wrote: > Hi Neelakanta, > > The reason you are getting these errors is that you didn't enable extended > names on server side. > You must add to immnd.conf: > export SA_ENABLE_EXTENDED_NAMES=1 > > I'll add this line to immnd.conf. > > Best regards, > Zoran > > > -Original Message- > From: Neelakanta Reddy [mailto:reddy.neelaka...@oracle.com] > Sent: den 23 juli 2014 16:48 > To: Zoran Milinkovic > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 0 of 4] Review Request for imm: add support for long DN > [#886] > > Hi zoran, > > Reviewed and tested the patch. >have the same comments as AndersBj. > Regarding the patch 4 "add test cases for long DN" > Ack for the first three patches. > > But for patch 4: > Ack, when immomtest and immoitest is fixed. > > The test cases are not running: > > I enabled longdDns, Following are the errors.: > > immcfg -m -a longDnsAllowed=1 opensafImm=opensafImm,safApp=safImmService > > root@Slot-3 ~]# immoitest --longDn 7 > > Suite 7: Long DN > 1 FAILEDSA_AIS_OK - Object create callback (expected > SA_AIS_OK, got SA_AIS_ERR_NAME_TOO_LONG) > error: in test_saImmOiLongDn.c at 52: SA_AIS_ERR_NAME_TOO_LONG, expected > SA_AIS_OK - exiting > > > syslog: > > Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO implementer for class > 'TestClassConfig' is test_saImmOiLongDn.c => class extent is safe. > Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO ERR_NAME_TOO_LONG: RDN > attribute has long name. Support for extended names is not enabled > Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO implementer for class > 'TestClassConfig' is released => class extent is UNSAFE > > > [root@Slot-3 ~]# immomtest --longDn 8 > > Suite 8: Long DN > error: in test_saImmOmLongDn.c at 91: SA_AIS_ERR_NOT_EXIST, expected > SA_AIS_OK - exiting > > > syslog: > > Jul 23 20:11:56 Slot-3 osafimmnd[3056]: NO ERR_LIBRARY: attr 'attr' of > type SaNameT is too long:307. Extended names is not enabled > Jul 23 20:11:56 Slot-3 osafimmnd[3056]: NO ERR_INVALID_PARAM: Problem > with new class 'LongDnClass' > > > /Neel. > > On Monday 14 July 2014 08:10 PM, Zoran Milinkovic wrote: >> Summary: imm: add support for long DN [#886] >> Review request for Trac Ticket(s): 886 >> Peer Reviewer(s): Neelakanta, Anders >> Pull request to: Zoran >> Affected branch(es): default(4.5) >> Development branch: default(4.5) >> >> >> Impacted area Impact y/n >> >>Docsn >>Build systemn >>RPM/packaging n >>Configuration files n >>Startup scripts n >>SAF servicesy >>OpenSAF servicesn >>Core libraries n >>Samples n >>Tests y >>Other n >> >> >> Comments (indicate scope for each "y" above): >> - >> >> changeset 60ba40f3a2b55b564b73630d38bb594ac4eeced7 >> Author: Zoran Milinkovic >> Date:Mon, 14 Jul 2014 16:29:00 +0200 >> >> imm: add support for long DN to IMM service [#886] >> >> The patch contains a code for the support long DNs in IMM service >> >> changeset 6c8c17209176a70e3d922d5294cf02dd489590bb >> Author: Zoran Milinkovic >> Date:Mon, 14 Jul 2014 16:30:49 +0200 >> >> imm: add support for long DN to IMM library [#886] >> >> The patch contains a code for the support long DNs in IMM library >> >> changeset 54ee330a6132782883d3df036f99298df9153dad >> Author: Zoran Milinkovic >> Date:Mon, 14 Jul 2014 16:31:40 +0200 >> >> imm: add support for long DN to IMM tools [#886] >> >> The patch contains a code for the support long DNs in IMM tools >> >> changeset e003b7c7c3e7054811ec237ba48d4562b7105432 >> Author: Zoran Milinkovic >> Date:Mon, 14 Jul 2014 16:35:03 +0200 >> >> imm: add test cases for long DN [#886] >> >> New test cases have been added to immomtest and immoitest. If >> SA_ENABLE_EXTENDED_NAMES is not set, immomtest and immoitest can use >> --longDn parameter to set SA_ENABLE_EXTENDED_NAMES for testing. >> >> >> Added Files: >> >>tests/immsv/implementer/test_saImmOiLongDn.c >>tests/immsv/management/test_saImmOmLongDn.c >> >> >> Complete diffstat: >> -- >>osaf/libs/agents/saf/imma/Makefile.am|4 +- >>osaf/libs/agents/saf/imma/imma_cb.h |1 + >>osaf/libs/agents/saf/imma/imma_db.c |3 + >>osaf/libs/agents/saf/imma/imma_init.c| 28 --- >>osaf/libs/agents/saf/imma/imma_oi_api.c | 139 >> ++- >>osaf/libs/agents
Re: [devel] [PATCH 1 of 1] IMM: Dumpimg resource utilization data for IMMsv [#35]
Hi Neel.
There is some valuable work done in this patch and most of it is ok.
But in its current state I have to give it a NACK.
It needs some adjustments and extension before it can be pushed.
There are two main problems:
1) The PBE is the main OI for the object involved and we actually want
to be able to reach the PBE
(not just the IMMND) to also probe the PBE for resource measurements.
Only the empty OK reply
on PBE resources needs to be implemented in #35, but the problem now is
that this patch shortcircuits the admop
inside the IMMND so that it never reaches the PBE. Instead the admin-op
should reach the PBE and
the PBE should just reply OK for now. In the future the PBE would scan
the list of params for values
that where owned by the PBE and set those vales in a reply.
2) The control flow fetches the IMMND params at the requesting ndoe and
it does so on the invocation.
It should normally wait for the reply (from the PBE) and only fetch the
(local) IMMND params immediately
if there is no PBE (it is disabled).
The good news is that the added new method
ImmModel::resourceDisplay(...) should be ok as is.
But it would usually be invoked later (still only at the IMMND where the
om-client resides).
This is how I would like to see the data flow (assuming the PBE is enabled):
A) The admin-op is invoked in the normal way. It reaches the PBE. The
PBE recognizes
the new resource admin-op and just replies OK. Later we add support for
PBE params
which will be added by the PBE.
B) The reply is received normally (the PBE could be remote so the reply
could be from remote),
and caught in immnd_evt_pbe_admop_rsp(). Inside if(reqConn) { ... }
there should be a check if this
is the new special admin-op. (The same long condition that you currently
have in proc_admop().
If this case matches, then invoke ImmModel::resourceDisplay(...) to
fetch the parameters that you have implemented.
Note that here we could potentiually be appending IMMND parameters to an
existing list of PBE parameters.
For the case where there is no PBE, then the call should bounce sort of
like it does now, by returning ERR_REPAIR_PENDING
which is caught in proc_admop() which invokes
ImmModl::resourceDisplay(). But this would be the rarer 0PBE case.
Some additional detailed review comments below.
reddy.neelaka...@oracle.com wrote:
> osaf/services/saf/immsv/immnd/ImmModel.cc | 279
> -
> osaf/services/saf/immsv/immnd/ImmModel.hh | 10 +-
> osaf/services/saf/immsv/immnd/immnd_evt.c | 43 -
> osaf/services/saf/immsv/immnd/immnd_init.h |6 +-
> osaf/tools/safimm/immadm/imm_admin.c | 13 +-
> 5 files changed, 333 insertions(+), 18 deletions(-)
>
>
> The enhancement dumps the IMMsv resource utilization data using IMM Admin
> Operation.
> The IMM AdminOperation must be directed to immsv object
> opensafImm=opensafImm,safApp=safImmService.
>
> Presently two types of operationNames for dumping resource utilization is
> supported:
>
> display -- returns the terse output for the requested resource. The number of
> requested resource will be returned.
>
Clarify to: The allocation count of the resource will be returned,
"number" can be misread as some kind of id.
>The supported resource are implementers, adminowners, ccbhandles and
> searchhandels.
>
For ccbs its not really ccbHandles but just ccbs. I suggest the resource
name: 'ccbs'..
Remeber that one ccb-handle can produce a chain of ccb's and even after
a ccb-handle is closed,
the ccb(s) may linger in the system and will be included in the count.
It is the number of "lingering" ccbs that is printed.
Also even when there are zero ccbs lingering in the server, there may be
valid ccb-handles at clients.
Note: It is the correct count that is printed. It is just the
explanation of what is counted that is not quite correct here.
Similarly for "search-handles" I suggest: 'searches'.
> Eg:
>
> immadm -O display -p resource:SA_STRING_T:implementers
> opensafImm=opensafImm,safApp=safImmService
>
> The output is the number of implementers present:
> Name Type Value(s)
>
> count SA_INT64_T 9 (0x9)
>
> displayverbose -- returns the verbose output for the requested resource. The
> verbose output is supported for the resources implementers and adminowners.
> If the number of resource is greater than 127 then the verbose output is
> displayed to syslog.
>
> Eg:
> immadm -O displayverbose -p resource:SA_STRING_T:implementers
> opensafImm=opensafImm,safApp=safImmService
> [using admin-owner: 'safImmService']
> Object: opensafImm=opensafImm,safApp=safImmService
> Name Type Value(s)
>
> safLogService
Re: [devel] [PATCH 0 of 4] Review Request for imm: add support for long DN [#886]
Hi Neelakanta, The reason you are getting these errors is that you didn't enable extended names on server side. You must add to immnd.conf: export SA_ENABLE_EXTENDED_NAMES=1 I'll add this line to immnd.conf. Best regards, Zoran -Original Message- From: Neelakanta Reddy [mailto:reddy.neelaka...@oracle.com] Sent: den 23 juli 2014 16:48 To: Zoran Milinkovic Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 0 of 4] Review Request for imm: add support for long DN [#886] Hi zoran, Reviewed and tested the patch. have the same comments as AndersBj. Regarding the patch 4 "add test cases for long DN" Ack for the first three patches. But for patch 4: Ack, when immomtest and immoitest is fixed. The test cases are not running: I enabled longdDns, Following are the errors.: immcfg -m -a longDnsAllowed=1 opensafImm=opensafImm,safApp=safImmService root@Slot-3 ~]# immoitest --longDn 7 Suite 7: Long DN 1 FAILEDSA_AIS_OK - Object create callback (expected SA_AIS_OK, got SA_AIS_ERR_NAME_TOO_LONG) error: in test_saImmOiLongDn.c at 52: SA_AIS_ERR_NAME_TOO_LONG, expected SA_AIS_OK - exiting syslog: Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO implementer for class 'TestClassConfig' is test_saImmOiLongDn.c => class extent is safe. Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO ERR_NAME_TOO_LONG: RDN attribute has long name. Support for extended names is not enabled Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO implementer for class 'TestClassConfig' is released => class extent is UNSAFE [root@Slot-3 ~]# immomtest --longDn 8 Suite 8: Long DN error: in test_saImmOmLongDn.c at 91: SA_AIS_ERR_NOT_EXIST, expected SA_AIS_OK - exiting syslog: Jul 23 20:11:56 Slot-3 osafimmnd[3056]: NO ERR_LIBRARY: attr 'attr' of type SaNameT is too long:307. Extended names is not enabled Jul 23 20:11:56 Slot-3 osafimmnd[3056]: NO ERR_INVALID_PARAM: Problem with new class 'LongDnClass' /Neel. On Monday 14 July 2014 08:10 PM, Zoran Milinkovic wrote: > Summary: imm: add support for long DN [#886] > Review request for Trac Ticket(s): 886 > Peer Reviewer(s): Neelakanta, Anders > Pull request to: Zoran > Affected branch(es): default(4.5) > Development branch: default(4.5) > > > Impacted area Impact y/n > > Docsn > Build systemn > RPM/packaging n > Configuration files n > Startup scripts n > SAF servicesy > OpenSAF servicesn > Core libraries n > Samples n > Tests y > Other n > > > Comments (indicate scope for each "y" above): > - > > changeset 60ba40f3a2b55b564b73630d38bb594ac4eeced7 > Author: Zoran Milinkovic > Date: Mon, 14 Jul 2014 16:29:00 +0200 > > imm: add support for long DN to IMM service [#886] > > The patch contains a code for the support long DNs in IMM service > > changeset 6c8c17209176a70e3d922d5294cf02dd489590bb > Author: Zoran Milinkovic > Date: Mon, 14 Jul 2014 16:30:49 +0200 > > imm: add support for long DN to IMM library [#886] > > The patch contains a code for the support long DNs in IMM library > > changeset 54ee330a6132782883d3df036f99298df9153dad > Author: Zoran Milinkovic > Date: Mon, 14 Jul 2014 16:31:40 +0200 > > imm: add support for long DN to IMM tools [#886] > > The patch contains a code for the support long DNs in IMM tools > > changeset e003b7c7c3e7054811ec237ba48d4562b7105432 > Author: Zoran Milinkovic > Date: Mon, 14 Jul 2014 16:35:03 +0200 > > imm: add test cases for long DN [#886] > > New test cases have been added to immomtest and immoitest. If > SA_ENABLE_EXTENDED_NAMES is not set, immomtest and immoitest can use > --longDn parameter to set SA_ENABLE_EXTENDED_NAMES for testing. > > > Added Files: > > tests/immsv/implementer/test_saImmOiLongDn.c > tests/immsv/management/test_saImmOmLongDn.c > > > Complete diffstat: > -- > osaf/libs/agents/saf/imma/Makefile.am|4 +- > osaf/libs/agents/saf/imma/imma_cb.h |1 + > osaf/libs/agents/saf/imma/imma_db.c |3 + > osaf/libs/agents/saf/imma/imma_init.c| 28 --- > osaf/libs/agents/saf/imma/imma_oi_api.c | 139 > ++- > osaf/libs/agents/saf/imma/imma_om_api.c | 228 > - > osaf/libs/agents/saf/imma/imma_proc.c| 134 > -- > osaf/libs/common/immsv/Makefile.am |1 + > osaf/libs/common/immsv/immpbe_dump.cc| 30 +++ > osaf/libs/common/immsv/immsv_evt.c | 111 > ++- > osaf/libs/common/immsv/include/immsv_api.h |5 + >
Re: [devel] [PATCH 0 of 4] Review Request for imm: add support for long DN [#886]
Hi zoran, Reviewed and tested the patch. have the same comments as AndersBj. Regarding the patch 4 "add test cases for long DN" Ack for the first three patches. But for patch 4: Ack, when immomtest and immoitest is fixed. The test cases are not running: I enabled longdDns, Following are the errors.: immcfg -m -a longDnsAllowed=1 opensafImm=opensafImm,safApp=safImmService root@Slot-3 ~]# immoitest --longDn 7 Suite 7: Long DN 1 FAILEDSA_AIS_OK - Object create callback (expected SA_AIS_OK, got SA_AIS_ERR_NAME_TOO_LONG) error: in test_saImmOiLongDn.c at 52: SA_AIS_ERR_NAME_TOO_LONG, expected SA_AIS_OK - exiting syslog: Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO implementer for class 'TestClassConfig' is test_saImmOiLongDn.c => class extent is safe. Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO ERR_NAME_TOO_LONG: RDN attribute has long name. Support for extended names is not enabled Jul 23 20:10:35 Slot-3 osafimmnd[3056]: NO implementer for class 'TestClassConfig' is released => class extent is UNSAFE [root@Slot-3 ~]# immomtest --longDn 8 Suite 8: Long DN error: in test_saImmOmLongDn.c at 91: SA_AIS_ERR_NOT_EXIST, expected SA_AIS_OK - exiting syslog: Jul 23 20:11:56 Slot-3 osafimmnd[3056]: NO ERR_LIBRARY: attr 'attr' of type SaNameT is too long:307. Extended names is not enabled Jul 23 20:11:56 Slot-3 osafimmnd[3056]: NO ERR_INVALID_PARAM: Problem with new class 'LongDnClass' /Neel. On Monday 14 July 2014 08:10 PM, Zoran Milinkovic wrote: > Summary: imm: add support for long DN [#886] > Review request for Trac Ticket(s): 886 > Peer Reviewer(s): Neelakanta, Anders > Pull request to: Zoran > Affected branch(es): default(4.5) > Development branch: default(4.5) > > > Impacted area Impact y/n > > Docsn > Build systemn > RPM/packaging n > Configuration files n > Startup scripts n > SAF servicesy > OpenSAF servicesn > Core libraries n > Samples n > Tests y > Other n > > > Comments (indicate scope for each "y" above): > - > > changeset 60ba40f3a2b55b564b73630d38bb594ac4eeced7 > Author: Zoran Milinkovic > Date: Mon, 14 Jul 2014 16:29:00 +0200 > > imm: add support for long DN to IMM service [#886] > > The patch contains a code for the support long DNs in IMM service > > changeset 6c8c17209176a70e3d922d5294cf02dd489590bb > Author: Zoran Milinkovic > Date: Mon, 14 Jul 2014 16:30:49 +0200 > > imm: add support for long DN to IMM library [#886] > > The patch contains a code for the support long DNs in IMM library > > changeset 54ee330a6132782883d3df036f99298df9153dad > Author: Zoran Milinkovic > Date: Mon, 14 Jul 2014 16:31:40 +0200 > > imm: add support for long DN to IMM tools [#886] > > The patch contains a code for the support long DNs in IMM tools > > changeset e003b7c7c3e7054811ec237ba48d4562b7105432 > Author: Zoran Milinkovic > Date: Mon, 14 Jul 2014 16:35:03 +0200 > > imm: add test cases for long DN [#886] > > New test cases have been added to immomtest and immoitest. If > SA_ENABLE_EXTENDED_NAMES is not set, immomtest and immoitest can use > --longDn parameter to set SA_ENABLE_EXTENDED_NAMES for testing. > > > Added Files: > > tests/immsv/implementer/test_saImmOiLongDn.c > tests/immsv/management/test_saImmOmLongDn.c > > > Complete diffstat: > -- > osaf/libs/agents/saf/imma/Makefile.am|4 +- > osaf/libs/agents/saf/imma/imma_cb.h |1 + > osaf/libs/agents/saf/imma/imma_db.c |3 + > osaf/libs/agents/saf/imma/imma_init.c| 28 --- > osaf/libs/agents/saf/imma/imma_oi_api.c | 139 > ++- > osaf/libs/agents/saf/imma/imma_om_api.c | 228 > - > osaf/libs/agents/saf/imma/imma_proc.c| 134 > -- > osaf/libs/common/immsv/Makefile.am |1 + > osaf/libs/common/immsv/immpbe_dump.cc| 30 +++ > osaf/libs/common/immsv/immsv_evt.c | 111 > ++- > osaf/libs/common/immsv/include/immsv_api.h |5 + > osaf/services/saf/immsv/immd/Makefile.am |2 +- > osaf/services/saf/immsv/immd/immd_amf.c |3 +- > osaf/services/saf/immsv/immd/immd_evt.c |3 +- > osaf/services/saf/immsv/immloadd/Makefile.am |1 + > osaf/services/saf/immsv/immloadd/imm_loader.cc | 54 ++ > osaf/services/saf/immsv/immnd/ImmModel.cc| 240 > - > osaf/services/saf/immsv/immnd/Makefil
Re: [devel] [PATCH 1 of 1] amfd: replace patricia tree with stl::maps for node_lists [#713]
Ack, code review only/Regards HansN
-Original Message-
From: nagendr...@oracle.com [mailto:nagendr...@oracle.com]
Sent: den 23 juli 2014 13:01
To: Hans Feldt; Hans Nordebäck; praveen.malv...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: [PATCH 1 of 1] amfd: replace patricia tree with stl::maps for
node_lists [#713]
osaf/services/saf/amf/amfd/include/node.h | 2 +-
osaf/services/saf/amf/amfd/main.cc| 10 +-
osaf/services/saf/amf/amfd/ndfsm.cc | 49 +++---
3 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/osaf/services/saf/amf/amfd/include/node.h
b/osaf/services/saf/amf/amfd/include/node.h
--- a/osaf/services/saf/amf/amfd/include/node.h
+++ b/osaf/services/saf/amf/amfd/include/node.h
@@ -62,7 +62,6 @@ typedef enum {
/* Fail-over Node List */
typedef struct avd_fail_over_node {
- NCS_PATRICIA_NODE tree_node_id_node;
SaClmNodeIdT node_id;
} AVD_FAIL_OVER_NODE;
@@ -147,6 +146,7 @@ struct NodeNameCompare: public std::bina
extern AmfDb *node_name_db; extern AmfDb *node_id_db;
+extern AmfDb *node_list_db;
typedef struct avd_ng_tag {
diff --git a/osaf/services/saf/amf/amfd/main.cc
b/osaf/services/saf/amf/amfd/main.cc
--- a/osaf/services/saf/amf/amfd/main.cc
+++ b/osaf/services/saf/amf/amfd/main.cc
@@ -380,7 +380,8 @@ static void handle_event_in_failover_sta
m_AVD_EVT_QUEUE_ENQUEUE(cb, queue_evt);
}
- if (cb->node_list.n_nodes == 0) {
+ std::map::const_iterator it =
node_list_db->begin();
+ if (it == node_list_db->end()) {
AVD_EVT_QUEUE *queue_evt;
/* We have received the info from all the nodes. */ @@ -455,7
+456,6 @@ static void rda_cb(uint32_t notused, PCS static uint32_t
initialize(void) {
AVD_CL_CB *cb = avd_cb;
- NCS_PATRICIA_PARAMS patricia_params = {0};
int rc = NCSCC_RC_FAILURE;
SaVersionT ntfVersion = { 'A', 0x01, 0x01 };
SaAmfHAStateT role;
@@ -524,12 +524,6 @@ static uint32_t initialize(void)
}
}
- patricia_params.key_size = sizeof(SaClmNodeIdT);
- if (ncs_patricia_tree_init(&cb->node_list, &patricia_params) !=
NCSCC_RC_SUCCESS) {
- LOG_ER("ncs_patricia_tree_init FAILED");
- goto done;
- }
-
/* get the node id of the node on which the AVD is running. */
cb->node_id_avd = m_NCS_GET_NODE_ID;
diff --git a/osaf/services/saf/amf/amfd/ndfsm.cc
b/osaf/services/saf/amf/amfd/ndfsm.cc
--- a/osaf/services/saf/amf/amfd/ndfsm.cc
+++ b/osaf/services/saf/amf/amfd/ndfsm.cc
@@ -29,6 +29,8 @@
#include
#include
+AmfDb *node_list_db = 0; /* SaClmNodeIdT
index */
+
/*
* Function: avd_node_up_func
*
@@ -350,13 +352,14 @@ void avd_mds_avnd_down_evh(AVD_CL_CB *cb
/* Find if node is there in the f-over node list.
* If yes then remove entry
*/
- AVD_FAIL_OVER_NODE *node_fovr;
- node_fovr = (AVD_FAIL_OVER_NODE
*)ncs_patricia_tree_get(&cb->node_list,
- (uint8_t *)&evt->info.node_id);
-
- if (NULL != node_fovr) {
- ncs_patricia_tree_del(&cb->node_list,
&node_fovr->tree_node_id_node);
- delete node_fovr;
+ for (std::map::const_iterator
it = node_list_db->begin();
+ it != node_list_db->end(); it++) {
+ AVD_FAIL_OVER_NODE *node_fovr = it->second;
+ if (node_fovr->node_id == evt->info.node_id) {
+ node_list_db->erase(node_fovr->node_id);
+ delete node_fovr;
+ break;
+ }
}
}
TRACE_LEAVE();
@@ -421,12 +424,8 @@ void avd_fail_over_event(AVD_CL_CB *cb)
node_to_add = new AVD_FAIL_OVER_NODE();
node_to_add->node_id = avnd->node_info.nodeId;
- node_to_add->tree_node_id_node.key_info = (uint8_t
*)&(node_to_add->node_id);
- node_to_add->tree_node_id_node.bit = 0;
- node_to_add->tree_node_id_node.left =
NCS_PATRICIA_NODE_NULL;
- node_to_add->tree_node_id_node.right =
NCS_PATRICIA_NODE_NULL;
- if (ncs_patricia_tree_add(&cb->node_list,
&node_to_add->tree_node_id_node) != NCSCC_RC_SUCCESS) {
+ if (node_list_db->insert(avnd->node_info.nodeId,
node_to_add) !=
+NCSCC_RC_SUCCESS) {
/* log an error */
delete node_to_add;
return;
@@ -467,7 +466,6 @@ void avd_ack_nack_evh(AVD_CL_CB *cb, AVD
AVD_AVND *avnd;
AVD_SU *su_ptr;
AVD_SU_SI_R
[devel] [PATCH 0 of 1] Review Request for amfd: replace patricia tree with stl::maps for node_lists [#713]
Summary: amfd: replace patricia tree with stl::maps for node_lists [#713] Review request for Trac Ticket(s): #713 Peer Reviewer(s): Hans N, Hans F, Praveen Pull request to: <> Affected branch(es): default Development branch: default Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesy Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - <> changeset 3e72c67901650427412ee8d73e1b1cec07fd3f45 Author: Nagendra Kumar Date: Wed, 23 Jul 2014 16:27:47 +0530 amfd: replace patricia tree with stl::maps for node_lists [#713] Complete diffstat: -- osaf/services/saf/amf/amfd/include/node.h | 2 +- osaf/services/saf/amf/amfd/main.cc| 10 ++ osaf/services/saf/amf/amfd/ndfsm.cc | 49 + 3 files changed, 28 insertions(+), 33 deletions(-) Testing Commands: - started opensaf Testing, Expected Results: -- Conditions of Submission: - Ack from reviewers Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. -- Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1 of 1] amfd: replace patricia tree with stl::maps for node_lists [#713]
osaf/services/saf/amf/amfd/include/node.h | 2 +-
osaf/services/saf/amf/amfd/main.cc| 10 +-
osaf/services/saf/amf/amfd/ndfsm.cc | 49 +++---
3 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/osaf/services/saf/amf/amfd/include/node.h
b/osaf/services/saf/amf/amfd/include/node.h
--- a/osaf/services/saf/amf/amfd/include/node.h
+++ b/osaf/services/saf/amf/amfd/include/node.h
@@ -62,7 +62,6 @@ typedef enum {
/* Fail-over Node List */
typedef struct avd_fail_over_node {
- NCS_PATRICIA_NODE tree_node_id_node;
SaClmNodeIdT node_id;
} AVD_FAIL_OVER_NODE;
@@ -147,6 +146,7 @@ struct NodeNameCompare: public std::bina
extern AmfDb *node_name_db;
extern AmfDb *node_id_db;
+extern AmfDb *node_list_db;
typedef struct avd_ng_tag {
diff --git a/osaf/services/saf/amf/amfd/main.cc
b/osaf/services/saf/amf/amfd/main.cc
--- a/osaf/services/saf/amf/amfd/main.cc
+++ b/osaf/services/saf/amf/amfd/main.cc
@@ -380,7 +380,8 @@ static void handle_event_in_failover_sta
m_AVD_EVT_QUEUE_ENQUEUE(cb, queue_evt);
}
- if (cb->node_list.n_nodes == 0) {
+ std::map::const_iterator it =
node_list_db->begin();
+ if (it == node_list_db->end()) {
AVD_EVT_QUEUE *queue_evt;
/* We have received the info from all the nodes. */
@@ -455,7 +456,6 @@ static void rda_cb(uint32_t notused, PCS
static uint32_t initialize(void)
{
AVD_CL_CB *cb = avd_cb;
- NCS_PATRICIA_PARAMS patricia_params = {0};
int rc = NCSCC_RC_FAILURE;
SaVersionT ntfVersion = { 'A', 0x01, 0x01 };
SaAmfHAStateT role;
@@ -524,12 +524,6 @@ static uint32_t initialize(void)
}
}
- patricia_params.key_size = sizeof(SaClmNodeIdT);
- if (ncs_patricia_tree_init(&cb->node_list, &patricia_params) !=
NCSCC_RC_SUCCESS) {
- LOG_ER("ncs_patricia_tree_init FAILED");
- goto done;
- }
-
/* get the node id of the node on which the AVD is running. */
cb->node_id_avd = m_NCS_GET_NODE_ID;
diff --git a/osaf/services/saf/amf/amfd/ndfsm.cc
b/osaf/services/saf/amf/amfd/ndfsm.cc
--- a/osaf/services/saf/amf/amfd/ndfsm.cc
+++ b/osaf/services/saf/amf/amfd/ndfsm.cc
@@ -29,6 +29,8 @@
#include
#include
+AmfDb *node_list_db = 0; /* SaClmNodeIdT
index */
+
/*
* Function: avd_node_up_func
*
@@ -350,13 +352,14 @@ void avd_mds_avnd_down_evh(AVD_CL_CB *cb
/* Find if node is there in the f-over node list.
* If yes then remove entry
*/
- AVD_FAIL_OVER_NODE *node_fovr;
- node_fovr = (AVD_FAIL_OVER_NODE
*)ncs_patricia_tree_get(&cb->node_list,
- (uint8_t *)&evt->info.node_id);
-
- if (NULL != node_fovr) {
- ncs_patricia_tree_del(&cb->node_list,
&node_fovr->tree_node_id_node);
- delete node_fovr;
+ for (std::map::const_iterator
it = node_list_db->begin();
+ it != node_list_db->end(); it++) {
+ AVD_FAIL_OVER_NODE *node_fovr = it->second;
+ if (node_fovr->node_id == evt->info.node_id) {
+ node_list_db->erase(node_fovr->node_id);
+ delete node_fovr;
+ break;
+ }
}
}
TRACE_LEAVE();
@@ -421,12 +424,8 @@ void avd_fail_over_event(AVD_CL_CB *cb)
node_to_add = new AVD_FAIL_OVER_NODE();
node_to_add->node_id = avnd->node_info.nodeId;
- node_to_add->tree_node_id_node.key_info = (uint8_t
*)&(node_to_add->node_id);
- node_to_add->tree_node_id_node.bit = 0;
- node_to_add->tree_node_id_node.left =
NCS_PATRICIA_NODE_NULL;
- node_to_add->tree_node_id_node.right =
NCS_PATRICIA_NODE_NULL;
- if (ncs_patricia_tree_add(&cb->node_list,
&node_to_add->tree_node_id_node) != NCSCC_RC_SUCCESS) {
+ if (node_list_db->insert(avnd->node_info.nodeId,
node_to_add) != NCSCC_RC_SUCCESS) {
/* log an error */
delete node_to_add;
return;
@@ -467,7 +466,6 @@ void avd_ack_nack_evh(AVD_CL_CB *cb, AVD
AVD_AVND *avnd;
AVD_SU *su_ptr;
AVD_SU_SI_REL *rel_ptr;
- AVD_FAIL_OVER_NODE *node_fovr;
AVD_DND_MSG *n2d_msg;
TRACE_ENTER();
@@ -476,17 +474,20 @@ void avd_ack_nack_evh(AVD_CL_CB *cb, AVD
/* Find if node is there in the f-over node list. If yes then remove
entry
* and process the message. Else just return.
*/
- if (NULL != (node_f
