NFC requires SOCK_SEQPACKET for connections to nfcd

RESOLVED FIXED

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: tzimmermann, Assigned: tzimmermann)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(2 attachments)

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
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?
Flags: needinfo?(dlee)
Flags: needinfo?(allstars.chh)
Summary: NFC requires SOCK_SEQPACKET for connection to nfcd → NFC requires SOCK_SEQPACKET for connections to nfcd
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.
Flags: needinfo?(dlee)
go for it,
but I'd suggest that we do this after work week so we can focus on fixing bugs if any.
Flags: needinfo?(allstars.chh)
Thanks for the infos. Taking this bug...
Assignee: nobody → tzimmermann
Status: NEW → ASSIGNED
SOCK_STREAM is still the default for compatibility with the current Gecko. The option 'S' (as in sequential) enables SOCK_SEQPACKET.
Attachment #8620853 - Flags: review?(dlee)
Passing '-S' to nfcd enables SOCK_SEQPACKET on the nfcd side.
Attachment #8620854 - Flags: review?(allstars.chh)
(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.
Attachment #8620853 - Flags: review?(dlee) → review+
Attachment #8620854 - Flags: review?(allstars.chh) → review+
Thanks! I merged the nfcd pull request. Gecko can follow in a few days when people have pulled the nfcd changes.
please send notice to dev-b2g/dev-gaia, otherwise NFC might be broken after OTA.
https://hg.mozilla.org/mozilla-central/rev/45edcf7858b3
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.