[Intel-wired-lan] [tnguy-next-queue:main] BUILD SUCCESS 4e41231249f4083a095085ff86e317e29313c2c3
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git main branch HEAD: 4e41231249f4083a095085ff86e317e29313c2c3 Merge branch '100GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue elapsed time: 1693m configs tested: 205 configs skipped: 4 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alpha allnoconfiggcc-14.2.0 alphaallyesconfigclang-21 alphaallyesconfiggcc-14.2.0 alpha defconfiggcc-14.2.0 arc allmodconfigclang-18 arc allnoconfiggcc-14.2.0 arc allyesconfigclang-18 arc defconfiggcc-14.2.0 arc randconfig-001-20250213gcc-13.2.0 arc randconfig-001-20250213gcc-14.2.0 arc randconfig-002-20250213gcc-13.2.0 arc randconfig-002-20250213gcc-14.2.0 arm allmodconfigclang-18 arm allnoconfiggcc-14.2.0 arm allyesconfigclang-18 arm defconfiggcc-14.2.0 arm randconfig-001-20250213clang-17 arm randconfig-001-20250213gcc-14.2.0 arm randconfig-002-20250213clang-15 arm randconfig-002-20250213gcc-14.2.0 arm randconfig-003-20250213clang-21 arm randconfig-003-20250213gcc-14.2.0 arm randconfig-004-20250213gcc-14.2.0 arm64allmodconfigclang-18 arm64 allnoconfiggcc-14.2.0 arm64 defconfiggcc-14.2.0 arm64 randconfig-001-20250213clang-19 arm64 randconfig-001-20250213gcc-14.2.0 arm64 randconfig-002-20250213gcc-14.2.0 arm64 randconfig-003-20250213gcc-14.2.0 arm64 randconfig-004-20250213clang-21 arm64 randconfig-004-20250213gcc-14.2.0 csky allnoconfiggcc-14.2.0 cskydefconfiggcc-14.2.0 csky randconfig-001-20250213clang-21 csky randconfig-001-20250213gcc-14.2.0 csky randconfig-002-20250213clang-21 csky randconfig-002-20250213gcc-14.2.0 hexagon allmodconfigclang-21 hexagon allnoconfiggcc-14.2.0 hexagon allyesconfigclang-18 hexagon allyesconfigclang-21 hexagon defconfiggcc-14.2.0 hexagon randconfig-001-20250213clang-21 hexagon randconfig-002-20250213clang-18 hexagon randconfig-002-20250213clang-21 i386 allmodconfigclang-19 i386 allmodconfiggcc-12 i386 allnoconfigclang-19 i386 allnoconfiggcc-12 i386 allyesconfigclang-19 i386 allyesconfiggcc-12 i386buildonly-randconfig-001-20250213clang-19 i386buildonly-randconfig-001-20250213gcc-12 i386buildonly-randconfig-002-20250213clang-19 i386buildonly-randconfig-003-20250213clang-19 i386buildonly-randconfig-004-20250213clang-19 i386buildonly-randconfig-005-20250213clang-19 i386buildonly-randconfig-006-20250213clang-19 i386defconfigclang-19 i386 randconfig-001-20250213clang-19 i386 randconfig-002-20250213clang-19 i386 randconfig-003-20250213clang-19 i386 randconfig-004-20250213clang-19 i386 randconfig-005-20250213clang-19 i386 randconfig-006-20250213clang-19 i386 randconfig-007-20250213clang-19 i386 randconfig-011-20250213gcc-12 i386 randconfig-012-20250213gcc-12 i386 randconfig-013-20250213gcc-12 i386 randconfig-014-20250213gcc-12 i386 randconfig-015-20250213gcc-12 i386 randconfig-016-20250213gcc-12 i386 randconfig-017-20250213gcc-12 loongarchallmodconfiggcc-14.2.0 loongarch allnoconfiggcc-14.2.0 loongarch defconfiggcc-14.2.0 loongarch randconfig-001-20250213clang-21
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Vladimir Oltean
> Sent: Wednesday, February 12, 2025 11:01 PM
> To: Faizal Rahim
> Cc: Nguyen, Anthony L ; Kitszel, Przemyslaw
> ; Andrew Lunn ;
> David S . Miller ; Eric Dumazet
> ; Jakub Kicinski ; Paolo Abeni
> ; Maxime Coquelin ;
> Alexandre Torgue ; Simon Horman
> ; Russell King ; Alexei
> Starovoitov ; Daniel Borkmann ;
> Jesper Dangaard Brouer ; John Fastabend
> ; Furong Xu <0x1...@gmail.com>; Russell King
> ; Serge Semin ;
> Xiaolei Wang ; Suraj Jaiswal
> ; Kory Maincent ;
> Gal Pressman ; Jesper Nilsson ;
> Andrew Halaney ; Choong Yong Liang
> ; Kunihiko Hayashi
> ; Gomes, Vinicius
> ; intel-wired-...@lists.osuosl.org;
> net...@vger.kernel.org; linux-ker...@vger.kernel.org; linux-stm32@st-md-
> mailman.stormreply.com; linux-arm-ker...@lists.infradead.org;
> b...@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for
> Frame Preemption feature in IGC
Please start commit title from slam letters:
Igc: add ...
> On Mon, Feb 10, 2025 at 02:01:58AM -0500, Faizal Rahim wrote:
> > Introduces support for the FPE feature in the IGC driver.
> >
> > The patches aligns with the upstream FPE API:
> >
> https://patchwork.kernel.org/project/netdevbpf/cover/20230220122343.1
> 1
> > 56614-1-vladimir.olt...@nxp.com/
> >
> https://patchwork.kernel.org/project/netdevbpf/cover/20230119122705.7
> 3
> > 054-1-vladimir.olt...@nxp.com/
> >
> > It builds upon earlier work:
> >
> https://patchwork.kernel.org/project/netdevbpf/cover/20220520011538.1
> 0
> > 9-1-vinicius.go...@intel.com/
> >
> > The patch series adds the following functionalities to the IGC driver:
> > a) Configure FPE using `ethtool --set-mm`.
> > b) Display FPE settings via `ethtool --show-mm`.
> > c) View FPE statistics using `ethtool --include-statistics --show-mm'.
> > e) Enable preemptible/express queue with `fp`:
> >tc qdisc add ... root taprio \
> >fp E E P P
>
> Any reason why you are only enabling the preemptible traffic classes with
> taprio, and not with mqprio as well? I see there will have to be some work
> harmonizing igc's existing understanding of ring priorities with what Kurt
> did in
> 9f3297511dae ("igc: Add MQPRIO offload support"), and I was kind of
> expecting to see a proposal for that as part of this.
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On 14/2/2025 3:12 am, Kurt Kanzenbach wrote: On Thu Feb 13 2025, Vladimir Oltean wrote: So, confusingly to me, it seems like one operating mode is fundamentally different from the other, and something will have to change if both will be made to behave the same. What will change? You say mqprio will behave like taprio, but I think if anything, mqprio is the one which does the right thing, in igc_tsn_tx_arb(), and taprio seems to use the default Tx arbitration scheme? Correct. taprio is using the default scheme. mqprio configures it to what ever the user provided (in igc_tsn_tx_arb()). I don't think I'm on the same page as you guys, because to me, it is just odd that the P traffic classes would be the first ones with mqprio, but the last ones with taprio. I think we are on the same page here. At the end both have to behave the same. Either by using igc_tsn_tx_arb() for taprio too or only using the default scheme for both (and thereby keeping broken_mqprio). Whatever Faizal implements I'll match the behavior with mqprio. Hi Kurt & Vladimir, After reading Vladimir's reply on tc, hw queue, and socket priority mapping for both taprio and mqprio, I agree they should follow the same priority scheme for consistency—both in code and command usage (i.e., taprio, mqprio, and fpe in both configurations). Since igc_tsn_tx_arb() ensures a standard mapping of tc, socket priority, and hardware queue priority, I'll enable taprio to use igc_tsn_tx_arb() in a separate patch submission. I'll split the changes based on Vladimir's suggestion. First part - ethtool-mm related: igc: Add support to get frame preemption statistics via ethtool igc: Add support to get MAC Merge data via ethtool igc: Add support to set tx-min-frag-size igc: Add support for frame preemption verification igc: Set the RX packet buffer size for TSN mode igc: Optimize TX packet buffer utilization igc: Rename xdp_get_tx_ring() for non-XDP usage net: ethtool: mm: Extract stmmac verification logic into a common library Second part: igc: Add support for preemptible traffic class in taprio and mqprio igc: Use igc_tsn_tx_arb() for taprio queue priority scheme igc: Kurt's patch on mqprio to use normal TSN Tx mode Kurt can keep igc_tsn_tx_arb() for his mqprio patch, so preemptible tc should work the same for both taprio and mqprio. I'm suggesting to include Kurt's patch in the second part since there's some dependency and potential code conflict, even though it mixes different functional changes in the same series. Does this sound good to you?
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On 14/2/2025 12:20 pm, Abdul Rahim, Faizal wrote: On 14/2/2025 3:12 am, Kurt Kanzenbach wrote: On Thu Feb 13 2025, Vladimir Oltean wrote: So, confusingly to me, it seems like one operating mode is fundamentally different from the other, and something will have to change if both will be made to behave the same. What will change? You say mqprio will behave like taprio, but I think if anything, mqprio is the one which does the right thing, in igc_tsn_tx_arb(), and taprio seems to use the default Tx arbitration scheme? Correct. taprio is using the default scheme. mqprio configures it to what ever the user provided (in igc_tsn_tx_arb()). I don't think I'm on the same page as you guys, because to me, it is just odd that the P traffic classes would be the first ones with mqprio, but the last ones with taprio. I think we are on the same page here. At the end both have to behave the same. Either by using igc_tsn_tx_arb() for taprio too or only using the default scheme for both (and thereby keeping broken_mqprio). Whatever Faizal implements I'll match the behavior with mqprio. Hi Kurt & Vladimir, After reading Vladimir's reply on tc, hw queue, and socket priority mapping for both taprio and mqprio, I agree they should follow the same priority scheme for consistency—both in code and command usage (i.e., taprio, mqprio, and fpe in both configurations). Since igc_tsn_tx_arb() ensures a standard mapping of tc, socket priority, and hardware queue priority, I'll enable taprio to use igc_tsn_tx_arb() in a separate patch submission. I'll split the changes based on Vladimir's suggestion. First part - ethtool-mm related: igc: Add support to get frame preemption statistics via ethtool igc: Add support to get MAC Merge data via ethtool igc: Add support to set tx-min-frag-size igc: Add support for frame preemption verification igc: Set the RX packet buffer size for TSN mode igc: Optimize TX packet buffer utilization igc: Rename xdp_get_tx_ring() for non-XDP usage net: ethtool: mm: Extract stmmac verification logic into a common library Second part: igc: Add support for preemptible traffic class in taprio and mqprio igc: Use igc_tsn_tx_arb() for taprio queue priority scheme igc: Kurt's patch on mqprio to use normal TSN Tx mode Kurt can keep igc_tsn_tx_arb() for his mqprio patch, so preemptible tc should work the same for both taprio and mqprio. I'm suggesting to include Kurt's patch in the second part since there's some dependency and potential code conflict, even though it mixes different functional changes in the same series. I forgot that the second part patch: igc: Add support for preemptible traffic class in taprio and mqprio depends on the first part ethtool-mm, which would delay Kurt's patch. So Kurt, if you'd prefer to submit yours first, that's okay too.
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On Thu Feb 13 2025, Abdul Rahim, Faizal wrote:
> On 13/2/2025 6:01 am, Vladimir Oltean wrote:
>> On Mon, Feb 10, 2025 at 02:01:58AM -0500, Faizal Rahim wrote:
>>> Introduces support for the FPE feature in the IGC driver.
>>>
>>> The patches aligns with the upstream FPE API:
>>> https://patchwork.kernel.org/project/netdevbpf/cover/20230220122343.1156614-1-vladimir.olt...@nxp.com/
>>> https://patchwork.kernel.org/project/netdevbpf/cover/20230119122705.73054-1-vladimir.olt...@nxp.com/
>>>
>>> It builds upon earlier work:
>>> https://patchwork.kernel.org/project/netdevbpf/cover/20220520011538.109-1-vinicius.go...@intel.com/
>>>
>>> The patch series adds the following functionalities to the IGC driver:
>>> a) Configure FPE using `ethtool --set-mm`.
>>> b) Display FPE settings via `ethtool --show-mm`.
>>> c) View FPE statistics using `ethtool --include-statistics --show-mm'.
>>> e) Enable preemptible/express queue with `fp`:
>>> tc qdisc add ... root taprio \
>>> fp E E P P
>>
>> Any reason why you are only enabling the preemptible traffic classes
>> with taprio, and not with mqprio as well? I see there will have to be
>> some work harmonizing igc's existing understanding of ring priorities
>> with what Kurt did in 9f3297511dae ("igc: Add MQPRIO offload support"),
>> and I was kind of expecting to see a proposal for that as part of this.
>>
>
> I was planning to enable fpe + mqprio separately since it requires extra
> effort to explore mqprio with preemptible rings, ring priorities, and
> testing to ensure it works properly and there are no regressions.
Well, my idea was to move the current mqprio offload implementation from
legacy TSN Tx mode to the normal TSN Tx mode. Then, taprio and mqprio
can share the same code (with or without fpe). I have a draft patch
ready for that. What do you think about it?
Thanks,
Kurt
signature.asc
Description: PGP signature
Re: [Intel-wired-lan] [PATCH net-next v8 3/6] net: napi: add CPU affinity to napi_config
On 2/11/25 10:06 PM, Ahmed Zaki wrote:
> @@ -394,10 +395,8 @@ struct napi_struct {
> struct list_headdev_list;
> struct hlist_node napi_hash_node;
> int irq;
> -#ifdef CONFIG_RFS_ACCEL
> struct irq_affinity_notify notify;
> int napi_rmap_idx;
> -#endif
I'm sorry for the late doubt, but it's not clear to me why you need to
add the #ifdef in the previous patch ?!?
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 209296cef3cd..d2c942bbd5e6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6871,28 +6871,39 @@ void netif_queue_set_napi(struct net_device *dev,
> unsigned int queue_index,
> }
> EXPORT_SYMBOL(netif_queue_set_napi);
>
> -#ifdef CONFIG_RFS_ACCEL
> static void
> -netif_irq_cpu_rmap_notify(struct irq_affinity_notify *notify,
> - const cpumask_t *mask)
> +netif_napi_irq_notify(struct irq_affinity_notify *notify,
> + const cpumask_t *mask)
> {
> struct napi_struct *napi =
> container_of(notify, struct napi_struct, notify);
> +#ifdef CONFIG_RFS_ACCEL
> struct cpu_rmap *rmap = napi->dev->rx_cpu_rmap;
> int err;
> +#endif
>
> - err = cpu_rmap_update(rmap, napi->napi_rmap_idx, mask);
> - if (err)
> - netdev_warn(napi->dev, "RMAP update failed (%d)\n",
> - err);
> + if (napi->config && napi->dev->irq_affinity_auto)
> + cpumask_copy(&napi->config->affinity_mask, mask);
> +
> +#ifdef CONFIG_RFS_ACCEL
> + if (napi->dev->rx_cpu_rmap_auto) {
> + err = cpu_rmap_update(rmap, napi->napi_rmap_idx, mask);
> + if (err)
> + netdev_warn(napi->dev, "RMAP update failed (%d)\n",
> + err);
> + }
> +#endif
Minor nit: if you provide a netif_rx_cpu_rmap() helper returning
dev->rx_cpu_rmap or NULL for !CONFIG_RFS_ACCEL build, you can avoid the
above 2 ifdefs and possibly more below.
> @@ -6915,7 +6926,6 @@ static int napi_irq_cpu_rmap_add(struct napi_struct
> *napi, int irq)
> if (rc)
> goto err_set;
>
> - set_bit(NAPI_STATE_HAS_NOTIFIER, &napi->state);
Minor nit: I think it would be better if the previous patch would add
directly this line in netif_napi_set_irq_locked() (avoding the removal
here).
/P
Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: Fix deinitializing VF in error path
On 13.02.2025 11:55, Simon Horman wrote:
> On Tue, Feb 11, 2025 at 06:43:21PM +0100, Marcin Szycik wrote:
>> If ice_ena_vfs() fails after calling ice_create_vf_entries(), it frees
>> all VFs without removing them from snapshot PF-VF mailbox list, leading
>> to list corruption.
>>
>> Reproducer:
>> devlink dev eswitch set $PF1_PCI mode switchdev
>> ip l s $PF1 up
>> ip l s $PF1 promisc on
>> sleep 1
>> echo 1 > /sys/class/net/$PF1/device/sriov_numvfs
>
> Should the line above be "echo 0" to remove the VFs before creating VFs
> below (I'm looking at sriov_numvfs_store())?
Both "echo 1" commands fail (I'm fixing it in patch 2/2), that's why there's
no "echo 0" in between. Also, in this minimal example I'm assuming no VFs
were initially present.
Thanks for reviewing!
Marcin
>> sleep 1
>> echo 1 > /sys/class/net/$PF1/device/sriov_numvfs
>>
>> Trace (minimized):
>> list_add corruption. next->prev should be prev (8882e241c6f0), but was
>> . (next=888455da1330).
>> kernel BUG at lib/list_debug.c:29!
>> RIP: 0010:__list_add_valid_or_report+0xa6/0x100
>>ice_mbx_init_vf_info+0xa7/0x180 [ice]
>>ice_initialize_vf_entry+0x1fa/0x250 [ice]
>>ice_sriov_configure+0x8d7/0x1520 [ice]
>>? __percpu_ref_switch_mode+0x1b1/0x5d0
>>? __pfx_ice_sriov_configure+0x10/0x10 [ice]
>>
>> Sometimes a KASAN report can be seen instead with a similar stack trace:
>> BUG: KASAN: use-after-free in __list_add_valid_or_report+0xf1/0x100
>>
>> VFs are added to this list in ice_mbx_init_vf_info(), but only removed
>> in ice_free_vfs(). Move the removing to ice_free_vf_entries(), which is
>> also being called in other places where VFs are being removed (including
>> ice_free_vfs() itself).
>>
>> Fixes: 8cd8a6b17d27 ("ice: move VF overflow message count into struct
>> ice_mbx_vf_info")
>> Reported-by: Sujai Buvaneswaran
>> Closes:
>> https://lore.kernel.org/intel-wired-lan/ph0pr11mb50138b635f2e5ceb7075325d96...@ph0pr11mb5013.namprd11.prod.outlook.com
>> Reviewed-by: Martyna Szapar-Mudlaw
>> Signed-off-by: Marcin Szycik
>
> The comment above notwithstanding, I agree that this addresses the
> bug you have described.
>
> Reviewed-by: Simon Horman
>
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On 13/2/2025 8:01 pm, Kurt Kanzenbach wrote:
On Thu Feb 13 2025, Abdul Rahim, Faizal wrote:
On 13/2/2025 6:01 am, Vladimir Oltean wrote:
On Mon, Feb 10, 2025 at 02:01:58AM -0500, Faizal Rahim wrote:
Introduces support for the FPE feature in the IGC driver.
The patches aligns with the upstream FPE API:
https://patchwork.kernel.org/project/netdevbpf/cover/20230220122343.1156614-1-vladimir.olt...@nxp.com/
https://patchwork.kernel.org/project/netdevbpf/cover/20230119122705.73054-1-vladimir.olt...@nxp.com/
It builds upon earlier work:
https://patchwork.kernel.org/project/netdevbpf/cover/20220520011538.109-1-vinicius.go...@intel.com/
The patch series adds the following functionalities to the IGC driver:
a) Configure FPE using `ethtool --set-mm`.
b) Display FPE settings via `ethtool --show-mm`.
c) View FPE statistics using `ethtool --include-statistics --show-mm'.
e) Enable preemptible/express queue with `fp`:
tc qdisc add ... root taprio \
fp E E P P
Any reason why you are only enabling the preemptible traffic classes
with taprio, and not with mqprio as well? I see there will have to be
some work harmonizing igc's existing understanding of ring priorities
with what Kurt did in 9f3297511dae ("igc: Add MQPRIO offload support"),
and I was kind of expecting to see a proposal for that as part of this.
I was planning to enable fpe + mqprio separately since it requires extra
effort to explore mqprio with preemptible rings, ring priorities, and
testing to ensure it works properly and there are no regressions.
Well, my idea was to move the current mqprio offload implementation from
legacy TSN Tx mode to the normal TSN Tx mode. Then, taprio and mqprio
can share the same code (with or without fpe). I have a draft patch
ready for that. What do you think about it?
Thanks,
Kurt
Hi Kurt,
I’m okay with including it in this series and testing fpe + mqprio, but I’m
not sure if others might be concerned about adding different functional
changes in this fpe series.
Hi Vladimir,
Any thoughts on this ?
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On Thu, Feb 13, 2025 at 08:54:18PM +0800, Abdul Rahim, Faizal wrote: > > Well, my idea was to move the current mqprio offload implementation from > > legacy TSN Tx mode to the normal TSN Tx mode. Then, taprio and mqprio > > can share the same code (with or without fpe). I have a draft patch > > ready for that. What do you think about it? > > Hi Kurt, > > I’m okay with including it in this series and testing fpe + mqprio, but I’m > not sure if others might be concerned about adding different functional > changes in this fpe series. > > Hi Vladimir, > Any thoughts on this ? Well, what do you think of my split proposal from here, essentially drawing the line for the first patch set at just ethtool mm? https://lore.kernel.org/netdev/20250213110653.iqy5magn27jyfnwh@skbuf/
Re: [Intel-wired-lan] [PATCH net-next v9 0/4] fix the DMA API misuse problem for page_pool
On 2025/2/13 2:53, Matthew Wilcox wrote: > On Wed, Feb 12, 2025 at 05:25:47PM +0800, Yunsheng Lin wrote: >> This patchset fix the dma API misuse problem as mentioned in [1]. >> >> 1. >> https://lore.kernel.org/lkml/8067f204-1380-4d37-8ffd-007fc6f26...@kernel.org/T/ > > That's a very long and complicated thread. I gave up. You need to > provide a proper description of the problem. The description of the problem is in the commit log of patch 2 as something below: "Networking driver with page_pool support may hand over page still with dma mapping to network stack and try to reuse that page after network stack is done with it and passes it back to page_pool to avoid the penalty of dma mapping/unmapping. With all the caching in the network stack, some pages may be held in the network stack without returning to the page_pool soon enough, and with VF disable causing the driver unbound, the page_pool does not stop the driver from doing it's unbounding work, instead page_pool uses workqueue to check if there is some pages coming back from the network stack periodically, if there is any, it will do the dma unmmapping related cleanup work. As mentioned in [1], attempting DMA unmaps after the driver has already unbound may leak resources or at worst corrupt memory. Fundamentally, the page pool code cannot allow DMA mappings to outlive the driver they belong to." The description of the fixing is also in the commit log of patch 2 as below: "By using the 'struct page_pool_item' referenced by page->pp_item, page_pool is not only able to keep track of the inflight page to do dma unmmaping if some pages are still handled in networking stack when page_pool_destroy() is called, and networking stack is also able to find the page_pool owning the page when returning pages back into page_pool: 1. When a page is added to the page_pool, an item is deleted from pool->hold_items and set the 'pp_netmem' pointing to that page and set item->state and item->pp_netmem accordingly in order to keep track of that page, refill from pool->release_items when pool->hold_items is empty or use the item from pool->slow_items when fast items run out. 2. When a page is released from the page_pool, it is able to tell which page_pool this page belongs to by masking off the lower bits of the pointer to page_pool_item *item, as the 'struct page_pool_item_block' is stored in the top of a struct page. And after clearing the pp_item->state', the item for the released page is added back to pool->release_items so that it can be reused for new pages or just free it when it is from the pool->slow_items. 3. When page_pool_destroy() is called, item->state is used to tell if a specific item is being used/dma mapped or not by scanning all the item blocks in pool->item_blocks, then item->netmem can be used to do the dma unmmaping if the corresponding inflight page is dma mapped." it is worth to mention that the changing of page->pp to page->pp_item for the above fix may be able to enable the decoupling page_pool from using the metadata of 'struct page' if folios only provide a memdesc pointer to the page_pool subsystem in the future as pp_item may be used as the metadata replacement of existing 'struct page'. >
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On 13/2/2025 9:00 pm, Vladimir Oltean wrote: On Thu, Feb 13, 2025 at 08:54:18PM +0800, Abdul Rahim, Faizal wrote: Well, my idea was to move the current mqprio offload implementation from legacy TSN Tx mode to the normal TSN Tx mode. Then, taprio and mqprio can share the same code (with or without fpe). I have a draft patch ready for that. What do you think about it? Hi Kurt, I’m okay with including it in this series and testing fpe + mqprio, but I’m not sure if others might be concerned about adding different functional changes in this fpe series. Hi Vladimir, Any thoughts on this ? Well, what do you think of my split proposal from here, essentially drawing the line for the first patch set at just ethtool mm? https://lore.kernel.org/netdev/20250213110653.iqy5magn27jyfnwh@skbuf/ Honestly, after reconsidering, I’d prefer to keep the current series as is (without Kurt’s patch), assuming you’re okay with enabling mqprio + fpe later rather than at the same time as taprio + fpe. There likely won’t be any additional work needed for mqprio + fpe after Kurt’s patch is accepted, since it will mostly reuse the taprio code flow. If I were to split it, the structure would look something like this: First part of fpe series: igc: Add support to get frame preemption statistics via ethtool igc: Add support to get MAC Merge data via ethtool igc: Add support to set tx-min-frag-size igc: Add support for frame preemption verification igc: Set the RX packet buffer size for TSN mode igc: Optimize the TX packet buffer utilization igc: Rename xdp_get_tx_ring() for non-XDP usage net: ethtool: mm: Extract stmmac verification logic into a common library Second part of fpe: igc: Add support for preemptible traffic class in taprio I don’t think Kurt’s patch should be included in my second part of fpe, as it’s not logically related. Another approach would be to wait for Kurt’s patch to be accepted first, then submit the second part and verify both taprio + mqprio. However, that would delay i226 from having a basic fpe feature working as a whole, which I'd really like to avoid.
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On Thu Feb 13 2025, Abdul Rahim, Faizal wrote: > On 13/2/2025 9:00 pm, Vladimir Oltean wrote: >> On Thu, Feb 13, 2025 at 08:54:18PM +0800, Abdul Rahim, Faizal wrote: Well, my idea was to move the current mqprio offload implementation from legacy TSN Tx mode to the normal TSN Tx mode. Then, taprio and mqprio can share the same code (with or without fpe). I have a draft patch ready for that. What do you think about it? >>> >>> Hi Kurt, >>> >>> I’m okay with including it in this series and testing fpe + mqprio, but I’m >>> not sure if others might be concerned about adding different functional >>> changes in this fpe series. >>> >>> Hi Vladimir, >>> Any thoughts on this ? >> >> Well, what do you think of my split proposal from here, essentially >> drawing the line for the first patch set at just ethtool mm? >> https://lore.kernel.org/netdev/20250213110653.iqy5magn27jyfnwh@skbuf/ >> > > Honestly, after reconsidering, I’d prefer to keep the current series as is > (without Kurt’s patch), assuming you’re okay with enabling mqprio + fpe > later rather than at the same time as taprio + fpe. There likely won’t be > any additional work needed for mqprio + fpe after Kurt’s patch is accepted, > since it will mostly reuse the taprio code flow. I think so. After switching the Tx mode mqprio will basically be a special case of taprio with a dummy Qbv schedule. Also the driver currently rejects mqprio with hardware offloading and preemptible_tcs set. So, I do not see any issues in merging your fpe series first. I can handle the mqprio part afterwards. Thanks, Kurt signature.asc Description: PGP signature
Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: Fix deinitializing VF in error path
On Tue, Feb 11, 2025 at 06:43:21PM +0100, Marcin Szycik wrote:
> If ice_ena_vfs() fails after calling ice_create_vf_entries(), it frees
> all VFs without removing them from snapshot PF-VF mailbox list, leading
> to list corruption.
>
> Reproducer:
> devlink dev eswitch set $PF1_PCI mode switchdev
> ip l s $PF1 up
> ip l s $PF1 promisc on
> sleep 1
> echo 1 > /sys/class/net/$PF1/device/sriov_numvfs
Should the line above be "echo 0" to remove the VFs before creating VFs
below (I'm looking at sriov_numvfs_store())?
> sleep 1
> echo 1 > /sys/class/net/$PF1/device/sriov_numvfs
>
> Trace (minimized):
> list_add corruption. next->prev should be prev (8882e241c6f0), but was
> . (next=888455da1330).
> kernel BUG at lib/list_debug.c:29!
> RIP: 0010:__list_add_valid_or_report+0xa6/0x100
>ice_mbx_init_vf_info+0xa7/0x180 [ice]
>ice_initialize_vf_entry+0x1fa/0x250 [ice]
>ice_sriov_configure+0x8d7/0x1520 [ice]
>? __percpu_ref_switch_mode+0x1b1/0x5d0
>? __pfx_ice_sriov_configure+0x10/0x10 [ice]
>
> Sometimes a KASAN report can be seen instead with a similar stack trace:
> BUG: KASAN: use-after-free in __list_add_valid_or_report+0xf1/0x100
>
> VFs are added to this list in ice_mbx_init_vf_info(), but only removed
> in ice_free_vfs(). Move the removing to ice_free_vf_entries(), which is
> also being called in other places where VFs are being removed (including
> ice_free_vfs() itself).
>
> Fixes: 8cd8a6b17d27 ("ice: move VF overflow message count into struct
> ice_mbx_vf_info")
> Reported-by: Sujai Buvaneswaran
> Closes:
> https://lore.kernel.org/intel-wired-lan/ph0pr11mb50138b635f2e5ceb7075325d96...@ph0pr11mb5013.namprd11.prod.outlook.com
> Reviewed-by: Martyna Szapar-Mudlaw
> Signed-off-by: Marcin Szycik
The comment above notwithstanding, I agree that this addresses the
bug you have described.
Reviewed-by: Simon Horman
Re: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: Avoid setting default Rx VSI twice in switchdev setup
On Tue, Feb 11, 2025 at 06:43:22PM +0100, Marcin Szycik wrote:
> As part of switchdev environment setup, uplink VSI is configured as
> default for both Tx and Rx. Default Rx VSI is also used by promiscuous
> mode. If promisc mode is enabled and an attempt to enter switchdev mode
> is made, the setup will fail because Rx VSI is already configured as
> default (rule exists).
>
> Reproducer:
> devlink dev eswitch set $PF1_PCI mode switchdev
> ip l s $PF1 up
> ip l s $PF1 promisc on
> echo 1 > /sys/class/net/$PF1/device/sriov_numvfs
>
> In switchdev setup, use ice_set_dflt_vsi() instead of plain
> ice_cfg_dflt_vsi(), which avoids repeating setting default VSI for Rx if
> it's already configured.
>
> Fixes: 50d62022f455 ("ice: default Tx rule instead of to queue")
> Reported-by: Sujai Buvaneswaran
> Closes:
> https://lore.kernel.org/intel-wired-lan/ph0pr11mb50138b635f2e5ceb7075325d96...@ph0pr11mb5013.namprd11.prod.outlook.com
> Reviewed-by: Martyna Szapar-Mudlaw
> Signed-off-by: Marcin Szycik
Reviewed-by: Simon Horman
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On Thu, Feb 13, 2025 at 11:00:32AM +0200, Vladimir Oltean wrote: > On Thu, Feb 13, 2025 at 02:12:47PM +0800, Abdul Rahim, Faizal wrote: > > I was planning to enable fpe + mqprio separately since it requires extra > > effort to explore mqprio with preemptible rings, ring priorities, and > > testing to ensure it works properly and there are no regressions. > > > > I’m really hoping that fpe + mqprio doesn’t have to be enabled together in > > this series to keep things simple. It could be added later—adding it now > > would introduce additional complexity and delay this series further, which > > is focused on enabling basic, working fpe on i226. > > > > Would that be okay with you? > > But why would the mqprio params of taprio be handled differently than > the dedicated mqprio qdisc? Why isn't the additional complexity you > mention also needed for taprio? When I got convinced to expose > preemptible TCs through separate UAPI in mqprio in taprio, it wasn't my > understanding that drivers would be reacting differently depending on > which Qdisc the configuration comes from. If you want to reduce the complexity of individual patch sets, I guess you can keep this one for just the MAC Merge layer (ethtool), and then group common handling of preemptible traffic classes (both mqprio and taprio) in the next one.
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On Thu, Feb 13, 2025 at 02:12:47PM +0800, Abdul Rahim, Faizal wrote: > I was planning to enable fpe + mqprio separately since it requires extra > effort to explore mqprio with preemptible rings, ring priorities, and > testing to ensure it works properly and there are no regressions. > > I’m really hoping that fpe + mqprio doesn’t have to be enabled together in > this series to keep things simple. It could be added later—adding it now > would introduce additional complexity and delay this series further, which > is focused on enabling basic, working fpe on i226. > > Would that be okay with you? But why would the mqprio params of taprio be handled differently than the dedicated mqprio qdisc? Why isn't the additional complexity you mention also needed for taprio? When I got convinced to expose preemptible TCs through separate UAPI in mqprio in taprio, it wasn't my understanding that drivers would be reacting differently depending on which Qdisc the configuration comes from.
Re: [Intel-wired-lan] [PATCH iwl-next v3 03/14] ixgbe: add handler for devlink .info_get()
From: Jiri Pirko Sent: Wednesday, February 12, 2025 4:05 PM >Wed, Feb 12, 2025 at 02:14:02PM +0100, jedrzej.jagiel...@intel.com wrote: >>Provide devlink .info_get() callback implementation to allow the >>driver to report detailed version information. The following info >>is reported: >> >> "serial_number" -> The PCI DSN of the adapter >> "fw.bundle_id" -> Unique identifier for the combined flash image >> "fw.undi" -> Version of the Option ROM containing the UEFI driver >> "board.id" -> The PBA ID string > >Do you have some board.serial_number by any chance? This could be handy >in some cases, for example if you have a board with multiple nic asics. Unfortunately i cannot spot nothing more for identification than the PCI DSN number.
Re: [Intel-wired-lan] [iwl-next, rdma v3 00/24] Add RDMA support for Intel IPU E2000 (GEN3)
On 2/10/25 12:09, Leon Romanovsky wrote: On Mon, Feb 10, 2025 at 11:41:31AM +0100, Przemek Kitszel wrote: On 2/7/25 20:49, Tatyana Nikolova wrote: This patch series is based on 6.14-rc1 and includes both netdev and RDMA patches for ease of review. It can also be viewed here [1]. A shared pull request will be sent for patches 1-7 following review. [...] TLDR of my mail: could be take 1st patch prior to the rest? V2 RFC series is at https://lwn.net/Articles/987141/. code there was mostly the same, and noone commented, I bet due to the sheer size of the series It was very optimistic to expect for a review during holiday season and merge window, especially series of 25 patches which are marked as RFC. that's true so, given most of the patches will go via your tree, how do you want to split us the existing ones into series? a) 1st, idpf, rdma b) 1st, rest c) all together In any case I will do a review too of course Thanks
Re: [Intel-wired-lan] [iwl-next v9 5/9] ice, irdma: move interrupts code to irdma
On 03.12.2024 07:58, Michal Swiatkowski wrote:
> Move responsibility of MSI-X requesting for RDMA feature from ice driver
> to irdma driver. It is done to allow simple fallback when there is not
> enough MSI-X available.
>
> Change amount of MSI-X used for control from 4 to 1, as it isn't needed
> to have more than one MSI-X for this purpose.
Hi, I'm observing KASAN reports or kernel panic when attempting to remove irdma
with this patchset, most probably this patch being the culprit, since it touches
functions from splat.
Reproducer:
sudo rmmod irdma
Minified splat(s):
BUG: KASAN: use-after-free in irdma_remove+0x257/0x2d0 [irdma]
Call Trace:
? __pfx__raw_spin_lock_irqsave+0x10/0x10
? kfree+0x253/0x450
? irdma_remove+0x257/0x2d0 [irdma]
kasan_report+0xed/0x120
? irdma_remove+0x257/0x2d0 [irdma]
irdma_remove+0x257/0x2d0 [irdma]
auxiliary_bus_remove+0x56/0x80
device_release_driver_internal+0x371/0x530
? kernfs_put.part.0+0x147/0x310
driver_detach+0xbf/0x180
bus_remove_driver+0x11b/0x2a0
auxiliary_driver_unregister+0x1a/0x50
irdma_exit_module+0x40/0x4c [irdma]
Oops: general protection fault, probably for non-canonical address
0xdc00: [#1] PREEMPT SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x-0x0007]
RIP: 0010:ice_free_rdma_qvector+0x2a/0xa0 [ice]
Call Trace:
? ice_free_rdma_qvector+0x2a/0xa0 [ice]
irdma_remove+0x179/0x2d0 [irdma]
auxiliary_bus_remove+0x56/0x80
device_release_driver_internal+0x371/0x530
? kobject_put+0x61/0x4b0
driver_detach+0xbf/0x180
bus_remove_driver+0x11b/0x2a0
auxiliary_driver_unregister+0x1a/0x50
irdma_exit_module+0x40/0x4c [irdma]
> Reviewed-by: Jacob Keller
> Signed-off-by: Michal Swiatkowski
> ---
> drivers/infiniband/hw/irdma/main.h | 3 ++
> drivers/net/ethernet/intel/ice/ice.h | 1 -
> include/linux/net/intel/iidc.h | 2 +
> drivers/infiniband/hw/irdma/hw.c | 2 -
> drivers/infiniband/hw/irdma/main.c | 46 -
> drivers/net/ethernet/intel/ice/ice_idc.c | 64 ++--
> drivers/net/ethernet/intel/ice/ice_irq.c | 3 +-
> 7 files changed, 65 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/infiniband/hw/irdma/main.h
> b/drivers/infiniband/hw/irdma/main.h
> index 9f0ed6e84471..ef9a9b79d711 100644
> --- a/drivers/infiniband/hw/irdma/main.h
> +++ b/drivers/infiniband/hw/irdma/main.h
> @@ -117,6 +117,9 @@ extern struct auxiliary_driver i40iw_auxiliary_drv;
>
> #define IRDMA_IRQ_NAME_STR_LEN (64)
>
> +#define IRDMA_NUM_AEQ_MSIX 1
> +#define IRDMA_MIN_MSIX 2
> +
> enum init_completion_state {
> INVALID_STATE = 0,
> INITIAL_STATE,
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index f497f7d6eb71..14a90c916d43 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -97,7 +97,6 @@
> #define ICE_MIN_LAN_OICR_MSIX1
> #define ICE_MIN_MSIX (ICE_MIN_LAN_TXRX_MSIX + ICE_MIN_LAN_OICR_MSIX)
> #define ICE_FDIR_MSIX2
> -#define ICE_RDMA_NUM_AEQ_MSIX4
> #define ICE_NO_VSI 0x
> #define ICE_VSI_MAP_CONTIG 0
> #define ICE_VSI_MAP_SCATTER 1
> diff --git a/include/linux/net/intel/iidc.h b/include/linux/net/intel/iidc.h
> index 1c1332e4df26..13274c3def66 100644
> --- a/include/linux/net/intel/iidc.h
> +++ b/include/linux/net/intel/iidc.h
> @@ -78,6 +78,8 @@ int ice_del_rdma_qset(struct ice_pf *pf, struct
> iidc_rdma_qset_params *qset);
> int ice_rdma_request_reset(struct ice_pf *pf, enum iidc_reset_type
> reset_type);
> int ice_rdma_update_vsi_filter(struct ice_pf *pf, u16 vsi_id, bool enable);
> void ice_get_qos_params(struct ice_pf *pf, struct iidc_qos_params *qos);
> +int ice_alloc_rdma_qvector(struct ice_pf *pf, struct msix_entry *entry);
> +void ice_free_rdma_qvector(struct ice_pf *pf, struct msix_entry *entry);
>
> /* Structure representing auxiliary driver tailored information about the
> core
> * PCI dev, each auxiliary driver using the IIDC interface will have an
> diff --git a/drivers/infiniband/hw/irdma/hw.c
> b/drivers/infiniband/hw/irdma/hw.c
> index ad50b77282f8..69ce1862eabe 100644
> --- a/drivers/infiniband/hw/irdma/hw.c
> +++ b/drivers/infiniband/hw/irdma/hw.c
> @@ -498,8 +498,6 @@ static int irdma_save_msix_info(struct irdma_pci_f *rf)
> iw_qvlist->num_vectors = rf->msix_count;
> if (rf->msix_count <= num_online_cpus())
> rf->msix_shared = true;
> - else if (rf->msix_count > num_online_cpus() + 1)
> - rf->msix_count = num_online_cpus() + 1;
>
> pmsix = rf->msix_entries;
> for (i = 0, ceq_idx = 0; i < rf->msix_count; i++, iw_qvinfo++) {
> diff --git a/drivers/infiniband/hw/irdma/main.c
> b/drivers/infiniband/hw/irdma/main.c
> index 3f13200ff71b..1ee8969595d3 100644
> --- a/drivers/infiniband/hw/irdma/main.c
> +++ b
[Intel-wired-lan] [tnguy-next-queue:dev-queue] BUILD SUCCESS dd529eeb4eb8cc6aaee6cb24a7e366a8938df4e2
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue branch HEAD: dd529eeb4eb8cc6aaee6cb24a7e366a8938df4e2 ixgbe: add PTP support for E610 device elapsed time: 1449m configs tested: 119 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alpha allnoconfiggcc-14.2.0 alphaallyesconfiggcc-14.2.0 arc allmodconfiggcc-13.2.0 arc allnoconfiggcc-13.2.0 arc allyesconfiggcc-13.2.0 arc axs103_defconfiggcc-13.2.0 arc haps_hs_smp_defconfiggcc-13.2.0 arc randconfig-001-20250213gcc-13.2.0 arc randconfig-002-20250213gcc-13.2.0 arm allnoconfigclang-17 arm allyesconfiggcc-14.2.0 arm netwinder_defconfiggcc-14.2.0 arm randconfig-001-20250213clang-17 arm randconfig-002-20250213clang-15 arm randconfig-003-20250213clang-21 arm randconfig-004-20250213gcc-14.2.0 arm rpc_defconfigclang-17 arm64allmodconfigclang-18 arm64 allnoconfiggcc-14.2.0 arm64 randconfig-001-20250213clang-19 arm64 randconfig-002-20250213gcc-14.2.0 arm64 randconfig-003-20250213gcc-14.2.0 arm64 randconfig-004-20250213clang-21 csky allnoconfiggcc-14.2.0 csky randconfig-001-20250213gcc-14.2.0 csky randconfig-002-20250213gcc-14.2.0 hexagon allmodconfigclang-21 hexagon allnoconfigclang-21 hexagon allyesconfigclang-18 hexagon randconfig-001-20250213clang-21 hexagon randconfig-002-20250213clang-18 i386 allmodconfiggcc-12 i386 allnoconfiggcc-12 i386 allyesconfiggcc-12 i386buildonly-randconfig-001-20250213gcc-12 i386buildonly-randconfig-002-20250213clang-19 i386buildonly-randconfig-003-20250213clang-19 i386buildonly-randconfig-004-20250213clang-19 i386buildonly-randconfig-005-20250213clang-19 i386buildonly-randconfig-006-20250213clang-19 i386defconfigclang-19 loongarchallmodconfiggcc-14.2.0 loongarch allnoconfiggcc-14.2.0 loongarch randconfig-001-20250213gcc-14.2.0 loongarch randconfig-002-20250213gcc-14.2.0 m68k allmodconfiggcc-14.2.0 m68k allnoconfiggcc-14.2.0 m68k allyesconfiggcc-14.2.0 microblazeallnoconfiggcc-14.2.0 mips allnoconfiggcc-14.2.0 mipsbcm47xx_defconfigclang-21 mips ci20_defconfigclang-19 mips rt305x_defconfigclang-19 nios2 allnoconfiggcc-14.2.0 nios2 randconfig-001-20250213gcc-14.2.0 nios2 randconfig-002-20250213gcc-14.2.0 openrisc allnoconfiggcc-14.2.0 openrisc allyesconfiggcc-14.2.0 openriscdefconfiggcc-14.2.0 parisc allmodconfiggcc-14.2.0 pariscallnoconfiggcc-14.2.0 parisc allyesconfiggcc-14.2.0 parisc defconfiggcc-14.2.0 pariscrandconfig-001-20250213gcc-14.2.0 pariscrandconfig-002-20250213gcc-14.2.0 powerpc allmodconfiggcc-14.2.0 powerpc allnoconfiggcc-14.2.0 powerpcamigaone_defconfiggcc-14.2.0 powerpc randconfig-001-20250213clang-17 powerpc randconfig-002-20250213gcc-14.2.0 powerpc randconfig-003-20250213gcc-14.2.0 powerpc tqm8541_defconfigclang-15 powerpc64 randconfig-001-20250213clang-19 powerpc64 randconfig-002-20250213clang-21 powerpc64 randconfig-003-20250213gcc-14.2.0 riscv allnoconfiggcc-14.2.0 riscv defconfigclang-19 riscv randconfig-001-20250213clang-19 riscv
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On Thu Feb 13 2025, Vladimir Oltean wrote: > So, confusingly to me, it seems like one operating mode is fundamentally > different from the other, and something will have to change if both will > be made to behave the same. What will change? You say mqprio will behave > like taprio, but I think if anything, mqprio is the one which does the > right thing, in igc_tsn_tx_arb(), and taprio seems to use the default Tx > arbitration scheme? Correct. taprio is using the default scheme. mqprio configures it to what ever the user provided (in igc_tsn_tx_arb()). > I don't think I'm on the same page as you guys, because to me, it is > just odd that the P traffic classes would be the first ones with > mqprio, but the last ones with taprio. I think we are on the same page here. At the end both have to behave the same. Either by using igc_tsn_tx_arb() for taprio too or only using the default scheme for both (and thereby keeping broken_mqprio). Whatever Faizal implements I'll match the behavior with mqprio. Thanks, Kurt signature.asc Description: PGP signature
[Intel-wired-lan] [PATCH iwl-net] iavf: fix circular lock dependency with netdev_lock
ail to register the netdevice, then we re-acquire
the adapter critical lock to finish the transition back to
__IAVF_INIT_CONFIG_ADAPTER.
This ensures every call where both netdev_lock and the adapter->crit_lock
are acquired under the same ordering.
Fixes: afc664987ab3 ("eth: iavf: extend the netdev_lock usage")
Signed-off-by: Jacob Keller
Tested-by: Przemek Kitszel
Reviewed-by: Przemek Kitszel
---
Changes in v2:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v1:
https://lore.kernel.org/r/20250213-jk-iavf-abba-lock-crash-v1-1-787d7c652...@intel.com
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
b/drivers/net/ethernet/intel/iavf/iavf_main.c
index
852e5b62f0a5dc038c0e5c0f76541870e69384ac..6faa62bced3a2ccb935219ba0726275c8ae60365
100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1983,7 +1983,7 @@ static int iavf_reinit_interrupt_scheme(struct
iavf_adapter *adapter, bool runni
static void iavf_finish_config(struct work_struct *work)
{
struct iavf_adapter *adapter;
- bool netdev_released = false;
+ bool locks_released = false;
int pairs, err;
adapter = container_of(work, struct iavf_adapter, finish_config);
@@ -2012,19 +2012,22 @@ static void iavf_finish_config(struct work_struct *work)
netif_set_real_num_tx_queues(adapter->netdev, pairs);
if (adapter->netdev->reg_state != NETREG_REGISTERED) {
+ mutex_unlock(&adapter->crit_lock);
netdev_unlock(adapter->netdev);
- netdev_released = true;
+ locks_released = true;
err = register_netdevice(adapter->netdev);
if (err) {
dev_err(&adapter->pdev->dev, "Unable to
register netdev (%d)\n",
err);
/* go back and try again.*/
+ mutex_lock(&adapter->crit_lock);
iavf_free_rss(adapter);
iavf_free_misc_irq(adapter);
iavf_reset_interrupt_capability(adapter);
iavf_change_state(adapter,
__IAVF_INIT_CONFIG_ADAPTER);
+ mutex_unlock(&adapter->crit_lock);
goto out;
}
}
@@ -2040,9 +2043,10 @@ static void iavf_finish_config(struct work_struct *work)
}
out:
- mutex_unlock(&adapter->crit_lock);
- if (!netdev_released)
+ if (!locks_released) {
+ mutex_unlock(&adapter->crit_lock);
netdev_unlock(adapter->netdev);
+ }
rtnl_unlock();
}
---
base-commit: 348f968b89bfeec0bb53dd82dba58b94d97fbd34
change-id: 20250213-jk-iavf-abba-lock-crash-300fbb33ae99
Best regards,
--
Jacob Keller
Re: [Intel-wired-lan] [PATCH net-next v8 3/6] net: napi: add CPU affinity to napi_config
On 2025-02-13 5:26 a.m., Paolo Abeni wrote:
On 2/11/25 10:06 PM, Ahmed Zaki wrote:
@@ -394,10 +395,8 @@ struct napi_struct {
struct list_headdev_list;
struct hlist_node napi_hash_node;
int irq;
-#ifdef CONFIG_RFS_ACCEL
struct irq_affinity_notify notify;
int napi_rmap_idx;
-#endif
I'm sorry for the late doubt, but it's not clear to me why you need to
add the #ifdef in the previous patch ?!?
It was there to make the code consistent, since the rmap and the
notifier were only needed for ARFS.
It can be removed, although I am not sure if there would be any warnings
since on !CONFIG_ARFS_ACCEL the fields would never be used.
diff --git a/net/core/dev.c b/net/core/dev.c
index 209296cef3cd..d2c942bbd5e6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6871,28 +6871,39 @@ void netif_queue_set_napi(struct net_device *dev,
unsigned int queue_index,
}
EXPORT_SYMBOL(netif_queue_set_napi);
-#ifdef CONFIG_RFS_ACCEL
static void
-netif_irq_cpu_rmap_notify(struct irq_affinity_notify *notify,
- const cpumask_t *mask)
+netif_napi_irq_notify(struct irq_affinity_notify *notify,
+ const cpumask_t *mask)
{
struct napi_struct *napi =
container_of(notify, struct napi_struct, notify);
+#ifdef CONFIG_RFS_ACCEL
struct cpu_rmap *rmap = napi->dev->rx_cpu_rmap;
int err;
+#endif
- err = cpu_rmap_update(rmap, napi->napi_rmap_idx, mask);
- if (err)
- netdev_warn(napi->dev, "RMAP update failed (%d)\n",
- err);
+ if (napi->config && napi->dev->irq_affinity_auto)
+ cpumask_copy(&napi->config->affinity_mask, mask);
+
+#ifdef CONFIG_RFS_ACCEL
+ if (napi->dev->rx_cpu_rmap_auto) {
+ err = cpu_rmap_update(rmap, napi->napi_rmap_idx, mask);
+ if (err)
+ netdev_warn(napi->dev, "RMAP update failed (%d)\n",
+ err);
+ }
+#endif
Minor nit: if you provide a netif_rx_cpu_rmap() helper returning
dev->rx_cpu_rmap or NULL for !CONFIG_RFS_ACCEL build, you can avoid the
above 2 ifdefs and possibly more below.
Thanks, I will add this if there is a new version.
@@ -6915,7 +6926,6 @@ static int napi_irq_cpu_rmap_add(struct napi_struct
*napi, int irq)
if (rc)
goto err_set;
- set_bit(NAPI_STATE_HAS_NOTIFIER, &napi->state);
Minor nit: I think it would be better if the previous patch would add
directly this line in netif_napi_set_irq_locked() (avoding the removal
here).
yes, it just made more sense for that patch.
Re: [Intel-wired-lan] [PATCH iwl-next v3 02/14] ixgbe: add initial devlink support
On 2/12/25 16:47, Jagielski, Jedrzej wrote:
From: Jiri Pirko
Sent: Wednesday, February 12, 2025 4:09 PM
Wed, Feb 12, 2025 at 02:14:01PM +0100, jedrzej.jagiel...@intel.com wrote:
Add an initial support for devlink interface to ixgbe driver.
Similarly to i40e driver the implementation doesn't enable
devlink to manage device-wide configuration. Devlink instance
is created for each physical function of PCIe device.
Create separate directory for devlink related ixgbe files
and use naming scheme similar to the one used in the ice driver.
Add a stub for Documentation, to be extended by further patches.
Reviewed-by: Mateusz Polchlopek
Signed-off-by: Jedrzej Jagielski
+int ixgbe_allocate_devlink(struct ixgbe_adapter *adapter)
+{
+ struct ixgbe_devlink_priv *devlink_private;
+ struct device *dev = &adapter->pdev->dev;
+ struct devlink *devlink;
+
+ devlink = devlink_alloc(&ixgbe_devlink_ops,
+ sizeof(*devlink_private), dev);
+ if (!devlink)
+ return -ENOMEM;
+
+ devlink_private = devlink_priv(devlink);
+ devlink_private->adapter = adapter;
struct ixgbe_adapter * should be returned by devlink_priv(), that is the
idea, to let devlink allocate the driver private for you.
Using ixgbe_devlink_priv here is a workaround which i decided to introduce
to mitigate the fact that ixgbe_adapter is used to alloc netdev with
alloc_etherdev_mq().
This would require general ixgbe refactoring.
We will try to do that, pending a retest before for new submission ;)
Re: [Intel-wired-lan] [PATCH iwl-net] idpf: check error for register_netdev() on init
On 2/12/2025 10:21 AM, Simon Horman wrote:
On Mon, Feb 10, 2025 at 06:38:51PM -0800, Emil Tantilov wrote:
Current init logic ignores the error code from register_netdev(),
which will cause WARN_ON() on attempt to unregister it, if there was one,
and there is no info for the user that the creation of the netdev failed.
WARNING: CPU: 89 PID: 6902 at net/core/dev.c:11512
unregister_netdevice_many_notify+0x211/0x1a10
...
[ 3707.563641] unregister_netdev+0x1c/0x30
[ 3707.563656] idpf_vport_dealloc+0x5cf/0xce0 [idpf]
[ 3707.563684] idpf_deinit_task+0xef/0x160 [idpf]
[ 3707.563712] idpf_vc_core_deinit+0x84/0x320 [idpf]
[ 3707.563739] idpf_remove+0xbf/0x780 [idpf]
[ 3707.563769] pci_device_remove+0xab/0x1e0
[ 3707.563786] device_release_driver_internal+0x371/0x530
[ 3707.563803] driver_detach+0xbf/0x180
[ 3707.563816] bus_remove_driver+0x11b/0x2a0
[ 3707.563829] pci_unregister_driver+0x2a/0x250
Introduce an error check and log the vport number and error code.
On removal make sure to check VPORT_REG_NETDEV flag prior to calling
unregister and free on the netdev.
Add local variables for idx, vport_config and netdev for readability.
Fixes: 0fe45467a104 ("idpf: add create vport and netdev configuration")
Reviewed-by: Madhu Chittim
Suggested-by: Tony Nguyen
Signed-off-by: Emil Tantilov
---
drivers/net/ethernet/intel/idpf/idpf_lib.c | 27 ++
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c
b/drivers/net/ethernet/intel/idpf/idpf_lib.c
...
@@ -1536,12 +1540,17 @@ void idpf_init_task(struct work_struct *work)
}
for (index = 0; index < adapter->max_vports; index++) {
- if (adapter->netdevs[index] &&
- !test_bit(IDPF_VPORT_REG_NETDEV,
- adapter->vport_config[index]->flags)) {
- register_netdev(adapter->netdevs[index]);
- set_bit(IDPF_VPORT_REG_NETDEV,
- adapter->vport_config[index]->flags);
+ struct idpf_vport_config *vport_config =
adapter->vport_config[index];
+ struct net_device *netdev = adapter->netdevs[index];
+
+ if (netdev && !test_bit(IDPF_VPORT_REG_NETDEV,
vport_config->flags)) {
+ err = register_netdev(netdev);
+ if (err) {
+ dev_err(&pdev->dev, "failed to register netdev for
vport %d: %pe\n",
+ index, ERR_PTR(err));
+ continue;
+ }
+ set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags);
}
}
Hi Emil,
I'm wondering if we could reduce indentation and lines longer
than 80 characters in the above like this (completely untested!):
I was mostly trying to focus on the fix itself, since this patch is -net
bound. The >80 line came about from the introduction of the local netdev
and it seemed cleaner to keep it in one line. I can just split the check
as in the original code.
for (index = 0; index < adapter->max_vports; index++) {
struct idpf_vport_config *vport_config =
adapter->vport_config[index];
struct net_device *netdev = adapter->netdevs[index];
if (!netdev ||
test_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags))
continue;
Again, because its mainly to add the error checking I am not sure if its
OK to re-shuffle the logic.
err = register_netdev(netdev);
if (err) {
dev_err(&pdev->dev, "failed to register netdev for vport %d:
%pe\n",
index, ERR_PTR(err));
continue;
}
set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags);
}
Don't mind re-spinning (and testing) v2 with the proposed change, if
it's not infringing on the guidelines for submission to -net.
Thanks,
Emil
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption feature in IGC
On Thu, Feb 13, 2025 at 03:12:35PM +0100, Kurt Kanzenbach wrote:
> On Thu Feb 13 2025, Abdul Rahim, Faizal wrote:
> > On 13/2/2025 9:00 pm, Vladimir Oltean wrote:
> >> On Thu, Feb 13, 2025 at 08:54:18PM +0800, Abdul Rahim, Faizal wrote:
> Well, my idea was to move the current mqprio offload implementation from
> legacy TSN Tx mode to the normal TSN Tx mode. Then, taprio and mqprio
> can share the same code (with or without fpe). I have a draft patch
> ready for that. What do you think about it?
> >>>
> >>> Hi Kurt,
> >>>
> >>> I’m okay with including it in this series and testing fpe + mqprio, but
> >>> I’m
> >>> not sure if others might be concerned about adding different functional
> >>> changes in this fpe series.
> >>>
> >>> Hi Vladimir,
> >>> Any thoughts on this ?
> >>
> >> Well, what do you think of my split proposal from here, essentially
> >> drawing the line for the first patch set at just ethtool mm?
> >> https://lore.kernel.org/netdev/20250213110653.iqy5magn27jyfnwh@skbuf/
> >>
> >
> > Honestly, after reconsidering, I’d prefer to keep the current series as is
> > (without Kurt’s patch), assuming you’re okay with enabling mqprio + fpe
> > later rather than at the same time as taprio + fpe. There likely won’t be
> > any additional work needed for mqprio + fpe after Kurt’s patch is accepted,
> > since it will mostly reuse the taprio code flow.
>
> I think so. After switching the Tx mode mqprio will basically be a
> special case of taprio with a dummy Qbv schedule. Also the driver
> currently rejects mqprio with hardware offloading and preemptible_tcs
> set. So, I do not see any issues in merging your fpe series first. I can
> handle the mqprio part afterwards.
>
> Thanks,
> Kurt
Currently, igc sets tc_taprio_caps :: broken_mqprio = true, meaning that
higher scheduling priority is given to smaller TXQ indices. This is a
special case, as normally speaking, higher scheduler priority is given
to higher traffic classes, both in mqprio and in normal taprio (see
taprio_dequeue_txq_priority() vs taprio_dequeue_tc_priority()).
In commit 9f3297511dae ("igc: Add MQPRIO offload support") you document
the intended mqprio usage pattern:
tc qdisc replace dev ${INTERFACE} handle 100 parent root mqprio num_tc 4 \
map 0 0 0 0 0 1 2 3 0 0 0 0 0 0 0 0 \
queues 1@0 1@1 1@2 1@3 \
hw 1
Applying the transformations described in
https://man7.org/linux/man-pages/man8/tc-mqprio.8.html, it looks like this:
┌┬┬───┐
│Prio│ tc │ queue │
├┼┼───┤
│ 0 │ 0 │ 0 │
│ 1 │ 0 │ 0 │
│ 2 │ 0 │ 0 │
│ 3 │ 0 │ 0 │
│ 4 │ 0 │ 0 │
│ 5 │ 1 │ 1 │
│ 6 │ 2 │ 2 │
│ 7 │ 3 │ 3 │
│ 8 │ 0 │ 0 │
│ 9 │ 0 │ 0 │
│ 10 │ 0 │ 0 │
│ 11 │ 0 │ 0 │
│ 12 │ 0 │ 0 │
│ 13 │ 0 │ 0 │
│ 14 │ 0 │ 0 │
│ 15 │ 0 │ 0 │
└┴┴───┘
In this model, prio 7 goes to TXQ 3, and since I assume prio 7 is a high
priority, it makes me think TXQ 3 is the highest priority queue (I don't
have a lot of spare time to search for i216 documentation and enlighten
myself).
Then we have Faizal's example from patch 7/9:
https://lore.kernel.org/netdev/20250210070207.2615418-8-faizal.abdul.ra...@linux.intel.com/
a) 1:1 TC-to-Queue Mapping
$ sudo tc qdisc replace dev enp1s0 parent root handle 100 \
taprio num_tc 4 map 3 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3 \
queues 1@0 1@1 1@2 1@3 base-time 0 sched-entry S F 10 \
fp E E P P
b) Non-1:1 TC-to-Queue Mapping
$ sudo tc qdisc replace dev enp1s0 parent root handle 100 \
taprio num_tc 3 map 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 2
queues 2@0 1@2 1@3
fp E E P
┌┬┬───┐ ┌┬┬┐
│Prio│ tc │ queue │ │Prio│ tc │ queue │
├┼┼───┤ ├┼┼┤
│ 0 │ 3 │ 3 │ │ 0 │ 2 │ 3 │
│ 1 │ 2 │ 2 │ │ 1 │ 1 │ 2 │
│ 2 │ 1 │ 1 │ │ 2 │ 0 │ 0 or 1 │
│ 3 │ 0 │ 0 │ │ 3 │ 2 │ 3 │
│ 4 │ 3 │ 3 │ │ 4 │ 2 │ 3 │
│ 5 │ 3 │ 3 │ │ 5 │ 2 │ 3 │
│ 6 │ 3 │ 3 │ │ 6 │ 2 │ 3 │
│ 7 │ 3 │ 3 │ │ 7 │ 2 │ 3 │
│ 8 │ 3 │ 3 │ │ 8 │ 2 │ 3 │
│ 9 │ 3 │ 3 │ │ 9 │ 2 │ 3 │
│ 10 │ 3 │ 3 │ │ 10 │ 2 │ 3 │
│ 11 │ 3 │ 3 │ │ 11 │ 2 │ 3 │
│ 12 │ 3 │ 3 │ │ 12 │ 2 │ 3 │
│ 13 │ 3 │ 3 │ │ 13 │ 2 │ 3 │
│ 14 │ 3 │ 3 │ │ 14 │ 2 │ 3 │
│ 15 │ 3 │ 3 │ │ 15 │ 2 │ 3 │
└┴┴───┘ └┴┴┘
case a case b
In these cases, Faizal leaves us a hint that the preemptible traffic
classes are the ones with the lower scheduling priority (TC2 an
Re: [Intel-wired-lan] [iwl-next v9 5/9] ice, irdma: move interrupts code to irdma
On Thu, Feb 13, 2025 at 08:20:31PM +0100, Marcin Szycik wrote: > > > On 03.12.2024 07:58, Michal Swiatkowski wrote: > > Move responsibility of MSI-X requesting for RDMA feature from ice driver > > to irdma driver. It is done to allow simple fallback when there is not > > enough MSI-X available. > > > > Change amount of MSI-X used for control from 4 to 1, as it isn't needed > > to have more than one MSI-X for this purpose. > > Hi, I'm observing KASAN reports or kernel panic when attempting to remove > irdma > with this patchset, most probably this patch being the culprit, since it > touches > functions from splat. > > Reproducer: > sudo rmmod irdma > > Minified splat(s): > BUG: KASAN: use-after-free in irdma_remove+0x257/0x2d0 [irdma] > Call Trace: > >? __pfx__raw_spin_lock_irqsave+0x10/0x10 >? kfree+0x253/0x450 >? irdma_remove+0x257/0x2d0 [irdma] >kasan_report+0xed/0x120 >? irdma_remove+0x257/0x2d0 [irdma] >irdma_remove+0x257/0x2d0 [irdma] >auxiliary_bus_remove+0x56/0x80 >device_release_driver_internal+0x371/0x530 >? kernfs_put.part.0+0x147/0x310 >driver_detach+0xbf/0x180 >bus_remove_driver+0x11b/0x2a0 >auxiliary_driver_unregister+0x1a/0x50 >irdma_exit_module+0x40/0x4c [irdma] > > Oops: general protection fault, probably for non-canonical address > 0xdc00: [#1] PREEMPT SMP KASAN NOPTI > KASAN: null-ptr-deref in range [0x-0x0007] > RIP: 0010:ice_free_rdma_qvector+0x2a/0xa0 [ice] > Call Trace: >? ice_free_rdma_qvector+0x2a/0xa0 [ice] >irdma_remove+0x179/0x2d0 [irdma] >auxiliary_bus_remove+0x56/0x80 >device_release_driver_internal+0x371/0x530 >? kobject_put+0x61/0x4b0 >driver_detach+0xbf/0x180 >bus_remove_driver+0x11b/0x2a0 >auxiliary_driver_unregister+0x1a/0x50 >irdma_exit_module+0x40/0x4c [irdma] Thanks, I will work on it. >
[Intel-wired-lan] [PATCH iwl-net v2] iavf: fix circular lock dependency with netdev_lock
ail to register the netdevice, then we re-acquire
the adapter critical lock to finish the transition back to
__IAVF_INIT_CONFIG_ADAPTER.
This ensures every call where both netdev_lock and the adapter->crit_lock
are acquired under the same ordering.
Fixes: afc664987ab3 ("eth: iavf: extend the netdev_lock usage")
Signed-off-by: Jacob Keller
Tested-by: Przemek Kitszel
Reviewed-by: Przemek Kitszel
---
Changes in v2:
- Add Intel Wired LAN for patchwork integration
- Link to v1:
https://lore.kernel.org/r/20250213-jk-iavf-abba-lock-crash-v1-1-d4850f6a0...@intel.com
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
b/drivers/net/ethernet/intel/iavf/iavf_main.c
index
852e5b62f0a5dc038c0e5c0f76541870e69384ac..6faa62bced3a2ccb935219ba0726275c8ae60365
100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1983,7 +1983,7 @@ static int iavf_reinit_interrupt_scheme(struct
iavf_adapter *adapter, bool runni
static void iavf_finish_config(struct work_struct *work)
{
struct iavf_adapter *adapter;
- bool netdev_released = false;
+ bool locks_released = false;
int pairs, err;
adapter = container_of(work, struct iavf_adapter, finish_config);
@@ -2012,19 +2012,22 @@ static void iavf_finish_config(struct work_struct *work)
netif_set_real_num_tx_queues(adapter->netdev, pairs);
if (adapter->netdev->reg_state != NETREG_REGISTERED) {
+ mutex_unlock(&adapter->crit_lock);
netdev_unlock(adapter->netdev);
- netdev_released = true;
+ locks_released = true;
err = register_netdevice(adapter->netdev);
if (err) {
dev_err(&adapter->pdev->dev, "Unable to
register netdev (%d)\n",
err);
/* go back and try again.*/
+ mutex_lock(&adapter->crit_lock);
iavf_free_rss(adapter);
iavf_free_misc_irq(adapter);
iavf_reset_interrupt_capability(adapter);
iavf_change_state(adapter,
__IAVF_INIT_CONFIG_ADAPTER);
+ mutex_unlock(&adapter->crit_lock);
goto out;
}
}
@@ -2040,9 +2043,10 @@ static void iavf_finish_config(struct work_struct *work)
}
out:
- mutex_unlock(&adapter->crit_lock);
- if (!netdev_released)
+ if (!locks_released) {
+ mutex_unlock(&adapter->crit_lock);
netdev_unlock(adapter->netdev);
+ }
rtnl_unlock();
}
---
base-commit: 348f968b89bfeec0bb53dd82dba58b94d97fbd34
change-id: 20250213-jk-iavf-abba-lock-crash-300fbb33ae99
Best regards,
--
Jacob Keller
