On 5/7/2024 12:18 PM, Willem de Bruijn wrote:
> Abhishek Chauhan (ABC) wrote:
>>
>>
>> On 5/7/2024 4:39 AM, Willem de Bruijn wrote:
>>> Martin KaFai Lau wrote:
>>>> On 5/3/24 8:13 PM, Abhishek Chauhan wrote:
>>>>> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
>>>>> index fe86cadfa85b..c3d852eecb01 100644
>>>>> --- a/net/ipv4/ip_output.c
>>>>> +++ b/net/ipv4/ip_output.c
>>>>> @@ -1457,7 +1457,10 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
>>>>>
>>>>> skb->priority = (cork->tos != -1) ? cork->priority: READ_ONCE(sk->sk_priority);
>>>>> skb->mark = cork->mark;
>>>>> - skb->tstamp = cork->transmit_time;
>>>>> + if (sk_is_tcp(sk))
>>>>
>>>> This seems not catching all IPPROTO_TCP case. In particular, the percpu
>>>> "ipv4_tcp_sk" is SOCK_RAW. sk_is_tcp() is checking SOCK_STREAM:
>>>>
>>>> void __init tcp_v4_init(void)
>>>> {
>>>>
>>>> /* ... */
>>>> res = inet_ctl_sock_create(&sk, PF_INET, SOCK_RAW,
>>>> IPPROTO_TCP, &init_net);
>>>>
>>>> /* ... */
>>>> }
>>>>
>>>> "while :; do ./test_progs -t tc_redirect/tc_redirect_dtime || break; done"
>>>> failed pretty often exactly in this case.
>>>>
>>>
>>> Interesting. The TCP stack opens non TCP sockets.
>>>
>>> Initializing sk->sk_clockid for this socket should address that.
>>>
>> Willem, Are you suggesting your point from the previous patch ?
>>
>
> No, just for this custom socket to initialize the sk_clockid. It is
> not a TCP socket, but only used by TCP.
Thanks Willem,
Noted! Which means there are only two places these custom RAW tcp socket
are getting called
1. tcp_ipv4.c
2. tcp_ipv6.c
I will take care of initializing sk_clockid to monotonic in the next patch
in the above two files.
Let me know if i missed out anything.
