osgi / bugzilla-archive

Archive of OSGi Alliance Specification Bugzilla bugs. The Specification Bugzilla system was decommissioned with the move to GitHub. The issues in this repository are imported from the Specification Bugzilla system for archival purposes.
0 stars 1 forks source link

RFC-149 fault code review #1664

Closed bjhargrave closed 13 years ago

bjhargrave commented 13 years ago

Original bug ID: BZ#1796 From: wlupton@2wire.com Reported version: R4 V4.3

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

Created attachment 161 Fault Code Review (Excel)

Ikuo asked me to review the fault code mapping in section 6.5.1.

I have pasted the table into Excel, sorted it by TR-069 fault code, and added a column of Remarks. Please see the attachment.

In addition, I note that the following fault codes are not used:

Attached file: Fault Code Review.xlsx (application/vnd.openxmlformats-officedocument.spreadsheetml.sheet, 83957 bytes) Description: Fault Code Review (Excel)

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

Created attachment 162 Fault Code Review (PDF)

Attached file: Fault Code Review.pdf (application/pdf, 61315 bytes) Description: Fault Code Review (PDF)

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Created attachment 163 Fault Code Review by ProSyst, 12.11.2010

A quick review of the mapping between the DmtException error codes and TR fault codes by ProSyst.

Attached file: Fault Code Review_12112010_ProSyst.xls (application/vnd.ms-excel, 72112 bytes) Description: Fault Code Review by ProSyst, 12.11.2010

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Let me clarify the meaning of "Server" and "Client" of Type in Table 65 of TR-069 Amendment2.

