[PATCH net-next] net: thunder: make tx software timestamp independent

[PATCH net-next] net: thunder: make tx software timestamp independent

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

skb_tx_timestamp() is used for tx software timestamp enabled by
SOF_TIMESTAMPING_TX_SOFTWARE while SKBTX_HW_TSTAMP is controlled by
SOF_TIMESTAMPING_TX_HARDWARE. As it clearly shows they are different
timestamps in two dimensions, this patch makes the software one
standalone.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 06397cc8bb36..d368f381b6de 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -1389,11 +1389,11 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
 		this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
 	}
 
+	skb_tx_timestamp(skb);
+
 	/* Check if timestamp is requested */
-	if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
-		skb_tx_timestamp(skb);
+	if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
 		return;
-	}
 
 	/* Tx timestamping not supported along with TSO, so ignore request */
 	if (skb_shinfo(skb)->gso_size)
-- 
2.43.5

Re: [PATCH net-next] net: thunder: make tx software timestamp independent

[Willem de Bruijn]

Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> skb_tx_timestamp() is used for tx software timestamp enabled by
> SOF_TIMESTAMPING_TX_SOFTWARE while SKBTX_HW_TSTAMP is controlled by
> SOF_TIMESTAMPING_TX_HARDWARE. As it clearly shows they are different
> timestamps in two dimensions, this patch makes the software one
> standalone.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> index 06397cc8bb36..d368f381b6de 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> @@ -1389,11 +1389,11 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
>  		this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
>  	}
>  
> +	skb_tx_timestamp(skb);
> +
>  	/* Check if timestamp is requested */

Nit: check if hw timestamp is requested.

> -	if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> -		skb_tx_timestamp(skb);
> +	if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
>  		return;
> -	}

The SO_TIMESTAMPING behavior around both software and hardware
timestamps is a bit odd.

Unless SOF_TIMESTAMPING_OPT_TX_SWHW is set, by default a driver will
only return software if no hardware timestamp is also requested.

Through the following in __skb_tstamp_tx

        if (!hwtstamps && !(tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) &&
            skb_shinfo(orig_skb)->tx_flags & SKBTX_IN_PROGRESS)
                return;

There really is no good reason to have this dependency. But it is
historical and all drivers should implement the same behavior.

This automatically happens if the software timestamp request
skb_tx_timestamp is called after the hardware timestamp request
is configured, i.e., after SKBTX_IN_PROGRESS is set. That usually
happens because the software timestamp is requests as close to kicking
the doorbell as possible.

In this driver, that would be not in nicvf_sq_add_hdr_subdesc, but
just before calling nicvf_sq_doorbell. Unfortunately, there are two
callers, TSO and non-TSO.

>  
>  	/* Tx timestamping not supported along with TSO, so ignore request */
>  	if (skb_shinfo(skb)->gso_size)
> -- 
> 2.43.5
> 

Re: [PATCH net-next] net: thunder: make tx software timestamp independent

[Jason Xing]

Hi Willem,

On Wed, May 7, 2025 at 9:23 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > skb_tx_timestamp() is used for tx software timestamp enabled by
> > SOF_TIMESTAMPING_TX_SOFTWARE while SKBTX_HW_TSTAMP is controlled by
> > SOF_TIMESTAMPING_TX_HARDWARE. As it clearly shows they are different
> > timestamps in two dimensions, this patch makes the software one
> > standalone.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >  drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > index 06397cc8bb36..d368f381b6de 100644
> > --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > @@ -1389,11 +1389,11 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
> >               this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
> >       }
> >
> > +     skb_tx_timestamp(skb);
> > +
> >       /* Check if timestamp is requested */
>
> Nit: check if hw timestamp is requested.

Thanks for the review. Will change it.

>
> > -     if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> > -             skb_tx_timestamp(skb);
> > +     if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
> >               return;
> > -     }
>
> The SO_TIMESTAMPING behavior around both software and hardware
> timestamps is a bit odd.

Just a little bit. The reason why I looked into this driver is because
I was reviewing this recent patch[1]. Then I found that the thunder
driver uses the HW flag to test if we can generate a software
timestamp which is also a little bit odd. Software timestamp function
is controlled by the SW flag or SWHW flag instead of the pure HW flag.

[1]: https://lore.kernel.org/all/20250506215508.3611977-1-stfomichev@gmail.com/

>
> Unless SOF_TIMESTAMPING_OPT_TX_SWHW is set, by default a driver will
> only return software if no hardware timestamp is also requested.

Sure thing. SOF_TIMESTAMPING_OPT_TX_SWHW can be used in this case as
well as patch [1].

>
> Through the following in __skb_tstamp_tx
>
>         if (!hwtstamps && !(tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) &&
>             skb_shinfo(orig_skb)->tx_flags & SKBTX_IN_PROGRESS)
>                 return;
>
> There really is no good reason to have this dependency. But it is
> historical and all drivers should implement the same behavior.

As you said, this morning when I was reviewing patch[1], I noticed
that thunder code is not that consistent with others.

>
> This automatically happens if the software timestamp request
> skb_tx_timestamp is called after the hardware timestamp request
> is configured, i.e., after SKBTX_IN_PROGRESS is set. That usually
> happens because the software timestamp is requests as close to kicking
> the doorbell as possible.

Right. In most cases, they implemented in such an order:

                if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
                        skb_shinfo(skb)->tx_flags |=
SKBTX_IN_PROGRESS;

                skb_tx_timestamp(skb);

Should I adjust this patch to have the same behavior in the next
revision like below[2]? Then we can get the conclusion:1) if only the
HW or SW flag is set, nothing changes and only corresponding timestamp
will be generated, 2) if HW and SW are set without the HWSW flag, it
will check the HW first. In non TSO mode, If the non outstanding skb
misses the HW timestamp, then the software timestamp will be
generated, 3) if HW and SW and HWSW are set with the HWSW flag, two
types of timestamp can be generated. To put it in a simpler way, after
[2] patch, thunder driver works like other drivers. Or else, without
[2], the HWSW flag doesn't even work.

[2]
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 06397cc8bb36..4be562ead392 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -1389,28 +1389,24 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic,
struct snd_queue *sq, int qentry,
                this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
        }

-       /* Check if timestamp is requested */
-       if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
-               skb_tx_timestamp(skb);
-               return;
-       }

-       /* Tx timestamping not supported along with TSO, so ignore request */
-       if (skb_shinfo(skb)->gso_size)
-               return;
-
-       /* HW supports only a single outstanding packet to timestamp */
-       if (!atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1))
-               return;
-
-       /* Mark the SKB for later reference */
-       skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+       /* Check if hw timestamp is requested */
+       if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
+           /* Tx timestamping not supported along with TSO, so ignore
request */
+           !skb_shinfo(skb)->gso_size &&
+           /* HW supports only a single outstanding packet to timestamp */
+           atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1)) {
+               /* Mark the SKB for later reference */
+               skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+
+               /* Finally enable timestamp generation
+                * Since 'post_cqe' is also set, two CQEs will be posted
+                * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
+                */
+               hdr->tstmp = 1;
+       }

-       /* Finally enable timestamp generation
-        * Since 'post_cqe' is also set, two CQEs will be posted
-        * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
-        */
-       hdr->tstmp = 1;
+       skb_tx_timestamp(skb);
 }

 /* SQ GATHER subdescriptor

Thanks,
Jason

>
> In this driver, that would be not in nicvf_sq_add_hdr_subdesc, but
> just before calling nicvf_sq_doorbell. Unfortunately, there are two
> callers, TSO and non-TSO.
>
> >
> >       /* Tx timestamping not supported along with TSO, so ignore request */
> >       if (skb_shinfo(skb)->gso_size)
> > --
> > 2.43.5
> >
>
>

Re: [PATCH net-next] net: thunder: make tx software timestamp independent

[Willem de Bruijn]

Jason Xing wrote:
> Hi Willem,
> 
> On Wed, May 7, 2025 at 9:23 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Jason Xing wrote:
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > skb_tx_timestamp() is used for tx software timestamp enabled by
> > > SOF_TIMESTAMPING_TX_SOFTWARE while SKBTX_HW_TSTAMP is controlled by
> > > SOF_TIMESTAMPING_TX_HARDWARE. As it clearly shows they are different
> > > timestamps in two dimensions, this patch makes the software one
> > > standalone.
> > >
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > ---
> > >  drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > > index 06397cc8bb36..d368f381b6de 100644
> > > --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > > +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > > @@ -1389,11 +1389,11 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
> > >               this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
> > >       }
> > >
> > > +     skb_tx_timestamp(skb);
> > > +
> > >       /* Check if timestamp is requested */
> >
> > Nit: check if hw timestamp is requested.
> 
> Thanks for the review. Will change it.
> 
> >
> > > -     if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> > > -             skb_tx_timestamp(skb);
> > > +     if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
> > >               return;
> > > -     }
> >
> > The SO_TIMESTAMPING behavior around both software and hardware
> > timestamps is a bit odd.
> 
> Just a little bit. The reason why I looked into this driver is because
> I was reviewing this recent patch[1]. Then I found that the thunder
> driver uses the HW flag to test if we can generate a software
> timestamp which is also a little bit odd. Software timestamp function
> is controlled by the SW flag or SWHW flag instead of the pure HW flag.
> 
> [1]: https://lore.kernel.org/all/20250506215508.3611977-1-stfomichev@gmail.com/
> 
> >
> > Unless SOF_TIMESTAMPING_OPT_TX_SWHW is set, by default a driver will
> > only return software if no hardware timestamp is also requested.
> 
> Sure thing. SOF_TIMESTAMPING_OPT_TX_SWHW can be used in this case as
> well as patch [1].
> 
> >
> > Through the following in __skb_tstamp_tx
> >
> >         if (!hwtstamps && !(tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) &&
> >             skb_shinfo(orig_skb)->tx_flags & SKBTX_IN_PROGRESS)
> >                 return;
> >
> > There really is no good reason to have this dependency. But it is
> > historical and all drivers should implement the same behavior.
> 
> As you said, this morning when I was reviewing patch[1], I noticed
> that thunder code is not that consistent with others.
> 
> >
> > This automatically happens if the software timestamp request
> > skb_tx_timestamp is called after the hardware timestamp request
> > is configured, i.e., after SKBTX_IN_PROGRESS is set. That usually
> > happens because the software timestamp is requests as close to kicking
> > the doorbell as possible.
> 
> Right. In most cases, they implemented in such an order:
> 
>                 if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
>                         skb_shinfo(skb)->tx_flags |=
> SKBTX_IN_PROGRESS;
> 
>                 skb_tx_timestamp(skb);
> 
> Should I adjust this patch to have the same behavior in the next
> revision like below[2]?

Best is just to do what other drivers do with the software
timestamp: take it as late as possible in ndo_start_xmit,
meaning just before ringing the doorbell.

For this driver, that is in two places, as said.

> Then we can get the conclusion:1) if only the
> HW or SW flag is set, nothing changes and only corresponding timestamp
> will be generated, 2) if HW and SW are set without the HWSW flag, it
> will check the HW first. In non TSO mode, If the non outstanding skb
> misses the HW timestamp, then the software timestamp will be
> generated, 3) if HW and SW and HWSW are set with the HWSW flag, two
> types of timestamp can be generated. To put it in a simpler way, after
> [2] patch, thunder driver works like other drivers. Or else, without
> [2], the HWSW flag doesn't even work.
> 
> [2]
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> index 06397cc8bb36..4be562ead392 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> @@ -1389,28 +1389,24 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic,
> struct snd_queue *sq, int qentry,
>                 this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
>         }
> 
> -       /* Check if timestamp is requested */
> -       if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> -               skb_tx_timestamp(skb);
> -               return;
> -       }
> 
> -       /* Tx timestamping not supported along with TSO, so ignore request */
> -       if (skb_shinfo(skb)->gso_size)
> -               return;
> -
> -       /* HW supports only a single outstanding packet to timestamp */
> -       if (!atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1))
> -               return;
> -
> -       /* Mark the SKB for later reference */
> -       skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> +       /* Check if hw timestamp is requested */
> +       if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
> +           /* Tx timestamping not supported along with TSO, so ignore
> request */
> +           !skb_shinfo(skb)->gso_size &&
> +           /* HW supports only a single outstanding packet to timestamp */
> +           atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1)) {
> +               /* Mark the SKB for later reference */
> +               skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> +
> +               /* Finally enable timestamp generation
> +                * Since 'post_cqe' is also set, two CQEs will be posted
> +                * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
> +                */
> +               hdr->tstmp = 1;
> +       }
> 
> -       /* Finally enable timestamp generation
> -        * Since 'post_cqe' is also set, two CQEs will be posted
> -        * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
> -        */
> -       hdr->tstmp = 1;
> +       skb_tx_timestamp(skb);
>  }
> 
>  /* SQ GATHER subdescriptor
> 
> Thanks,
> Jason
> 
> >
> > In this driver, that would be not in nicvf_sq_add_hdr_subdesc, but
> > just before calling nicvf_sq_doorbell. Unfortunately, there are two
> > callers, TSO and non-TSO.
> >
> > >
> > >       /* Tx timestamping not supported along with TSO, so ignore request */
> > >       if (skb_shinfo(skb)->gso_size)
> > > --
> > > 2.43.5
> > >
> >
> >

Re: [PATCH net-next] net: thunder: make tx software timestamp independent

[Jason Xing]

On Thu, May 8, 2025 at 1:49 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > Hi Willem,
> >
> > On Wed, May 7, 2025 at 9:23 PM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > Jason Xing wrote:
> > > > From: Jason Xing <kernelxing@tencent.com>
> > > >
> > > > skb_tx_timestamp() is used for tx software timestamp enabled by
> > > > SOF_TIMESTAMPING_TX_SOFTWARE while SKBTX_HW_TSTAMP is controlled by
> > > > SOF_TIMESTAMPING_TX_HARDWARE. As it clearly shows they are different
> > > > timestamps in two dimensions, this patch makes the software one
> > > > standalone.
> > > >
> > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > > ---
> > > >  drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > > > index 06397cc8bb36..d368f381b6de 100644
> > > > --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > > > +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > > > @@ -1389,11 +1389,11 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
> > > >               this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
> > > >       }
> > > >
> > > > +     skb_tx_timestamp(skb);
> > > > +
> > > >       /* Check if timestamp is requested */
> > >
> > > Nit: check if hw timestamp is requested.
> >
> > Thanks for the review. Will change it.
> >
> > >
> > > > -     if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> > > > -             skb_tx_timestamp(skb);
> > > > +     if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
> > > >               return;
> > > > -     }
> > >
> > > The SO_TIMESTAMPING behavior around both software and hardware
> > > timestamps is a bit odd.
> >
> > Just a little bit. The reason why I looked into this driver is because
> > I was reviewing this recent patch[1]. Then I found that the thunder
> > driver uses the HW flag to test if we can generate a software
> > timestamp which is also a little bit odd. Software timestamp function
> > is controlled by the SW flag or SWHW flag instead of the pure HW flag.
> >
> > [1]: https://lore.kernel.org/all/20250506215508.3611977-1-stfomichev@gmail.com/
> >
> > >
> > > Unless SOF_TIMESTAMPING_OPT_TX_SWHW is set, by default a driver will
> > > only return software if no hardware timestamp is also requested.
> >
> > Sure thing. SOF_TIMESTAMPING_OPT_TX_SWHW can be used in this case as
> > well as patch [1].
> >
> > >
> > > Through the following in __skb_tstamp_tx
> > >
> > >         if (!hwtstamps && !(tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) &&
> > >             skb_shinfo(orig_skb)->tx_flags & SKBTX_IN_PROGRESS)
> > >                 return;
> > >
> > > There really is no good reason to have this dependency. But it is
> > > historical and all drivers should implement the same behavior.
> >
> > As you said, this morning when I was reviewing patch[1], I noticed
> > that thunder code is not that consistent with others.
> >
> > >
> > > This automatically happens if the software timestamp request
> > > skb_tx_timestamp is called after the hardware timestamp request
> > > is configured, i.e., after SKBTX_IN_PROGRESS is set. That usually
> > > happens because the software timestamp is requests as close to kicking
> > > the doorbell as possible.
> >
> > Right. In most cases, they implemented in such an order:
> >
> >                 if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
> >                         skb_shinfo(skb)->tx_flags |=
> > SKBTX_IN_PROGRESS;
> >
> >                 skb_tx_timestamp(skb);
> >
> > Should I adjust this patch to have the same behavior in the next
> > revision like below[2]?
>
> Best is just to do what other drivers do with the software
> timestamp: take it as late as possible in ndo_start_xmit,
> meaning just before ringing the doorbell.
>
> For this driver, that is in two places, as said.

