Skip to content

[~] #820 fix active_connection_id_limit: only exclude self-generated CID per RFC 9000 §18.2#821

Open
dreamwind1985 wants to merge 2 commits into
mainfrom
fix/fix_issue_820
Open

[~] #820 fix active_connection_id_limit: only exclude self-generated CID per RFC 9000 §18.2#821
dreamwind1985 wants to merge 2 commits into
mainfrom
fix/fix_issue_820

Conversation

@dreamwind1985

Copy link
Copy Markdown
Collaborator

Summary

Per RFC 9000 §18.2, active_connection_id_limit is the maximum number of connection IDs from the peer that an endpoint is willing to store, and it includes the connection ID received during the handshake.

The previous fix (#777, commits 26f2c50 and 1db5fd5) incorrectly excluded all handshake CIDs from the limit, citing a non-existent RFC §5.1.1 quote. This caused xquic to issue too many NEW_CONNECTION_ID frames, exceeding the peer's actual limit and triggering CONNECTION_ID_LIMIT_ERROR in nginx, ngtcp2, quiche, etc.

Root Cause

xquic's dcid_set contains two handshake CIDs:

  1. The client's self-generated initial DCID — NOT from the peer, should NOT count toward the limit
  2. The server's SCID from the Initial response — IS from the peer, should count toward the limit

The previous fix excluded both, but only #1 should be excluded.

Fix

  • Redefine original_cid_cnt to track only self-generated CIDs (not from the peer), not all handshake CIDs
  • Remove the mark_original call for the server's SCID (pkt->pkt_scid) in xqc_conn.c, since it IS from the peer and counts toward the limit
  • Update countable_cnt comments to reference RFC 9000 §18.2 correctly

Verification

After fix, for active_connection_id_limit=8 (nginx default):

  • countable = (unused + used) - original_cid_cnt = (7 + 2) - 1 = 8 == limit
  • nclient_ids = 1(handshake) + 7(NCID) = 8 <= 8

Interop test results (xquic vs nginx): 14/16 pass (2 unrelated failures, 2 unsupported)

Closes #820

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: active_connection_id_limit causes interoperability issues after #776

1 participant