Re: [Intel-wired-lan] [PATCH net v1] ice: do not reserve resources for RDMA when disabled
On 11/14/24 01:00, jbran...@kernel.org wrote:
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.
Do this by avoiding enabling the capability when scanning hardware
capabilities.
Fixes: d25a0fc41c1f ("ice: Initialize RDMA support")
CC: Dave Ertman
Signed-off-by: Jesse Brandeburg
---
Hi Jesse, it's good to hear back from you :)
we are already working on resolving the issue of miss-allocating
too many resources (would be good to know what beyond MSI-x'es
you care about) for RDMA in the default (likely non-RDMA heavy) case.
Here is a series from Michal that lets user to manage it a bit:
https://lore.kernel.org/netdev/20241114122009.97416-3-michal.swiatkow...@linux.intel.com/T/
and we want to post another one later that changes defaults from the
current "grab a lot when there are many CPUs" policy to more resonable
[Intel-wired-lan] [PATCH iwl-net v2] ice: fix PHY timestamp extraction for ETH56G
Fix incorrect PHY timestamp extraction for ETH56G.
It's better to use FIELD_PREP() than manual shift.
Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
Reviewed-by: Przemek Kitszel
Reviewed-by: Simon Horman
Signed-off-by: Przemyslaw Korba
---
Changelog
v2:
remove legal footer
v1:
https://lore.kernel.org/intel-wired-lan/20241107113257.466286-1-przemyslaw.ko...@intel.com
---
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 3 ++-
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 5 ++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 75c68b0325e7..3d45e4ed90b6 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -1553,7 +1553,8 @@ static int ice_read_ptp_tstamp_eth56g(struct ice_hw *hw,
u8 port, u8 idx,
* lower 8 bits in the low register, and the upper 32 bits in the high
* register.
*/
- *tstamp = ((u64)hi) << TS_PHY_HIGH_S | ((u64)lo & TS_PHY_LOW_M);
+ *tstamp = FIELD_PREP(TS_PHY_HIGH_M, hi) |
+ FIELD_PREP(TS_PHY_LOW_M, lo);
return 0;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 6cedc1a906af..4c8b84571344 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -663,9 +663,8 @@ static inline u64 ice_get_base_incval(struct ice_hw *hw)
#define TS_HIGH_M 0xFF
#define TS_HIGH_S 32
-#define TS_PHY_LOW_M 0xFF
-#define TS_PHY_HIGH_M 0x
-#define TS_PHY_HIGH_S 8
+#define TS_PHY_LOW_M GENMASK(7, 0)
+#define TS_PHY_HIGH_M GENMASK_ULL(39, 8)
#define BYTES_PER_IDX_ADDR_L_U 8
#define BYTES_PER_IDX_ADDR_L 4
base-commit: 182ff3dabe8f127049c09660346cad492bcc0ceb
--
2.31.1
Re: [Intel-wired-lan] [PATCH net v1] ice: do not reserve resources for RDMA when disabled
On Fri, Nov 15, 2024 at 12:51 AM Przemek Kitszel
wrote:
>
> On 11/14/24 01:00, jbran...@kernel.org wrote:
> > 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.
> >
> > Do this by avoiding enabling the capability when scanning hardware
> > capabilities.
> >
> > Fixes: d25a0fc41c1f ("ice: Initialize RDMA support")
> > CC: Dave Ertman
> > Signed-off-by: Jesse Brandeburg
> > ---
>
> Hi Jesse, it's good to hear back from you :)
Hi Przemek! You too.
> we are already working on resolving the issue of miss-allocating
> too many resources (would be good to know what beyond MSI-x'es
> you care about) for RDMA in the default (likely non-RDMA heavy) case.
> Here is a series from Michal that lets user to manage it a bit:
> https://lore.kernel.org/netdev/20241114122009.97416-3-michal.swiatkow...@linux.intel.com/T/
I agree, but that whole series is far too big to backport to stable, right?
> and we want to post another one later that changes defaults from the
> current "grab a lot when there are many CPUs" policy to more resonable
I'm looking forward to those series, and in fact had been looking to
backport one of the patches from michal's series, but found that for
us at least with RDMA disabled this limit seemed far simpler, and also
I doubt it will conflict with the more complicated work of managing
features when all are enabled.
Please see my other reply to dave (and yes I'm replying from two
different accounts, as I'm figuring out the best way to work here at
my new job.)
Re: [Intel-wired-lan] [PATCH net v1] ice: do not reserve resources for RDMA when disabled
On 11/14/24 10:06 AM, Ertman, David M wrote: case ICE_AQC_CAPS_RDMA: - caps->rdma = (number == 1); + if (IS_ENABLED(CONFIG_INFINIBAND_IRDMA)) + caps->rdma = (number == 1); ice_debug(hw, ICE_DBG_INIT, "%s: rdma = %d\n", prefix, The HW caps struct should always accurately reflect the capabilities of the HW being probed. Since this why must it accurately reflect the capability of the hardware? The driver state and capability is a reflection of the combination of both, so I'm not sure what the point of your statement. is a kernel configuration (i.e. software) consideration, the more appropriate approach would be to control the PF flag "ICE_FLAG_RDMA_ENA" based on the kernel CONFIG setting. I started making the changes you suggested, but the ICE_FLAG_RDMA_ENA is blindly set by the LAG code, if the cap.rdma is enabled. see ice_set_rdma_cap(). This means the disable won't stick. Unless I'm misunderstanding something, ICE_FLAG_RDMA_ENA is used both as a gate and as a state, which is a design issue. This leaves no choice but to implement the way I did in this v1 patch. Do you see any other option to make a simple change that is safe for backporting to stable? Thanks, Jesse
Re: [Intel-wired-lan] [PATCH iwl-net 04/10] idpf: negotiate PTP capabilies and get PTP clock
On Wed, Nov 13, 2024 at 04:46:14PM +0100, Milena Olech wrote:
> PTP capabilities are negotiated using virtchnl command. Add get
> capabilities function, direct access to read the PTP clock time and
> direct access to read the cross timestamp - system time and PTP clock
> time. Set initial PTP capabilities exposed to the stack.
>
> Reviewed-by: Alexander Lobakin
> Signed-off-by: Milena Olech
...
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
...
> /**
> * struct idpf_ptp - PTP parameters
> * @info: structure defining PTP hardware capabilities
> * @clock: pointer to registered PTP clock device
> * @adapter: back pointer to the adapter
> + * @cmd: HW specific command masks
> + * @dev_clk_regs: the set of registers to access the device clock
> + * @caps: PTP capabilities negotiated with the Control Plane
> + * @get_dev_clk_time_access: access type for getting the device clock time
> + * @get_cross_tstamp_access: access type for the cross timestamping
> */
> struct idpf_ptp {
> struct ptp_clock_info info;
> struct ptp_clock *clock;
> struct idpf_adapter *adapter;
> + struct idpf_ptp_cmd cmd;
> + struct idpf_ptp_dev_clk_regs dev_clk_regs;
> + u32 caps;
> + enum idpf_ptp_access get_dev_clk_time_access:16;
> + enum idpf_ptp_access get_cross_tstamp_access:16;
> };
>
> +/**
> + * idpf_ptp_info_to_adapter - get driver adapter struct from ptp_clock_info
> + * @info: pointer to ptp_clock_info struct
Please in include a "Return:" section, as you have done elsewhere,
to document the return value of this function.
Flagged by ./scripts/kernel-doc -none -Wall
> + */
> +static inline struct idpf_adapter *
> +idpf_ptp_info_to_adapter(const struct ptp_clock_info *info)
> +{
> + const struct idpf_ptp *ptp = container_of_const(info, struct idpf_ptp,
> + info);
> + return ptp->adapter;
> +}
> +
> #if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
> int idpf_ptp_init(struct idpf_adapter *adapter);
> void idpf_ptp_release(struct idpf_adapter *adapter);
> +int idpf_ptp_get_caps(struct idpf_adapter *adapter);
> +void idpf_ptp_get_features_access(const struct idpf_adapter *adapter);
> #else /* CONFIG_PTP_1588_CLOCK */
> static inline int idpf_ptp_init(struct idpf_adapter *adpater)
> {
...
Re: [Intel-wired-lan] [PATCH iwl-net 01/10] idpf: initial PTP support
Dear Milena,
Thank you for your patch. It’d be great if you used a statement for the
summary/title by adding a verb (in imperative mood):
idpf: Add initial PTP support
Am 13.11.24 um 16:46 schrieb Milena Olech:
PTP feature is supported if the VIRTCHNL2_CAP_PTP is negotiated during the
capabilities recognition. Initial PTP support includes PTP initialization
and registration of the clock.
Maybe mention/paste the new debug messages, and document on what device
you tested it?
Kind regards,
Paul
Reviewed-by: Alexander Lobakin
Signed-off-by: Milena Olech
---
drivers/net/ethernet/intel/idpf/Kconfig | 1 +
drivers/net/ethernet/intel/idpf/Makefile | 1 +
drivers/net/ethernet/intel/idpf/idpf.h| 3 +
drivers/net/ethernet/intel/idpf/idpf_main.c | 4 +
drivers/net/ethernet/intel/idpf/idpf_ptp.c| 89 +++
drivers/net/ethernet/intel/idpf/idpf_ptp.h| 32 +++
.../net/ethernet/intel/idpf/idpf_virtchnl.c | 9 +-
7 files changed, 138 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/intel/idpf/idpf_ptp.c
create mode 100644 drivers/net/ethernet/intel/idpf/idpf_ptp.h
diff --git a/drivers/net/ethernet/intel/idpf/Kconfig
b/drivers/net/ethernet/intel/idpf/Kconfig
index 1addd663acad..2c359a8551c7 100644
--- a/drivers/net/ethernet/intel/idpf/Kconfig
+++ b/drivers/net/ethernet/intel/idpf/Kconfig
@@ -4,6 +4,7 @@
config IDPF
tristate "Intel(R) Infrastructure Data Path Function Support"
depends on PCI_MSI
+ depends on PTP_1588_CLOCK_OPTIONAL
select DIMLIB
select LIBETH
help
diff --git a/drivers/net/ethernet/intel/idpf/Makefile
b/drivers/net/ethernet/intel/idpf/Makefile
index 2ce01a0b5898..1f38a9d7125c 100644
--- a/drivers/net/ethernet/intel/idpf/Makefile
+++ b/drivers/net/ethernet/intel/idpf/Makefile
@@ -17,3 +17,4 @@ idpf-y := \
idpf_vf_dev.o
idpf-$(CONFIG_IDPF_SINGLEQ) += idpf_singleq_txrx.o
+idpf-$(CONFIG_PTP_1588_CLOCK) += idpf_ptp.o
diff --git a/drivers/net/ethernet/intel/idpf/idpf.h
b/drivers/net/ethernet/intel/idpf/idpf.h
index 66544faab710..2e8b14dd9d96 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -530,6 +530,7 @@ struct idpf_vc_xn_manager;
* @vector_lock: Lock to protect vector distribution
* @queue_lock: Lock to protect queue distribution
* @vc_buf_lock: Lock to protect virtchnl buffer
+ * @ptp: Storage for PTP-related data
*/
struct idpf_adapter {
struct pci_dev *pdev;
@@ -587,6 +588,8 @@ struct idpf_adapter {
struct mutex vector_lock;
struct mutex queue_lock;
struct mutex vc_buf_lock;
+
+ struct idpf_ptp *ptp;
};
/**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c
b/drivers/net/ethernet/intel/idpf/idpf_main.c
index db476b3314c8..22d9e2646444 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -163,6 +163,10 @@ static int idpf_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
goto err_free;
}
+ err = pci_enable_ptm(pdev, NULL);
+ if (err)
+ pci_dbg(pdev, "PCIe PTM is not supported by PCIe
bus/controller\n");
+
/* set up for high or low dma */
err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
if (err) {
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
new file mode 100644
index ..1ac6367f5989
--- /dev/null
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2024 Intel Corporation */
+
+#include "idpf.h"
+#include "idpf_ptp.h"
+
+/**
+ * idpf_ptp_create_clock - Create PTP clock device for userspace
+ * @adapter: Driver specific private structure
+ *
+ * This function creates a new PTP clock device.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int idpf_ptp_create_clock(const struct idpf_adapter *adapter)
+{
+ struct ptp_clock *clock;
+
+ /* Attempt to register the clock before enabling the hardware. */
+ clock = ptp_clock_register(&adapter->ptp->info,
+ &adapter->pdev->dev);
+ if (IS_ERR(clock)) {
+ pci_err(adapter->pdev, "PTP clock creation failed: %pe\n",
clock);
+ return PTR_ERR(clock);
+ }
+
+ adapter->ptp->clock = clock;
+
+ return 0;
+}
+
+/**
+ * idpf_ptp_init - Initialize PTP hardware clock support
+ * @adapter: Driver specific private structure
+ *
+ * Set up the device for interacting with the PTP hardware clock for all
+ * functions. Function will allocate and register a ptp_clock with the
+ * PTP_1588_CLOCK infrastructure.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int idpf_ptp_init(struct idpf_adapter *adapter)
+{
+ int err;
+
+ if (!idpf_is_cap_ena(adapter, IDPF_OTHER_CAPS, VIRTCHNL2_CAP_PTP)) {
+
Re: [Intel-wired-lan] [PATCH iwl-net 07/10] idpf: add Tx timestamp capabilities negotiation
On Wed, Nov 13, 2024 at 04:46:19PM +0100, Milena Olech wrote:
> Tx timestamp capabilities are negotiated for the uplink Vport.
> Driver receives information about the number of available Tx timestamp
> latches, the size of Tx timestamp value and the set of indexes used
> for Tx timestamping.
>
> Add function to get the Tx timestamp capabilities and parse the uplink
> vport flag.
>
> Co-developed-by: Emil Tantilov
> Signed-off-by: Emil Tantilov
> Co-developed-by: Pavan Kumar Linga
> Signed-off-by: Pavan Kumar Linga
> Reviewed-by: Alexander Lobakin
> Signed-off-by: Milena Olech
...
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
...
> +/**
> + * idpf_ptp_get_vport_tstamps_caps - Send virtchnl to get tstamps caps for
> vport
> + * @vport: Virtual port structure
> + *
> + * Send virtchnl get vport tstamps caps message to receive the set of tstamp
> + * capabilities per vport.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int idpf_ptp_get_vport_tstamps_caps(struct idpf_vport *vport)
...
> + for (u16 i = 0; i < tstamp_caps->num_entries; i++) {
> + u32 offset_l, offset_h;
It looks like the type of offset_l and offset_h should be __le32
to match the values that are stored in them.
Flagged by Sparse.
> +
> + ptp_tx_tstamp = ptp_tx_tstamps + i;
> + tx_tstamp_latch_caps = rcv_tx_tstamp_caps->tstamp_latches[i];
> +
> + if (tstamp_access != IDPF_PTP_DIRECT)
> + goto skip_offsets;
> +
> + offset_l = tx_tstamp_latch_caps.tx_latch_reg_offset_l;
> + offset_h = tx_tstamp_latch_caps.tx_latch_reg_offset_h;
> + ptp_tx_tstamp->tx_latch_reg_offset_l = le32_to_cpu(offset_l);
> + ptp_tx_tstamp->tx_latch_reg_offset_h = le32_to_cpu(offset_h);
> +
> +skip_offsets:
> + ptp_tx_tstamp->idx = tx_tstamp_latch_caps.index;
> +
> + list_add(&ptp_tx_tstamp->list_member,
> + &tstamp_caps->latches_free);
> +
> + tstamp_caps->tx_tstamp_status[i].state = IDPF_PTP_FREE;
> + }
...
[Intel-wired-lan] [PATCH v3 1/1] ixgbe: Correct BASE-BX10 compliance code
SFF-8472 (section 5.4 Transceiver Compliance Codes) defines bit 6 as
BASE-BX10. Bit 6 means a value of 0x40 (decimal 64).
The current value in the source code is 0x64, which appears to be a
mix-up of hex and decimal values. A value of 0x64 (binary 01100100)
incorrectly sets bit 2 (1000BASE-CX) and bit 5 (100BASE-FX) as well.
Fixes: 1b43e0d20f2d ("ixgbe: Add 1000BASE-BX support")
Signed-off-by: Tore Amundsen
Reviewed-by: Paul Menzel
Acked-by: Ernesto Castellotti
---
v2: Added Fixes tag as requested by Paul Menzel.
v3: Correct Fixes tag format and add Acked-By.
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
index 14aa2ca51f70..81179c60af4e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
@@ -40,7 +40,7 @@
#define IXGBE_SFF_1GBASESX_CAPABLE 0x1
#define IXGBE_SFF_1GBASELX_CAPABLE 0x2
#define IXGBE_SFF_1GBASET_CAPABLE 0x8
-#define IXGBE_SFF_BASEBX10_CAPABLE 0x64
+#define IXGBE_SFF_BASEBX10_CAPABLE 0x40
#define IXGBE_SFF_10GBASESR_CAPABLE0x10
#define IXGBE_SFF_10GBASELR_CAPABLE0x20
#define IXGBE_SFF_SOFT_RS_SELECT_MASK 0x8
--
2.43.0
Re: [Intel-wired-lan] [PATCH iwl-net 01/10] idpf: initial PTP support
On Wed, Nov 13, 2024 at 04:46:09PM +0100, Milena Olech wrote:
> PTP feature is supported if the VIRTCHNL2_CAP_PTP is negotiated during the
> capabilities recognition. Initial PTP support includes PTP initialization
> and registration of the clock.
>
> Reviewed-by: Alexander Lobakin
> Signed-off-by: Milena Olech
...
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> new file mode 100644
> index ..cb19988ca60f
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (C) 2024 Intel Corporation */
> +
> +#ifndef _IDPF_PTP_H
> +#define _IDPF_PTP_H
> +
> +#include
> +
> +/**
> + * struct idpf_ptp - PTP parameters
> + * @info: structure defining PTP hardware capabilities
> + * @clock: pointer to registered PTP clock device
> + * @adapter: back pointer to the adapter
> + */
> +struct idpf_ptp {
> + struct ptp_clock_info info;
> + struct ptp_clock *clock;
> + struct idpf_adapter *adapter;
> +};
> +
> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
> +int idpf_ptp_init(struct idpf_adapter *adapter);
> +void idpf_ptp_release(struct idpf_adapter *adapter);
> +#else /* CONFIG_PTP_1588_CLOCK */
> +static inline int idpf_ptp_init(struct idpf_adapter *adpater)
nit: adapter
> +{
> + return 0;
> +}
> +
> +static inline void idpf_ptp_release(struct idpf_adapter *adpater) { }
Ditto
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +#endif /* _IDPF_PTP_H */
...
Re: [Intel-wired-lan] [PATCH iwl-net 02/10] virtchnl: add PTP virtchnl definitions
On Wed, Nov 13, 2024 at 04:46:11PM +0100, Milena Olech wrote:
> PTP capabilities are negotiated using virtchnl commands. There are two
> available modes of the PTP support: direct and mailbox. When the direct
> access to PTP resources is negotiated, virtchnl messages returns a set
> of registers that allow read/write directly. When the mailbox access to
> PTP resources is negotiated, virtchnl messages are used to access
> PTP clock and to read the timestamp values.
>
> Virtchnl API covers both modes and exposes a set of PTP capabilities.
>
> Using virtchnl API, the driver recognizes also HW abilities - maximum
> adjustment of the clock and the basic increment value.
>
> Additionally, API allows to configure the secondary mailbox, dedicated
> exclusively for PTP purposes.
>
> Reviewed-by: Alexander Lobakin
> Signed-off-by: Milena Olech
> ---
> drivers/net/ethernet/intel/idpf/virtchnl2.h | 302
> 1 file changed, 302 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/idpf/virtchnl2.h
> b/drivers/net/ethernet/intel/idpf/virtchnl2.h
...
> +/**
> + * struct virtchnl2_ptp_set_dev_clk_time: Associated with message
> + * VIRTCHNL2_OP_PTP_SET_DEV_CLK_TIME.
> + * @dev_time_ns: Device time value expressed in nanoseconds to set
> + *
> + * PF/VF sends this message to set the time of the main timer.
> + */
> +struct virtchnl2_ptp_set_dev_clk_time {
> + __le64 dev_time_ns;
> +};
> +VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_ptp_set_dev_clk_time);
> +
> +/**
> + * struct virtchnl2_ptp_set_dev_clk_time: Associated with message
> + * VIRTCHNL2_OP_PTP_ADJ_DEV_CLK_FINE.
nit: struct virtchnl2_ptp_adj_dev_clk_fine:
Flagged by ./scripts/kernel-doc -none
> + * @incval: Source timer increment value per clock cycle
> + *
> + * PF/VF sends this message to adjust the frequency of the main timer by the
> + * indicated scaled ppm.
> + */
> +struct virtchnl2_ptp_adj_dev_clk_fine {
> + __le64 incval;
> +};
> +VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_ptp_adj_dev_clk_fine);
...
Re: [Intel-wired-lan] [PATCH iwl-net 07/10] idpf: add Tx timestamp capabilities negotiation
On Thu, Nov 14, 2024 at 03:49:27PM -0500, Willem de Bruijn wrote: > Milena Olech wrote: > > Tx timestamp capabilities are negotiated for the uplink Vport. > > Driver receives information about the number of available Tx timestamp > > latches, the size of Tx timestamp value and the set of indexes used > > for Tx timestamping. > > > > Add function to get the Tx timestamp capabilities and parse the uplink > > vport flag. > > > > Co-developed-by: Emil Tantilov > > Signed-off-by: Emil Tantilov > > Co-developed-by: Pavan Kumar Linga > > Signed-off-by: Pavan Kumar Linga > > Reviewed-by: Alexander Lobakin > > Signed-off-by: Milena Olech > > A few minor points. No big concerns from me. > > > struct idpf_vc_xn_manager; > > > > +#define idpf_for_each_vport(adapter, iter) \ > > + for (struct idpf_vport **__##iter = &(adapter)->vports[0], \ > > +*iter = *__##iter; \ > > +__##iter < &(adapter)->vports[(adapter)->num_alloc_vports]; \ > > +iter = *(++__##iter)) > > + > > Perhaps more readable to just use an int: > > for (int i = 0; iter = &(adapter)->vports[i], i < > (adapter)->num_alloc_vports; i++) > > > /** > > @@ -517,6 +524,60 @@ static int idpf_ptp_create_clock(const struct > > idpf_adapter *adapter) > > return 0; > > } > > > > +/** > > + * idpf_ptp_release_vport_tstamp - Release the Tx timestamps trakcers for a > > s/trakcers/trackers > > > +/** > > + * struct idpf_ptp_tx_tstamp - Parametrs for Tx timestamping > > s/Parametrs/Parameters > > > + * @list_member: the list member strutcure > > s/strutcure/Structure > > Please use a spell checker, don't rely on reviewers. To add to that: * Capabilities is misspelt in the subject * checkpatch.pl --codespell will spell-check the patch > > Also, going forward, IMHO documentation can be limited to APIs and > non-obvious functions/structs/fields. >
Re: [Intel-wired-lan] [PATCH iwl-net 01/10] idpf: initial PTP support
On 11/15/2024 2:44 PM, Paul Menzel wrote: > Dear Milena, > > >Thank you for your patch. It’d be great if you used a statement for the >summary/title by adding a verb (in imperative mood): > >idpf: Add initial PTP support > >Am 13.11.24 um 16:46 schrieb Milena Olech: >> PTP feature is supported if the VIRTCHNL2_CAP_PTP is negotiated during the >> capabilities recognition. Initial PTP support includes PTP initialization >> and registration of the clock. > >Maybe mention/paste the new debug messages, and document on what device >you tested it? > > >Kind regards, > >Paul Paul, Introduced PTP flow should work on all IDPF-compliant adapters. I've tested on the Intel IPU E2100 adapter. Regards, Milena
Re: [Intel-wired-lan] [PATCH v2 1/1] ixgbe: Correct BASE-BX10 compliance code
On Thu, 2024-11-14 at 19:50 +, Tore Amundsen wrote: > The current value in the source code is 0x64, which appears to be a > mix-up of hex and decimal values. A value of 0x64 (binary 01100100) > incorrectly sets bit 2 (1000BASE-CX) and bit 5 (100BASE-FX) as well. > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h > b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h > index 14aa2ca51f70..81179c60af4e 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h > @@ -40,7 +40,7 @@ > #define IXGBE_SFF_1GBASESX_CAPABLE 0x1 > #define IXGBE_SFF_1GBASELX_CAPABLE 0x2 > #define IXGBE_SFF_1GBASET_CAPABLE0x8 > -#define IXGBE_SFF_BASEBX10_CAPABLE 0x64 > +#define IXGBE_SFF_BASEBX10_CAPABLE 0x40 > #define IXGBE_SFF_10GBASESR_CAPABLE 0x10 > #define IXGBE_SFF_10GBASELR_CAPABLE 0x20 > #define IXGBE_SFF_SOFT_RS_SELECT_MASK0x8 LGMT. Acked-by: Ernesto Castellotti Kind regards, Ernesto
Re: [Intel-wired-lan] [PATCH iwl-next v4444] i40e: add ability to reset VF for Tx and Rx MDD events
> -Original Message-
> From: Michal Schmidt
> Sent: Monday, November 4, 2024 4:41 PM
> To: Loktionov, Aleksandr
> Cc: intel-wired-...@lists.osuosl.org; Nguyen, Anthony L
> ; net...@vger.kernel.org; Sokolowski,
> Jan ; Connolly, Padraig J
> ; Fijalkowski, Maciej
> ; Kitszel, Przemyslaw
>
> Subject: Re: [PATCH iwl-next v] i40e: add ability to reset VF
> for Tx and Rx MDD events
>
> On Tue, Oct 29, 2024 at 1:36 PM Aleksandr Loktionov
> wrote:
> > Implement "mdd-auto-reset-vf" priv-flag to handle Tx and Rx MDD
> events for VFs.
> > This flag is also used in other network adapters like ICE.
> >
> > Usage:
> > - "on" - The problematic VF will be automatically reset
> > if a malformed descriptor is detected.
> > - "off" - The problematic VF will be disabled.
> >
> > In cases where a VF sends malformed packets classified as
> malicious,
> > it can cause the Tx queue to freeze, rendering it unusable for
> several
> > minutes. When an MDD event occurs, this new implementation allows
> for
> > a graceful VF reset to quickly restore operational state.
> >
> > Currently, VF iqueues are disabled if an MDD event occurs. This
> patch
> > adds the
>
> s/iqueues/queues/
Thanx
>
> > ability to reset the VF if a Tx or Rx MDD event occurs. It also
> > includes MDD event logging throttling to avoid dmesg pollution
> and
> > unifies the format of Tx and Rx MDD messages.
> >
> > Note: Standard message rate limiting functions like
> > dev_info_ratelimited() do not meet our requirements. Custom rate
> > limiting is implemented, please see the code for details.
>
> I am not opposed to the custom rate-limiting, but have you also
> considered struct ratelimit_state, ratelimit_state_{init,exit}(),
> __ratelimit()?
Ok I'll try, I hope it won't change behavior too much.
> > Co-developed-by: Jan Sokolowski
> > Signed-off-by: Jan Sokolowski
> > Co-developed-by: Padraig J Connolly
>
> > Signed-off-by: Padraig J Connolly
> > Signed-off-by: Aleksandr Loktionov
>
> > ---
> > drivers/net/ethernet/intel/i40e/i40e.h| 4 +-
> > .../net/ethernet/intel/i40e/i40e_debugfs.c| 2 +-
> > .../net/ethernet/intel/i40e/i40e_ethtool.c| 2 +
> > drivers/net/ethernet/intel/i40e/i40e_main.c | 105
> --
> > .../ethernet/intel/i40e/i40e_virtchnl_pf.c| 2 +-
> > .../ethernet/intel/i40e/i40e_virtchnl_pf.h| 11 +-
> > 6 files changed, 111 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
> > b/drivers/net/ethernet/intel/i40e/i40e.h
> > index d546567..6d6683c 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e.h
> > +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> > @@ -87,6 +87,7 @@ enum i40e_state {
> > __I40E_SERVICE_SCHED,
> > __I40E_ADMINQ_EVENT_PENDING,
> > __I40E_MDD_EVENT_PENDING,
> > + __I40E_MDD_VF_PRINT_PENDING,
> > __I40E_VFLR_EVENT_PENDING,
> > __I40E_RESET_RECOVERY_PENDING,
> > __I40E_TIMEOUT_RECOVERY_PENDING, @@ -190,6 +191,7 @@ enum
> > i40e_pf_flags {
> > */
> > I40E_FLAG_TOTAL_PORT_SHUTDOWN_ENA,
> > I40E_FLAG_VF_VLAN_PRUNING_ENA,
> > + I40E_FLAG_MDD_AUTO_RESET_VF,
> > I40E_PF_FLAGS_NBITS,/* must be last */
> > };
> >
> > @@ -571,7 +573,7 @@ struct i40e_pf {
> > int num_alloc_vfs; /* actual number of VFs allocated
> */
> > u32 vf_aq_requests;
> > u32 arq_overflows; /* Not fatal, possibly indicative
> of problems */
> > -
> > + unsigned long last_printed_mdd_jiffies; /* MDD message
> rate
> > + limit */
> > /* DCBx/DCBNL capability for PF that indicates
> > * whether DCBx is managed by firmware or host
> > * based agent (LLDPAD). Also, indicates what diff --git
> > a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> > b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> > index abf624d..6a697bf 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> > @@ -721,7 +721,7 @@ static void i40e_dbg_dump_vf(struct i40e_pf
> *pf, int vf_id)
> > dev_info(&pf->pdev->dev, "vf %2d: VSI id=%d,
> seid=%d, qps=%d\n",
> > vf_id, vf->lan_vsi_id, vsi->seid, vf-
> >num_queue_pairs);
> > dev_info(&pf->pdev->dev, " num MDD=%lld\n",
> > -vf->num_mdd_events);
> > +vf->mdd_tx_events.count +
> > + vf->mdd_rx_events.count);
> > } else {
> > dev_info(&pf->pdev->dev, "invalid VF id %d\n",
> vf_id);
> > }
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > index 1d0d2e5..d146575 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > @@ -459,6 +459,8 @@ static const struct i40e_priv_flags
> i40e_gstrings_priv_flags[] = {
> > I40E_PRI
Re: [Intel-wired-lan] [PATCH iwl-net 04/10] idpf: negotiate PTP capabilies and get PTP clock
On 11/14/2024 9:57 PM, Willem de Bruijn wrote:
> Vadim Fedorenko wrote:
>
> > > +/**
> > > + * idpf_ptp_read_src_clk_reg_direct - Read directly the main timer value
> > > + * @adapter: Driver specific private structure
> > > + * @sts: Optional parameter for holding a pair of system timestamps from
> > > + *the system clock. Will be ignored when NULL is given.
> > > + *
> > > + * Return: the device clock time on success, -errno otherwise.
> > > + */
> > > +static u64 idpf_ptp_read_src_clk_reg_direct(struct idpf_adapter *adapter,
> > > + struct ptp_system_timestamp *sts)
> > > +{
> > > + struct idpf_ptp *ptp = adapter->ptp;
> > > + u32 hi, lo;
> > > +
> > > + /* Read the system timestamp pre PHC read */
> > > + ptp_read_system_prets(sts);
> > > +
> > > + idpf_ptp_enable_shtime(adapter);
> > > + lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
> > > +
> > > + /* Read the system timestamp post PHC read */
> > > + ptp_read_system_postts(sts);
> > > +
> > > + hi = readl(ptp->dev_clk_regs.dev_clk_ns_h);
> > > +
> > > + return ((u64)hi << 32) | lo;
> > > +}
> >
> > Am I right that idpf_ptp_enable_shtime() "freezes" the time in clk
> > registers and you can be sure that no changes will happen while you are
> > doing 2 transactions? If yes, then what does unfreeze it? Or does it
> > just copy new values to the registers and they will stay until the next
> > command?
>
> Yep, these are shadow registers.
>
> I guess they remain until overwritten on the next latch.
Right, these are shadow registers and the idea is to latch these values
exactly at the same time. As Willem mentioned, they remain until the next
command exec is performed.
Thanks,
Milena
[Intel-wired-lan] [PATCH iwl-next v5] i40e: add ability to reset VF for Tx and Rx MDD events
Implement "mdd-auto-reset-vf" priv-flag to handle Tx and Rx MDD events for VFs.
This flag is also used in other network adapters like ICE.
Usage:
- "on" - The problematic VF will be automatically reset
if a malformed descriptor is detected.
- "off" - The problematic VF will be disabled.
In cases where a VF sends malformed packets classified as malicious, it can
cause the Tx queue to freeze, rendering it unusable for several minutes. When
an MDD event occurs, this new implementation allows for a graceful VF reset to
quickly restore operational state.
Currently, VF queues are disabled if an MDD event occurs. This patch adds the
ability to reset the VF if a Tx or Rx MDD event occurs. It also includes MDD
event logging throttling to avoid dmesg pollution and unifies the format of
Tx and Rx MDD messages.
Note: Standard message rate limiting functions like dev_info_ratelimited()
do not meet our requirements. Custom rate limiting is implemented,
please see the code for details.
Co-developed-by: Jan Sokolowski
Signed-off-by: Jan Sokolowski
Co-developed-by: Padraig J Connolly
Signed-off-by: Padraig J Connolly
Signed-off-by: Aleksandr Loktionov
---
v4->v5 + documentation - 2nd clear_bit(__I40E_MDD_EVENT_PENDING) * rate limit
v3->v4 refactor two helper functions into one
v2->v3 fix compilation issue
v1->v2 fix compilation issue
---
.../device_drivers/ethernet/intel/i40e.rst| 12 ++
drivers/net/ethernet/intel/i40e/i40e.h| 4 +-
.../net/ethernet/intel/i40e/i40e_debugfs.c| 2 +-
.../net/ethernet/intel/i40e/i40e_ethtool.c| 2 +
drivers/net/ethernet/intel/i40e/i40e_main.c | 107 +++---
.../ethernet/intel/i40e/i40e_virtchnl_pf.c| 2 +-
.../ethernet/intel/i40e/i40e_virtchnl_pf.h| 11 +-
7 files changed, 123 insertions(+), 17 deletions(-)
diff --git a/Documentation/networking/device_drivers/ethernet/intel/i40e.rst
b/Documentation/networking/device_drivers/ethernet/intel/i40e.rst
index 4fbaa1a..53d9d58 100644
--- a/Documentation/networking/device_drivers/ethernet/intel/i40e.rst
+++ b/Documentation/networking/device_drivers/ethernet/intel/i40e.rst
@@ -299,6 +299,18 @@ Use ethtool to view and set link-down-on-close, as
follows::
ethtool --show-priv-flags ethX
ethtool --set-priv-flags ethX link-down-on-close [on|off]
+Setting the mdd-auto-reset-vf Private Flag
+--
+
+When the mdd-auto-reset-vf private flag is set to "on", the problematic VF will
+be automatically reset if a malformed descriptor is detected. If the flag is
+set to "off", the problematic VF will be disabled.
+
+Use ethtool to view and set mdd-auto-reset-vf, as follows::
+
+ ethtool --show-priv-flags ethX
+ ethtool --set-priv-flags ethX mdd-auto-reset-vf [on|off]
+
Viewing Link Messages
-
Link messages will not be displayed to the console if the distribution is
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
b/drivers/net/ethernet/intel/i40e/i40e.h
index d546567..ee5d8d6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -87,6 +87,7 @@ enum i40e_state {
__I40E_SERVICE_SCHED,
__I40E_ADMINQ_EVENT_PENDING,
__I40E_MDD_EVENT_PENDING,
+ __I40E_MDD_VF_PRINT_PENDING,
__I40E_VFLR_EVENT_PENDING,
__I40E_RESET_RECOVERY_PENDING,
__I40E_TIMEOUT_RECOVERY_PENDING,
@@ -190,6 +191,7 @@ enum i40e_pf_flags {
*/
I40E_FLAG_TOTAL_PORT_SHUTDOWN_ENA,
I40E_FLAG_VF_VLAN_PRUNING_ENA,
+ I40E_FLAG_MDD_AUTO_RESET_VF,
I40E_PF_FLAGS_NBITS,/* must be last */
};
@@ -571,7 +573,7 @@ struct i40e_pf {
int num_alloc_vfs; /* actual number of VFs allocated */
u32 vf_aq_requests;
u32 arq_overflows; /* Not fatal, possibly indicative of problems */
-
+ struct ratelimit_state mdd_message_rate_limit;
/* DCBx/DCBNL capability for PF that indicates
* whether DCBx is managed by firmware or host
* based agent (LLDPAD). Also, indicates what
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index abf624d..6a697bf 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -721,7 +721,7 @@ static void i40e_dbg_dump_vf(struct i40e_pf *pf, int vf_id)
dev_info(&pf->pdev->dev, "vf %2d: VSI id=%d, seid=%d, qps=%d\n",
vf_id, vf->lan_vsi_id, vsi->seid, vf->num_queue_pairs);
dev_info(&pf->pdev->dev, " num MDD=%lld\n",
-vf->num_mdd_events);
+vf->mdd_tx_events.count + vf->mdd_rx_events.count);
} else {
dev_info(&pf->pdev->dev, "invalid VF id %d\n", vf_id);
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 1d0d2e5..d146575 100644
--- a/
