On 3/14/24 1:53 PM, Abhishek Chauhan (ABC) wrote:
The bpf_convert_tstamp_{read,write} and the helper bpf_skb_set_tstamp need to be
changed to handle the new "user_delivery_time" bit anyway, e.g.
bpf_skb_set_tstamp(BPF_SKB_TSTAMP_DELIVERY_MONO) needs to clear the
"user_delivery_time" bit.
I think the "struct inet_frag_queue" also needs a new "user_delivery_time"
field. "mono_delivery_time" is already in there.
[ ... ]
I would think the first step is to revert this patch. I don't think much of the
current patch can be reused.
1. I will raise one patch to introduce rename mono_delivery_time to
tstamp_type
Right, I expect something like this:
struct sk_buff {
/* ... */
- __u8 mono_delivery_time:1;
+ __u8 tstamp_type:1;
/* ... */
};
2. I will introduce setting of userspace timestamp type as the second bit
whem transmit_time is set.
I expect the second patch should be introducing the enum first
enum skb_tstamp_type {
SKB_TSTAMP_TYPE_RX_REAL = 0, /* A RX (receive) time in real */
SKB_TSTAMP_TYPE_TX_MONO = 1, /* A TX (delivery) time in mono */
};
and start doing "skb->tstamp_type = SKB_TSTAMP_TYPE_TX_MONO;" instead of
"skb->tstamp_type = 1;"
and the same for "skb->tstamp_type = SKB_TSTAMP_TYPE_RX_REAL;" instead of
"skb->tstamp_type = 0;"
This one I am not sure but probably need to change the skb_set_delivery_time()
function signature also:
static inline void skb_set_delivery_time(struct sk_buff *skb, ktime_t kt,
- bool mono)
+ enum skb_tstamp_type tstamp_type)
The third patch is to change tstamp_type from 1 bit to 2 bits and add
SKB_TSTAMP_TYPE_TX_USER.
struct sk_buff {
/* ... */
- __u8 tstamp_type:1;
+ __u8 tstamp_type:2;
/* ... */
};
enum skb_tstamp_type {
SKB_TSTAMP_TYPE_RX_REAL = 0, /* A RX (receive) time in real */
SKB_TSTAMP_TYPE_TX_MONO = 1, /* A TX (delivery) time in mono */
+ SKB_TSTAMP_TYPE_TX_USER = 2, /* A TX (delivery) time and its clock
* is in skb->sk->sk_clockid.
*/
};
This will shift a bit out of the byte where tstamp_type lives. It should be the
"inner_protocol_type" bit by my hand count. Please check if it is directly used
in bpf instruction (filter.c). As far as I look, it is not, so should be fine.
Some details about bpf instruction accessible skb bit field here:
https://lore.kernel.org/all/20230321014115.997841-1-kuba@xxxxxxxxxx/
3. This will be a first step to make the design scalable.
4. Tomorrow if we have more timestamp to support, upstream community has to do is
update the enum and increase the bitfield from 2=>3 and so on.
I need help from Martin to test the patch which renames the mono_delivery_time
to tstamp_type (Which i feel should be straight forward as the value of the bit is 1)
The bpf change is not a no-op rename of mono_delivery_time. It needs to take
care of the new bit added to the tstamp_type. Please see the previous email (and
I also left it in the beginning of this email).
Thus, you need to compile the selftests/bpf/ and run it to verify the changes
when handling the new bit. The Documentation/bpf/bpf_devel_QA.rst has the howto
details. You probably only need the newer llvm (newer gcc should work also as
bpf CI has been using it) and the newer pahole. I can definitely help if there
is issue in running the test_progs in selftests/bpf or you have question on
making the changes in filter.c. To run the test: "./test_progs -t
tc_redirect/tc_redirect_dtime"
