[PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2022-02-07

[PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2022-02-07

[Tony Nguyen]

Corinna Vinschen says:

Fix the kernel warning "Missing unregister, handled but fix driver"
when running, e.g.,

  $ ethtool -G eth0 rx 1024

on igc.  Remove memset hack from igb and align igb code to igc.

The following are changes since commit ff62433883b3ab753a78954ecf46e2c514f5c407:
  net: dsa: mv88e6xxx: Unlock on error in mv88e6xxx_port_bridge_join()
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 1GbE

Corinna Vinschen (2):
  igc: avoid kernel warning when changing RX ring parameters
  igb: refactor XDP registration

 drivers/net/ethernet/intel/igb/igb_ethtool.c |  4 ----
 drivers/net/ethernet/intel/igb/igb_main.c    | 19 +++++++++++++------
 drivers/net/ethernet/intel/igc/igc_main.c    |  3 +++
 3 files changed, 16 insertions(+), 10 deletions(-)

-- 
2.31.1

[PATCH net-next 1/2] igc: avoid kernel warning when changing RX ring parameters

[Tony Nguyen]

From: Corinna Vinschen <vinschen@redhat.com>

Calling ethtool changing the RX ring parameters like this:

  $ ethtool -G eth0 rx 1024

on igc triggers kernel warnings like this:

[  225.198467] ------------[ cut here ]------------
[  225.198473] Missing unregister, handled but fix driver
[  225.198485] WARNING: CPU: 7 PID: 959 at net/core/xdp.c:168
xdp_rxq_info_reg+0x79/0xd0
[...]
[  225.198601] Call Trace:
[  225.198604]  <TASK>
[  225.198609]  igc_setup_rx_resources+0x3f/0xe0 [igc]
[  225.198617]  igc_ethtool_set_ringparam+0x30e/0x450 [igc]
[  225.198626]  ethnl_set_rings+0x18a/0x250
[  225.198631]  genl_family_rcv_msg_doit+0xca/0x110
[  225.198637]  genl_rcv_msg+0xce/0x1c0
[  225.198640]  ? rings_prepare_data+0x60/0x60
[  225.198644]  ? genl_get_cmd+0xd0/0xd0
[  225.198647]  netlink_rcv_skb+0x4e/0xf0
[  225.198652]  genl_rcv+0x24/0x40
[  225.198655]  netlink_unicast+0x20e/0x330
[  225.198659]  netlink_sendmsg+0x23f/0x480
[  225.198663]  sock_sendmsg+0x5b/0x60
[  225.198667]  __sys_sendto+0xf0/0x160
[  225.198671]  ? handle_mm_fault+0xb2/0x280
[  225.198676]  ? do_user_addr_fault+0x1eb/0x690
[  225.198680]  __x64_sys_sendto+0x20/0x30
[  225.198683]  do_syscall_64+0x38/0x90
[  225.198687]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  225.198693] RIP: 0033:0x7f7ae38ac3aa

igc_ethtool_set_ringparam() copies the igc_ring structure but neglects to
reset the xdp_rxq_info member before calling igc_setup_rx_resources().
This in turn calls xdp_rxq_info_reg() with an already registered xdp_rxq_info.

Make sure to unregister the xdp_rxq_info structure first in
igc_setup_rx_resources.

Fixes: 73f1071c1d29 ("igc: Add support for XDP_TX action")
Reported-by: Lennert Buytenhek <buytenh@arista.com>
Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Tested-by: Dvora Fuxbrumer <dvorax.fuxbrumer@linux.intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index b965fb809d84..74b2c590ed5d 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -505,6 +505,9 @@ int igc_setup_rx_resources(struct igc_ring *rx_ring)
 	u8 index = rx_ring->queue_index;
 	int size, desc_len, res;
 
+	/* XDP RX-queue info */
+	if (xdp_rxq_info_is_reg(&rx_ring->xdp_rxq))
+		xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
 	res = xdp_rxq_info_reg(&rx_ring->xdp_rxq, ndev, index,
 			       rx_ring->q_vector->napi.napi_id);
 	if (res < 0) {
-- 
2.31.1

[PATCH net-next 2/2] igb: refactor XDP registration

[Tony Nguyen]

From: Corinna Vinschen <vinschen@redhat.com>

On changing the RX ring parameters igb uses a hack to avoid a warning
when calling xdp_rxq_info_reg via igb_setup_rx_resources.  It just
clears the struct xdp_rxq_info content.

Instead, change this to unregister if we're already registered.  Align
code to the igc code.

Fixes: 9cbc948b5a20c ("igb: add XDP support")
Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Tested-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c |  4 ----
 drivers/net/ethernet/intel/igb/igb_main.c    | 19 +++++++++++++------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 51a2dcaf553d..2a5782063f4c 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -965,10 +965,6 @@ static int igb_set_ringparam(struct net_device *netdev,
 			memcpy(&temp_ring[i], adapter->rx_ring[i],
 			       sizeof(struct igb_ring));
 
-			/* Clear copied XDP RX-queue info */
-			memset(&temp_ring[i].xdp_rxq, 0,
-			       sizeof(temp_ring[i].xdp_rxq));
-
 			temp_ring[i].count = new_rx_count;
 			err = igb_setup_rx_resources(&temp_ring[i]);
 			if (err) {
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index bfa321e4003f..34b33b21e0dc 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4345,7 +4345,18 @@ int igb_setup_rx_resources(struct igb_ring *rx_ring)
 {
 	struct igb_adapter *adapter = netdev_priv(rx_ring->netdev);
 	struct device *dev = rx_ring->dev;
-	int size;
+	int size, res;
+
+	/* XDP RX-queue info */
+	if (xdp_rxq_info_is_reg(&rx_ring->xdp_rxq))
+		xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
+	res = xdp_rxq_info_reg(&rx_ring->xdp_rxq, rx_ring->netdev,
+			       rx_ring->queue_index, 0);
+	if (res < 0) {
+		dev_err(dev, "Failed to register xdp_rxq index %u\n",
+			rx_ring->queue_index);
+		return res;
+	}
 
 	size = sizeof(struct igb_rx_buffer) * rx_ring->count;
 
@@ -4368,14 +4379,10 @@ int igb_setup_rx_resources(struct igb_ring *rx_ring)
 
 	rx_ring->xdp_prog = adapter->xdp_prog;
 
-	/* XDP RX-queue info */
-	if (xdp_rxq_info_reg(&rx_ring->xdp_rxq, rx_ring->netdev,
-			     rx_ring->queue_index, 0) < 0)
-		goto err;
-
 	return 0;
 
 err:
+	xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
 	vfree(rx_ring->rx_buffer_info);
 	rx_ring->rx_buffer_info = NULL;
 	dev_err(dev, "Unable to allocate memory for the Rx descriptor ring\n");
-- 
2.31.1

Re: [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2022-02-07

[Jakub Kicinski]

On Mon,  7 Feb 2022 15:32:44 -0800 Tony Nguyen wrote:
> Corinna Vinschen says:
> 
> Fix the kernel warning "Missing unregister, handled but fix driver"
> when running, e.g.,
> 
>   $ ethtool -G eth0 rx 1024
> 
> on igc.  Remove memset hack from igb and align igb code to igc.

Why -next?

Re: [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2022-02-07

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

Hi Tony,

On Tue Feb 08 2022, Jakub Kicinski wrote:
> On Mon,  7 Feb 2022 15:32:44 -0800 Tony Nguyen wrote:
>> Corinna Vinschen says:
>> 
>> Fix the kernel warning "Missing unregister, handled but fix driver"
>> when running, e.g.,
>> 
>>   $ ethtool -G eth0 rx 1024
>> 
>> on igc.  Remove memset hack from igb and align igb code to igc.
>
> Why -next?

Can we get these patches into net, please? The mentioned igc problem
exists on v5.15-LTS too.

FWIW, Tested-by: Kurt Kanzenbach <kurt@linutronix.de>

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2022-02-07

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Tony Nguyen <anthony.l.nguyen@intel.com>:

On Mon,  7 Feb 2022 15:32:44 -0800 you wrote:
> Corinna Vinschen says:
> 
> Fix the kernel warning "Missing unregister, handled but fix driver"
> when running, e.g.,
> 
>   $ ethtool -G eth0 rx 1024
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] igc: avoid kernel warning when changing RX ring parameters
    https://git.kernel.org/netdev/net-next/c/453307b569a0
  - [net-next,2/2] igb: refactor XDP registration
    https://git.kernel.org/netdev/net-next/c/e62ad74aa534

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2022-02-07

[Nguyen, Anthony L]

On Wed, 2022-02-09 at 09:13 +0100, Kurt Kanzenbach wrote:
> Hi Tony,
> 
> On Tue Feb 08 2022, Jakub Kicinski wrote:
> > On Mon,  7 Feb 2022 15:32:44 -0800 Tony Nguyen wrote:
> > > Corinna Vinschen says:
> > > 
> > > Fix the kernel warning "Missing unregister, handled but fix
> > > driver"
> > > when running, e.g.,
> > > 
> > >   $ ethtool -G eth0 rx 1024
> > > 
> > > on igc.  Remove memset hack from igb and align igb code to igc.
> > 
> > Why -next?

As the original submission was targeting -next, I carried that forward.
Since the warning said it was handled, I thought it was ok to go there.

> Can we get these patches into net, please? The mentioned igc problem
> exists on v5.15-LTS too.

I'll follow the igc patch and submit a request to stable when it hits
Linus' tree. For igb, as it sounds like things are working, just not
with the preferred method, so I don't plan on sending that to stable.
Or is there an issue there that this patch needs to go to stable as
well?

Thanks,
Tony

> FWIW, Tested-by: Kurt Kanzenbach <kurt@linutronix.de>
> 
> Thanks,
> Kurt

Re: [PATCH net-next 0/2][pull request] 1GbE Intel Wired LAN Driver Updates 2022-02-07

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]

On Wed Feb 09 2022, Anthony L. Nguyen wrote:
> On Wed, 2022-02-09 at 09:13 +0100, Kurt Kanzenbach wrote:
>> Hi Tony,
>> 
>> On Tue Feb 08 2022, Jakub Kicinski wrote:
>> > On Mon,  7 Feb 2022 15:32:44 -0800 Tony Nguyen wrote:
>> > > Corinna Vinschen says:
>> > > 
>> > > Fix the kernel warning "Missing unregister, handled but fix
>> > > driver"
>> > > when running, e.g.,
>> > > 
>> > >   $ ethtool -G eth0 rx 1024
>> > > 
>> > > on igc.  Remove memset hack from igb and align igb code to igc.
>> > 
>> > Why -next?
>
> As the original submission was targeting -next, I carried that forward.
> Since the warning said it was handled, I thought it was ok to go there.
>
>> Can we get these patches into net, please? The mentioned igc problem
>> exists on v5.15-LTS too.
>
> I'll follow the igc patch and submit a request to stable when it hits
> Linus' tree.

OK, thanks!

> For igb, as it sounds like things are working, just not
> with the preferred method, so I don't plan on sending that to stable.
> Or is there an issue there that this patch needs to go to stable as
> well?

igb is fine, as it uses the memset() approach.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]