1173334 - NFC requires SOCK_SEQPACKET for connections to nfcd

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Thomas Zimmermann [:tzimmermann] [:tdz]]

NFC expects messages from nfcd to be complete and transmitted one-by-one. Otherwise it will fail with an assertion. [1]

But the current code uses SOCK_STREAM as its socket type. [2] It transfers data in arbitrary chunks. We should change the type to SOCK_SEQPACKET, which provides sequential transfers of individual messages. The Linux kernel will guarantee these properties. 

[1] https://dxr.mozilla.org/mozilla-central/source/dom/nfc/gonk/NfcService.cpp#279
[2] https://dxr.mozilla.org/mozilla-central/source/ipc/nfc/NfcConnector.cpp#26
[3] http://linux.die.net/man/2/socket

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hi Dimi, Yoshi,

I'd provide patches for this bug. But since the change has to be done in Gecko and nfcd, landing them will need a bit of coordination and probably break compatibility with older versions of either program.

Just wanted to ask for your feedback first. How stable and forward compatible is the NFC protocol?

Comment 2

[Dimi Lee [:dimi]]

Hi Thomas
Since we keep refining NFC architecture both in gecko and nfcd part, we don't guarantee that nfcd and gecko could work fine if there version are not match.For now we ask QA to do full flash when test NFC.

So in my opinion it should be fine to land patches break compatibility with older versions for now.

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

go for it,
but I'd suggest that we do this after work week so we can focus on fixing bugs if any.

Comment 4

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thanks for the infos. Taking this bug...

Comment 5

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Github pull request for platform_system_nfcd

SOCK_STREAM is still the default for compatibility with the current Gecko. The option 'S' (as in sequential) enables SOCK_SEQPACKET.

Comment 6

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [01] Bug 1173334: Open nfcd connections with socket type SOCK_SEQPACKET

Passing '-S' to nfcd enables SOCK_SEQPACKET on the nfcd side.

Comment 7

[Thomas Zimmermann [:tzimmermann] [:tdz]]

(In reply to Yoshi Huang[:allstars.chh] from comment #3)
> go for it,
> but I'd suggest that we do this after work week so we can focus on fixing
> bugs if any.

I'd ask to land this before Whistler because I'll be on PTO afterwards. But the patches are so trivial that I don't expect much problems.

Nfcd supports both socket types and is controlled by Gecko. So I'd suggest to land the nfcd side this week and Gecko's side next week. During the extra week, people will pick-up the nfcd changes and won't notice when Gecko switches.

Comment 8

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thanks! I merged the nfcd pull request. Gecko can follow in a few days when people have pulled the nfcd changes.

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

please send notice to dev-b2g/dev-gaia, otherwise NFC might be broken after OTA.

Comment 10

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Done.

Comment 11

[Thomas Zimmermann [:tzimmermann] [:tdz]]

https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=45edcf7858b3

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/45edcf7858b3