Jason Xing wrote:
> On Wed, Mar 19, 2025 at 3:10 AM Pauli Virtanen <pav@xxxxxx> wrote:
> >
> > Support enabling TX timestamping for some skbs, and track them until
> > packet completion. Generate software SCM_TSTAMP_COMPLETION when getting
> > completion report from hardware.
> >
> > Generate software SCM_TSTAMP_SND before sending to driver. Sending from
> > driver requires changes in the driver API, and drivers mostly are going
> > to send the skb immediately.
> >
> > Make the default situation with no COMPLETION TX timestamping more
> > efficient by only counting packets in the queue when there is nothing to
> > track. When there is something to track, we need to make clones, since
> > the driver may modify sent skbs.
Why count packets at all? And if useful separate from completions,
should that be a separate patch?
> > +void hci_conn_tx_queue(struct hci_conn *conn, struct sk_buff *skb)
> > +{
> > + struct tx_queue *comp = &conn->tx_q;
> > + bool track = false;
> > +
> > + /* Emit SND now, ie. just before sending to driver */
> > + if (skb_shinfo(skb)->tx_flags & SKBTX_SW_TSTAMP)
> > + __skb_tstamp_tx(skb, NULL, NULL, skb->sk, SCM_TSTAMP_SND);
>
> It's a bit strange that SCM_TSTAMP_SND is under the control of
> SKBTX_SW_TSTAMP. Can we use the same flag for both lines here
> directly? I suppose I would use SKBTX_SW_TSTAMP then.
This is the established behavior.
>
> > +
> > + /* COMPLETION tstamp is emitted for tracked skb later in Number of
> > + * Completed Packets event. Available only for flow controlled cases.
> > + *
> > + * TODO: SCO support without flowctl (needs to be done in drivers)
> > + */
> > + switch (conn->type) {
> > + case ISO_LINK:
> > + case ACL_LINK:
> > + case LE_LINK:
> > + break;
> > + case SCO_LINK:
> > + case ESCO_LINK:
> > + if (!hci_dev_test_flag(conn->hdev, HCI_SCO_FLOWCTL))
> > + return;
> > + break;
> > + default:
> > + return;
> > + }
> > +
> > + if (skb->sk && (skb_shinfo(skb)->tx_flags & SKBTX_COMPLETION_TSTAMP))
> > + track = true;
> > +
> > + /* If nothing is tracked, just count extra skbs at the queue head */
> > + if (!track && !comp->tracked) {
> > + comp->extra++;
> > + return;
> > + }
> > +
> > + if (track) {
> > + skb = skb_clone_sk(skb);
> > + if (!skb)
> > + goto count_only;
> > +
> > + comp->tracked++;
> > + } else {
> > + skb = skb_clone(skb, GFP_KERNEL);
> > + if (!skb)
> > + goto count_only;
> > + }
What is the difference between track and comp->tracked
