Re: [Intel-wired-lan] [PATCH v2 iwl-net 1/3] ice: Fix improper extts handling
> -Original Message-
> From: Intel-wired-lan On Behalf Of Karol
> Kolacinski
> Sent: Thursday, June 20, 2024 5:57 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: net...@vger.kernel.org; Kolacinski, Karol ;
> Nguyen, Anthony L ; Kitszel, Przemyslaw
> ; Keller, Jacob E ;
> Olech, Milena
> Subject: [Intel-wired-lan] [PATCH v2 iwl-net 1/3] ice: Fix improper extts
> handling
>
> From: Milena Olech
>
> Extts events are disabled and enabled by the application ts2phc.
> However, in case where the driver is removed when the application is running,
> channel remains enabled. As a result, in the next run of the app, two
> channels are enabled and the information "extts on unexpected channel" is
> printed to the user.
>
> To avoid that, extts events shall be disabled when PTP is released.
>
> Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
> Reviewed-by: Przemek Kitszel
> Co-developed-by: Jacob Keller
> Signed-off-by: Jacob Keller
> Signed-off-by: Milena Olech
> Signed-off-by: Karol Kolacinski
> ---
> V1 -> V2: removed extra space and fixed return value in
> ice_ptp_gpio_enable_e823()
>
> drivers/net/ethernet/intel/ice/ice_ptp.c | 106 ++-
> drivers/net/ethernet/intel/ice/ice_ptp.h | 8 ++
> 2 files changed, 92 insertions(+), 22 deletions(-)
>
Tested-by: Pucha Himasekhar Reddy (A
Contingent worker at Intel)
Re: [Intel-wired-lan] [PATCH v2 iwl-net 2/3] ice: Don't process extts if PTP is disabled
> -Original Message-
> From: Intel-wired-lan On Behalf Of Karol
> Kolacinski
> Sent: Thursday, June 20, 2024 5:57 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: Keller, Jacob E ; net...@vger.kernel.org;
> Kolacinski, Karol ; Nguyen, Anthony L
> ; Kitszel, Przemyslaw
>
> Subject: [Intel-wired-lan] [PATCH v2 iwl-net 2/3] ice: Don't process extts if
> PTP is disabled
>
> From: Jacob Keller
>
> The ice_ptp_extts_event() function can race with ice_ptp_release() and result
> in a NULL pointer dereference which leads to a kernel panic.
>
> Panic occurs because the ice_ptp_extts_event() function calls
> ptp_clock_event() with a NULL pointer. The ice driver has already released
> the PTP clock by the time the interrupt for the next external timestamp event
> occurs.
>
> To fix this, modify the ice_ptp_extts_event() function to check the PTP state
> and bail early if PTP is not ready.
>
> Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
> Reviewed-by: Przemek Kitszel
> Signed-off-by: Jacob Keller
> Signed-off-by: Karol Kolacinski
> ---
> V1 -> V2: removed unnecessary hunk of code and adjusted commit message
>
> drivers/net/ethernet/intel/ice/ice_ptp.c | 4
> 1 file changed, 4 insertions(+)
>
Tested-by: Pucha Himasekhar Reddy (A
Contingent worker at Intel)
Re: [Intel-wired-lan] [PATCH v2 iwl-net 3/3] ice: Reject pin requests with unsupported flags
> -Original Message-
> From: Intel-wired-lan On Behalf Of Karol
> Kolacinski
> Sent: Thursday, June 20, 2024 5:57 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: Keller, Jacob E ; net...@vger.kernel.org;
> Kolacinski, Karol ; Nguyen, Anthony L
> ; Kitszel, Przemyslaw
>
> Subject: [Intel-wired-lan] [PATCH v2 iwl-net 3/3] ice: Reject pin requests
> with unsupported flags
>
> From: Jacob Keller
>
> The driver receives requests for configuring pins via the .enable callback of
> the PTP clock object. These requests come into the driver with flags which
> modify the requested behavior from userspace. Current implementation in ice
> does not reject flags that it doesn't support.
> This causes the driver to incorrectly apply requests with such flags as
> PTP_PEROUT_DUTY_CYCLE, or any future flags added by the kernel which it is
> not yet aware of.
>
> Fix this by properly validating flags in both ice_ptp_cfg_perout and
> ice_ptp_cfg_extts. Ensure that we check by bit-wise negating supported flags
> rather than just checking and rejecting known un-supported flags.
> This is preferable, as it ensures better compatibility with future kernels.
>
> Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
> Reviewed-by: Przemek Kitszel
> Signed-off-by: Jacob Keller
> Signed-off-by: Karol Kolacinski
> ---
> V1 -> V2: adjusted indentation and added NULL config pointer check
>
> drivers/net/ethernet/intel/ice/ice_ptp.c | 38 ++--
> drivers/net/ethernet/intel/ice/ice_ptp.h | 1 +
> 2 files changed, 23 insertions(+), 16 deletions(-)
>
Tested-by: Pucha Himasekhar Reddy (A
Contingent worker at Intel)
Re: [Intel-wired-lan] [PATCH iwl-next v2 7/7] ice: Add tracepoint for adding and removing switch rules
On 6/24/24 16:45, Marcin Szycik wrote: Track the number of rules and recipes added to switch. Add a tracepoint to ice_aq_sw_rules(), which shows both rule and recipe count. This information can be helpful when designing a set of rules to program to the hardware, as it shows where the practical limit is. Actual limits are known (64 recipes, 32k rules), but it's hard to translate these values to how many rules the *user* can actually create, because of extra metadata being implicitly added, and recipe/rule chaining. Chaining combines several recipes/rules to create a larger recipe/rule, so one large rule added by the user might actually consume multiple rules from hardware perspective. Rule counter is simply incremented/decremented in ice_aq_sw_rules(), since all rules are added or removed via it. Counting recipes is harder, as recipes can't be removed (only overwritten). Recipes added via ice_aq_add_recipe() could end up being unused, when there is an error in later stages of rule creation. Instead, track the allocation and freeing of recipes, which should reflect the actual usage of recipes (if something fails after recipe(s) were created, caller should free them). Also, a number of recipes are loaded from NVM by default - initialize the recipe counter with the number of these recipes on switch initialization. Reviewed-by: Michal Swiatkowski Signed-off-by: Marcin Szycik --- drivers/net/ethernet/intel/ice/ice_common.c | 3 +++ drivers/net/ethernet/intel/ice/ice_switch.c | 22 +++-- drivers/net/ethernet/intel/ice/ice_trace.h | 18 + drivers/net/ethernet/intel/ice/ice_type.h | 2 ++ 4 files changed, 43 insertions(+), 2 deletions(-) Reviewed-by: Przemek Kitszel
Re: [Intel-wired-lan] [PATCH iwl-next v2 7/7] ice: Add tracepoint for adding and removing switch rules
Dear Marcin,
Thank you for your patch.
Am 24.06.24 um 16:45 schrieb Marcin Szycik:
Track the number of rules and recipes added to switch. Add a tracepoint to
ice_aq_sw_rules(), which shows both rule and recipe count. This information
can be helpful when designing a set of rules to program to the hardware, as
it shows where the practical limit is. Actual limits are known (64 recipes,
32k rules), but it's hard to translate these values to how many rules the
*user* can actually create, because of extra metadata being implicitly
added, and recipe/rule chaining. Chaining combines several recipes/rules to
create a larger recipe/rule, so one large rule added by the user might
actually consume multiple rules from hardware perspective.
Rule counter is simply incremented/decremented in ice_aq_sw_rules(), since
all rules are added or removed via it.
Counting recipes is harder, as recipes can't be removed (only overwritten).
Recipes added via ice_aq_add_recipe() could end up being unused, when
there is an error in later stages of rule creation. Instead, track the
allocation and freeing of recipes, which should reflect the actual usage of
recipes (if something fails after recipe(s) were created, caller should
free them). Also, a number of recipes are loaded from NVM by default -
initialize the recipe counter with the number of these recipes on switch
initialization.
Can you please add an example how to use the tracepoint?
Reviewed-by: Michal Swiatkowski
Signed-off-by: Marcin Szycik
---
drivers/net/ethernet/intel/ice/ice_common.c | 3 +++
drivers/net/ethernet/intel/ice/ice_switch.c | 22 +++--
drivers/net/ethernet/intel/ice/ice_trace.h | 18 +
drivers/net/ethernet/intel/ice/ice_type.h | 2 ++
4 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index 6abd1b3796ab..009716a12a26 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -934,6 +934,9 @@ static int ice_init_fltr_mgmt_struct(struct ice_hw *hw)
INIT_LIST_HEAD(&sw->vsi_list_map_head);
sw->prof_res_bm_init = 0;
+ /* Initialize recipe count with default recipes read from NVM */
+ sw->recp_cnt = ICE_SW_LKUP_LAST;
+
status = ice_init_def_sw_recp(hw);
if (status) {
devm_kfree(ice_hw_to_dev(hw), hw->switch_info);
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c
b/drivers/net/ethernet/intel/ice/ice_switch.c
index 27828cdfe085..3caafcdc301f 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -3,6 +3,7 @@
#include "ice_lib.h"
#include "ice_switch.h"
+#include "ice_trace.h"
#define ICE_ETH_DA_OFFSET 0
#define ICE_ETH_ETHTYPE_OFFSET12
@@ -1961,6 +1962,15 @@ ice_aq_sw_rules(struct ice_hw *hw, void *rule_list, u16
rule_list_sz,
hw->adminq.sq_last_status == ICE_AQ_RC_ENOENT)
status = -ENOENT;
+ if (!status) {
+ if (opc == ice_aqc_opc_add_sw_rules)
+ hw->switch_info->rule_cnt += num_rules;
+ else if (opc == ice_aqc_opc_remove_sw_rules)
+ hw->switch_info->rule_cnt -= num_rules;
+ }
+
+ trace_ice_aq_sw_rules(hw->switch_info);
+
return status;
}
@@ -2181,8 +2191,10 @@ int ice_alloc_recipe(struct ice_hw *hw, u16 *rid)
sw_buf->res_type = cpu_to_le16(res_type);
status = ice_aq_alloc_free_res(hw, sw_buf, buf_len,
ice_aqc_opc_alloc_res);
- if (!status)
+ if (!status) {
*rid = le16_to_cpu(sw_buf->elem[0].e.sw_resp);
+ hw->switch_info->recp_cnt++;
+ }
return status;
}
@@ -2196,7 +2208,13 @@ int ice_alloc_recipe(struct ice_hw *hw, u16 *rid)
*/
static int ice_free_recipe_res(struct ice_hw *hw, u16 rid)
{
- return ice_free_hw_res(hw, ICE_AQC_RES_TYPE_RECIPE, 1, &rid);
+ int status;
+
+ status = ice_free_hw_res(hw, ICE_AQC_RES_TYPE_RECIPE, 1, &rid);
+ if (!status)
+ hw->switch_info->recp_cnt--;
+
+ return status;
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_trace.h
b/drivers/net/ethernet/intel/ice/ice_trace.h
index 244cddd2a9ea..07aab6e130cd 100644
--- a/drivers/net/ethernet/intel/ice/ice_trace.h
+++ b/drivers/net/ethernet/intel/ice/ice_trace.h
@@ -330,6 +330,24 @@ DEFINE_EVENT(ice_esw_br_port_template,
TP_ARGS(port)
);
+DECLARE_EVENT_CLASS(ice_switch_stats_template,
+ TP_PROTO(struct ice_switch_info *sw_info),
+ TP_ARGS(sw_info),
+ TP_STRUCT__entry(__field(u16, rule_cnt)
+__field(u8, recp_cnt)),
+ TP_fast_assign(__entry->rule_cnt = sw_info->rule_cnt;
+ __entry->re
Re: [Intel-wired-lan] [PATCH iwl-next v2 01/14] cache: add __cacheline_group_{begin, end}_aligned() (+ couple more)
From: Przemek Kitszel
Date: Thu, 20 Jun 2024 17:15:53 +0200
> On 6/20/24 15:53, Alexander Lobakin wrote:
>> __cacheline_group_begin(), unfortunately, doesn't align the group
>> anyhow. If it is wanted, then you need to do something like
>>
>> __cacheline_group_begin(grp) __aligned(ALIGN)
>>
>> which isn't really convenient nor compact.
>> Add the _aligned() counterparts to align the groups automatically to
>> either the specified alignment (optional) or ``SMP_CACHE_BYTES``.
>> Note that the actual struct layout will then be (on x64 with 64-byte CL):
>>
>> struct x {
>> u32 y; // offset 0, size 4, padding 56
>> __cacheline_group_begin__grp; // offset 64, size 0
>> u32 z; // offset 64, size 4, padding 4
>> __cacheline_group_end__grp; // offset 72, size 0
>> __cacheline_group_pad__grp; // offset 72, size 0, padding 56
>> u32 w; // offset 128
>> };
>>
>> The end marker is aligned to long, so that you can assert the struct
>> size more strictly, but the offset of the next field in the structure
>> will be aligned to the group alignment, so that the next field won't
>> fall into the group it's not intended to.
>>
>> Add __LARGEST_ALIGN definition and LARGEST_ALIGN() macro.
>> __LARGEST_ALIGN is the value to which the compilers align fields when
>> __aligned_largest is specified. Sometimes, it might be needed to get
>> this value outside of variable definitions. LARGEST_ALIGN() is macro
>> which just aligns a value to __LARGEST_ALIGN.
>> Also add SMP_CACHE_ALIGN(), similar to L1_CACHE_ALIGN(), but using
>> ``SMP_CACHE_BYTES`` instead of ``L1_CACHE_BYTES`` as the former
>> also accounts L2, needed in some cases.
>>
>> Signed-off-by: Alexander Lobakin
>> ---
>> include/linux/cache.h | 59 +++
>> 1 file changed, 59 insertions(+)
>>
>
> [...]
>
>> +/**
>> + * __cacheline_group_begin_aligned - declare an aligned group start
>> + * @GROUP: name of the group
>> + * @...: optional group alignment
>
> didn't know that you could document "..." :)
>
>> + *
>> + * The following block inside a struct:
>> + *
>> + * __cacheline_group_begin_aligned(grp);
>> + * field a;
>> + * field b;
>> + * __cacheline_group_end_aligned(grp);
>> + *
>> + * will always be aligned to either the specified alignment or
>> + * ``SMP_CACHE_BYTES``.
>> + */
>> +#define __cacheline_group_begin_aligned(GROUP, ...) \
>> + __cacheline_group_begin(GROUP) \
>> + __aligned((__VA_ARGS__ + 0) ? : SMP_CACHE_BYTES)
>
> nice trick :) +0
The usual way to handle varargs. However, this one:
__cacheline_group_begin_aligned(grp, 63 & 31);
will trigger a compiler warning as it expands to
__aligned(63 & 31 + 0)
The compilers don't like bitops and arithmetic ops not separated by
parenthesis even in such simple case =\
Thanks,
Olek
[Intel-wired-lan] [PATCH net-next v9 04/13] mm: page_frag: add '_va' suffix to page_frag API
Currently the page_frag API is returning 'virtual address'
or 'va' when allocing and expecting 'virtual address' or
'va' as input when freeing.
As we are about to support new use cases that the caller
need to deal with 'struct page' or need to deal with both
'va' and 'struct page'. In order to differentiate the API
handling between 'va' and 'struct page', add '_va' suffix
to the corresponding API mirroring the page_pool_alloc_va()
API of the page_pool. So that callers expecting to deal with
va, page or both va and page may call page_frag_alloc_va*,
page_frag_alloc_pg*, or page_frag_alloc* API accordingly.
CC: Alexander Duyck
Signed-off-by: Yunsheng Lin
---
drivers/net/ethernet/google/gve/gve_rx.c | 4 ++--
drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +-
drivers/net/ethernet/intel/ice/ice_txrx.h | 2 +-
drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 2 +-
.../net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++--
.../marvell/octeontx2/nic/otx2_common.c | 2 +-
drivers/net/ethernet/mediatek/mtk_wed_wo.c| 4 ++--
drivers/nvme/host/tcp.c | 8 +++
drivers/nvme/target/tcp.c | 22 +--
drivers/vhost/net.c | 6 ++---
include/linux/page_frag_cache.h | 21 +-
include/linux/skbuff.h| 2 +-
kernel/bpf/cpumap.c | 2 +-
mm/page_frag_cache.c | 12 +-
mm/page_frag_test.c | 13 ++-
net/core/skbuff.c | 14 ++--
net/core/xdp.c| 2 +-
net/rxrpc/txbuf.c | 15 +++--
net/sunrpc/svcsock.c | 6 ++---
19 files changed, 74 insertions(+), 69 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve_rx.c
b/drivers/net/ethernet/google/gve/gve_rx.c
index acb73d4d0de6..b6c10100e462 100644
--- a/drivers/net/ethernet/google/gve/gve_rx.c
+++ b/drivers/net/ethernet/google/gve/gve_rx.c
@@ -729,7 +729,7 @@ static int gve_xdp_redirect(struct net_device *dev, struct
gve_rx_ring *rx,
total_len = headroom + SKB_DATA_ALIGN(len) +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
- frame = page_frag_alloc(&rx->page_cache, total_len, GFP_ATOMIC);
+ frame = page_frag_alloc_va(&rx->page_cache, total_len, GFP_ATOMIC);
if (!frame) {
u64_stats_update_begin(&rx->statss);
rx->xdp_alloc_fails++;
@@ -742,7 +742,7 @@ static int gve_xdp_redirect(struct net_device *dev, struct
gve_rx_ring *rx,
err = xdp_do_redirect(dev, &new, xdp_prog);
if (err)
- page_frag_free(frame);
+ page_frag_free_va(frame);
return err;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c
b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 8bb743f78fcb..399b317c509d 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -126,7 +126,7 @@ ice_unmap_and_free_tx_buf(struct ice_tx_ring *ring, struct
ice_tx_buf *tx_buf)
dev_kfree_skb_any(tx_buf->skb);
break;
case ICE_TX_BUF_XDP_TX:
- page_frag_free(tx_buf->raw_buf);
+ page_frag_free_va(tx_buf->raw_buf);
break;
case ICE_TX_BUF_XDP_XMIT:
xdp_return_frame(tx_buf->xdpf);
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h
b/drivers/net/ethernet/intel/ice/ice_txrx.h
index feba314a3fe4..6379f57d8228 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -148,7 +148,7 @@ static inline int ice_skb_pad(void)
* @ICE_TX_BUF_DUMMY: dummy Flow Director packet, unmap and kfree()
* @ICE_TX_BUF_FRAG: mapped skb OR &xdp_buff frag, only unmap DMA
* @ICE_TX_BUF_SKB: &sk_buff, unmap and consume_skb(), update stats
- * @ICE_TX_BUF_XDP_TX: &xdp_buff, unmap and page_frag_free(), stats
+ * @ICE_TX_BUF_XDP_TX: &xdp_buff, unmap and page_frag_free_va(), stats
* @ICE_TX_BUF_XDP_XMIT: &xdp_frame, unmap and xdp_return_frame(), stats
* @ICE_TX_BUF_XSK_TX: &xdp_buff on XSk queue, xsk_buff_free(), stats
*/
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
index 2719f0e20933..a1a41a14df0d 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
@@ -250,7 +250,7 @@ ice_clean_xdp_tx_buf(struct device *dev, struct ice_tx_buf
*tx_buf,
switch (tx_buf->type) {
case ICE_TX_BUF_XDP_TX:
- page_frag_free(tx_buf->raw_buf);
+ page_frag_free_va(tx_buf->raw_buf);
break;
case ICE_TX_BUF_XDP_XMIT:
xdp_return_frame_bulk(tx_buf->xdpf, bq);
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
b/drivers/net/ethernet/intel/ixgbevf/ix
[Intel-wired-lan] [PATCH net 1/1] igc: Fix double reset adapter triggered from a single taprio cmd
Following the implementation of "igc: Add TransmissionOverrun counter"
patch, when a taprio command is triggered by user, igc processes two
commands: TAPRIO_CMD_REPLACE followed by TAPRIO_CMD_STATS. However, both
commands unconditionally pass through igc_tsn_offload_apply() which
evaluates and triggers reset adapter. The double reset causes issues in
the calculation of adapter->qbv_count in igc.
TAPRIO_CMD_REPLACE command is expected to reset the adapter since it
activates qbv. It's unexpected for TAPRIO_CMD_STATS to do the same
because it doesn't configure any driver-specific TSN settings. So, the
evaluation in igc_tsn_offload_apply() isn't needed for TAPRIO_CMD_STATS.
To address this, commands parsing are relocated to
igc_tsn_enable_qbv_scheduling(). Commands that don't require an adapter
reset will exit after processing, thus avoiding igc_tsn_offload_apply().
Fixes: d3750076d464 ("igc: Add TransmissionOverrun counter")
Signed-off-by: Faizal Rahim
---
drivers/net/ethernet/intel/igc/igc_main.c | 33 ---
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
b/drivers/net/ethernet/intel/igc/igc_main.c
index 87b655b839c1..33069880c86c 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6310,21 +6310,6 @@ static int igc_save_qbv_schedule(struct igc_adapter
*adapter,
size_t n;
int i;
- switch (qopt->cmd) {
- case TAPRIO_CMD_REPLACE:
- break;
- case TAPRIO_CMD_DESTROY:
- return igc_tsn_clear_schedule(adapter);
- case TAPRIO_CMD_STATS:
- igc_taprio_stats(adapter->netdev, &qopt->stats);
- return 0;
- case TAPRIO_CMD_QUEUE_STATS:
- igc_taprio_queue_stats(adapter->netdev, &qopt->queue_stats);
- return 0;
- default:
- return -EOPNOTSUPP;
- }
-
if (qopt->base_time < 0)
return -ERANGE;
@@ -6433,7 +6418,23 @@ static int igc_tsn_enable_qbv_scheduling(struct
igc_adapter *adapter,
if (hw->mac.type != igc_i225)
return -EOPNOTSUPP;
- err = igc_save_qbv_schedule(adapter, qopt);
+ switch (qopt->cmd) {
+ case TAPRIO_CMD_REPLACE:
+ err = igc_save_qbv_schedule(adapter, qopt);
+ break;
+ case TAPRIO_CMD_DESTROY:
+ err = igc_tsn_clear_schedule(adapter);
+ break;
+ case TAPRIO_CMD_STATS:
+ igc_taprio_stats(adapter->netdev, &qopt->stats);
+ return 0;
+ case TAPRIO_CMD_QUEUE_STATS:
+ igc_taprio_queue_stats(adapter->netdev, &qopt->queue_stats);
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+
if (err)
return err;
--
2.25.1
Re: [Intel-wired-lan] [PATCH iwl-next v2 7/7] ice: Add tracepoint for adding and removing switch rules
On 25.06.2024 10:31, Paul Menzel wrote:
> Dear Marcin,
>
>
> Thank you for your patch.
>
> Am 24.06.24 um 16:45 schrieb Marcin Szycik:
>> Track the number of rules and recipes added to switch. Add a tracepoint to
>> ice_aq_sw_rules(), which shows both rule and recipe count. This information
>> can be helpful when designing a set of rules to program to the hardware, as
>> it shows where the practical limit is. Actual limits are known (64 recipes,
>> 32k rules), but it's hard to translate these values to how many rules the
>> *user* can actually create, because of extra metadata being implicitly
>> added, and recipe/rule chaining. Chaining combines several recipes/rules to
>> create a larger recipe/rule, so one large rule added by the user might
>> actually consume multiple rules from hardware perspective.
>>
>> Rule counter is simply incremented/decremented in ice_aq_sw_rules(), since
>> all rules are added or removed via it.
>>
>> Counting recipes is harder, as recipes can't be removed (only overwritten).
>> Recipes added via ice_aq_add_recipe() could end up being unused, when
>> there is an error in later stages of rule creation. Instead, track the
>> allocation and freeing of recipes, which should reflect the actual usage of
>> recipes (if something fails after recipe(s) were created, caller should
>> free them). Also, a number of recipes are loaded from NVM by default -
>> initialize the recipe counter with the number of these recipes on switch
>> initialization.
>
> Can you please add an example how to use the tracepoint?
Sure, will add to next version.
Example configuration:
cd /sys/kernel/tracing
echo function > current_tracer
echo ice_aq_sw_rules > set_ftrace_filter
echo ice_aq_sw_rules > set_event
echo 1 > tracing_on
cat trace
Sample output:
tc-4097[069] ...1. 787.595536: ice_aq_sw_rules <-ice_rem_adv_rule
tc-4097[069] . 787.595705: ice_aq_sw_rules: rules=9 recipes=15
tc-4098[057] ...1. 787.652033: ice_aq_sw_rules <-ice_add_adv_rule
tc-4098[057] . 787.652201: ice_aq_sw_rules: rules=10 recipes=16
Thanks,
Marcin
>> Reviewed-by: Michal Swiatkowski
>> Signed-off-by: Marcin Szycik
>> ---
>> drivers/net/ethernet/intel/ice/ice_common.c | 3 +++
>> drivers/net/ethernet/intel/ice/ice_switch.c | 22 +++--
>> drivers/net/ethernet/intel/ice/ice_trace.h | 18 +
>> drivers/net/ethernet/intel/ice/ice_type.h | 2 ++
>> 4 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
>> b/drivers/net/ethernet/intel/ice/ice_common.c
>> index 6abd1b3796ab..009716a12a26 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_common.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
>> @@ -934,6 +934,9 @@ static int ice_init_fltr_mgmt_struct(struct ice_hw *hw)
>> INIT_LIST_HEAD(&sw->vsi_list_map_head);
>> sw->prof_res_bm_init = 0;
>> + /* Initialize recipe count with default recipes read from NVM */
>> + sw->recp_cnt = ICE_SW_LKUP_LAST;
>> +
>> status = ice_init_def_sw_recp(hw);
>> if (status) {
>> devm_kfree(ice_hw_to_dev(hw), hw->switch_info);
>> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c
>> b/drivers/net/ethernet/intel/ice/ice_switch.c
>> index 27828cdfe085..3caafcdc301f 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
>> @@ -3,6 +3,7 @@
>> #include "ice_lib.h"
>> #include "ice_switch.h"
>> +#include "ice_trace.h"
>> #define ICE_ETH_DA_OFFSET 0
>> #define ICE_ETH_ETHTYPE_OFFSET 12
>> @@ -1961,6 +1962,15 @@ ice_aq_sw_rules(struct ice_hw *hw, void *rule_list,
>> u16 rule_list_sz,
>> hw->adminq.sq_last_status == ICE_AQ_RC_ENOENT)
>> status = -ENOENT;
>> + if (!status) {
>> + if (opc == ice_aqc_opc_add_sw_rules)
>> + hw->switch_info->rule_cnt += num_rules;
>> + else if (opc == ice_aqc_opc_remove_sw_rules)
>> + hw->switch_info->rule_cnt -= num_rules;
>> + }
>> +
>> + trace_ice_aq_sw_rules(hw->switch_info);
>> +
>> return status;
>> }
>> @@ -2181,8 +2191,10 @@ int ice_alloc_recipe(struct ice_hw *hw, u16 *rid)
>> sw_buf->res_type = cpu_to_le16(res_type);
>> status = ice_aq_alloc_free_res(hw, sw_buf, buf_len,
>> ice_aqc_opc_alloc_res);
>> - if (!status)
>> + if (!status) {
>> *rid = le16_to_cpu(sw_buf->elem[0].e.sw_resp);
>> + hw->switch_info->recp_cnt++;
>> + }
>> return status;
>> }
>> @@ -2196,7 +2208,13 @@ int ice_alloc_recipe(struct ice_hw *hw, u16 *rid)
>> */
>> static int ice_free_recipe_res(struct ice_hw *hw, u16 rid)
>> {
>> - return ice_free_hw_res(hw, ICE_AQC_RES_TYPE_RECIPE, 1, &rid);
>> + int status;
>> +
>> + status = ice_free_hw_res(hw, ICE_AQC_RES_TYPE_RECIPE, 1, &rid);
>> + if (!status)
>> + hw->switch_info->recp_cnt--;
>> +
>> + re
[Intel-wired-lan] [PATCH iwl-net v5] i40e: fix: remove needless retries of NVM update
Remove wrong EIO to EGAIN conversion and pass all errors as is.
After commit 230f3d53a547 ("i40e: remove i40e_status"), which should only
replace F/W specific error codes with Linux kernel generic, all EIO errors
suddenly started to be converted into EAGAIN which leads nvmupdate to retry
until it timeouts and sometimes fails after more than 20 minutes in the
middle of NVM update, so NVM becomes corrupted.
The bug affects users only at the time when they try to update NVM, and
only F/W versions that generate errors while nvmupdate. For example, X710DA2
with 0x8000ECB7 F/W is affected, but there are probably more...
Command for reproduction is just NVM update:
./nvmupdate64
In the log instead of:
i40e_nvmupd_exec_aq err I40E_ERR_ADMIN_QUEUE_ERROR aq_err I40E_AQ_RC_ENOMEM)
appears:
i40e_nvmupd_exec_aq err -EIO aq_err I40E_AQ_RC_ENOMEM
i40e: eeprom check failed (-5), Tx/Rx traffic disabled
The problematic code did silently convert EIO into EAGAIN which forced
nvmupdate to ignore EAGAIN error and retry the same operation until timeout.
That's why NVM update takes 20+ minutes to finish with the fail in the end.
Fixes: 230f3d53a547 ("i40e: remove i40e_status")
Co-developed-by: Kelvin Kang
Signed-off-by: Kelvin Kang
Reviewed-by: Arkadiusz Kubalewski
Signed-off-by: Aleksandr Loktionov
---
v4->v5 commit message update
https://lore.kernel.org/netdev/20240618132111.3193963-1-aleksandr.loktio...@intel.com/T/#u
v3->v4 commit message update
v2->v3 commit messege typos
v1->v2 commit message update
---
drivers/net/ethernet/intel/i40e/i40e_adminq.h | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
index ee86d2c..55b5bb8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
@@ -109,10 +109,6 @@ static inline int i40e_aq_rc_to_posix(int aq_ret, int
aq_rc)
-EFBIG, /* I40E_AQ_RC_EFBIG */
};
- /* aq_rc is invalid if AQ timed out */
- if (aq_ret == -EIO)
- return -EAGAIN;
-
if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]
return -ERANGE;
--
2.25.1
Re: [Intel-wired-lan] [PATCH net 1/1] igc: Fix double reset adapter triggered from a single taprio cmd
Added Vladimir and Husaini in CC.
On 25/6/2024 4:26 pm, Faizal Rahim wrote:
Following the implementation of "igc: Add TransmissionOverrun counter"
patch, when a taprio command is triggered by user, igc processes two
commands: TAPRIO_CMD_REPLACE followed by TAPRIO_CMD_STATS. However, both
commands unconditionally pass through igc_tsn_offload_apply() which
evaluates and triggers reset adapter. The double reset causes issues in
the calculation of adapter->qbv_count in igc.
TAPRIO_CMD_REPLACE command is expected to reset the adapter since it
activates qbv. It's unexpected for TAPRIO_CMD_STATS to do the same
because it doesn't configure any driver-specific TSN settings. So, the
evaluation in igc_tsn_offload_apply() isn't needed for TAPRIO_CMD_STATS.
To address this, commands parsing are relocated to
igc_tsn_enable_qbv_scheduling(). Commands that don't require an adapter
reset will exit after processing, thus avoiding igc_tsn_offload_apply().
Fixes: d3750076d464 ("igc: Add TransmissionOverrun counter")
Signed-off-by: Faizal Rahim
---
drivers/net/ethernet/intel/igc/igc_main.c | 33 ---
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
b/drivers/net/ethernet/intel/igc/igc_main.c
index 87b655b839c1..33069880c86c 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6310,21 +6310,6 @@ static int igc_save_qbv_schedule(struct igc_adapter
*adapter,
size_t n;
int i;
- switch (qopt->cmd) {
- case TAPRIO_CMD_REPLACE:
- break;
- case TAPRIO_CMD_DESTROY:
- return igc_tsn_clear_schedule(adapter);
- case TAPRIO_CMD_STATS:
- igc_taprio_stats(adapter->netdev, &qopt->stats);
- return 0;
- case TAPRIO_CMD_QUEUE_STATS:
- igc_taprio_queue_stats(adapter->netdev, &qopt->queue_stats);
- return 0;
- default:
- return -EOPNOTSUPP;
- }
-
if (qopt->base_time < 0)
return -ERANGE;
@@ -6433,7 +6418,23 @@ static int igc_tsn_enable_qbv_scheduling(struct
igc_adapter *adapter,
if (hw->mac.type != igc_i225)
return -EOPNOTSUPP;
- err = igc_save_qbv_schedule(adapter, qopt);
+ switch (qopt->cmd) {
+ case TAPRIO_CMD_REPLACE:
+ err = igc_save_qbv_schedule(adapter, qopt);
+ break;
+ case TAPRIO_CMD_DESTROY:
+ err = igc_tsn_clear_schedule(adapter);
+ break;
+ case TAPRIO_CMD_STATS:
+ igc_taprio_stats(adapter->netdev, &qopt->stats);
+ return 0;
+ case TAPRIO_CMD_QUEUE_STATS:
+ igc_taprio_queue_stats(adapter->netdev, &qopt->queue_stats);
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+
if (err)
return err;
--
2.25.1
