Re: [PATCH net-next v10 14/15] quic: add packet builder base

On 2/25/26 3:34 AM, Xin Long wrote:
> +/* Transmit a QUIC packet, possibly encrypting and bundling it. */
> +int quic_packet_xmit(struct sock *sk, struct sk_buff *skb)
> +{
> +     struct quic_packet *packet = quic_packet(sk);
> +     struct quic_skb_cb *cb = QUIC_SKB_CB(skb);
> +     struct net *net = sock_net(sk);
> +     int err;
> +
> +     /* Skip encryption if taglen == 0 (e.g., disable_1rtt_encryption). */
> +     if (!packet->taglen[quic_hdr(skb)->form])
> +             goto xmit;
> +
> +     cb->crypto_done = quic_packet_encrypt_done;
> +     /* Associate skb with sk to ensure sk is valid during async encryption completion. */
> +     WARN_ON(!skb_set_owner_sk_safe(skb, sk));

This is the TX path, how can sk refcout be 0 here? Possibly use
skb_set_owner_r() directly? At least use the WARN_ON_ONCE() variant and
add a comment documenting why is needed.

> +     err = quic_crypto_encrypt(quic_crypto(sk, packet->level), skb);
> +     if (err) {
> +             if (err != -EINPROGRESS) {
> +                     QUIC_INC_STATS(net, QUIC_MIB_PKT_ENCDROP);
> +                     kfree_skb(skb);
> +                     return err;
> +             }
> +             QUIC_INC_STATS(net, QUIC_MIB_PKT_ENCBACKLOGS);
> +             return err;
> +     }
> +     if (!cb->resume) /* Encryption completes synchronously. */
> +             QUIC_INC_STATS(net, QUIC_MIB_PKT_ENCFASTPATHS);
> +
> +xmit:
> +     if (quic_packet_bundle(sk, skb))
> +             quic_packet_flush(sk);
> +     return 0;
> +}
> +
> +/* Create and transmit a new QUIC packet. */
> +int quic_packet_create_and_xmit(struct sock *sk)
> +{
> +     struct quic_packet *packet = quic_packet(sk);
> +     struct sk_buff *skb;
> +     int err;
> +
> +     err = quic_packet_number_check(sk);
> +     if (err)
> +             goto err;
> +
> +     if (packet->level)
> +             skb = quic_packet_handshake_create(sk);
> +     else
> +             skb = quic_packet_app_create(sk);
> +     if (!skb) {
> +             err = -ENOMEM;
> +             goto err;
> +     }
> +
> +     err = quic_packet_xmit(sk, skb);
> +     if (err && err != -EINPROGRESS)
> +             goto err;
> +
> +     /* Return 1 if at least one ACK-eliciting (non-PING) frame was sent. */
> +     return !!packet->frames;
> +err:
> +     pr_debug("%s: err: %d\n", __func__, err);
> +     return 0;
> +}
> +
> +/* Flush any coalesced/bundled QUIC packets. */
> +void quic_packet_flush(struct sock *sk)
> +{
> +     struct quic_path_group *paths = quic_paths(sk);
> +     struct quic_packet *packet = quic_packet(sk);
> +
> +     if (packet->head) {
> +             quic_lower_xmit(sk, packet->head,
> +                             quic_path_daddr(paths, packet->path), &paths->fl);
> +             packet->head = NULL;
> +     }
> +}
> +
> +void quic_packet_init(struct sock *sk)
> +{
> +     struct quic_packet *packet = quic_packet(sk);
> +
> +     INIT_LIST_HEAD(&packet->frame_list);
> +     packet->taglen[0] = QUIC_TAG_LEN;
> +     packet->taglen[1] = QUIC_TAG_LEN;
> +     packet->mss[0] = QUIC_MIN_UDP_PAYLOAD;
> +     packet->mss[1] = QUIC_MIN_UDP_PAYLOAD;

The magic number above looks quite obscure, and AFAICS looking at struct
quick_packet comments have different meaning. Please use some macro instead.


/P