Oh, now I see your other core suggestion. Thanks!

Probably I will split it into a small series to make each patch only
do one thing.

Thanks,
Jason

>
> > Then we can get the conclusion:1) if only the
> > HW or SW flag is set, nothing changes and only corresponding timestamp
> > will be generated, 2) if HW and SW are set without the HWSW flag, it
> > will check the HW first. In non TSO mode, If the non outstanding skb
> > misses the HW timestamp, then the software timestamp will be
> > generated, 3) if HW and SW and HWSW are set with the HWSW flag, two
> > types of timestamp can be generated. To put it in a simpler way, after
> > [2] patch, thunder driver works like other drivers. Or else, without
> > [2], the HWSW flag doesn't even work.
> >
> > [2]
> > diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > index 06397cc8bb36..4be562ead392 100644
> > --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> > @@ -1389,28 +1389,24 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic,
> > struct snd_queue *sq, int qentry,
> >                 this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
> >         }
> >
> > -       /* Check if timestamp is requested */
> > -       if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> > -               skb_tx_timestamp(skb);
> > -               return;
> > -       }
> >
> > -       /* Tx timestamping not supported along with TSO, so ignore request */
> > -       if (skb_shinfo(skb)->gso_size)
> > -               return;
> > -
> > -       /* HW supports only a single outstanding packet to timestamp */
> > -       if (!atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1))
> > -               return;
> > -
> > -       /* Mark the SKB for later reference */
> > -       skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> > +       /* Check if hw timestamp is requested */
> > +       if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
> > +           /* Tx timestamping not supported along with TSO, so ignore
> > request */
> > +           !skb_shinfo(skb)->gso_size &&
> > +           /* HW supports only a single outstanding packet to timestamp */
> > +           atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1)) {
> > +               /* Mark the SKB for later reference */
> > +               skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> > +
> > +               /* Finally enable timestamp generation
> > +                * Since 'post_cqe' is also set, two CQEs will be posted
> > +                * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
> > +                */
> > +               hdr->tstmp = 1;
> > +       }
> >
> > -       /* Finally enable timestamp generation
> > -        * Since 'post_cqe' is also set, two CQEs will be posted
> > -        * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
> > -        */
> > -       hdr->tstmp = 1;
> > +       skb_tx_timestamp(skb);
> >  }
> >
> >  /* SQ GATHER subdescriptor
> >
> > Thanks,
> > Jason
> >
> > >
> > > In this driver, that would be not in nicvf_sq_add_hdr_subdesc, but
> > > just before calling nicvf_sq_doorbell. Unfortunately, there are two
> > > callers, TSO and non-TSO.
> > >
> > > >
> > > >       /* Tx timestamping not supported along with TSO, so ignore request */
> > > >       if (skb_shinfo(skb)->gso_size)
> > > > --
> > > > 2.43.5
> > > >
> > >
> > >
>
>