netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open()
@ 2025-12-18 12:13 mheib
  2025-12-18 12:13 ` [PATCH net v2 2/2] ice: drop udp_tunnel_get_rx_info() call from ndo_open() mheib
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: mheib @ 2025-12-18 12:13 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: anthony.l.nguyen, przemyslaw.kitszel, davem, aduyck, kuba, netdev,
	jacob.e.keller, Mohammad Heib, Aleksandr Loktionov

From: Mohammad Heib <mheib@redhat.com>

The i40e driver calls udp_tunnel_get_rx_info() during i40e_open().
This is redundant because UDP tunnel RX offload state is preserved
across device down/up cycles. The udp_tunnel core handles
synchronization automatically when required.

Furthermore, recent changes in the udp_tunnel infrastructure require
querying RX info while holding the udp_tunnel lock. Calling it
directly from the ndo_open path violates this requirement,
triggering the following lockdep warning:

  Call Trace:
   <TASK>
   ? __udp_tunnel_nic_assert_locked+0x39/0x40 [udp_tunnel]
   i40e_open+0x135/0x14f [i40e]
   __dev_open+0x121/0x2e0
   __dev_change_flags+0x227/0x270
   dev_change_flags+0x3d/0xb0
   devinet_ioctl+0x56f/0x860
   sock_do_ioctl+0x7b/0x130
   __x64_sys_ioctl+0x91/0xd0
   do_syscall_64+0x90/0x170
   ...
   </TASK>

Remove the redundant and unsafe call to udp_tunnel_get_rx_info() from
i40e_open() resolve the locking violation.

Fixes: 06a5f7f167c5 ("i40e: Move all UDP port notifiers to single function")
Signed-off-by: Mohammad Heib <mheib@redhat.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 50be0a60ae13..72358a34438b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -9029,7 +9029,6 @@ int i40e_open(struct net_device *netdev)
 						       TCP_FLAG_FIN |
 						       TCP_FLAG_CWR) >> 16);
 	wr32(&pf->hw, I40E_GLLAN_TSOMSK_L, be32_to_cpu(TCP_FLAG_CWR) >> 16);
-	udp_tunnel_get_rx_info(netdev);
 
 	return 0;
 }
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net v2 2/2] ice: drop udp_tunnel_get_rx_info() call from ndo_open()
  2025-12-18 12:13 [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open() mheib
@ 2025-12-18 12:13 ` mheib
  2025-12-18 12:42 ` [Intel-wired-lan] [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open() Paul Menzel
  2025-12-28  9:29 ` Paolo Abeni
  2 siblings, 0 replies; 5+ messages in thread
From: mheib @ 2025-12-18 12:13 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: anthony.l.nguyen, przemyslaw.kitszel, davem, aduyck, kuba, netdev,
	jacob.e.keller, Mohammad Heib, Aleksandr Loktionov

From: Mohammad Heib <mheib@redhat.com>

The ice driver calls udp_tunnel_get_rx_info() during ice_open_internal().
This is redundant because UDP tunnel RX offload state is preserved
across device down/up cycles. The udp_tunnel core handles
synchronization automatically when required.

Furthermore, recent changes in the udp_tunnel infrastructure require
querying RX info while holding the udp_tunnel lock. Calling it
directly from the ndo_open path violates this requirement,
triggering the following lockdep warning:

Call Trace:
  <TASK>
  ice_open_internal+0x253/0x350 [ice]
  __udp_tunnel_nic_assert_locked+0x86/0xb0 [udp_tunnel]
  __dev_open+0x2f5/0x880
  __dev_change_flags+0x44c/0x660
  netif_change_flags+0x80/0x160
  devinet_ioctl+0xd21/0x15f0
  inet_ioctl+0x311/0x350
  sock_ioctl+0x114/0x220
  __x64_sys_ioctl+0x131/0x1a0
  ...
  </TASK>

Remove the redundant and unsafe call to udp_tunnel_get_rx_info() from
ice_open_internal() to resolve the locking violation

Fixes: a4e82a81f573 ("ice: Add support for tunnel offloads")
Signed-off-by: Mohammad Heib <mheib@redhat.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 2533876f1a2f..1f94bdcbbba9 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -9633,9 +9633,6 @@ int ice_open_internal(struct net_device *netdev)
 		netdev_err(netdev, "Failed to open VSI 0x%04X on switch 0x%04X\n",
 			   vsi->vsi_num, vsi->vsw->sw_id);
 
-	/* Update existing tunnels information */
-	udp_tunnel_get_rx_info(netdev);
-
 	return err;
 }
 
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Intel-wired-lan] [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open()
  2025-12-18 12:13 [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open() mheib
  2025-12-18 12:13 ` [PATCH net v2 2/2] ice: drop udp_tunnel_get_rx_info() call from ndo_open() mheib
@ 2025-12-18 12:42 ` Paul Menzel
  2025-12-28  9:29 ` Paolo Abeni
  2 siblings, 0 replies; 5+ messages in thread
From: Paul Menzel @ 2025-12-18 12:42 UTC (permalink / raw)
  To: mheib
  Cc: intel-wired-lan, anthony.l.nguyen, przemyslaw.kitszel, davem,
	aduyck, kuba, netdev, jacob.e.keller, Aleksandr Loktionov

Dear Mohammad,


Thank you for your patch.

Am 18.12.25 um 13:13 schrieb mheib@redhat.com:
> From: Mohammad Heib <mheib@redhat.com>
> 
> The i40e driver calls udp_tunnel_get_rx_info() during i40e_open().
> This is redundant because UDP tunnel RX offload state is preserved
> across device down/up cycles. The udp_tunnel core handles
> synchronization automatically when required.
> 
> Furthermore, recent changes in the udp_tunnel infrastructure require
> querying RX info while holding the udp_tunnel lock. Calling it
> directly from the ndo_open path violates this requirement,
> triggering the following lockdep warning:
> 
>    Call Trace:
>     <TASK>
>     ? __udp_tunnel_nic_assert_locked+0x39/0x40 [udp_tunnel]
>     i40e_open+0x135/0x14f [i40e]
>     __dev_open+0x121/0x2e0
>     __dev_change_flags+0x227/0x270
>     dev_change_flags+0x3d/0xb0
>     devinet_ioctl+0x56f/0x860
>     sock_do_ioctl+0x7b/0x130
>     __x64_sys_ioctl+0x91/0xd0
>     do_syscall_64+0x90/0x170
>     ...
>     </TASK>
> 
> Remove the redundant and unsafe call to udp_tunnel_get_rx_info() from
> i40e_open() resolve the locking violation.
> 
> Fixes: 06a5f7f167c5 ("i40e: Move all UDP port notifiers to single function")
> Signed-off-by: Mohammad Heib <mheib@redhat.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_main.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 50be0a60ae13..72358a34438b 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -9029,7 +9029,6 @@ int i40e_open(struct net_device *netdev)
>   						       TCP_FLAG_FIN |
>   						       TCP_FLAG_CWR) >> 16);
>   	wr32(&pf->hw, I40E_GLLAN_TSOMSK_L, be32_to_cpu(TCP_FLAG_CWR) >> 16);
> -	udp_tunnel_get_rx_info(netdev);
>   
>   	return 0;
>   }

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-wired-lan] [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open()
  2025-12-18 12:13 [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open() mheib
  2025-12-18 12:13 ` [PATCH net v2 2/2] ice: drop udp_tunnel_get_rx_info() call from ndo_open() mheib
  2025-12-18 12:42 ` [Intel-wired-lan] [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open() Paul Menzel
@ 2025-12-28  9:29 ` Paolo Abeni
  2025-12-28 19:44   ` mohammad heib
  2 siblings, 1 reply; 5+ messages in thread
From: Paolo Abeni @ 2025-12-28  9:29 UTC (permalink / raw)
  To: mheib, intel-wired-lan, anthony.l.nguyen
  Cc: przemyslaw.kitszel, davem, aduyck, kuba, netdev, jacob.e.keller,
	Aleksandr Loktionov

On 12/18/25 1:13 PM, mheib@redhat.com wrote:
> From: Mohammad Heib <mheib@redhat.com>
> 
> The i40e driver calls udp_tunnel_get_rx_info() during i40e_open().
> This is redundant because UDP tunnel RX offload state is preserved
> across device down/up cycles. The udp_tunnel core handles
> synchronization automatically when required.
> 
> Furthermore, recent changes in the udp_tunnel infrastructure require
> querying RX info while holding the udp_tunnel lock. Calling it
> directly from the ndo_open path violates this requirement,
> triggering the following lockdep warning:
> 
>   Call Trace:
>    <TASK>
>    ? __udp_tunnel_nic_assert_locked+0x39/0x40 [udp_tunnel]
>    i40e_open+0x135/0x14f [i40e]
>    __dev_open+0x121/0x2e0
>    __dev_change_flags+0x227/0x270
>    dev_change_flags+0x3d/0xb0
>    devinet_ioctl+0x56f/0x860
>    sock_do_ioctl+0x7b/0x130
>    __x64_sys_ioctl+0x91/0xd0
>    do_syscall_64+0x90/0x170
>    ...
>    </TASK>
> 
> Remove the redundant and unsafe call to udp_tunnel_get_rx_info() from
> i40e_open() resolve the locking violation.
> 
> Fixes: 06a5f7f167c5 ("i40e: Move all UDP port notifiers to single function")
> Signed-off-by: Mohammad Heib <mheib@redhat.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

@Tony: I assume you prefer to take this series into your tree first.

@Mohammad: I think we don't need to packport this path in old kernels; I
guess a better fixes tag should point to the recent udp_tunnel
infrastructure changes.

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-wired-lan] [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open()
  2025-12-28  9:29 ` Paolo Abeni
@ 2025-12-28 19:44   ` mohammad heib
  0 siblings, 0 replies; 5+ messages in thread
From: mohammad heib @ 2025-12-28 19:44 UTC (permalink / raw)
  To: Paolo Abeni, intel-wired-lan, anthony.l.nguyen
  Cc: przemyslaw.kitszel, davem, aduyck, kuba, netdev, jacob.e.keller,
	Aleksandr Loktionov

HI Paolo,
Thank you for the review.

On 12/28/25 11:29 AM, Paolo Abeni wrote:
> On 12/18/25 1:13 PM, mheib@redhat.com wrote:
>> From: Mohammad Heib <mheib@redhat.com>
>>
>> The i40e driver calls udp_tunnel_get_rx_info() during i40e_open().
>> This is redundant because UDP tunnel RX offload state is preserved
>> across device down/up cycles. The udp_tunnel core handles
>> synchronization automatically when required.
>>
>> Furthermore, recent changes in the udp_tunnel infrastructure require
>> querying RX info while holding the udp_tunnel lock. Calling it
>> directly from the ndo_open path violates this requirement,
>> triggering the following lockdep warning:
>>
>>    Call Trace:
>>     <TASK>
>>     ? __udp_tunnel_nic_assert_locked+0x39/0x40 [udp_tunnel]
>>     i40e_open+0x135/0x14f [i40e]
>>     __dev_open+0x121/0x2e0
>>     __dev_change_flags+0x227/0x270
>>     dev_change_flags+0x3d/0xb0
>>     devinet_ioctl+0x56f/0x860
>>     sock_do_ioctl+0x7b/0x130
>>     __x64_sys_ioctl+0x91/0xd0
>>     do_syscall_64+0x90/0x170
>>     ...
>>     </TASK>
>>
>> Remove the redundant and unsafe call to udp_tunnel_get_rx_info() from
>> i40e_open() resolve the locking violation.
>>
>> Fixes: 06a5f7f167c5 ("i40e: Move all UDP port notifiers to single function")
>> Signed-off-by: Mohammad Heib <mheib@redhat.com>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> 
> @Tony: I assume you prefer to take this series into your tree first.
> 
> @Mohammad: I think we don't need to packport this path in old kernels; I
> guess a better fixes tag should point to the recent udp_tunnel
> infrastructure changes.

indeed we don't need this change in the old versions.
I updated the fixes tag to:
   - Fixes: 1ead7501094c ("udp_tunnel: remove rtnl_lock dependency").
This better reflects the change that exposed the problem, even though 
the fix itself is in the driver.


> 
> Thanks,
> 
> Paolo
> 
Thanks,


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-28 19:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-18 12:13 [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open() mheib
2025-12-18 12:13 ` [PATCH net v2 2/2] ice: drop udp_tunnel_get_rx_info() call from ndo_open() mheib
2025-12-18 12:42 ` [Intel-wired-lan] [PATCH net v2 1/2] i40e: drop udp_tunnel_get_rx_info() call from i40e_open() Paul Menzel
2025-12-28  9:29 ` Paolo Abeni
2025-12-28 19:44   ` mohammad heib

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).