FAILED: patch "[PATCH] igc: read before write to SRRCTL register" failed to apply to 6.1-stable tree

FAILED: patch "[PATCH] igc: read before write to SRRCTL register" failed to apply to 6.1-stable tree

[gregkh]

The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 3ce29c17dc847bf4245e16aad78a7617afa96297
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023050749-deskwork-snowboard-82cf@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..

Possible dependencies:

3ce29c17dc84 ("igc: read before write to SRRCTL register")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 3ce29c17dc847bf4245e16aad78a7617afa96297 Mon Sep 17 00:00:00 2001
From: Song Yoong Siang <yoong.siang.song@intel.com>
Date: Tue, 2 May 2023 08:48:06 -0700
Subject: [PATCH] igc: read before write to SRRCTL register

igc_configure_rx_ring() function will be called as part of XDP program
setup. If Rx hardware timestamp is enabled prio to XDP program setup,
this timestamp enablement will be overwritten when buffer size is
written into SRRCTL register.

Thus, this commit read the register value before write to SRRCTL
register. This commit is tested by using xdp_hw_metadata bpf selftest
tool. The tool enables Rx hardware timestamp and then attach XDP program
to igc driver. It will display hardware timestamp of UDP packet with
port number 9092. Below are detail of test steps and results.

Command on DUT:
  sudo ./xdp_hw_metadata <interface name>

Command on Link Partner:
  echo -n skb | nc -u -q1 <destination IPv4 addr> 9092

Result before this patch:
  skb hwtstamp is not found!

Result after this patch:
  found skb hwtstamp = 1677800973.642836757

Optionally, read PHC to confirm the values obtained are almost the same:
Command:
  sudo ./testptp -d /dev/ptp0 -g