For example, regarding SPV, can CPE use both 9000 (Server) and 9007 (Client)as Fault code in SPVResponse ? (I'm confused. Is it OK to ignore Type column in Table 65 ?)

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

Created an attachment (id=163) [details] Fault Code Review by ProSyst, 12.11.2010

A quick review of the mapping between the DmtException error codes and TR fault codes by ProSyst.

I had a quick look. One comment that stands out is that TR-069 9000 (method not supported) refers to the TR-069 method, not to a method on a node within the implementation. It should be returned only when the ACS attempts to invoke a method that the CPE did not list in its response to GetRPCMethods().

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

Let me clarify the meaning of "Server" and "Client" of Type in Table 65 of TR-069 Amendment2.

For example, regarding SPV, can CPE use both 9000 (Server) and 9007 (Client)as Fault code in SPVResponse ? (I'm confused. Is it OK to ignore Type column in Table 65 ?)

This is a SOAP thing and needn't concern us (it's intended to indicate whether the problem is on the client side - e.g. invalid argument - or server side - e.g. resources exceeded) . Please see the footnote: "The specified Type MUST be used to determine the value of the SOAP faultcode element as described in Section 3.5".

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

I got it. Thank you William.

Let me clarify the meaning of "Server" and "Client" of Type in Table 65 of TR-069 Amendment2.

For example, regarding SPV, can CPE use both 9000 (Server) and 9007 (Client)as Fault code in SPVResponse ? (I'm confused. Is it OK to ignore Type column in Table 65 ?)

This is a SOAP thing and needn't concern us (it's intended to indicate whether the problem is on the client side - e.g. invalid argument - or server side - e.g. resources exceeded) . Please see the footnote: "The specified Type MUST be used to determine the value of the SOAP faultcode element as described in Section 3.5".

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Created attachment 164 Fault Code Update by NTT, 16.11.2010

I had updated Fault Code Table.

Attached file: Fault Code Review_16112010_NTT.xls (application/vnd.ms-excel, 68811 bytes) Description: Fault Code Update by NTT, 16.11.2010

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

Sorry, haven't done extensive review (not made easy because I find it very hard to see the red!).

One comment: why is fault 9008 not used anywhere? I had said the following under PERMISSION_DENIED:

<9008 is "Attempt to set non-writable parameter". This is a very specific fault and is being used for too many things here I think. This is probably the only valid use?>

...and was thinking that it IS appropriate in this one case.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Created attachment 165 Fault Code Update by NTT, 17.11.2010

Sorry, haven't done extensive review (not made easy because I find it very hard to see the red!).

I made part in red bold. How is it ?

One comment: why is fault 9008 not used anywhere? I had said the following under PERMISSION_DENIED:

<9008 is "Attempt to set non-writable parameter". This is a very specific fault and is being used for too many things here I think. This is probably the only valid use?>

...and was thinking that it IS appropriate in this one case.

Sorry, it was my mistake.

I had updated the sheet. Please check it.

Attached file: Fault Code Review_17112010_NTT.xls (application/vnd.ms-excel, 68693 bytes) Description: Fault Code Update by NTT, 17.11.2010

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Created an attachment (id=163) [details] [details] Fault Code Review by ProSyst, 12.11.2010

A quick review of the mapping between the DmtException error codes and TR fault codes by ProSyst.

I had a quick look. One comment that stands out is that TR-069 9000 (method not supported) refers to the TR-069 method, not to a method on a node within the implementation. It should be returned only when the ACS attempts to invoke a method that the CPE did not list in its response to GetRPCMethods(). Ah, it's a pity.

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

I had updated the sheet. Please check it.

I have reviewed the bold text and here is some feedback (cell numbers are from <<Fault Code Update by NTT, 17.11.2010>>); both these items are very minor ... just bringing them to your attention:

D12: can now be combined with D10

C29/C30: looks strange to map this differently for CWMP Fault (9002) and SetParameterValuesFault (9001); is there a reason for this; the same comment applies in various other cases

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

I had updated the sheet. Please check it. The yellow cells look OK to me.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Created attachment 166 Fault Code Update by NTT, 19.11.2010

I had updated the sheet. Please check it.

I have reviewed the bold text and here is some feedback (cell numbers are from <<Fault Code Update by NTT, 17.11.2010>>); both these items are very minor ... just bringing them to your attention:

D12: can now be combined with D10

I had updated it.

C29/C30: looks strange to map this differently for CWMP Fault (9002) and SetParameterValuesFault (9001); is there a reason for this; the same comment applies in various other cases

Why do you think it is strange ? Cannot CPE send CWMP Fault(9002 Internal Error) with SetParameterValuesFault (9001) ?

Attached file: Fault Code Review_19112010_NTT.xls (application/vnd.ms-excel, 73243 bytes) Description: Fault Code Update by NTT, 19.11.2010

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

I just felt it was a little strange that the underlying error "TRANSACTION_ERROR" should sometimes be regarded as an "internal error" and sometimes return "request denied". If there is an underlying logic to this then fine. But if not then I think it would look better to choose one of 9001 and 9002.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

I just felt it was a little strange that the underlying error "TRANSACTION_ERROR" should sometimes be regarded as an "internal error" and sometimes return "request denied". If there is an underlying logic to this then fine. But if not then I think it would look better to choose one of 9001 and 9002.

Do you mean that:

CPE should send CWMP Fault(9002 Internal Error) with SetParameterValuesFault (9002 Internal Error) or CWMP Fault(9001 Permission denied) with SetParameterValuesFault (9001 Permission denied)?

I can accept either. We should choose the best one from the TR-069 perspective.

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

I just felt it was a little strange that the underlying error "TRANSACTION_ERROR" should sometimes be regarded as an "internal error" and sometimes return "request denied". If there is an underlying logic to this then fine. But if not then I think it would look better to choose one of 9001 and 9002.

Do you mean that:

CPE should send CWMP Fault(9002 Internal Error) with SetParameterValuesFault (9002 Internal Error) or CWMP Fault(9001 Permission denied) with SetParameterValuesFault (9001 Permission denied)?

I can accept either. We should choose the best one from the TR-069 perspective.

If SetParameterValuesFault is present, I think that the CWMP fault MUST be 9003 mustn't it?

"If there is a fault due to one or more parameters in error, the fault response for this method MUST include a SetParameterValuesFault element for each parameter in error. In this case, the primary fault code indicated for the overall fault response MUST be Invalid Arguments (9003)"

I wasn't talking about this case. I was only wondering why the DMT TRANSACTION_ERROR sometimes maps to 9002 (Internal Error) and sometimes to 9001 (Permission denied).

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

I just felt it was a little strange that the underlying error "TRANSACTION_ERROR" should sometimes be regarded as an "internal error" and sometimes return "request denied". If there is an underlying logic to this then fine. But if not then I think it would look better to choose one of 9001 and 9002.

Do you mean that:

CPE should send CWMP Fault(9002 Internal Error) with SetParameterValuesFault (9002 Internal Error) or CWMP Fault(9001 Permission denied) with SetParameterValuesFault (9001 Permission denied)?

I can accept either. We should choose the best one from the TR-069 perspective.

If SetParameterValuesFault is present, I think that the CWMP fault MUST be 9003 mustn't it?

"If there is a fault due to one or more parameters in error, the fault response for this method MUST include a SetParameterValuesFault element for each parameter in error. In this case, the primary fault code indicated for the overall fault response MUST be Invalid Arguments (9003)"

I wasn't talking about this case. I was only wondering why the DMT TRANSACTION_ERROR sometimes maps to 9002 (Internal Error) and sometimes to 9001 (Permission denied).

  • TRANSACTION_ERROR can be mapped to 9002 (Internal Error) in case of fail in the commit operation
  • TRANSACTION_ERROR can be mapped to 9001 (Request denied) in case of an updating method within an atomic session can not be executed because the underlying plugin is read-only or does not support atomic writing
bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

  • TRANSACTION_ERROR can be mapped to 9002 (Internal Error) in case of fail in the commit operation
  • TRANSACTION_ERROR can be mapped to 9001 (Request denied) in case of an updating method within an atomic session can not be executed because the underlying plugin is read-only or does not support atomic writing

OK. Great. That's the "underlying logic" that I was looking for.

Just checking, but in the "underlying plugin is read-only" case, will the PA know that this is the reason for the failure? If so, would 9008 (Attempt to set non-writable parameter) be appropriate?

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Just checking, but in the "underlying plugin is read-only" case, will the PA know that this is the reason for the failure? Yes, he does. "- TRANSACTION_ERROR can be mapped to 9002 (Internal Error) in case of fail in the commit operation" - this means that the fail is in the execution of commit method "- TRANSACTION_ERROR can be mapped to 9001 (Request denied) in case of an updating method within an atomic session can not be executed because the underlying plugin is read-only or does not support atomic writing" - this means that the fail is set methods.

If so, would 9008 (Attempt to set non-writable parameter) be appropriate? No, TRANSACTION_ERROR can be thrown when the plug-in is read/write but no atomic. In this case, the parameter is writeable but not in atomic session.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Now I understand what William and Evgeni means respectively.

  1. The cwmp fault code if any error happens for SPV, it must be 9003(Invalid Arguments).
  2. Even if the same reason of DmtException was thrown, SetParameterValuesFault code can be changed according to what kind of operation throws the DmtException. For example, "- TRANSACTION_ERROR can be mapped to 9002 (Internal Error) in case of fail in the commit operation" - this means that the fail is in the execution of commit method "- TRANSACTION_ERROR can be mapped to 9001 (Request denied) in case of an updating method within an atomic session can not be executed because the underlying plugin is read-only or does not support atomic writing" - this means that the fail is set methods.

Taking those into consideration, I'll update the sheet later.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Created attachment 167 Fault Code Update by NTT, 20.11.2010

We need to separate the following cases:

  1. SetParameterValuesFault code for SetParameterValues RPC.
  2. CWMP Fault code for AddObject or DeleteObject RPC,
  3. CWMP Fault code for GetParameterValues or GetParameterNames RPC. RFC149 has no recommendation for the ohter RPCs.

Therefore, I had completely updated the sheet. Could you check them ? Especially William and Evgeni.

Attached file: Fault_Code_Review_20112010_NTT.xls (application/octet-stream, 44853 bytes) Description: Fault Code Update by NTT, 20.11.2010

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Created an attachment (id=167) [details] Fault Code Update by NTT, 20.11.2010 The new table design is more readable, thanks. I have two minor questions:

  1. A.3.2.2 GetParameterValues: " The following fault codes are defined for this method: 9001, 9002, 9003, 9004, 9005. " Can someone gives an examples about 9004? That code is not available in GetParameterNames.
  2. Depends on the first question answer, but NODE_ALREADY_EXISTS -> 9004 Resources exceeded mapping is possible in that scenario:
    • ./A/B/ cardinality is 0..10
    • there are 10 nodes like ./A/B/1, ..., ./A/B/10 The next AddObject RPC should fail with 9004.
bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Created an attachment (id=167) [details] [details] Fault Code Update by NTT, 20.11.2010 The new table design is more readable, thanks.

So Evgeni agreed the cells I wrote in red on yellow background except 9004 questions. How about William or John (2Wire) ?

I have two minor questions:

  1. A.3.2.2 GetParameterValues: " The following fault codes are defined for this method: 9001, 9002, 9003, 9004,
  2. " Can someone gives an examples about 9004? That code is not available in GetParameterNames.

"9004 Resources exceeded (when used in association with SetParameterValues, this MUST NOT be used to indicate parameters in error)" can be used only for any of SetParameterValues, AddObject, and GetParameterValues. I can imagine a case of AddObject but cannot ones of others.

  1. Depends on the first question answer, but NODE_ALREADY_EXISTS -> 9004 Resources exceeded mapping is possible in that scenario:
    • ./A/B/ cardinality is 0..10
    • there are 10 nodes like ./A/B/1, ..., ./A/B/10 The next AddObject RPC should fail with 9004.

For AddObject, I agree. However,but not NODE_ALREADY_EXIST but METADATA_MISMATCH, because "117.13.7.8 public static final int METADATA_MISMATCH" says --- begin quote ---- • any node creation or deletion that would violate the cardinality constraints --- end quote ----

So how about this? :

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Created an attachment (id=167) [details] [details] [details] Fault Code Update by NTT, 20.11.2010 The new table design is more readable, thanks.

So Evgeni agreed the cells I wrote in red on yellow background except 9004 questions. How about William or John (2Wire) ?

I have two minor questions:

  1. A.3.2.2 GetParameterValues: " The following fault codes are defined for this method: 9001, 9002, 9003, 9004,
  2. " Can someone gives an examples about 9004? That code is not available in GetParameterNames.

"9004 Resources exceeded (when used in association with SetParameterValues, this MUST NOT be used to indicate parameters in error)" can be used only for any of SetParameterValues, AddObject, and GetParameterValues. I can imagine a case of AddObject but cannot ones of others. William or John, can you give an example here?

  1. Depends on the first question answer, but NODE_ALREADY_EXISTS -> 9004 Resources exceeded mapping is possible in that scenario:
    • ./A/B/ cardinality is 0..10
    • there are 10 nodes like ./A/B/1, ..., ./A/B/10 The next AddObject RPC should fail with 9004.

For AddObject, I agree. However,but not NODE_ALREADY_EXIST but METADATA_MISMATCH, because "117.13.7.8 public static final int METADATA_MISMATCH" says --- begin quote ---- • any node creation or deletion that would violate the cardinality constraints --- end quote ---- Good quote. So how about this? :

  • If a DmtSession#createXxxNode(node) throws METADATA_MISMATCH, PA can should check whether the MetaNode#getMaxOccurence() <= the size of array of DmtSession#getChildNodeNames() against the parent node. If true, 9004 should be chosen for the CWMP fault code. Those steps are OK to me. Can we set them as a note or recommendation?
bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

I had a very quick look, and it looks good.

I noticed a couple of "no recommendation" entries that appear to relate to error codes that should never occur for that RPC, e.g. GetParameterValues and ROLLBACK_FAILED. Perhaps could add "(not expected to occur)"? Or don't bother. Up to you.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Created attachment 168 Fault Code Update by NTT, 23.11.2010

I had a very quick look, and it looks good. I noticed a couple of "no recommendation" entries that appear to relate to error codes that should never occur for that RPC, e.g. GetParameterValues and ROLLBACK_FAILED. Perhaps could add "(not expected to occur)"? Or don't bother. Up to you.

Thanks William.

Although I had written in the old sheet that "A protocol adapter never receives this Exception for either GetParameterValues or GetParameterNames RPC.", I had updated the description ot "A protocol adapter is not expected to receive this Exception for either GetParameterValues or GetParameterNames RPC.".

The updated part are written in red.

Attached file: Fault_Code_Review_23112010_NTT.xls (application/vnd.ms-excel, 44249 bytes) Description: Fault Code Update by NTT, 23.11.2010

bjhargrave commented 13 years ago

Comment author: jblackford@2wire.com

I have two minor questions:

  1. A.3.2.2 GetParameterValues: " The following fault codes are defined for this method: 9001, 9002, 9003, 9004,
  2. " Can someone gives an examples about 9004? That code is not available in GetParameterNames.

"9004 Resources exceeded (when used in association with SetParameterValues, this MUST NOT be used to indicate parameters in error)" can be used only for any of SetParameterValues, AddObject, and GetParameterValues. I can imagine a case of AddObject but cannot ones of others. William or John, can you give an example here?

You could get a 9004 on a GetParameterValues from a CPE if the ACS asked for too many parameters and the resulting GetParameterValuesResponse XML was too large for the CPE to generate within memory. Remember, this was from about 7 years ago when many CPE were resource constrained and weren't designed to support remote management protocols based off of SOAP and XML.

A 9004 on a SetParameterValues is an unlikely scenario, but it would be based off of the size of the incoming XML and the CPE not being able to parse it all.

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

I have two minor questions:

  1. A.3.2.2 GetParameterValues: " The following fault codes are defined for this method: 9001, 9002, 9003, 9004,
  2. " Can someone gives an examples about 9004? That code is not available in GetParameterNames.

"9004 Resources exceeded (when used in association with SetParameterValues, this MUST NOT be used to indicate parameters in error)" can be used only for any of SetParameterValues, AddObject, and GetParameterValues. I can imagine a case of AddObject but cannot ones of others. William or John, can you give an example here?

You could get a 9004 on a GetParameterValues from a CPE if the ACS asked for too many parameters and the resulting GetParameterValuesResponse XML was too large for the CPE to generate within memory. Remember, this was from about 7 years ago when many CPE were resource constrained and weren't designed to support remote management protocols based off of SOAP and XML.

A 9004 on a SetParameterValues is an unlikely scenario, but it would be based off of the size of the incoming XML and the CPE not being able to parse it all. Thanks. The confusion comes from GetParameterNames error codes. The same arguments are valid, but 9004 is not available.

bjhargrave commented 13 years ago

Comment author: wlupton@2wire.com

Thanks. The confusion comes from GetParameterNames error codes. The same arguments are valid, but 9004 is not available.

Yes. That was deliberate, the idea being not to let a CPE refuse a request to return the names of all the parameters in the data model. If necessary, a memory-limited CPE would need to stream the response.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Created attachment 169 Fault Code Update by NTT, 25.11.2010

I had updated sheet, including

Attached file: Fault_Code_Review_25112010_NTT.xls (application/vnd.ms-excel, 45288 bytes) Description: Fault Code Update by NTT, 25.11.2010

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Close this bug, because the RFC149 ver. 1.5.0 has reflect the result of it.