[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250709190559883SkXisbe8RTqw6jz6kycBo@zte.com.cn>
Date: Wed, 9 Jul 2025 19:05:59 +0800 (CST)
From: <fan.yu9@....com.cn>
To: <edumazet@...gle.com>
Cc: <xu.xin16@....com.cn>, <kuba@...nel.org>, <kuniyu@...zon.com>,
<ncardwell@...gle.com>, <davem@...emloft.net>, <horms@...nel.org>,
<dsahern@...nel.org>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-trace-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2] tcp: extend tcp_retransmit_skb tracepoint with failure reasons
> > > >
> > > > -/*
> > > > - * tcp event with arguments sk and skb
> > > > - *
> > > > - * Note: this class requires a valid sk pointer; while skb pointer could
> > > > - * be NULL.
> > > > - */
> > > > -DECLARE_EVENT_CLASS(tcp_event_sk_skb,
> > > > +#define TCP_RETRANSMIT_QUIT_REASON \
> > > > + ENUM(TCP_RETRANS_ERR_DEFAULT, "retransmit terminate unexpectedly") \
> > > > + ENUM(TCP_RETRANS_SUCCESS, "retransmit successfully") \
> > > > + ENUM(TCP_RETRANS_IN_HOST_QUEUE, "packet still queued in driver") \
> > > > + ENUM(TCP_RETRANS_END_SEQ_ERROR, "invalid end sequence") \
> > > > + ENUM(TCP_RETRANS_TRIM_HEAD_NOMEM, "trim head no memory") \
> > > > + ENUM(TCP_RETRANS_UNCLONE_NOMEM, "skb unclone keeptruesize no memory") \
> > > > + ENUM(TCP_RETRANS_FRAG_NOMEM, "fragment no memory") \
> > >
> > > Do we really need 3 + 1 different 'NOMEMORY' status ?
> >
> > Yes, different "NOMEM" status pinpoint exact failure stages in packet retransmission,
> > which helps distinguish which process triggered it. Beneficia
>
> ENOMEM is ENOMEM. Honestly I fail to see why it matters.
>
> If this was the case, we would have thousands of different ENOMEM errnos.
Hi Eric,
Thanks for reviewing the patch and your valid feedback – I agree with your point
that excessive ENOMEM granularity could create unnecessary redundancy in the kernel.
My initial intent was to differentiate allocation failures in TCP retransmission
(TRIM_HEAD, UNCLONE, FRAG) to pinpoint exact failure points during debugging,
since these errors occur at distinct stages with different stack contexts.
However, I recognize that ENOMEM itself sufficiently conveys the core issue.
We will consolidate these into a unified TCP_RETRANS_NOMEM in v4 of the patch.
Appreciate your time and expertise!
<div class="zcontentRow"><p>> > > ></p><p>> > > > -/*</p><p>> > > > - * tcp event with arguments sk and skb</p><p>> > > > - *</p><p>> > > > - * Note: this class requires a valid sk pointer; while skb pointer could</p><p>> > > > - * be NULL.</p><p>> > > > - */</p><p>> > > > -DECLARE_EVENT_CLASS(tcp_event_sk_skb,</p><p>> > > > +#define TCP_RETRANSMIT_QUIT_REASON \</p><p>> > > > + ENUM(TCP_RETRANS_ERR_DEFAULT, "retransmit terminate unexpectedly") \</p><p>> > > > + ENUM(TCP_RETRANS_SUCCESS, "retransmit successfully") \</p><p>> > > > + ENUM(TCP_RETRANS_IN_HOST_QUEUE, "packet still queued in driver") \</p><p>> > > > + ENUM(TCP_RETRANS_END_SEQ_ERROR, "invalid end sequence") \</p><p>> > > > + ENUM(TCP_RETRANS_TRIM_HEAD_NOMEM, "trim head no memory") \</p><p>> > > > + ENUM(TCP_RETRANS_UNCLONE_NOMEM, "skb unclone keeptruesize no memory") \</p><p>> > > > + ENUM(TCP_RETRANS_FRAG_NOMEM, "fragment no memory") \</p><p>> > ></p><p>> > > Do we really need 3 + 1 different 'NOMEMORY' status ?</p><p>> ></p><p>> > Yes, different "NOMEM" status pinpoint exact failure stages in packet retransmission,</p><p>> > which helps distinguish which process triggered it. Beneficia</p><p>></p><p>> ENOMEM is ENOMEM. Honestly I fail to see why it matters.</p><p>></p><p>> If this was the case, we would have thousands of different ENOMEM errnos.</p><p><br></p><p>Hi Eric,</p><p><br></p><p>Thanks for reviewing the patch and your valid feedback – I agree with your point</p><p>that excessive ENOMEM granularity could create unnecessary redundancy in the kernel.</p><p><br></p><p>My initial intent was to differentiate allocation failures in TCP retransmission</p><p>(TRIM_HEAD, UNCLONE, FRAG) to pinpoint exact failure points during debugging,</p><p>since these errors occur at distinct stages with different stack contexts.</p><p>However, I recognize that ENOMEM itself sufficiently conveys the core issue.</p><p><br></p><p>We will consolidate these into a unified TCP_RETRANS_NOMEM in v4 of the patch.</p><p>Appreciate your time and expertise!</p><p><br></p><p><br></p></div>
Powered by blists - more mailing lists