Result:
  clock time: 1677800973.913598978 or Fri Mar  3 07:49:33 2023

Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
Cc: <stable@vger.kernel.org> # 5.14+
Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Jesper Dangaard Brouer <brouer@redhat.com>
Tested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Tested-by: Naama Meir <naamax.meir@linux.intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/intel/igc/igc_base.h b/drivers/net/ethernet/intel/igc/igc_base.h
index 7a992befca24..9f3827eda157 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.h
+++ b/drivers/net/ethernet/intel/igc/igc_base.h
@@ -87,8 +87,13 @@ union igc_adv_rx_desc {
 #define IGC_RXDCTL_SWFLUSH		0x04000000 /* Receive Software Flush */
 
 /* SRRCTL bit definitions */
-#define IGC_SRRCTL_BSIZEPKT_SHIFT		10 /* Shift _right_ */
-#define IGC_SRRCTL_BSIZEHDRSIZE_SHIFT		2  /* Shift _left_ */
-#define IGC_SRRCTL_DESCTYPE_ADV_ONEBUF	0x02000000
+#define IGC_SRRCTL_BSIZEPKT_MASK	GENMASK(6, 0)
+#define IGC_SRRCTL_BSIZEPKT(x)		FIELD_PREP(IGC_SRRCTL_BSIZEPKT_MASK, \
+					(x) / 1024) /* in 1 KB resolution */
+#define IGC_SRRCTL_BSIZEHDR_MASK	GENMASK(13, 8)
+#define IGC_SRRCTL_BSIZEHDR(x)		FIELD_PREP(IGC_SRRCTL_BSIZEHDR_MASK, \
+					(x) / 64) /* in 64 bytes resolution */
+#define IGC_SRRCTL_DESCTYPE_MASK	GENMASK(27, 25)
+#define IGC_SRRCTL_DESCTYPE_ADV_ONEBUF	FIELD_PREP(IGC_SRRCTL_DESCTYPE_MASK, 1)
 
 #endif /* _IGC_BASE_H */
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index ba49728be919..1c4676882082 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -640,8 +640,11 @@ static void igc_configure_rx_ring(struct igc_adapter *adapter,
 	else
 		buf_size = IGC_RXBUFFER_2048;
 
-	srrctl = IGC_RX_HDR_LEN << IGC_SRRCTL_BSIZEHDRSIZE_SHIFT;
-	srrctl |= buf_size >> IGC_SRRCTL_BSIZEPKT_SHIFT;
+	srrctl = rd32(IGC_SRRCTL(reg_idx));
+	srrctl &= ~(IGC_SRRCTL_BSIZEPKT_MASK | IGC_SRRCTL_BSIZEHDR_MASK |
+		    IGC_SRRCTL_DESCTYPE_MASK);
+	srrctl |= IGC_SRRCTL_BSIZEHDR(IGC_RX_HDR_LEN);
+	srrctl |= IGC_SRRCTL_BSIZEPKT(buf_size);
 	srrctl |= IGC_SRRCTL_DESCTYPE_ADV_ONEBUF;
 
 	wr32(IGC_SRRCTL(reg_idx), srrctl);

Re: FAILED: patch "[PATCH] igc: read before write to SRRCTL register" failed to apply to 6.1-stable tree

[Florian Bezdeka]

Hi all,

On 07.05.23 08:44, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 6.1-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 
> To reproduce the conflict and resubmit, you may use the following commands:
> 
> git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> git checkout FETCH_HEAD
> git cherry-pick -x 3ce29c17dc847bf4245e16aad78a7617afa96297
> # <resolve conflicts, build, test, etc.>
> git commit -s
> git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023050749-deskwork-snowboard-82cf@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..

Is someone already working on that? I would love to see this patch in
6.1. If no further activities are planned I might have the option/time
to supply a backport as well.

Regards,
Florian

> 
> Possible dependencies:
> 
> 3ce29c17dc84 ("igc: read before write to SRRCTL register")
> 
> thanks,
> 
> greg k-h
> 
> ------------------ original commit in Linus's tree ------------------
> 
> From 3ce29c17dc847bf4245e16aad78a7617afa96297 Mon Sep 17 00:00:00 2001
> From: Song Yoong Siang <yoong.siang.song@intel.com>
> Date: Tue, 2 May 2023 08:48:06 -0700
> Subject: [PATCH] igc: read before write to SRRCTL register
> 
> igc_configure_rx_ring() function will be called as part of XDP program
> setup. If Rx hardware timestamp is enabled prio to XDP program setup,
> this timestamp enablement will be overwritten when buffer size is
> written into SRRCTL register.
> 
> Thus, this commit read the register value before write to SRRCTL
> register. This commit is tested by using xdp_hw_metadata bpf selftest
> tool. The tool enables Rx hardware timestamp and then attach XDP program
> to igc driver. It will display hardware timestamp of UDP packet with
> port number 9092. Below are detail of test steps and results.
> 
> Command on DUT:
>   sudo ./xdp_hw_metadata <interface name>
> 
> Command on Link Partner:
>   echo -n skb | nc -u -q1 <destination IPv4 addr> 9092
> 
> Result before this patch:
>   skb hwtstamp is not found!
> 
> Result after this patch:
>   found skb hwtstamp = 1677800973.642836757
> 
> Optionally, read PHC to confirm the values obtained are almost the same:
> Command:
>   sudo ./testptp -d /dev/ptp0 -g
> Result:
>   clock time: 1677800973.913598978 or Fri Mar  3 07:49:33 2023
> 
> Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> Cc: <stable@vger.kernel.org> # 5.14+
> Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Tested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Tested-by: Naama Meir <naamax.meir@linux.intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_base.h b/drivers/net/ethernet/intel/igc/igc_base.h
> index 7a992befca24..9f3827eda157 100644
> --- a/drivers/net/ethernet/intel/igc/igc_base.h
> +++ b/drivers/net/ethernet/intel/igc/igc_base.h
> @@ -87,8 +87,13 @@ union igc_adv_rx_desc {
>  #define IGC_RXDCTL_SWFLUSH		0x04000000 /* Receive Software Flush */
>  
>  /* SRRCTL bit definitions */
> -#define IGC_SRRCTL_BSIZEPKT_SHIFT		10 /* Shift _right_ */
> -#define IGC_SRRCTL_BSIZEHDRSIZE_SHIFT		2  /* Shift _left_ */
> -#define IGC_SRRCTL_DESCTYPE_ADV_ONEBUF	0x02000000
> +#define IGC_SRRCTL_BSIZEPKT_MASK	GENMASK(6, 0)
> +#define IGC_SRRCTL_BSIZEPKT(x)		FIELD_PREP(IGC_SRRCTL_BSIZEPKT_MASK, \
> +					(x) / 1024) /* in 1 KB resolution */
> +#define IGC_SRRCTL_BSIZEHDR_MASK	GENMASK(13, 8)
> +#define IGC_SRRCTL_BSIZEHDR(x)		FIELD_PREP(IGC_SRRCTL_BSIZEHDR_MASK, \
> +					(x) / 64) /* in 64 bytes resolution */
> +#define IGC_SRRCTL_DESCTYPE_MASK	GENMASK(27, 25)
> +#define IGC_SRRCTL_DESCTYPE_ADV_ONEBUF	FIELD_PREP(IGC_SRRCTL_DESCTYPE_MASK, 1)
>  
>  #endif /* _IGC_BASE_H */
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index ba49728be919..1c4676882082 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -640,8 +640,11 @@ static void igc_configure_rx_ring(struct igc_adapter *adapter,
>  	else
>  		buf_size = IGC_RXBUFFER_2048;
>  
> -	srrctl = IGC_RX_HDR_LEN << IGC_SRRCTL_BSIZEHDRSIZE_SHIFT;
> -	srrctl |= buf_size >> IGC_SRRCTL_BSIZEPKT_SHIFT;
> +	srrctl = rd32(IGC_SRRCTL(reg_idx));
> +	srrctl &= ~(IGC_SRRCTL_BSIZEPKT_MASK | IGC_SRRCTL_BSIZEHDR_MASK |
> +		    IGC_SRRCTL_DESCTYPE_MASK);
> +	srrctl |= IGC_SRRCTL_BSIZEHDR(IGC_RX_HDR_LEN);
> +	srrctl |= IGC_SRRCTL_BSIZEPKT(buf_size);
>  	srrctl |= IGC_SRRCTL_DESCTYPE_ADV_ONEBUF;
>  
>  	wr32(IGC_SRRCTL(reg_idx), srrctl);
> 

Re: FAILED: patch "[PATCH] igc: read before write to SRRCTL register" failed to apply to 6.1-stable tree

[Greg KH]

On Thu, May 11, 2023 at 12:01:36AM +0200, Florian Bezdeka wrote:
> Hi all,
> 
> On 07.05.23 08:44, gregkh@linuxfoundation.org wrote:
> > 
> > The patch below does not apply to the 6.1-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> > 
> > To reproduce the conflict and resubmit, you may use the following commands:
> > 
> > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> > git checkout FETCH_HEAD
> > git cherry-pick -x 3ce29c17dc847bf4245e16aad78a7617afa96297
> > # <resolve conflicts, build, test, etc.>
> > git commit -s
> > git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023050749-deskwork-snowboard-82cf@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
> 
> Is someone already working on that? I would love to see this patch in
> 6.1. If no further activities are planned I might have the option/time
> to supply a backport as well.

Please supply a backport, I don't think anyone is working on it :)

RE: FAILED: patch "[PATCH] igc: read before write to SRRCTL register" failed to apply to 6.1-stable tree

[Song, Yoong Siang]

On Thursday, May 11, 2023 6:46 AM , Greg KH <gregkh@linuxfoundation.org> wrote:
>On Thu, May 11, 2023 at 12:01:36AM +0200, Florian Bezdeka wrote:
>> Hi all,
>>
>> On 07.05.23 08:44, gregkh@linuxfoundation.org wrote:
>> >
>> > The patch below does not apply to the 6.1-stable tree.
>> > If someone wants it applied there, or to any other stable or
>> > longterm tree, then please email the backport, including the
>> > original git commit id to <stable@vger.kernel.org>.
>> >
>> > To reproduce the conflict and resubmit, you may use the following commands:
>> >
>> > git fetch
>> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/
>> > linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x
>> > 3ce29c17dc847bf4245e16aad78a7617afa96297
>> > # <resolve conflicts, build, test, etc.> git commit -s git
>> > send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023050749-
>deskwork-snowboard-82cf@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
>>
>> Is someone already working on that? I would love to see this patch in
>> 6.1. If no further activities are planned I might have the option/time
>> to supply a backport as well.
>
>Please supply a backport, I don't think anyone is working on it :)

Hi Florian,

I not yet got plan to backport the patch, so I am more than happy
if you could supply a backport.

Most probably the issue is due to missing "#include <linux/bitfield.h>".

Will you do it for 5.15 and 6.2 as well?

Thanks & Regards
Siang

Re: FAILED: patch "[PATCH] igc: read before write to SRRCTL register" failed to apply to 6.1-stable tree

[florian]

Am 2023-05-11 08:56, schrieb Song, Yoong Siang:
> On Thursday, May 11, 2023 6:46 AM , Greg KH 
> <gregkh@linuxfoundation.org> wrote:
>> On Thu, May 11, 2023 at 12:01:36AM +0200, Florian Bezdeka wrote:
>>> Hi all,
>>> 
>>> On 07.05.23 08:44, gregkh@linuxfoundation.org wrote:
>>> >
>>> > The patch below does not apply to the 6.1-stable tree.
>>> > If someone wants it applied there, or to any other stable or
>>> > longterm tree, then please email the backport, including the
>>> > original git commit id to <stable@vger.kernel.org>.
>>> >
>>> > To reproduce the conflict and resubmit, you may use the following commands:
>>> >
>>> > git fetch
>>> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/
>>> > linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x
>>> > 3ce29c17dc847bf4245e16aad78a7617afa96297
>>> > # <resolve conflicts, build, test, etc.> git commit -s git
>>> > send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023050749-
>> deskwork-snowboard-82cf@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
>>> 
>>> Is someone already working on that? I would love to see this patch in
>>> 6.1. If no further activities are planned I might have the 
>>> option/time
>>> to supply a backport as well.
>> 
>> Please supply a backport, I don't think anyone is working on it :)
> 
> Hi Florian,
> 
> I not yet got plan to backport the patch, so I am more than happy
> if you could supply a backport.
> 
> Most probably the issue is due to missing "#include 
> <linux/bitfield.h>".

Exactly.

The build failure:

In file included from drivers/net/ethernet/intel/igc/igc_hw.h:17,
                  from drivers/net/ethernet/intel/igc/igc.h:17,
                  from drivers/net/ethernet/intel/igc/igc_main.c:19:
drivers/net/ethernet/intel/igc/igc_main.c: In function 
‘igc_configure_rx_ring’:
drivers/net/ethernet/intel/igc/igc_base.h:92:41: error: implicit 
declaration of function ‘FIELD_PREP’ 
[-Werror=implicit-function-declaration]
    92 | #define IGC_SRRCTL_BSIZEHDR(x)          
FIELD_PREP(IGC_SRRCTL_BSIZEHDR_MASK, \
       |                                         ^~~~~~~~~~
drivers/net/ethernet/intel/igc/igc_main.c:647:19: note: in expansion of 
macro ‘IGC_SRRCTL_BSIZEHDR’
   647 |         srrctl |= IGC_SRRCTL_BSIZEHDR(IGC_RX_HDR_LEN);
       |


For 6.3 on-wards we have the following include chain:

In file included from ./include/net/xdp.h:11,
                  from ./include/linux/netdevice.h:43,
                  from ./include/linux/if_vlan.h:10,
                  from drivers/net/ethernet/intel/igc/igc_main.c:6:

I think <linux/bitfield.h> is available "by accident" and your mainline 
patch
is faulty. igc_base.h now depends on bitfield.h but you forgot to 
include it.

How do we deal with that? I think it should be fixed in mainline as 
well.

I fear that adding the missing include in igc_base.h within the backport
breaks any further auto-backporting for the igc driver as patches might
not apply cleanly when stable diverges from mainline.

Florian

> 
> Will you do it for 5.15 and 6.2 as well?
> 
> Thanks & Regards
> Siang

RE: FAILED: patch "[PATCH] igc: read before write to SRRCTL register" failed to apply to 6.1-stable tree

[Song, Yoong Siang]

On Tuesday, May 16, 2023 2:14 AM, florian@bezdeka.de <florian@bezdeka.de> wrote:
>Am 2023-05-11 08:56, schrieb Song, Yoong Siang:
>> On Thursday, May 11, 2023 6:46 AM , Greg KH
>> <gregkh@linuxfoundation.org> wrote:
>>> On Thu, May 11, 2023 at 12:01:36AM +0200, Florian Bezdeka wrote:
>>>> Hi all,
>>>>
>>>> On 07.05.23 08:44, gregkh@linuxfoundation.org wrote:
>>>> >
>>>> > The patch below does not apply to the 6.1-stable tree.
>>>> > If someone wants it applied there, or to any other stable or
>>>> > longterm tree, then please email the backport, including the
>>>> > original git commit id to <stable@vger.kernel.org>.
>>>> >
>>>> > To reproduce the conflict and resubmit, you may use the following
>commands:
>>>> >
>>>> > git fetch
>>>> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/
>>>> > linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x
>>>> > 3ce29c17dc847bf4245e16aad78a7617afa96297
>>>> > # <resolve conflicts, build, test, etc.> git commit -s git
>>>> > send-email --to '<stable@vger.kernel.org>' --in-reply-to
>>>> > '2023050749-
>>> deskwork-snowboard-82cf@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
>>>>
>>>> Is someone already working on that? I would love to see this patch
>>>> in 6.1. If no further activities are planned I might have the
>>>> option/time to supply a backport as well.
>>>
>>> Please supply a backport, I don't think anyone is working on it :)
>>
>> Hi Florian,
>>
>> I not yet got plan to backport the patch, so I am more than happy if
>> you could supply a backport.
>>
>> Most probably the issue is due to missing "#include
>> <linux/bitfield.h>".
>
>Exactly.
>
>The build failure:
>
>In file included from drivers/net/ethernet/intel/igc/igc_hw.h:17,
>                  from drivers/net/ethernet/intel/igc/igc.h:17,
>                  from drivers/net/ethernet/intel/igc/igc_main.c:19:
>drivers/net/ethernet/intel/igc/igc_main.c: In function
>‘igc_configure_rx_ring’:
>drivers/net/ethernet/intel/igc/igc_base.h:92:41: error: implicit declaration of
>function ‘FIELD_PREP’
>[-Werror=implicit-function-declaration]
>    92 | #define IGC_SRRCTL_BSIZEHDR(x)
>FIELD_PREP(IGC_SRRCTL_BSIZEHDR_MASK, \
>       |                                         ^~~~~~~~~~
>drivers/net/ethernet/intel/igc/igc_main.c:647:19: note: in expansion of macro
>‘IGC_SRRCTL_BSIZEHDR’
>   647 |         srrctl |= IGC_SRRCTL_BSIZEHDR(IGC_RX_HDR_LEN);
>       |
>
>
>For 6.3 on-wards we have the following include chain:
>
>In file included from ./include/net/xdp.h:11,
>                  from ./include/linux/netdevice.h:43,
>                  from ./include/linux/if_vlan.h:10,
>                  from drivers/net/ethernet/intel/igc/igc_main.c:6:
>
>I think <linux/bitfield.h> is available "by accident" and your mainline patch is
>faulty. igc_base.h now depends on bitfield.h but you forgot to include it.
>
>How do we deal with that? I think it should be fixed in mainline as well.
>
>I fear that adding the missing include in igc_base.h within the backport breaks any
>further auto-backporting for the igc driver as patches might not apply cleanly
>when stable diverges from mainline.
>
>Florian

Yes, I agree that proper fix should be adding "#include <linux/bitfield.h>"
into igc_base.h. Do you got plan to submit the fix upstream?

Recently, there is a bug fix patch merged into bpf-next. That patch will add
"#include <linux/bitfield.h>" into igc.h, which I think should indirectly solve
the problem. Thus, backport this patch will be alternate solution:
https://lore.kernel.org/all/168182464270.616355.11391652654430626584.stgit@firesoul/

Thanks & Regards
Siang

>
>>
>> Will you do it for 5.15 and 6.2 as well?
>>
>> Thanks & Regards
>> Siang