808860 - B2G RIL: ril_worker should handle ipaddr w/wo carrying class info.

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Vincent Liu[:vliu]]

In our ril_worker, we can only deal with ipaddr which carrying class info in readDataCall_v6(). In Android reference code desing, it can handle ipaddr w/wo carrying class info to fulfill general case. We should add it in ril_worker.

Comment 1

[Vincent Liu[:vliu]]

When I figured out the Android reference code , it considered the case for IPv4 and IPv6 and then assigned different value. 

   addrPrefixLen = (ia instanceof Inet4Address) ? 32 : 128;

There is a question on my mind that do we have any method or variable in ril_worker to identify if the ipaddr belongs to IPv4 or IPv6?

Comment 2

[Shian-Yow Wu [:swu]]

Currently we only consider IPv4 case in readDataCall_v6().  So let's handle the IPv4 case for missing class info.  The IPv4/Ipv6 checking can be added later when IPv6 been implemented there, by filing another bug for IPv6 support.

Comment 3

[Vincent Liu[:vliu]]

Attachment: WIP V1. r=vicamo

Hi Vicamo,

Can you please review the patch and give me any feedback. Thanks.

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 679508 [details] [diff] [review]
WIP V1. r=vicamo

Review of attachment 679508 [details] [diff] [review]:
-----------------------------------------------------------------

Great! Since that's in ril_worker and is a simple function, could you please also have some xpcshell-based test cases for it? You can find some examples in dom/system/gonk/tests.

::: dom/system/gonk/ril_worker.js
@@ +4785,5 @@
>      let prefix_len = options.ipaddr.split("/")[1];
> +    if (!prefix_len) {
> +      // We assume point to point for IPv4 protocol.
> +      prefix_len = 32;
> +    }

I think you can do:

  let prefix_len;
  [options.ip, prefix_len] = options.ipaddr.split("/");
  ...

Then you don't have to call split() twice.

Comment 5

[Vincent Liu[:vliu]]

Attachment: xpc-shell test file.

Hi Vicamo,

Can you please give me some feedbacks for my test program? Thanks.

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 683018 [details] [diff] [review]
xpc-shell test file.

>function newUint8Worker() {
>  ...
>}

Please move this function from test_ril_worker_icc.js into header_helpers.js instead.

>
>/**
> * Verify readDataCall_v6#test_ipaddr_with_without_class
> */
>add_test(function test_ipaddr_with_without_class() {
>  let worker = newUint8Worker();
>  let buf = worker.Buf;
>  let array_with_class = [...];
>  let array_without_class = [...];

The two arrays are so terrible that I think no one but you can actually understand it. Please do as done in test_ril_worker_icc.js, utilize GsmPDUHelper.writeHexOctet().

>  worker.RIL.readDataCall_v6(options);
>  worker.RIL.readDataCall_v6(options);

Please also turn the main testing process into a function, so that we can add more similar test cases easily.