Skip to content

Commit

Permalink
net: skb: free up one bit in tx_flags
Browse files Browse the repository at this point in the history
The linked series wants to add skb tx completion timestamps.
That needs a bit in skb_shared_info.tx_flags, but all are in use.

A per-skb bit is only needed for features that are configured on a
per packet basis. Per socket features can be read from sk->sk_tsflags.

Per packet tsflags can be set in sendmsg using cmsg, but only those in
SOF_TIMESTAMPING_TX_RECORD_MASK.

Per packet tsflags can also be set without cmsg by sandwiching a
send inbetween two setsockopts:

    val |= SOF_TIMESTAMPING_$FEATURE;
    setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING, &val, sizeof(val));
    write(fd, buf, sz);
    val &= ~SOF_TIMESTAMPING_$FEATURE;
    setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING, &val, sizeof(val));

Changing a datapath test from skb_shinfo(skb)->tx_flags to
skb->sk->sk_tsflags can change behavior in that case, as the tx_flags
is written before the second setsockopt updates sk_tsflags.

Therefore, only bits can be reclaimed that cannot be set by cmsg and
are also highly unlikely to be used to target individual packets
otherwise.

Free up the bit currently used for SKBTX_HW_TSTAMP_USE_CYCLES. This
selects between clock and free running counter source for HW TX
timestamps. It is probable that all packets of the same socket will
always use the same source.

Link: https://lore.kernel.org/netdev/cover.1739988644.git.pav@iki.fi/
Signed-off-by: Willem de Bruijn <willemb@google.com>
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
Link: https://patch.msgid.link/20250225023416.2088705-1-willemdebruijn.kernel@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Willem de Bruijn authored and Jakub Kicinski committed Feb 27, 2025
1 parent 80c4a00 commit e6116fc
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 16 deletions.
4 changes: 2 additions & 2 deletions drivers/net/ethernet/engleder/tsnep_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -852,8 +852,8 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
struct skb_shared_hwtstamps hwtstamps;
u64 timestamp;

if (skb_shinfo(entry->skb)->tx_flags &
SKBTX_HW_TSTAMP_USE_CYCLES)
if (entry->skb->sk &&
READ_ONCE(entry->skb->sk->sk_tsflags) & SOF_TIMESTAMPING_BIND_PHC)
timestamp =
__le64_to_cpu(entry->desc_wb->counter);
else
Expand Down
3 changes: 2 additions & 1 deletion drivers/net/ethernet/intel/igc/igc_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1668,7 +1668,8 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
if (igc_request_tx_tstamp(adapter, skb, &tstamp_flags)) {
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
tx_flags |= IGC_TX_FLAGS_TSTAMP | tstamp_flags;
if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_USE_CYCLES)
if (skb->sk &&
READ_ONCE(skb->sk->sk_tsflags) & SOF_TIMESTAMPING_BIND_PHC)
tx_flags |= IGC_TX_FLAGS_TSTAMP_TIMER_1;
} else {
adapter->tx_hwtstamp_skipped++;
Expand Down
5 changes: 2 additions & 3 deletions include/linux/skbuff.h
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,8 @@ enum {
/* device driver is going to provide hardware time stamp */
SKBTX_IN_PROGRESS = 1 << 2,

/* generate hardware time stamp based on cycles if supported */
SKBTX_HW_TSTAMP_USE_CYCLES = 1 << 3,
/* reserved */
SKBTX_RESERVED = 1 << 3,

/* generate wifi status information (where possible) */
SKBTX_WIFI_STATUS = 1 << 4,
Expand All @@ -500,7 +500,6 @@ enum {
SKBTX_SCHED_TSTAMP | \
SKBTX_BPF)
#define SKBTX_ANY_TSTAMP (SKBTX_HW_TSTAMP | \
SKBTX_HW_TSTAMP_USE_CYCLES | \
SKBTX_ANY_SW_TSTAMP)

/* Definitions for flags in struct skb_shared_info */
Expand Down
11 changes: 1 addition & 10 deletions net/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -680,18 +680,9 @@ void __sock_tx_timestamp(__u32 tsflags, __u8 *tx_flags)
{
u8 flags = *tx_flags;

if (tsflags & SOF_TIMESTAMPING_TX_HARDWARE) {
if (tsflags & SOF_TIMESTAMPING_TX_HARDWARE)
flags |= SKBTX_HW_TSTAMP_NOBPF;

/* PTP hardware clocks can provide a free running cycle counter
* as a time base for virtual clocks. Tell driver to use the
* free running cycle counter for timestamp if socket is bound
* to virtual clock.
*/
if (tsflags & SOF_TIMESTAMPING_BIND_PHC)
flags |= SKBTX_HW_TSTAMP_USE_CYCLES;
}

if (tsflags & SOF_TIMESTAMPING_TX_SOFTWARE)
flags |= SKBTX_SW_TSTAMP;

Expand Down

0 comments on commit e6116fc

Please sign in to comment.