Skip to content

Client supports one in-flight request, correlates by payload type, buffers one unsolicited frame — blocks periodic DIDs and misattributes late replies #8

Description

@JustinKovacich

Problem

The client can have exactly one request outstanding, matches responses by
DoIP payload type rather than by anything identifying the request, and keeps
one slot for anything unsolicited. Together these make server-push and
request pipelining impossible, and they are the mechanism behind a class of
misattribution bug we have already hit in the field.

All three, in src/client_inner.rs at 0e7969b:

active_request: Option<ControlMessage>,              // :100  — one slot, not a queue
pending_diagnostic_response: Option<OwnedMessage>,   // :121  — one message, not a queue
// :292 — correlation is "next diagnostic message wins"
// Use a placeholder message - we just want any diagnostic response
self.active_request = Some(ControlMessage::AwaitResponse(
    OwnedMessage::default(),
    response,
));
// :461 — and an unsolicited frame overwrites whatever was buffered
if matches!(received_message.payload, OwnedPayload::DiagnosticMessage(_)) {
    debug!("Buffering diagnostic response (no active request): {:?}", received_message);
    self.pending_diagnostic_response = Some(received_message);

A second request also supersedes the first rather than queueing behind it.

What it blocks

UDS periodic DIDs — F200F203, services 0x2A / 0x2C. This is the
protocol's server-push family: arm a DID and the ECU sends it on a schedule. Any
consumer that wants pushed telemetry has to poll instead, which is what
iris_diagnostics_client does today and documents as the supported shape.

Crucially, this is not merely a missing feature — arming a periodic DID today
would be actively wrong.
Because correlation is "next diagnostic message
wins", periodic frames arriving between a request and its response would be
consumed as that response. The caller would get a periodic payload where it
asked for something else, silently. So the limitation is load-bearing: it has to
stay until correlation changes.

Request pipelining, for the same reason.

It is also the mechanism behind a real field bug

#7 fixed a deadline that abandoned requests too early. The
damage was not the failed read — it was that the abandoned request's reply
arrived later with no waiter registered, got buffered, and answered the next
request. On a DID-catalog sweep against a real sensor, one slow identifier
misattributed the six reads after it, surfacing as short-payload decode
errors, DiagnosticMessageNacks and wrong-DID echoes on identifiers that were
perfectly healthy. It cost a day to trace and produced a firmware bug report that
had to be retracted.

#7 removed that particular trigger. It did not remove the hazard: any request
abandoned for any reason — a cancellation, a real transport error — still leaves
a reply that will be handed to whoever asks next.

Consumers currently defend against this by reconnecting on transport failure,
which works but is a blunt instrument for what is really a correlation problem.

Shape of a fix

  1. Correlate on identity, not payload type. A diagnostic message carries
    source and target logical addresses, and the UDS payload carries SID and (for
    0x22 / 0x2E) the DID. Matching a response to its request on those makes
    the "next message wins" behaviour impossible.
  2. A queue rather than a slot, so a second request does not displace the
    first and pipelining becomes expressible.
  3. A separate path for unsolicited frames — periodic DIDs, and anything else
    the entity sends unprompted — rather than a single overwritten buffer.
    A Stream alongside the request/response API would fit.
  4. Only then does arming 0x2A / 0x2C become safe to expose.

(1) alone would close the misattribution hazard and is the smallest useful step;
(3) is what unlocks periodic DIDs.

Worth noting ISO 13400 does not forbid multiple messages in flight — the limits
are UDS-side (P2/P2*) and whatever the entity supports. This is our own design
constraint, not the standard's.

Why file it now

The limitation is documented customer-facing in
iris_diagnostics_client's module docs and in the Iris SDK README, as the reason
its fault-monitoring examples poll rather than subscribe. We are telling
consumers this is the supported shape with nothing recording a path out of it.
This issue is that record; the two should stay in sync if the design changes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions