Re: [Intel-wired-lan] [PATCH net-next 12/16] idpf: implement XDP_SETUP_PROG in ndo_bpf for splitq
From: Maciej Fijalkowski Date: Fri, 7 Mar 2025 15:16:48 +0100 > On Wed, Mar 05, 2025 at 05:21:28PM +0100, Alexander Lobakin wrote: >> From: Michal Kubiak >> >> Implement loading/removing XDP program using .ndo_bpf callback >> in the split queue mode. Reconfigure and restart the queues if needed >> (!!old_prog != !!new_prog), otherwise, just update the pointers. >> >> Signed-off-by: Michal Kubiak >> Signed-off-by: Alexander Lobakin >> --- >> drivers/net/ethernet/intel/idpf/idpf_txrx.h | 4 +- >> drivers/net/ethernet/intel/idpf/xdp.h | 7 ++ >> drivers/net/ethernet/intel/idpf/idpf_lib.c | 1 + >> drivers/net/ethernet/intel/idpf/idpf_txrx.c | 4 + >> drivers/net/ethernet/intel/idpf/xdp.c | 114 >> 5 files changed, 129 insertions(+), 1 deletion(-) >> > > (...) > >> + >> +/** >> + * idpf_xdp_setup_prog - handle XDP program install/remove requests >> + * @vport: vport to configure >> + * @xdp: request data (program, extack) >> + * >> + * Return: 0 on success, -errno on failure. >> + */ >> +static int >> +idpf_xdp_setup_prog(struct idpf_vport *vport, const struct netdev_bpf *xdp) >> +{ >> +const struct idpf_netdev_priv *np = netdev_priv(vport->netdev); >> +struct bpf_prog *old, *prog = xdp->prog; >> +struct idpf_vport_config *cfg; >> +int ret; >> + >> +cfg = vport->adapter->vport_config[vport->idx]; >> +if (!vport->num_xdp_txq && vport->num_txq == cfg->max_q.max_txq) { >> +NL_SET_ERR_MSG_MOD(xdp->extack, >> + "No Tx queues available for XDP, please >> decrease the number of regular SQs"); >> +return -ENOSPC; >> +} >> + >> +if (test_bit(IDPF_REMOVE_IN_PROG, vport->adapter->flags) || > > IN_PROG is a bit unfortunate here as it mixes with 'prog' :P Authentic idpf dictionary ¯\_(ツ)_/¯ > >> +!!vport->xdp_prog == !!prog) { >> +if (np->state == __IDPF_VPORT_UP) >> +idpf_copy_xdp_prog_to_qs(vport, prog); >> + >> +old = xchg(&vport->xdp_prog, prog); >> +if (old) >> +bpf_prog_put(old); >> + >> +cfg->user_config.xdp_prog = prog; >> + >> +return 0; >> +} >> + >> +old = cfg->user_config.xdp_prog; >> +cfg->user_config.xdp_prog = prog; >> + >> +ret = idpf_initiate_soft_reset(vport, IDPF_SR_Q_CHANGE); >> +if (ret) { >> +NL_SET_ERR_MSG_MOD(xdp->extack, >> + "Could not reopen the vport after XDP >> setup"); >> + >> +if (prog) >> +bpf_prog_put(prog); > > aren't you missing this for prog->NULL conversion? you have this for > hot-swap case (prog->prog). This path (soft_reset) handles NULL => prog and prog => NULL. This branch in particular handles errors during the soft reset, when we need to restore the original prog and put the new one. What you probably meant is that I don't have bpf_prog_put(old) in case everything went well below? Breh =\ > >> + >> +cfg->user_config.xdp_prog = old; >> +} >> + >> +return ret; >> +} Thanks, Olek
Re: [Intel-wired-lan] [iwl-net v3 3/5] ice: validate queue quanta parameters to prevent OOB access
> -Original Message- > From: Intel-wired-lan On Behalf Of > Martyna Szapar-Mudlaw > Sent: Tuesday, March 4, 2025 12:09 PM > To: intel-wired-...@lists.osuosl.org > Cc: net...@vger.kernel.org; Glaza, Jan ; Jagielski, > Jedrzej > ; Simon Horman ; Martyna > Szapar-Mudlaw > Subject: [Intel-wired-lan] [iwl-net v3 3/5] ice: validate queue quanta > parameters > to prevent OOB access > > From: Jan Glaza > > Add queue wraparound prevention in quanta configuration. > Ensure end_qid does not overflow by validating start_qid and num_queues. > > Fixes: 015307754a19 ("ice: Support VF queue rate limit and quanta size > configuration") > Reviewed-by: Jedrzej Jagielski > Reviewed-by: Simon Horman > Signed-off-by: Jan Glaza > Signed-off-by: Martyna Szapar-Mudlaw mud...@linux.intel.com> > --- > drivers/net/ethernet/intel/ice/ice_virtchnl.c | 13 + > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c > b/drivers/net/ethernet/intel/ice/ice_virtchnl.c > index 343f2b4b0dc5..adb1bf12542f 100644 > --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c > +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c > @@ -1903,13 +1903,21 @@ static int ice_vc_cfg_q_bw(struct ice_vf *vf, u8 Tested-by: Rafal Romanowski
Re: [Intel-wired-lan] [iwl-net v3 4/5] ice: fix input validation for virtchnl BW
> -Original Message- > From: Intel-wired-lan On Behalf Of > Martyna Szapar-Mudlaw > Sent: Tuesday, March 4, 2025 12:09 PM > To: intel-wired-...@lists.osuosl.org > Cc: net...@vger.kernel.org; Czapnik, Lukasz ; > Jagielski, Jedrzej ; Simon Horman > ; Martyna Szapar-Mudlaw mud...@linux.intel.com> > Subject: [Intel-wired-lan] [iwl-net v3 4/5] ice: fix input validation for > virtchnl BW > > From: Lukasz Czapnik > > Add missing validation of tc and queue id values sent by a VF in > ice_vc_cfg_q_bw(). > Additionally fixed logged value in the warning message, where max_tx_rate was > incorrectly referenced instead of min_tx_rate. > Also correct error handling in this function by properly exiting when invalid > configuration is detected. > > Fixes: 015307754a19 ("ice: Support VF queue rate limit and quanta size > configuration") > Reviewed-by: Jedrzej Jagielski > Reviewed-by: Simon Horman > Signed-off-by: Lukasz Czapnik > Co-developed-by: Martyna Szapar-Mudlaw mud...@linux.intel.com> > Signed-off-by: Martyna Szapar-Mudlaw mud...@linux.intel.com> > --- > drivers/net/ethernet/intel/ice/ice_virtchnl.c | 24 --- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c > b/drivers/net/ethernet/intel/ice/ice_virtchnl.c > index adb1bf12542f..824ef849b0ea 100644 > --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c > +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c > @@ -1865,15 +1865,33 @@ static int ice_vc_cfg_q_bw(struct ice_vf *vf, u8 Tested-by: Rafal Romanowski
Re: [Intel-wired-lan] [iwl-net v3 1/5] virtchnl: make proto and filter action count unsigned
> -Original Message- > From: Intel-wired-lan On Behalf Of > Szapar-Mudlaw, Martyna > Sent: Tuesday, March 4, 2025 2:12 PM > To: Paul Menzel ; Glaza, Jan > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Jagielski, > Jedrzej > ; Simon Horman ; Lobakin, > Aleksander > Subject: Re: [Intel-wired-lan] [iwl-net v3 1/5] virtchnl: make proto and > filter > action count unsigned > > > > On 3/4/2025 12:51 PM, Paul Menzel wrote: > > Dear Martyna, > > > > > > Thank you for your quick reply. > > > > Am 04.03.25 um 12:45 schrieb Szapar-Mudlaw, Martyna: > > > >> On 3/4/2025 12:15 PM, Paul Menzel wrote: > > > >>> Am 04.03.25 um 12:08 schrieb Martyna Szapar-Mudlaw: > From: Jan Glaza > > The count field in virtchnl_proto_hdrs and > virtchnl_filter_action_set should never be negative while still > being valid. Changing it from int to u32 ensures proper handling of > values in virtchnl messages in driverrs and prevents unintended behavior. > In its current signed form, a negative count does not trigger an > error in ice driver but instead results in it being treated as 0. > This can lead to unexpected outcomes when processing messages. > By using u32, any invalid values will correctly trigger -EINVAL, > making error detection more robust. > > Fixes: 1f7ea1cd6a374 ("ice: Enable FDIR Configure for AVF") > Reviewed-by: Jedrzej Jagielski > Reviewed-by: Simon Horman > Signed-off-by: Jan Glaza > Signed-off-by: Martyna Szapar-Mudlaw mud...@linux.intel.com> > --- > include/linux/avf/virtchnl.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/avf/virtchnl.h b/include/linux/avf/ > virtchnl.h index 4811b9a14604..cf0afa60e4a7 100644 > --- a/include/linux/avf/virtchnl.h > +++ b/include/linux/avf/virtchnl.h > @@ -1343,7 +1343,7 @@ struct virtchnl_proto_hdrs { Tested-by: Rafal Romanowski
Re: [Intel-wired-lan] [PATCH net-next 03/16] libeth: add a couple of XDP helpers (libeth_xdp)
From: Maciej Fijalkowski Date: Tue, 11 Mar 2025 15:05:38 +0100 > On Wed, Mar 05, 2025 at 05:21:19PM +0100, Alexander Lobakin wrote: >> "Couple" is a bit humbly... Add the following functionality to libeth: >> >> * XDP shared queues managing >> * XDP_TX bulk sending infra >> * .ndo_xdp_xmit() infra >> * adding buffers to &xdp_buff >> * running XDP prog and managing its verdict >> * completing XDP Tx buffers >> >> Suggested-by: Maciej Fijalkowski # lots of >> stuff >> Signed-off-by: Alexander Lobakin > > Patch is really big and I'm not sure how to trim this TBH to make my > comments bearable. I know this is highly optimized but it's rather hard to > follow with all of the callbacks, defines/aligns and whatnot. Any chance > to chop this commit a bit? Sometimes "highly optimized" code means "not really readable". See PeterZ's code :D I mean, I'm not able to write it to look more readable without hurting object code or not provoking code duplications. Maybe it's an art which I don't possess. I tried by best and left the documentation, even with pseudo-examples. Sorry if it doesn't help =\ > > Timers and locking logic could be pulled out to separate patches I think. > You don't ever say what improvement gave you the __LIBETH_WORD_ACCESS > approach. You've put a lot of thought onto this work and I feel like this I don't record/remember all of the perf changes. Couple percent for sure. Plus lighter object code. I can recall ~ -50-60 bytes in libeth_xdp_process_buff(), even though there's only 1 64-bit write replacing 2 32-bit writes. When there's a lot, like descriptor filling, it was 100+ bytes off, esp. when unrolling. > is not explained/described thoroughly. What would be nice to see is to > have this in the separate commit as well with a comment like 'this gave me > +X% performance boost on Y workload'. That would be probably a non-zero > effort to restructure it but generally while jumping back and forth Yeah it would be quite a big. I had a bit of hard time splitting it into 2 commits (XDP and XSk) from one, that request would cost a bunch more. Dunno if it would make sense at all? Defines, alignments etc, won't go away. Same for "head-scratching moments". Moreover, sometimes splitting the code borns more questions as it feels incomplete until the last patch and then there'll be a train of replies like "this will be added/changes in patch number X", which I don't like to do :s I mean, I would like to not sacrifice time splitting it only for the sake of split, depends on how critical this is and what it would give. > through this code I had a lot of head-scratching moments. > >> --- >> drivers/net/ethernet/intel/libeth/Kconfig | 10 +- >> drivers/net/ethernet/intel/libeth/Makefile |7 +- >> include/net/libeth/types.h | 106 +- >> drivers/net/ethernet/intel/libeth/priv.h | 26 + >> include/net/libeth/tx.h| 30 +- >> include/net/libeth/xdp.h | 1827 >> drivers/net/ethernet/intel/libeth/tx.c | 38 + >> drivers/net/ethernet/intel/libeth/xdp.c| 431 + >> 8 files changed, 2467 insertions(+), 8 deletions(-) >> create mode 100644 drivers/net/ethernet/intel/libeth/priv.h >> create mode 100644 include/net/libeth/xdp.h >> create mode 100644 drivers/net/ethernet/intel/libeth/tx.c >> create mode 100644 drivers/net/ethernet/intel/libeth/xdp.c Thanks, Olek
[Intel-wired-lan] [PATCH iwl-next v10 00/14] igc: Add support for Frame Preemption feature in IGC
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) Block setting preemptible tc in taprio since it is not supported yet. Existing code already blocks it in mqprio. Tested: Enabled CONFIG_PROVE_LOCKING, CONFIG_DEBUG_ATOMIC_SLEEP, CONFIG_DMA_API_DEBUG, and CONFIG_KASAN 1) selftests 2) netdev down/up cycles 3) suspend/resume cycles 4) fpe verification No bugs or unusual dmesg logs were observed. Ran 1), 2) and 3) with and without the patch series, compared dmesg and selftest logs — no differences found. Change Log: v10: - Add Vladimir Reviewed-by for patch 2, 3, 6, 8, 10-14 - Add Fu Rong Reviewed-by for patch 3 - Fix compilation error in intel lkp-test, snippet of error: "aarch64-linux-ld: drivers/net/ethernet/intel/igc/igc_main.o: in function `igc_clean_tx_irq': drivers/net/ethernet/intel/igc/igc_main.c:3157:(.text+0xec44): undefined reference to `ethtool_mmsv_event_handle" which randomize .config options (CONFIG_IGC=y, disable CONFIG_ETHTOOL_NETLINK). config -> https://download.01.org/0day-ci/archive/20250314/202503141646.6yybvfae-...@intel.com/config Fix at patch 2 and 10 by adding "depends on ETHTOOL_NETLINK" in stmmac and igc Kconfig. v8 -> v9: - Add Vladimir and Fu Rong Reviewed-by, patch 1 - Remove Faizal Co-developed-by for patch 2 - Remove code that reset verification state when link is down, patch 2 (Vladimir) - Refactor TX buffer and RX buffer magic constants with macro for each field. patch 5-9 (Vladimir) - s/Rest adapter/Adapter reset/ in commit description, patch 10 (Vladimir) - Replace igc_fpe_is_verify_or_response() and igc_fpe_lp_event_status() with igc_fpe_handle_mpacket(), patch 10 (Vladimir) - Comments on separate lines from code in igc_fpe_get_supported_frag_size(), patch 11 (Vladimir) - Remove mistakenly added igc_fpe_get_supported_frag_size(), patch 10 (Vladimir) - Remove max limit 256 of tx-min-size changes, patch 11 (Vladimir) v7 -> v8: - Reject SMD-V and SMD-R if the payload contains non-zero values (Vladimir, Chwee Lin) - Move resetting verification state when link is down to a new patch 3/11 (Vladimir) - Move frag_size related handling outside of spin_lock_irq_save(), patch 1/11 (Vladimir) - Renamed IGC_PRMEXPRCNT to IGC_PRMEXCPRCNT, to align with i226 SW User Manual, patch 11/11 (Chwee Lin) - Use IGC_PRMEXPRCNT_OOO_SMDC for frame smd errors instead of frame assembly errors, 11/11 (Chwee Lin) v6 -> v7: - Squash the cpu param to the prev line (Przemek Kitszel) - Use igc_ prefix for fpe_t (Przemek Kitszel) - Move new ops to different line in igc_ethtool_ops (Przemek Kitszel) - Documentation for igc_enable_empty_addr_recv (): rx -> Rx (Przemek Kitszel) - Documentation for igc_enable_empty_addr_recv (): s/IGC/the driver/ (Przemek Kitszel) - Change preferred style of init, from { }, to {} (Przemek Kitszel) - Remove inclusion of umbrella header in igc_tsn.c (Przemek Kitszel) - End enum with "," in igc_txd_popts_type (Przemek Kitszel) - Remove unnecessary braces in igc_fpe_is_verify_or_response() (Przemek Kitszel) v5 -> v6: - Added Tested-by: Furong Xu for patch 1/9 (Vladimir, Furong Xu) - Updated logic in ethtool_mmsv_link_state_handle() (Vladimir, Furong Xu) - Swap sequence of function call in stmmac_set_mm() (Furong Xu) - Log an error if igc_enable_empty_addr_recv() fails (Vladimir) - Move the patch ".. Block setting preemptible traffic .." before ".. Add support to get MAC Merge data .." (Vladimir) - Move mmsv function kernel-doc from .h to .c file (Vladimir) v4 -> v5: - Remove "igc: Add support for preemptible traffic class in taprio" patch (Vladimir) - Add a new patch "igc: Block setting preemptible traffic classes in taprio" (Vladimir) - Add kernel-doc for mmsv api (Vladimir) - olininfo_status to use host byte order (Simon) - status_error should host byte type (Simon) - Some code was misplaced in the wrong patch (Vladimir) - Mix of tabs and spaces in patch description (Vladimir) - Created igc_is_pmac_enabled() to reduce code repetition (Vladimir) v3 -> v4: - Fix compilation warnings introduced by this patch series v2 -> v3: - Implement configure_tx() mmsv callback (Vladimir) - Use static_branch_inc() and static_branch_dec() (Vladimir) - Add adapter->fpe.mmsv.pmac_enabled as extra check (Vladimir) - Remove unnecessary error check in igc_fpe_init_tx_descriptor() (Vladimir) - Additional places to use FIELD_PREP() instead
[Intel-wired-lan] [PATCH iwl-next v10 04/14] igc: rename xdp_get_tx_ring() for non-xdp usage
Renamed xdp_get_tx_ring() function to a more generic name for use in upcoming frame preemption patches. Signed-off-by: Faizal Rahim --- drivers/net/ethernet/intel/igc/igc.h | 2 +- drivers/net/ethernet/intel/igc/igc_main.c | 9 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h index cd1d7b6c1782..ba7c55d2dc85 100644 --- a/drivers/net/ethernet/intel/igc/igc.h +++ b/drivers/net/ethernet/intel/igc/igc.h @@ -737,7 +737,7 @@ struct igc_nfc_rule *igc_get_nfc_rule(struct igc_adapter *adapter, u32 location); int igc_add_nfc_rule(struct igc_adapter *adapter, struct igc_nfc_rule *rule); void igc_del_nfc_rule(struct igc_adapter *adapter, struct igc_nfc_rule *rule); - +struct igc_ring *igc_get_tx_ring(struct igc_adapter *adapter, int cpu); void igc_ptp_init(struct igc_adapter *adapter); void igc_ptp_reset(struct igc_adapter *adapter); void igc_ptp_suspend(struct igc_adapter *adapter); diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index 472f009630c9..99123eef610b 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -2464,8 +2464,7 @@ static int igc_xdp_init_tx_descriptor(struct igc_ring *ring, return -ENOMEM; } -static struct igc_ring *igc_xdp_get_tx_ring(struct igc_adapter *adapter, - int cpu) +struct igc_ring *igc_get_tx_ring(struct igc_adapter *adapter, int cpu) { int index = cpu; @@ -2489,7 +2488,7 @@ static int igc_xdp_xmit_back(struct igc_adapter *adapter, struct xdp_buff *xdp) if (unlikely(!xdpf)) return -EFAULT; - ring = igc_xdp_get_tx_ring(adapter, cpu); + ring = igc_get_tx_ring(adapter, cpu); nq = txring_txq(ring); __netif_tx_lock(nq, cpu); @@ -2566,7 +2565,7 @@ static void igc_finalize_xdp(struct igc_adapter *adapter, int status) struct igc_ring *ring; if (status & IGC_XDP_TX) { - ring = igc_xdp_get_tx_ring(adapter, cpu); + ring = igc_get_tx_ring(adapter, cpu); nq = txring_txq(ring); __netif_tx_lock(nq, cpu); @@ -6779,7 +6778,7 @@ static int igc_xdp_xmit(struct net_device *dev, int num_frames, if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) return -EINVAL; - ring = igc_xdp_get_tx_ring(adapter, cpu); + ring = igc_get_tx_ring(adapter, cpu); nq = txring_txq(ring); __netif_tx_lock(nq, cpu); -- 2.34.1
[Intel-wired-lan] [PATCH iwl-net v2] idpf: fix adapter NULL pointer dereference on reboot
With SRIOV enabled, idpf ends up calling into idpf_remove() twice. First via idpf_shutdown() and then again when idpf_remove() calls into sriov_disable(), because the VF devices use the idpf driver, hence the same remove routine. When that happens, it is possible for the adapter to be NULL from the first call to idpf_remove(), leading to a NULL pointer dereference. echo 1 > /sys/class/net//device/sriov_numvfs reboot BUG: kernel NULL pointer dereference, address: 0020 ... RIP: 0010:idpf_remove+0x22/0x1f0 [idpf] ... ? idpf_remove+0x22/0x1f0 [idpf] ? idpf_remove+0x1e4/0x1f0 [idpf] pci_device_remove+0x3f/0xb0 device_release_driver_internal+0x19f/0x200 pci_stop_bus_device+0x6d/0x90 pci_stop_and_remove_bus_device+0x12/0x20 pci_iov_remove_virtfn+0xbe/0x120 sriov_disable+0x34/0xe0 idpf_sriov_configure+0x58/0x140 [idpf] idpf_remove+0x1b9/0x1f0 [idpf] idpf_shutdown+0x12/0x30 [idpf] pci_device_shutdown+0x35/0x60 device_shutdown+0x156/0x200 ... Replace the direct idpf_remove() call in idpf_shutdown() with idpf_vc_core_deinit() and idpf_deinit_dflt_mbx(), which perform the bulk of the cleanup, such as stopping the init task, freeing IRQs, destroying the vports and freeing the mailbox. This avoids the calls to sriov_disable() in addition to a small netdev cleanup, and destroying workqueues, which don't seem to be required on shutdown. Reported-by: Yuying Ma Fixes: e850efed5e15 ("idpf: add module register and probe functionality") Reviewed-by: Madhu Chittim Signed-off-by: Emil Tantilov --- Changelog: v2: - Updated the description to clarify the path leading up to the crash, and the difference in the logic between remove and shutdown as result of this change. v1: https://lore.kernel.org/intel-wired-lan/20250307003956.22018-1-emil.s.tanti...@intel.com/ --- drivers/net/ethernet/intel/idpf/idpf_main.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c index b6c515d14cbf..bec4a02c5373 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_main.c +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c @@ -87,7 +87,11 @@ static void idpf_remove(struct pci_dev *pdev) */ static void idpf_shutdown(struct pci_dev *pdev) { - idpf_remove(pdev); + struct idpf_adapter *adapter = pci_get_drvdata(pdev); + + cancel_delayed_work_sync(&adapter->vc_event_task); + idpf_vc_core_deinit(adapter); + idpf_deinit_dflt_mbx(adapter); if (system_state == SYSTEM_POWER_OFF) pci_set_power_state(pdev, PCI_D3hot); -- 2.17.2
[Intel-wired-lan] [PATCH iwl-next v10 12/14] igc: block setting preemptible traffic class in taprio
Since preemptible tc implementation is not ready yet, block it from being set in taprio. The existing code already blocks it in mqprio. Reviewed-by: Vladimir Oltean Signed-off-by: Faizal Rahim --- drivers/net/ethernet/intel/igc/igc_main.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index 71c377cb7a88..fc945c5b7c70 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -6486,6 +6486,10 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter, if (!validate_schedule(adapter, qopt)) return -EINVAL; + /* preemptible isn't supported yet */ + if (qopt->mqprio.preemptible_tcs) + return -EOPNOTSUPP; + igc_ptp_read(adapter, &now); if (igc_tsn_is_taprio_activated_by_user(adapter) && -- 2.34.1
[Intel-wired-lan] [PATCH iwl-next v10 05/14] igc: rename I225_RXPBSIZE_DEFAULT and I225_TXPBSIZE_DEFAULT
Rename RX and TX packet buffer size macros in preparation for an upcoming patch that will refactor buffer size handling using FIELD_PREP and GENMASK. Changes: - Rename I225_RXPBSIZE_DEFAULT to IGC_RXPBSIZE_EXP_BMC_DEFAULT. The EXP_BMC suffix explicitly indicates Express and BMC buffer default values, improving readability and reusability for the upcoming changes, while also better reflecting the current buffer allocations. - Rename I225_TXPBSIZE_DEFAULT to IGC_TXPBSIZE_DEFAULT. These registers apply to both i225 and i226, so using the IGC prefix aligns with existing macro naming conventions. Signed-off-by: Faizal Rahim --- drivers/net/ethernet/intel/igc/igc_defines.h | 7 --- drivers/net/ethernet/intel/igc/igc_main.c| 4 ++-- drivers/net/ethernet/intel/igc/igc_tsn.c | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h index 8e449904aa7d..b6744ece64f0 100644 --- a/drivers/net/ethernet/intel/igc/igc_defines.h +++ b/drivers/net/ethernet/intel/igc/igc_defines.h @@ -396,9 +396,10 @@ #define IGC_RCTL_PMCF 0x0080 /* pass MAC control frames */ #define IGC_RCTL_SECRC 0x0400 /* Strip Ethernet CRC */ -#define I225_RXPBSIZE_DEFAULT 0x00A2 /* RXPBSIZE default */ -#define I225_TXPBSIZE_DEFAULT 0x0414 /* TXPBSIZE default */ -#define IGC_RXPBS_CFG_TS_EN0x8000 /* Timestamp in Rx buffer */ +/* RXPBSIZE default value for Express and BMC buffer */ +#define IGC_RXPBSIZE_EXP_BMC_DEFAULT 0x00A2 +#define IGC_TXPBSIZE_DEFAULT 0x0414 /* TXPBSIZE default */ +#define IGC_RXPBS_CFG_TS_EN0x8000 /* Timestamp in Rx buffer */ #define IGC_TXPBSIZE_TSN 0x04145145 /* 5k bytes buffer for each queue */ diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index 99123eef610b..6f0110e3ac22 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -7156,8 +7156,8 @@ static int igc_probe(struct pci_dev *pdev, } /* configure RXPBSIZE and TXPBSIZE */ - wr32(IGC_RXPBS, I225_RXPBSIZE_DEFAULT); - wr32(IGC_TXPBS, I225_TXPBSIZE_DEFAULT); + wr32(IGC_RXPBS, IGC_RXPBSIZE_EXP_BMC_DEFAULT); + wr32(IGC_TXPBS, IGC_TXPBSIZE_DEFAULT); timer_setup(&adapter->watchdog_timer, igc_watchdog, 0); timer_setup(&adapter->phy_info_timer, igc_update_phy_info, 0); diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c index 1e44374ca1ff..498741d83ca6 100644 --- a/drivers/net/ethernet/intel/igc/igc_tsn.c +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c @@ -136,7 +136,7 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter) int i; wr32(IGC_GTXOFFSET, 0); - wr32(IGC_TXPBS, I225_TXPBSIZE_DEFAULT); + wr32(IGC_TXPBS, IGC_TXPBSIZE_DEFAULT); wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_DEFAULT); if (igc_is_device_id_i226(hw)) -- 2.34.1
[Intel-wired-lan] [PATCH iwl-next v10 08/14] igc: use FIELD_PREP and GENMASK for existing RX packet buffer size
Prepare for an upcoming patch that modifies the RX buffer size in TSN mode. Refactor IGC_RXPBSIZE_EXP_BMC_DEFAULT and IGC_RXPBS_CFG_TS_EN using FIELD_PREP and GENMASK to improve clarity and maintainability. Refactor both macros for consistency, even though the upcoming patch only use IGC_RXPBSIZE_EXP_BMC_DEFAULT. The newly introduced macros follow the naming from the i226 SW User Manual for easy reference. I've tested IGC_RXPBSIZE_EXP_BMC_DEFAULT and IGC_RXPBS_CFG_TS_EN before and after the refactoring, and their values remain unchanged. Reviewed-by: Vladimir Oltean Signed-off-by: Faizal Rahim --- drivers/net/ethernet/intel/igc/igc_defines.h | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h index db937931c646..3564d15df57b 100644 --- a/drivers/net/ethernet/intel/igc/igc_defines.h +++ b/drivers/net/ethernet/intel/igc/igc_defines.h @@ -396,9 +396,20 @@ #define IGC_RCTL_PMCF 0x0080 /* pass MAC control frames */ #define IGC_RCTL_SECRC 0x0400 /* Strip Ethernet CRC */ -/* RXPBSIZE default value for Express and BMC buffer */ -#define IGC_RXPBSIZE_EXP_BMC_DEFAULT 0x00A2 -#define IGC_RXPBS_CFG_TS_EN0x8000 /* Timestamp in Rx buffer */ +/* Mask for RX packet buffer size */ +#define IGC_RXPBSIZE_EXP_MASK GENMASK(5, 0) +#define IGC_BMC2OSPBSIZE_MASK GENMASK(11, 6) +/* Mask for timestamp in RX buffer */ +#define IGC_RXPBS_CFG_TS_EN_MASK GENMASK(31, 31) +/* High-priority RX packet buffer size (KB). Used for Express traffic when preemption is enabled */ +#define IGC_RXPBSIZE_EXP(x)FIELD_PREP(IGC_RXPBSIZE_EXP_MASK, (x)) +/* BMC to OS packet buffer size in KB */ +#define IGC_BMC2OSPBSIZE(x)FIELD_PREP(IGC_BMC2OSPBSIZE_MASK, (x)) +/* Enable RX packet buffer for timestamp descriptor, saving 16 bytes per packet if set */ +#define IGC_RXPBS_CFG_TS_ENFIELD_PREP(IGC_RXPBS_CFG_TS_EN_MASK, 1) +/* Default value following I225/I226 SW User Manual Section 8.3.1 */ +#define IGC_RXPBSIZE_EXP_BMC_DEFAULT ( \ + IGC_RXPBSIZE_EXP(34) | IGC_BMC2OSPBSIZE(2)) /* Mask for TX packet buffer size */ #define IGC_TXPB0SIZE_MASK GENMASK(5, 0) -- 2.34.1
[Intel-wired-lan] [PATCH iwl-next v10 11/14] igc: add support to set tx-min-frag-size
Add support for setting tx-min-frag-size via the set_mm callback in igc. If the requested value is unsupported, round it up to the smallest supported i226 size (64, 128, 192, 256) and send a netlink message to inform the user. Co-developed-by: Vinicius Costa Gomes Signed-off-by: Vinicius Costa Gomes Reviewed-by: Vladimir Oltean Signed-off-by: Faizal Rahim --- drivers/net/ethernet/intel/igc/igc.h | 1 + drivers/net/ethernet/intel/igc/igc_defines.h | 1 + drivers/net/ethernet/intel/igc/igc_ethtool.c | 5 +++ drivers/net/ethernet/intel/igc/igc_tsn.c | 38 ++-- drivers/net/ethernet/intel/igc/igc_tsn.h | 1 + 5 files changed, 43 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h index d970968f3233..d61e2915ccb4 100644 --- a/drivers/net/ethernet/intel/igc/igc.h +++ b/drivers/net/ethernet/intel/igc/igc.h @@ -42,6 +42,7 @@ void igc_ethtool_set_ops(struct net_device *); struct igc_fpe_t { struct ethtool_mmsv mmsv; + u32 tx_min_frag_size; }; enum igc_mac_filter_type { diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h index 542c7c8802a0..80e0b5ed8b1a 100644 --- a/drivers/net/ethernet/intel/igc/igc_defines.h +++ b/drivers/net/ethernet/intel/igc/igc_defines.h @@ -583,6 +583,7 @@ #define IGC_TQAVCTRL_PREEMPT_ENA 0x0002 #define IGC_TQAVCTRL_ENHANCED_QAV 0x0008 #define IGC_TQAVCTRL_FUTSCDDIS 0x0080 +#define IGC_TQAVCTRL_MIN_FRAG_MASK 0xC000 #define IGC_TXQCTL_QUEUE_MODE_LAUNCHT 0x0001 #define IGC_TXQCTL_STRICT_CYCLE0x0002 diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c index b64d5c6c1d20..529654ccd83f 100644 --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c @@ -1789,6 +1789,11 @@ static int igc_ethtool_set_mm(struct net_device *netdev, struct igc_adapter *adapter = netdev_priv(netdev); struct igc_fpe_t *fpe = &adapter->fpe; + fpe->tx_min_frag_size = igc_fpe_get_supported_frag_size(cmd->tx_min_frag_size); + if (fpe->tx_min_frag_size != cmd->tx_min_frag_size) + NL_SET_ERR_MSG_MOD(extack, + "tx-min-frag-size value set is unsupported. Rounded up to supported value (64, 128, 192, 256)"); + if (fpe->mmsv.pmac_enabled != cmd->pmac_enabled) { if (cmd->pmac_enabled) static_branch_inc(&igc_fpe_enabled); diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c index 8a1725688381..215bf8e38328 100644 --- a/drivers/net/ethernet/intel/igc/igc_tsn.c +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c @@ -6,6 +6,12 @@ #include "igc_hw.h" #include "igc_tsn.h" +#define MIN_MULTPLIER_TX_MIN_FRAG 0 +#define MAX_MULTPLIER_TX_MIN_FRAG 3 +/* Frag size is based on the Section 8.12.2 of the SW User Manual */ +#define TX_MIN_FRAG_SIZE 64 +#define TX_MAX_FRAG_SIZE (TX_MIN_FRAG_SIZE * (MAX_MULTPLIER_TX_MIN_FRAG + 1)) + DEFINE_STATIC_KEY_FALSE(igc_fpe_enabled); static int igc_fpe_init_smd_frame(struct igc_ring *ring, @@ -128,6 +134,7 @@ static const struct ethtool_mmsv_ops igc_mmsv_ops = { void igc_fpe_init(struct igc_adapter *adapter) { + adapter->fpe.tx_min_frag_size = TX_MIN_FRAG_SIZE; ethtool_mmsv_init(&adapter->fpe.mmsv, adapter->netdev, &igc_mmsv_ops); } @@ -297,7 +304,7 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter) tqavctrl = rd32(IGC_TQAVCTRL); tqavctrl &= ~(IGC_TQAVCTRL_TRANSMIT_MODE_TSN | IGC_TQAVCTRL_ENHANCED_QAV | IGC_TQAVCTRL_FUTSCDDIS | - IGC_TQAVCTRL_PREEMPT_ENA); + IGC_TQAVCTRL_PREEMPT_ENA | IGC_TQAVCTRL_MIN_FRAG_MASK); wr32(IGC_TQAVCTRL, tqavctrl); @@ -343,12 +350,35 @@ static void igc_tsn_set_retx_qbvfullthreshold(struct igc_adapter *adapter) wr32(IGC_RETX_CTL, retxctl); } +static u8 igc_fpe_get_frag_size_mult(const struct igc_fpe_t *fpe) +{ + u8 mult = (fpe->tx_min_frag_size / TX_MIN_FRAG_SIZE) - 1; + + return clamp_t(u8, mult, MIN_MULTPLIER_TX_MIN_FRAG, + MAX_MULTPLIER_TX_MIN_FRAG); +} + +u32 igc_fpe_get_supported_frag_size(u32 frag_size) +{ + const u32 supported_sizes[] = {64, 128, 192, 256}; + + /* Find the smallest supported size that is >= frag_size */ + for (int i = 0; i < ARRAY_SIZE(supported_sizes); i++) { + if (frag_size <= supported_sizes[i]) + return supported_sizes[i]; + } + + /* Should not happen */ + return TX_MAX_FRAG_SIZE; +} + static int igc_tsn_enable_offload(struct igc_adapter *adapter) { struct igc_hw *hw = &adapter->hw; u32 tqavctrl, baset_l, baset_h; u32 sec, nsec, cycle;
[Intel-wired-lan] [PATCH iwl-next v10 14/14] igc: add support to get frame preemption statistics via ethtool
Implemented "ethtool --include-statistics --show-mm" callback for IGC. Tested preemption scenario to check preemption statistics: 1) Trigger verification handshake on both boards: $ sudo ethtool --set-mm enp1s0 pmac-enabled on $ sudo ethtool --set-mm enp1s0 tx-enabled on $ sudo ethtool --set-mm enp1s0 verify-enabled on 2) Set preemptible or express queue in taprio for tx board: $ 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 3) Send large size packets on preemptible queue 4) Send small size packets on express queue to preempt packets in preemptible queue 5) Show preemption statistics on the receiving board: $ ethtool --include-statistics --show-mm enp1s0 MAC Merge layer state for enp1s0: pMAC enabled: on TX enabled: on TX active: on TX minimum fragment size: 64 RX minimum fragment size: 60 Verify enabled: on Verify time: 128 Max verify time: 128 Verification status: SUCCEEDED Statistics: MACMergeFrameAssErrorCount: 0 MACMergeFrameSmdErrorCount: 0 MACMergeFrameAssOkCount: 511 MACMergeFragCountRx: 764 MACMergeFragCountTx: 0 MACMergeHoldCount: 0 Co-developed-by: Vinicius Costa Gomes Signed-off-by: Vinicius Costa Gomes Co-developed-by: Chwee-Lin Choong Signed-off-by: Chwee-Lin Choong Reviewed-by: Vladimir Oltean Signed-off-by: Faizal Rahim --- drivers/net/ethernet/intel/igc/igc_ethtool.c | 40 drivers/net/ethernet/intel/igc/igc_regs.h| 16 2 files changed, 56 insertions(+) diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c index fd4b4b332309..324a27a5bef9 100644 --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c @@ -1819,6 +1819,45 @@ static int igc_ethtool_set_mm(struct net_device *netdev, return igc_tsn_offload_apply(adapter); } +/** + * igc_ethtool_get_frame_ass_error - Get the frame assembly error count. + * @reg_value: Register value for IGC_PRMEXCPRCNT + * Return: The count of frame assembly errors. + */ +static u64 igc_ethtool_get_frame_ass_error(u32 reg_value) +{ + /* Out of order statistics */ + u32 ooo_frame_cnt, ooo_frag_cnt; + u32 miss_frame_frag_cnt; + + ooo_frame_cnt = FIELD_GET(IGC_PRMEXCPRCNT_OOO_FRAME_CNT, reg_value); + ooo_frag_cnt = FIELD_GET(IGC_PRMEXCPRCNT_OOO_FRAG_CNT, reg_value); + miss_frame_frag_cnt = FIELD_GET(IGC_PRMEXCPRCNT_MISS_FRAME_FRAG_CNT, reg_value); + + return ooo_frame_cnt + ooo_frag_cnt + miss_frame_frag_cnt; +} + +static u64 igc_ethtool_get_frame_smd_error(u32 reg_value) +{ + return FIELD_GET(IGC_PRMEXCPRCNT_OOO_SMDC, reg_value); +} + +static void igc_ethtool_get_mm_stats(struct net_device *dev, +struct ethtool_mm_stats *stats) +{ + struct igc_adapter *adapter = netdev_priv(dev); + struct igc_hw *hw = &adapter->hw; + u32 reg_value; + + reg_value = rd32(IGC_PRMEXCPRCNT); + + stats->MACMergeFrameAssErrorCount = igc_ethtool_get_frame_ass_error(reg_value); + stats->MACMergeFrameSmdErrorCount = igc_ethtool_get_frame_smd_error(reg_value); + stats->MACMergeFrameAssOkCount = rd32(IGC_PRMPTDRCNT); + stats->MACMergeFragCountRx = rd32(IGC_PRMEVNTRCNT); + stats->MACMergeFragCountTx = rd32(IGC_PRMEVNTTCNT); +} + static int igc_ethtool_get_link_ksettings(struct net_device *netdev, struct ethtool_link_ksettings *cmd) { @@ -2115,6 +2154,7 @@ static const struct ethtool_ops igc_ethtool_ops = { .set_link_ksettings = igc_ethtool_set_link_ksettings, .self_test = igc_ethtool_diag_test, .get_mm = igc_ethtool_get_mm, + .get_mm_stats = igc_ethtool_get_mm_stats, .set_mm = igc_ethtool_set_mm, }; diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h index 12ddc5793651..f343c6bfc6be 100644 --- a/drivers/net/ethernet/intel/igc/igc_regs.h +++ b/drivers/net/ethernet/intel/igc/igc_regs.h @@ -222,6 +222,22 @@ #define IGC_FTQF(_n) (0x059E0 + (4 * (_n))) /* 5-tuple Queue Fltr */ +/* Time sync registers - preemption statistics */ +#define IGC_PRMPTDRCNT 0x04284 /* Good RX Preempted Packets */ +#define IGC_PRMEVNTTCNT0x04298 /* TX Preemption event counter */ +#define IGC_PRMEVNTRCNT0x0429C /* RX Preemption event counter */ + + /* Preemption Exception Counter */ + #define IGC_PRMEXCPRCNT 0x42A0 +/* Received out of order packets with SMD-C */ +#define IGC_PRMEXCPRCNT_OOO_SMDC 0x00FF +/* Received out of order packets with SMD-C and wrong Frame CNT */ +#define IGC
[Intel-wired-lan] [PATCH iwl-next v10 01/14] net: stmmac: move frag_size handling out of spin_lock
The upcoming patch will extract verification logic into a new module, MMSV (MAC Merge Software Verification). MMSV will handle most FPE fields, except frag_size. It introduces its own lock (mmsv->lock), replacing fpe_cfg->lock. Since frag_size handling remains in the driver, the existing rtnl_lock() is sufficient. Move frag_size handling out of spin_lock_irq_save() to keep the upcoming patch a pure refactoring without behavior changes. Signed-off-by: Faizal Rahim Reviewed-by: Vladimir Oltean Reviewed-by: Furong Xu <0x1...@gmail.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index 918a32f8fda8..cfe5aea24549 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -1216,6 +1216,10 @@ static int stmmac_get_mm(struct net_device *ndev, if (!stmmac_fpe_supported(priv)) return -EOPNOTSUPP; + state->rx_min_frag_size = ETH_ZLEN; + frag_size = stmmac_fpe_get_add_frag_size(priv); + state->tx_min_frag_size = ethtool_mm_frag_size_add_to_min(frag_size); + spin_lock_irqsave(&priv->fpe_cfg.lock, flags); state->max_verify_time = STMMAC_FPE_MM_MAX_VERIFY_TIME_MS; @@ -1224,7 +1228,6 @@ static int stmmac_get_mm(struct net_device *ndev, state->verify_time = priv->fpe_cfg.verify_time; state->tx_enabled = priv->fpe_cfg.tx_enabled; state->verify_status = priv->fpe_cfg.status; - state->rx_min_frag_size = ETH_ZLEN; /* FPE active if common tx_enabled and * (verification success or disabled(forced)) @@ -1236,9 +1239,6 @@ static int stmmac_get_mm(struct net_device *ndev, else state->tx_active = false; - frag_size = stmmac_fpe_get_add_frag_size(priv); - state->tx_min_frag_size = ethtool_mm_frag_size_add_to_min(frag_size); - spin_unlock_irqrestore(&priv->fpe_cfg.lock, flags); return 0; @@ -1258,6 +1258,8 @@ static int stmmac_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg, if (err) return err; + stmmac_fpe_set_add_frag_size(priv, frag_size); + /* Wait for the verification that's currently in progress to finish */ timer_shutdown_sync(&fpe_cfg->verify_timer); @@ -1271,7 +1273,6 @@ static int stmmac_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg, if (!cfg->verify_enabled) fpe_cfg->status = ETHTOOL_MM_VERIFY_STATUS_DISABLED; - stmmac_fpe_set_add_frag_size(priv, frag_size); stmmac_fpe_apply(priv); spin_unlock_irqrestore(&fpe_cfg->lock, flags); -- 2.34.1
[Intel-wired-lan] [PATCH iwl-next v10 03/14] net: ethtool: mm: reset verification status when link is down
When the link partner goes down, "ethtool --show-mm" still displays "Verification status: SUCCEEDED," reflecting a previous state that is no longer valid. Reset the verification status to ensure it reflects the current state. Reviewed-by: Furong Xu <0x1...@gmail.com> Reviewed-by: Vladimir Oltean Signed-off-by: Faizal Rahim --- net/ethtool/mm.c | 4 1 file changed, 4 insertions(+) diff --git a/net/ethtool/mm.c b/net/ethtool/mm.c index bfd988464d7d..ad9b40034003 100644 --- a/net/ethtool/mm.c +++ b/net/ethtool/mm.c @@ -415,6 +415,10 @@ void ethtool_mmsv_link_state_handle(struct ethtool_mmsv *mmsv, bool up) /* New link => maybe new partner => new verification process */ ethtool_mmsv_apply(mmsv); } else { + /* Reset the reported verification state while the link is down */ + if (mmsv->verify_enabled) + mmsv->status = ETHTOOL_MM_VERIFY_STATUS_INITIAL; + /* No link or pMAC not enabled */ ethtool_mmsv_configure_pmac(mmsv, false); ethtool_mmsv_configure_tx(mmsv, false); -- 2.34.1
[Intel-wired-lan] [PATCH iwl-next v10 06/14] igc: use FIELD_PREP and GENMASK for existing TX packet buffer size
In preparation for an upcoming patch that will modify the TX buffer size in TSN mode, replace IGC_TXPBSIZE_TSN and IGC_TXPBSIZE_DEFAULT implementation with new macros that utilizes FIELD_PREP and GENMASK for clarity. The newly introduced macros follow the naming from the i226 SW User Manual for easy reference. I've tested IGC_TXPBSIZE_TSN and IGC_TXPBSIZE_DEFAULT before and after the refactoring, and their values remain unchanged. Reviewed-by: Vladimir Oltean Signed-off-by: Faizal Rahim --- drivers/net/ethernet/intel/igc/igc_defines.h | 23 ++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h index b6744ece64f0..b180e1497cc5 100644 --- a/drivers/net/ethernet/intel/igc/igc_defines.h +++ b/drivers/net/ethernet/intel/igc/igc_defines.h @@ -398,10 +398,29 @@ /* RXPBSIZE default value for Express and BMC buffer */ #define IGC_RXPBSIZE_EXP_BMC_DEFAULT 0x00A2 -#define IGC_TXPBSIZE_DEFAULT 0x0414 /* TXPBSIZE default */ #define IGC_RXPBS_CFG_TS_EN0x8000 /* Timestamp in Rx buffer */ -#define IGC_TXPBSIZE_TSN 0x04145145 /* 5k bytes buffer for each queue */ +/* Mask for TX packet buffer size */ +#define IGC_TXPB0SIZE_MASK GENMASK(5, 0) +#define IGC_TXPB1SIZE_MASK GENMASK(11, 6) +#define IGC_TXPB2SIZE_MASK GENMASK(17, 12) +#define IGC_TXPB3SIZE_MASK GENMASK(23, 18) +/* Mask for OS to BMC packet buffer size */ +#define IGC_OS2BMCPBSIZE_MASK GENMASK(29, 24) +/* TX Packet buffer size in KB */ +#define IGC_TXPB0SIZE(x) FIELD_PREP(IGC_TXPB0SIZE_MASK, (x)) +#define IGC_TXPB1SIZE(x) FIELD_PREP(IGC_TXPB1SIZE_MASK, (x)) +#define IGC_TXPB2SIZE(x) FIELD_PREP(IGC_TXPB2SIZE_MASK, (x)) +#define IGC_TXPB3SIZE(x) FIELD_PREP(IGC_TXPB3SIZE_MASK, (x)) +/* OS to BMC packet buffer size in KB */ +#define IGC_OS2BMCPBSIZE(x)FIELD_PREP(IGC_OS2BMCPBSIZE_MASK, (x)) +/* Default value following I225/I226 SW User Manual Section 8.3.2 */ +#define IGC_TXPBSIZE_DEFAULT ( \ + IGC_TXPB0SIZE(20) | IGC_TXPB1SIZE(0) | IGC_TXPB2SIZE(0) | \ + IGC_TXPB3SIZE(0) | IGC_OS2BMCPBSIZE(4)) +#define IGC_TXPBSIZE_TSN ( \ + IGC_TXPB0SIZE(5) | IGC_TXPB1SIZE(5) | IGC_TXPB2SIZE(5) | \ + IGC_TXPB3SIZE(5) | IGC_OS2BMCPBSIZE(4)) #define IGC_DTXMXPKTSZ_TSN 0x19 /* 1600 bytes of max TX DMA packet size */ #define IGC_DTXMXPKTSZ_DEFAULT 0x98 /* 9728-byte Jumbo frames */ -- 2.34.1
[Intel-wired-lan] [PATCH iwl-next v10 13/14] igc: add support to get MAC Merge data via ethtool
Implement "ethtool --show-mm" callback for IGC. Tested with command: $ ethtool --show-mm enp1s0. MAC Merge layer state for enp1s0: pMAC enabled: on TX enabled: on TX active: on TX minimum fragment size: 64 RX minimum fragment size: 60 Verify enabled: on Verify time: 128 Max verify time: 128 Verification status: SUCCEEDED Verified that the fields value are retrieved correctly. Reviewed-by: Vladimir Oltean Signed-off-by: Faizal Rahim --- drivers/net/ethernet/intel/igc/igc_ethtool.c | 14 ++ drivers/net/ethernet/intel/igc/igc_tsn.h | 1 + 2 files changed, 15 insertions(+) diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c index 529654ccd83f..fd4b4b332309 100644 --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c @@ -1782,6 +1782,19 @@ static int igc_ethtool_set_eee(struct net_device *netdev, return 0; } +static int igc_ethtool_get_mm(struct net_device *netdev, + struct ethtool_mm_state *cmd) +{ + struct igc_adapter *adapter = netdev_priv(netdev); + struct igc_fpe_t *fpe = &adapter->fpe; + + ethtool_mmsv_get_mm(&fpe->mmsv, cmd); + cmd->tx_min_frag_size = fpe->tx_min_frag_size; + cmd->rx_min_frag_size = IGC_RX_MIN_FRAG_SIZE; + + return 0; +} + static int igc_ethtool_set_mm(struct net_device *netdev, struct ethtool_mm_cfg *cmd, struct netlink_ext_ack *extack) @@ -2101,6 +2114,7 @@ static const struct ethtool_ops igc_ethtool_ops = { .get_link_ksettings = igc_ethtool_get_link_ksettings, .set_link_ksettings = igc_ethtool_set_link_ksettings, .self_test = igc_ethtool_diag_test, + .get_mm = igc_ethtool_get_mm, .set_mm = igc_ethtool_set_mm, }; diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h b/drivers/net/ethernet/intel/igc/igc_tsn.h index 58fe5f0773d7..c2a77229207b 100644 --- a/drivers/net/ethernet/intel/igc/igc_tsn.h +++ b/drivers/net/ethernet/intel/igc/igc_tsn.h @@ -4,6 +4,7 @@ #ifndef _IGC_TSN_H_ #define _IGC_TSN_H_ +#define IGC_RX_MIN_FRAG_SIZE 60 #define SMD_FRAME_SIZE 60 enum igc_txd_popts_type { -- 2.34.1
Re: [Intel-wired-lan] [PATCH iwl-net v3] ixgbe: fix media type detection for E610 device
> -Original Message- > From: Intel-wired-lan On Behalf Of > Piotr Kwapulinski > Sent: Friday, February 21, 2025 9:19 PM > To: intel-wired-...@lists.osuosl.org > Cc: net...@vger.kernel.org; pmen...@molgen.mpg.de; and...@lunn.ch; > Kwapulinski, Piotr ; Kitszel, Przemyslaw > > Subject: [Intel-wired-lan] [PATCH iwl-net v3] ixgbe: fix media type detection > for E610 device > > The commit 23c0e5a16bcc ("ixgbe: Add link management support for E610 > device") introduced incorrect media type detection for E610 device. It > reproduces when advertised speed is modified after driver reload. Clear the > previous outdated PHY type high value. > > Reproduction steps: > modprobe ixgbe > ethtool -s eth0 advertise 0x1 modprobe -r ixgbe modprobe > ixgbe ethtool -s eth0 advertise 0x1 Result before the fix: > netlink error: link settings update failed netlink error: Invalid argument > Result > after the fix: > No output error > > Fixes: 23c0e5a16bcc ("ixgbe: Add link management support for E610 device") > Reviewed-by: Przemek Kitszel > Reviewed-by: Paul Menzel > Signed-off-by: Piotr Kwapulinski > --- > v1 -> v2 > More commit message details and reproduction steps added > v2 -> v3 > More details in reproduction steps added > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > Tested-by: Bharath R
Re: [Intel-wired-lan] [PATCH intel-net v2] ice: fix reservation of resources for RDMA when disabled
> -Original Message- > From: Intel-wired-lan On Behalf Of Jesse > Brandeburg > Sent: 06 March 2025 23:27 > To: intel-wired-...@lists.osuosl.org > Cc: Brandeburg, Jesse ; net...@vger.kernel.org; > kernel-t...@cloudflare.com; jbran...@kernel.org; l...@kernel.org; Kitszel, > Przemyslaw ; Ertman, David M > > Subject: [Intel-wired-lan] [PATCH intel-net v2] ice: fix reservation of > resources for RDMA when disabled > > From: Jesse Brandeburg > > If the CONFIG_INFINIBAND_IRDMA symbol is not enabled as a module or a > built-in, then don't let the driver reserve resources for RDMA. The result of > this change is a large savings in resources for older kernels, and a cleaner > driver configuration for the IRDMA=n case for old and new kernels. > > Implement this by avoiding enabling the RDMA capability when scanning > hardware capabilities. > > Note: Loading the out-of-tree irdma driver in connection to the in-kernel ice > driver, is not supported, and should not be attempted, especially when > disabling IRDMA in the kernel config. > > Fixes: d25a0fc41c1f ("ice: Initialize RDMA support") > Signed-off-by: Jesse Brandeburg > Acked-by: Dave Ertman > --- > v2: resend with acks, add note about oot irdma (Leon), reword commit message > v1: > https://lore.kernel.org/netdev/20241114000105.703740-1-jbran...@kernel.org/ > --- > drivers/net/ethernet/intel/ice/ice_common.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Tested-by: Rinitha S (A Contingent worker at Intel)
Re: [Intel-wired-lan] [iwl-next v4 1/1] iidc/ice/irdma: Update IDC to support multiple consumers
On Fri, Mar 14, 2025 at 06:18:00PM -0700, Samudrala, Sridhar wrote: > > > On 3/14/2025 11:12 AM, Leon Romanovsky wrote: > > On Thu, Mar 13, 2025 at 04:38:39PM -0700, Samudrala, Sridhar wrote: > > > > > > > > > On 3/2/2025 12:26 AM, Leon Romanovsky wrote: > > > > On Wed, Feb 26, 2025 at 11:01:52PM +, Ertman, David M wrote: > > > > > > > > > > > > > > > > -Original Message- > > > > > > From: Leon Romanovsky > > > > > > Sent: Wednesday, February 26, 2025 10:50 AM > > > > > > To: Ertman, David M > > > > > > Cc: Nikolova, Tatyana E ; > > > > > > j...@nvidia.com; > > > > > > intel-wired-...@lists.osuosl.org; linux-r...@vger.kernel.org; > > > > > > net...@vger.kernel.org > > > > > > Subject: Re: [iwl-next v4 1/1] iidc/ice/irdma: Update IDC to > > > > > > support multiple > > > > > > consumers > > > > > > > > > > > > On Wed, Feb 26, 2025 at 05:36:44PM +, Ertman, David M wrote: > > > > > > > > -Original Message- > > > > > > > > From: Leon Romanovsky > > > > > > > > Sent: Monday, February 24, 2025 11:56 PM > > > > > > > > To: Nikolova, Tatyana E > > > > > > > > Cc: j...@nvidia.com; intel-wired-...@lists.osuosl.org; linux- > > > > > > > > r...@vger.kernel.org; net...@vger.kernel.org; Ertman, David M > > > > > > > > > > > > > > > > Subject: Re: [iwl-next v4 1/1] iidc/ice/irdma: Update IDC to > > > > > > > > support > > > > > > multiple > > > > > > > > consumers > > > > > > > > > > > > > > > > On Mon, Feb 24, 2025 at 11:04:28PM -0600, Tatyana Nikolova > > > > > > > > wrote: > > > > > > > > > From: Dave Ertman > > > > > > > > > > > > > > > > > > To support RDMA for E2000 product, the idpf driver will use > > > > > > > > > the IDC > > > > > > > > > interface with the irdma auxiliary driver, thus becoming a > > > > > > > > > second > > > > > > > > > consumer of it. This requires the IDC be updated to support > > > > > > > > > multiple > > > > > > > > > consumers. The use of exported symbols no longer makes sense > > > > > > because it > > > > > > > > > will require all core drivers (ice/idpf) that can interface > > > > > > > > > with irdma > > > > > > > > > auxiliary driver to be loaded even if hardware is not present > > > > > > > > > for those > > > > > > > > > drivers. > > > > > > > > > > > > > > > > In auxiliary bus world, the code drivers (ice/idpf) need to > > > > > > > > created > > > > > > > > auxiliary devices only if specific device present. That > > > > > > > > auxiliary device > > > > > > > > will trigger the load of specific module (irdma in our case). > > > > > > > > > > > > > > > > EXPORT_SYMBOL won't trigger load of irdma driver, but the > > > > > > > > opposite is > > > > > > > > true, load of irdma will trigger load of ice/idpf drivers > > > > > > > > (depends on > > > > > > > > their exported symbol). > > > > > > > > > > > > > > > > > > > > > > > > > > To address this, implement an ops struct that will be > > > > > > > > > universal set of > > > > > > > > > naked function pointers that will be populated by each core > > > > > > > > > driver for > > > > > > > > > the irdma auxiliary driver to call. > > > > > > > > > > > > > > > > No, we worked very hard to make proper HW discovery and driver > > > > > > autoload, > > > > > > > > let's not return back. For now, it is no-go. > > > > > > > > > > > > > > Hi Leon, > > > > > > > > > > > > > > I am a little confused about what the problem here is. The main > > > > > > > issue I pull > > > > > > > from your response is: Removing exported symbols will stop > > > > > > > ice/idpf from > > > > > > > autoloading when irdma loads. Is this correct or did I miss your > > > > > > > point? > > > > > > > > > > > > It is one of the main points. > > > > > > > > > > > > > > > > > > > > But, if there is an ice or idpf supported device present in the > > > > > > > system, the > > > > > > > appropriate driver will have already been loaded anyway (and gone > > > > > > through its > > > > > > > probe flow to create auxiliary devices). If it is not loaded, > > > > > > > then the system > > > > > > owner > > > > > > > has either unloaded it manually or blacklisted it. This would > > > > > > > not cause an > > > > > > issue > > > > > > > anyway, since irdma and ice/idpf can load in any order. > > > > > > > > > > > > There are two assumptions above, which both not true. > > > > > > 1. Users never issue "modprobe irdma" command alone and always will > > > > > > call > > > > > > to whole chain "modprobe ice ..." before. > > > > > > 2. You open-code module subsystem properly with reference counters, > > > > > > ownership and locks to protect from function pointers to be > > > > > > set/clear > > > > > > dynamically. > > > > > > > > > > Ah, I see your reasoning now. Our goal was to make the two modules > > > > > independent, > > > > > with no prescribed load order mandated, and utilize the auxiliary bus > > > > > and device subsystem > > > > > to handle load order and unload of one or the other module. The
Re: [Intel-wired-lan] [PATCH iwl-next v2 3/3] ice: enable timesync operation on 2xNAC E825 devices
On Mon, Mar 10, 2025 at 01:24:39PM +0100, Grzegorz Nitka wrote: > From: Karol Kolacinski > > According to the E825C specification, SBQ address for ports on a single > complex is device 2 for PHY 0 and device 13 for PHY1. > For accessing ports on a dual complex E825C (so called 2xNAC mode), > the driver should use destination device 2 (referred as phy_0) for > the current complex PHY and device 13 (referred as phy_0_peer) for > peer complex PHY. > > Differentiate SBQ destination device by checking if current PF port > number is on the same PHY as target port number. > > Adjust 'ice_get_lane_number' function to provide unique port number for > ports from PHY1 in 'dual' mode config (by adding fixed offset for PHY1 > ports). Cache this value in ice_hw struct. > > Introduce ice_get_primary_hw wrapper to get access to timesync register > not available from second NAC. > > Reviewed-by: Przemek Kitszel > Signed-off-by: Karol Kolacinski > Co-developed-by: Grzegorz Nitka > Signed-off-by: Grzegorz Nitka Reviewed-by: Simon Horman
Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side band access workaround for E825
On Mon, Mar 10, 2025 at 12:36:31PM +, Nitka, Grzegorz wrote: > > -Original Message- > > From: Intel-wired-lan On Behalf Of > > Nitka, Grzegorz > > Sent: Tuesday, March 4, 2025 2:04 PM > > To: Paul Menzel ; Kolacinski, Karol > > > > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Kitszel, > > Przemyslaw ; Michal Swiatkowski > > > > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side > > band access workaround for E825 > > > > > -Original Message- > > > From: Paul Menzel > > > Sent: Friday, February 21, 2025 10:16 PM > > > To: Nitka, Grzegorz ; Kolacinski, Karol > > > > > > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Kitszel, > > > Przemyslaw ; Michal Swiatkowski > > > > > > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side > > > band access workaround for E825 > > > > > > Dear Grzegorz, dear Karol, > > > > > > > > > Thank you for your patch. > > > > > > Am 21.02.25 um 13:31 schrieb Grzegorz Nitka: > > > > From: Karol Kolacinski > > > > > > > > Due to the bug in FW/NVM autoload mechanism (wrong default > > > > SB_REM_DEV_CTL register settings), the access to peer PHY and CGU > > > > clients was disabled by default. > > > > > > I’d add a blank line between the paragraphs. > > > > > > > As the workaround solution, the register value was overwritten by the > > > > driver at the probe or reset handling. > > > > Remove workaround as it's not needed anymore. The fix in autoload > > > > procedure has been provided with NVM 3.80 version. > > > > > > Is this compatible with Linux’ no regression policy? People might only > > > update the Linux kernel and not the firmware. > > > > > > How did you test this, and how can others test this? > > > > > > > Reviewed-by: Michal Swiatkowski > > > > Reviewed-by: Przemek Kitszel > > > > Signed-off-by: Karol Kolacinski > > > > Signed-off-by: Grzegorz Nitka > > > > --- > > > > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 23 - > > > > 1 file changed, 23 deletions(-) > > > > > > > > > Kind regards, > > > > > > Paul > > > > > > > > > > Dear Paul, first of all thank you for your review and apologize for late > > response (simply, I was out previous week). > > > > Removing that workaround was a conscious decision. > > Rationale for that is, that the 'problematic' workaround was provided > > in very early product development stage (~ year ago). Now, especially > > when E825-C product was just officially announced, the customer shall > > use official, approved SW ingredients. > > > > Here are more details on this: > > - introduction to E825-C devices was provided in kernel 6.6, to allow > > selected customers for early E825-C enablement. At that time E825-C > > product family was in early phase (kind of Alpha), and one of the > > consequences, from today's perspective, is that it included 'unwanted' > > workarounds like this. > > > > - this change applies to E825-C products only, which is SoC product, not > > a regular NIC card. What I'd like to emphasize here, it requires even > > more > > customer support from Intel company in terms of providing matching > > SW stack (like BIOS, firmware, drivers etc.). > > > > I see two possibilities now: > > 1) if the regression policy you mentioned is inviolable, keep the workaround > >in the kernel code like it is today. Maybe we could add NVM version > > checker > >and apply register updates for older NVMs only. > >But, in my opinion, it would lead to keeping a dead code. There shouldn't > > be > >official FW (I hope I won't regret these words) on the market which > > requires > >this workaround. > > > > 2) remove the workaround like it's implemented in this patch and improve > >commit message to clarify all potential doubts/questions from the user > >perspective. > > > > What's your thoughts? > > > > Kind regards > > > > Grzegorz > > > > I've just submitted v2 of this series, but no changes in this area yet > (except adding > blank line suggestion) > I'm waiting for feedback or confirmation if above justification is sufficient. > I'll submit one more if needed. Hi Grzegorz, Sorry for not commenting on this earlier, your question has been hanging out on the ML for a while now. >From my point of view the key part of your justification is that Intel has sufficient control of the SW stack and availability of (SoC) devices such that in practice users would not see a regression. And in my view this is entirely reasonable and the approach taken by this patch is fine. I do, however, think some text regarding this belongs in the patch description. And I'll respond in that vein to v3 [*]. [*] https://lore.kernel.org/netdev/20250310122439.3327908-2-grzegorz.ni...@intel.com/
Re: [Intel-wired-lan] [PATCH iwl-next v2 1/3] ice: remove SW side band access workaround for E825
On Mon, Mar 10, 2025 at 01:24:37PM +0100, Grzegorz Nitka wrote: > From: Karol Kolacinski > > Due to the bug in FW/NVM autoload mechanism (wrong default > SB_REM_DEV_CTL register settings), the access to peer PHY and CGU > clients was disabled by default. > > As the workaround solution, the register value was overwritten by the > driver at the probe or reset handling. > Remove workaround as it's not needed anymore. The fix in autoload > procedure has been provided with NVM 3.80 version. Hi Grzegorz, As per my belated comment on v2 [*], I think it would be worth adding a brief comment here regarding this not being expected to cause a regression in HW+FW seen in the wild. [*] https://lore.kernel.org/netdev/20250317163359.gc688...@kernel.org/ > > Reviewed-by: Michal Swiatkowski > Reviewed-by: Przemek Kitszel > Signed-off-by: Karol Kolacinski > Signed-off-by: Grzegorz Nitka ...
Re: [Intel-wired-lan] [iwl-net v3 2/5] ice: stop truncating queue ids when checking
> -Original Message- > From: Intel-wired-lan On Behalf Of > Martyna Szapar-Mudlaw > Sent: Tuesday, March 4, 2025 12:09 PM > To: intel-wired-...@lists.osuosl.org > Cc: net...@vger.kernel.org; Glaza, Jan ; Loktionov, > Aleksandr ; Jagielski, Jedrzej > ; Simon Horman ; Martyna > Szapar-Mudlaw > Subject: [Intel-wired-lan] [iwl-net v3 2/5] ice: stop truncating queue ids > when > checking > > From: Jan Glaza > > Queue IDs can be up to 4096, fix invalid check to stop truncating IDs to 8 > bits. > > Fixes: bf93bf791cec8 ("ice: introduce ice_virtchnl.c and ice_virtchnl.h") > Reviewed-by: Aleksandr Loktionov > Reviewed-by: Jedrzej Jagielski > Reviewed-by: Simon Horman > Signed-off-by: Jan Glaza > Signed-off-by: Martyna Szapar-Mudlaw mud...@linux.intel.com> > --- > drivers/net/ethernet/intel/ice/ice_virtchnl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c > b/drivers/net/ethernet/intel/ice/ice_virtchnl.c > index b6285433307c..343f2b4b0dc5 100644 > --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c > +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c > @@ -565,7 +565,7 @@ bool ice_vc_isvalid_vsi_id(struct ice_vf *vf, u16 vsi_id) Tested-by: Rafal Romanowski
Re: [Intel-wired-lan] [PATCH net-next 11/16] idpf: prepare structures to support XDP
From: Maciej Fijalkowski Date: Fri, 7 Mar 2025 14:27:13 +0100 > On Wed, Mar 05, 2025 at 05:21:27PM +0100, Alexander Lobakin wrote: >> From: Michal Kubiak >> >> Extend basic structures of the driver (e.g. 'idpf_vport', 'idpf_*_queue', >> 'idpf_vport_user_config_data') by adding members necessary to support XDP. >> Add extra XDP Tx queues needed to support XDP_TX and XDP_REDIRECT actions >> without interfering with regular Tx traffic. >> Also add functions dedicated to support XDP initialization for Rx and >> Tx queues and call those functions from the existing algorithms of >> queues configuration. [...] >> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c >> b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c >> index 59b1a1a09996..1ca322bfe92f 100644 >> --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c >> +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c >> @@ -186,9 +186,11 @@ static void idpf_get_channels(struct net_device *netdev, >> { >> struct idpf_netdev_priv *np = netdev_priv(netdev); >> struct idpf_vport_config *vport_config; >> +const struct idpf_vport *vport; >> u16 num_txq, num_rxq; >> u16 combined; >> >> +vport = idpf_netdev_to_vport(netdev); >> vport_config = np->adapter->vport_config[np->vport_idx]; >> >> num_txq = vport_config->user_config.num_req_tx_qs; >> @@ -202,8 +204,8 @@ static void idpf_get_channels(struct net_device *netdev, >> ch->max_rx = vport_config->max_q.max_rxq; >> ch->max_tx = vport_config->max_q.max_txq; >> >> -ch->max_other = IDPF_MAX_MBXQ; >> -ch->other_count = IDPF_MAX_MBXQ; >> +ch->max_other = IDPF_MAX_MBXQ + vport->num_xdp_txq; >> +ch->other_count = IDPF_MAX_MBXQ + vport->num_xdp_txq; > > That's new I think. Do you explain somewhere that other `other` will carry > xdpq count? Otherwise how would I know to interpret this value? Where? :D > > Also from what I see num_txq carries (txq + xdpq) count. How is that > affecting the `combined` from ethtool_channels? No changes in combined/Ethtool, num_txq is not used there. Stuff like req_txq_num includes skb queues only. > >> >> ch->combined_count = combined; >> ch->rx_count = num_rxq - combined; >> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c >> b/drivers/net/ethernet/intel/idpf/idpf_lib.c >> index 2594ca38e8ca..0f4edc9cd1ad 100644 > > (...) > >> + >> +/** >> + * __idpf_xdp_rxq_info_init - Setup XDP RxQ info for a given Rx queue >> + * @rxq: Rx queue for which the resources are setup >> + * @arg: flag indicating if the HW works in split queue mode >> + * >> + * Return: 0 on success, negative on failure. >> + */ >> +static int __idpf_xdp_rxq_info_init(struct idpf_rx_queue *rxq, void *arg) >> +{ >> +const struct idpf_vport *vport = rxq->q_vector->vport; >> +bool split = idpf_is_queue_model_split(vport->rxq_model); >> +const struct page_pool *pp; >> +int err; >> + >> +err = __xdp_rxq_info_reg(&rxq->xdp_rxq, vport->netdev, rxq->idx, >> + rxq->q_vector->napi.napi_id, >> + rxq->rx_buf_size); >> +if (err) >> +return err; >> + >> +pp = split ? rxq->bufq_sets[0].bufq.pp : rxq->pp; >> +xdp_rxq_info_attach_page_pool(&rxq->xdp_rxq, pp); >> + >> +if (!split) >> +return 0; > > why do you care about splitq model if on next patch you don't allow > XDP_SETUP_PROG for that? This function is called unconditionally for both queue models. If we don't account it here, we'd break regular traffic flow. (singleq will be removed soon, don't take it seriously anyway) [...] >> +int idpf_vport_xdpq_get(const struct idpf_vport *vport) >> +{ >> +struct libeth_xdpsq_timer **timers __free(kvfree) = NULL; > > please bear with me here - so this array will exist as long as there is a > single timers[i] allocated? even though it's a local var? No problem. No, this array will be freed when the function exits. This array is an array of pointers to iterate in a loop and assign timers to queues. When we exit this function, it's no longer needed. I can't place the whole array on the stack since I don't know the actual queue count + it can be really big (1024 pointers * 8 = 8 Kb, even 128 or 256 queues is already 1-2 Kb). The actual timers are allocated separately and NUMA-locally below. > > this way you avoid the need to store it in vport? > >> +struct net_device *dev; >> +u32 sqs; >> + >> +if (!idpf_xdp_is_prog_ena(vport)) >> +return 0; >> + >> +timers = kvcalloc(vport->num_xdp_txq, sizeof(*timers), GFP_KERNEL); >> +if (!timers) >> +return -ENOMEM; >> + >> +for (u32 i = 0; i < vport->num_xdp_txq; i++) { >> +timers[i] = kzalloc_node(sizeof(*timers[i]), GFP_KERNEL, >> + cpu_to_mem(i)); >> +if (!timers[i]) { >> +for (int j = i - 1; j >= 0; j--) >> +kfree(timers[j]);
Re: [Intel-wired-lan] [iwl-net v3 5/5] ice: fix using untrusted value of pkt_len in ice_vc_fdir_parse_raw()
> -Original Message- > From: Intel-wired-lan On Behalf Of > Martyna Szapar-Mudlaw > Sent: Tuesday, March 4, 2025 12:09 PM > To: intel-wired-...@lists.osuosl.org > Cc: net...@vger.kernel.org; Polchlopek, Mateusz > ; Kitszel, Przemyslaw > ; Martyna Szapar-Mudlaw mud...@linux.intel.com> > Subject: [Intel-wired-lan] [iwl-net v3 5/5] ice: fix using untrusted value of > pkt_len > in ice_vc_fdir_parse_raw() > > From: Mateusz Polchlopek > > Fix using the untrusted value of proto->raw.pkt_len in function > ice_vc_fdir_parse_raw() by verifying if it does not exceed the > VIRTCHNL_MAX_SIZE_RAW_PACKET value. > > Fixes: 99f419df8a5c ("ice: enable FDIR filters from raw binary patterns for > VFs") > Reviewed-by: Przemek Kitszel > Signed-off-by: Mateusz Polchlopek > Signed-off-by: Martyna Szapar-Mudlaw mud...@linux.intel.com> > --- > .../ethernet/intel/ice/ice_virtchnl_fdir.c| 24 --- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c > b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c > index 14e3f0f89c78..9be4bd717512 100644 > --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c > +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c > @@ -832,21 +832,27 @@ ice_vc_fdir_parse_raw(struct ice_vf *vf, Tested-by: Rafal Romanowski
[Intel-wired-lan] [PATCH][next][V2] ice: make const read-only array dflt_rules static
Don't populate the const read-only array dflt_rules on the stack at run time, instead make it static. Signed-off-by: Colin Ian King --- V2: Remove additional changes not related to this commit. --- drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c index 1d118171de37..aceec184e89b 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c @@ -1605,7 +1605,7 @@ void ice_fdir_replay_fltrs(struct ice_pf *pf) */ int ice_fdir_create_dflt_rules(struct ice_pf *pf) { - const enum ice_fltr_ptype dflt_rules[] = { + static const enum ice_fltr_ptype dflt_rules[] = { ICE_FLTR_PTYPE_NONF_IPV4_TCP, ICE_FLTR_PTYPE_NONF_IPV4_UDP, ICE_FLTR_PTYPE_NONF_IPV6_TCP, ICE_FLTR_PTYPE_NONF_IPV6_UDP, }; -- 2.49.0
[Intel-wired-lan] [PATCH][next] ice: make const read-only array dflt_rules static
Don't populate the const read-only array dflt_rules on the stack at run time, instead make it static. Signed-off-by: Colin Ian King --- drivers/gpu/drm/i915/intel_memory_region.c| 2 +- drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c index d40ee1b42110..7f4102edc75b 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.c +++ b/drivers/gpu/drm/i915/intel_memory_region.c @@ -62,7 +62,7 @@ static int iopagetest(struct intel_memory_region *mem, resource_size_t offset, const void *caller) { - const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 }; + static const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 }; void __iomem *va; int err; int i; diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c index 1d118171de37..aceec184e89b 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c @@ -1605,7 +1605,7 @@ void ice_fdir_replay_fltrs(struct ice_pf *pf) */ int ice_fdir_create_dflt_rules(struct ice_pf *pf) { - const enum ice_fltr_ptype dflt_rules[] = { + static const enum ice_fltr_ptype dflt_rules[] = { ICE_FLTR_PTYPE_NONF_IPV4_TCP, ICE_FLTR_PTYPE_NONF_IPV4_UDP, ICE_FLTR_PTYPE_NONF_IPV6_TCP, ICE_FLTR_PTYPE_NONF_IPV6_UDP, }; -- 2.47.2
Re: [Intel-wired-lan] [PATCH][next] ice: make const read-only array dflt_rules static
Ignore, managed to mangle two commits into one. On 17/03/2025 14:46, Colin Ian King wrote: Don't populate the const read-only array dflt_rules on the stack at run time, instead make it static. Signed-off-by: Colin Ian King --- drivers/gpu/drm/i915/intel_memory_region.c| 2 +- drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c index d40ee1b42110..7f4102edc75b 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.c +++ b/drivers/gpu/drm/i915/intel_memory_region.c @@ -62,7 +62,7 @@ static int iopagetest(struct intel_memory_region *mem, resource_size_t offset, const void *caller) { - const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 }; + static const u8 val[] = { 0x0, 0xa5, 0xc3, 0xf0 }; void __iomem *va; int err; int i; diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c index 1d118171de37..aceec184e89b 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c @@ -1605,7 +1605,7 @@ void ice_fdir_replay_fltrs(struct ice_pf *pf) */ int ice_fdir_create_dflt_rules(struct ice_pf *pf) { - const enum ice_fltr_ptype dflt_rules[] = { + static const enum ice_fltr_ptype dflt_rules[] = { ICE_FLTR_PTYPE_NONF_IPV4_TCP, ICE_FLTR_PTYPE_NONF_IPV4_UDP, ICE_FLTR_PTYPE_NONF_IPV6_TCP, ICE_FLTR_PTYPE_NONF_IPV6_UDP, }; OpenPGP_0x68C287DFC6A80226.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature