Re: [Intel-wired-lan] [PATCH iwl-net] ice: Fix PF with enabled XDP going no-carrier after reset
>-Original Message-
>From: Intel-wired-lan On Behalf Of
>Zaremba, Larysa
>Sent: Tuesday, December 12, 2023 2:59 PM
>To: intel-wired-...@lists.osuosl.org
>Cc: Zaremba, Larysa ; net...@vger.kernel.org;
>Brandeburg, Jesse ; Nguyen, Anthony L
>; Kitszel, Przemyslaw
>; Keller, Jacob E ;
>Michal Swiatkowski
>Subject: [Intel-wired-lan] [PATCH iwl-net] ice: Fix PF with enabled XDP going
>no-carrier after reset
>
>Commit 6624e780a577fc596788 ("ice: split ice_vsi_setup into smaller
>functions") has refactored a bunch of code involved in PFR. In this process, TC
>queue number adjustment for XDP was lost. Bring it back.
>
>Lack of such adjustment causes interface to go into no-carrier after a reset,
>if
>XDP program is attached, with the following message:
>
>ice :b1:00.0: Failed to set LAN Tx queue context, error: -22 ice
>:b1:00.0 ens801f0np0: Failed to open VSI 0x0006 on switch 0x0001 ice
>:b1:00.0: enable VSI failed, err -22, VSI index 0, type ICE_VSI_PF ice
>:b1:00.0: PF VSI rebuild failed: -22 ice :b1:00.0: Rebuild failed,
>unload and reload driver
>
>Fixes: 6624e780a577 ("ice: split ice_vsi_setup into smaller functions")
>Reviewed-by: Przemek Kitszel
>Signed-off-by: Larysa Zaremba
>---
> drivers/net/ethernet/intel/ice/ice_lib.c | 3 +++
> 1 file changed, 3 insertions(+)
>
Tested-by: Chandan Kumar Rout (A Contingent Worker at
Intel)
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/7] Add PFCP filter support
On 15.12.2023 17:49, Jakub Kicinski wrote: > On Fri, 15 Dec 2023 11:11:23 +0100 Alexander Lobakin wrote: >> Ping? :s >> Or should we resubmit? > > Can you wait for next merge window instead? > We're getting flooded with patches as everyone seemingly tries to get > their own (i.e. the most important!) work merged before the end of > the year. The set of PRs from the bitmap tree which Linus decided > not to pull is not empty. So we'd have to go figure out what's exactly > is in that branch we're supposed to pull, and whether it's fine. > It probably is, but you see, this is a problem which can be solved by > waiting, and letting Linus pull it himself. While the 150 patches we're > getting a day now have to be looked at. Let's wait to the next window then. Thanks, Marcin ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [PATCH iwl-next v5 0/2] ixgbe: Refactor ixgbe internal status
A small 2 patches series removing currently used internal status codes in ixgbe driver and converting them to the regular ones. 1st patch deals specifically with overtemp error code, the 2nd one refactors the rest of the error codes. Jedrzej Jagielski (2): ixgbe: Refactor overtemp event handling ixgbe: Refactor returning internal error codes .../net/ethernet/intel/ixgbe/ixgbe_82598.c| 36 ++--- .../net/ethernet/intel/ixgbe/ixgbe_82599.c| 61 --- .../net/ethernet/intel/ixgbe/ixgbe_common.c | 145 - .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 42 +++-- drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c | 34 ++-- drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 1 - drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 105 ++-- drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h | 2 +- .../net/ethernet/intel/ixgbe/ixgbe_sriov.c| 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 43 + drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c | 44 ++--- drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 150 +- 13 files changed, 309 insertions(+), 358 deletions(-) -- 2.31.1 ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [PATCH iwl-next v5 1/2] ixgbe: Refactor overtemp event handling
Currently ixgbe driver is notified of overheating events
via internal IXGBE_ERR_OVERTEMP error code.
Change the approach for handle_lasi() to use freshly introduced
is_overtemp function parameter which set when such event occurs.
Change check_overtemp() to bool and return true if overtemp
event occurs.
Reviewed-by: Przemek Kitszel
Signed-off-by: Jedrzej Jagielski
---
v2: change aproach to use additional function parameter to notify when overheat
v4: change check_overtemp to bool
v5: adress Simon's comments
https://lore.kernel.org/netdev/20231217093800.gx6...@kernel.org/
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++-
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 21 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 4 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 41 +++
5 files changed, 43 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 227415d61efc..687e2b0bb968 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2756,7 +2756,6 @@ static void ixgbe_check_overtemp_subtask(struct
ixgbe_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
u32 eicr = adapter->interrupt_event;
- s32 rc;
if (test_bit(__IXGBE_DOWN, &adapter->state))
return;
@@ -2790,14 +2789,13 @@ static void ixgbe_check_overtemp_subtask(struct
ixgbe_adapter *adapter)
}
/* Check if this is not due to overtemp */
- if (hw->phy.ops.check_overtemp(hw) != IXGBE_ERR_OVERTEMP)
+ if (!hw->phy.ops.check_overtemp(hw))
return;
break;
case IXGBE_DEV_ID_X550EM_A_1G_T:
case IXGBE_DEV_ID_X550EM_A_1G_T_L:
- rc = hw->phy.ops.check_overtemp(hw);
- if (rc != IXGBE_ERR_OVERTEMP)
+ if (!hw->phy.ops.check_overtemp(hw))
return;
break;
default:
@@ -7938,7 +7936,7 @@ static void ixgbe_service_timer(struct timer_list *t)
static void ixgbe_phy_interrupt_subtask(struct ixgbe_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
- u32 status;
+ bool overtemp;
if (!(adapter->flags2 & IXGBE_FLAG2_PHY_INTERRUPT))
return;
@@ -7948,11 +7946,9 @@ static void ixgbe_phy_interrupt_subtask(struct
ixgbe_adapter *adapter)
if (!hw->phy.ops.handle_lasi)
return;
- status = hw->phy.ops.handle_lasi(&adapter->hw);
- if (status != IXGBE_ERR_OVERTEMP)
- return;
-
- e_crit(drv, "%s\n", ixgbe_overheat_msg);
+ hw->phy.ops.handle_lasi(&adapter->hw, &overtemp);
+ if (overtemp)
+ e_crit(drv, "%s\n", ixgbe_overheat_msg);
}
static void ixgbe_reset_subtask(struct ixgbe_adapter *adapter)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index ca31638c6fb8..73b9c35acfd7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -407,8 +407,7 @@ s32 ixgbe_reset_phy_generic(struct ixgbe_hw *hw)
return status;
/* Don't reset PHY if it's shut down due to overtemp. */
- if (!hw->phy.reset_if_overtemp &&
- (IXGBE_ERR_OVERTEMP == hw->phy.ops.check_overtemp(hw)))
+ if (!hw->phy.reset_if_overtemp && hw->phy.ops.check_overtemp(hw))
return 0;
/* Blocked by MNG FW so bail */
@@ -2746,22 +2745,24 @@ static void ixgbe_i2c_bus_clear(struct ixgbe_hw *hw)
* @hw: pointer to hardware structure
*
* Checks if the LASI temp alarm status was triggered due to overtemp
+ *
+ * Return true when an overtemp event detected, otherwise false.
**/
-s32 ixgbe_tn_check_overtemp(struct ixgbe_hw *hw)
+bool ixgbe_tn_check_overtemp(struct ixgbe_hw *hw)
{
u16 phy_data = 0;
+ u32 status;
if (hw->device_id != IXGBE_DEV_ID_82599_T3_LOM)
- return 0;
+ return false;
/* Check that the LASI temp alarm status was triggered */
- hw->phy.ops.read_reg(hw, IXGBE_TN_LASI_STATUS_REG,
-MDIO_MMD_PMAPMD, &phy_data);
-
- if (!(phy_data & IXGBE_TN_LASI_STATUS_TEMP_ALARM))
- return 0;
+ status = hw->phy.ops.read_reg(hw, IXGBE_TN_LASI_STATUS_REG,
+ MDIO_MMD_PMAPMD, &phy_data);
+ if (status)
+ return false;
- return IXGBE_ERR_OVERTEMP;
+ return !!(phy_data & IXGBE_TN_LASI_STATUS_TEMP_ALARM);
}
/** ixgbe_set_copper_phy_power - Control power for copper phy
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
index 6544c4539c0d..ef72729d7c93 100644
--- a/drivers/net/
[Intel-wired-lan] [PATCH iwl-next v5 2/2] ixgbe: Refactor returning internal error codes
Change returning codes to the kernel ones instead of
the internal ones for the entire ixgbe driver.
Reviewed-by: Jacob Keller
Reviewed-by: Przemek Kitszel
Reviewed-by: Simon Horman
Signed-off-by: Jedrzej Jagielski
---
v3: do not use ENOSYS; rebase
---
.../net/ethernet/intel/ixgbe/ixgbe_82598.c| 36 ++---
.../net/ethernet/intel/ixgbe/ixgbe_82599.c| 61
.../net/ethernet/intel/ixgbe/ixgbe_common.c | 145 --
.../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 26 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c | 34 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 84 +-
.../net/ethernet/intel/ixgbe/ixgbe_sriov.c| 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 39 -
drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c | 44 +++---
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 109 ++---
12 files changed, 266 insertions(+), 317 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
index 0470b69d834c..6835d5f18753 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
@@ -123,14 +123,14 @@ static s32 ixgbe_init_phy_ops_82598(struct ixgbe_hw *hw)
if (ret_val)
return ret_val;
if (hw->phy.sfp_type == ixgbe_sfp_type_unknown)
- return IXGBE_ERR_SFP_NOT_SUPPORTED;
+ return -EOPNOTSUPP;
/* Check to see if SFP+ module is supported */
ret_val = ixgbe_get_sfp_init_sequence_offsets(hw,
&list_offset,
&data_offset);
if (ret_val)
- return IXGBE_ERR_SFP_NOT_SUPPORTED;
+ return -EOPNOTSUPP;
break;
default:
break;
@@ -213,7 +213,7 @@ static s32 ixgbe_get_link_capabilities_82598(struct
ixgbe_hw *hw,
break;
default:
- return IXGBE_ERR_LINK_SETUP;
+ return -EIO;
}
return 0;
@@ -283,7 +283,7 @@ static s32 ixgbe_fc_enable_82598(struct ixgbe_hw *hw)
/* Validate the water mark configuration */
if (!hw->fc.pause_time)
- return IXGBE_ERR_INVALID_LINK_SETTINGS;
+ return -EINVAL;
/* Low water mark of zero causes XOFF floods */
for (i = 0; i < MAX_TRAFFIC_CLASS; i++) {
@@ -292,7 +292,7 @@ static s32 ixgbe_fc_enable_82598(struct ixgbe_hw *hw)
if (!hw->fc.low_water[i] ||
hw->fc.low_water[i] >= hw->fc.high_water[i]) {
hw_dbg(hw, "Invalid water mark
configuration\n");
- return IXGBE_ERR_INVALID_LINK_SETTINGS;
+ return -EINVAL;
}
}
}
@@ -369,7 +369,7 @@ static s32 ixgbe_fc_enable_82598(struct ixgbe_hw *hw)
break;
default:
hw_dbg(hw, "Flow control param set incorrectly\n");
- return IXGBE_ERR_CONFIG;
+ return -EIO;
}
/* Set 802.3x based flow control settings. */
@@ -438,7 +438,7 @@ static s32 ixgbe_start_mac_link_82598(struct ixgbe_hw *hw,
msleep(100);
}
if (!(links_reg & IXGBE_LINKS_KX_AN_COMP)) {
- status = IXGBE_ERR_AUTONEG_NOT_COMPLETE;
+ status = -EIO;
hw_dbg(hw, "Autonegotiation did not
complete.\n");
}
}
@@ -478,7 +478,7 @@ static s32 ixgbe_validate_link_ready(struct ixgbe_hw *hw)
if (timeout == IXGBE_VALIDATE_LINK_READY_TIMEOUT) {
hw_dbg(hw, "Link was indicated but link is down\n");
- return IXGBE_ERR_LINK_SETUP;
+ return -EIO;
}
return 0;
@@ -594,7 +594,7 @@ static s32 ixgbe_setup_mac_link_82598(struct ixgbe_hw *hw,
speed &= link_capabilities;
if (speed == IXGBE_LINK_SPEED_UNKNOWN)
- return IXGBE_ERR_LINK_SETUP;
+ return -EINVAL;
/* Set KX4/KX support according to speed requested */
else if (link_mode == IXGBE_AUTOC_LMS_KX4_AN ||
@@ -701,9 +701,9 @@ static s32 ixgbe_reset_hw_82598(struct ixgbe_hw *hw)
/* Init PHY and function pointers, perform SFP setup */
phy_status = hw->phy.ops.init(hw);
- if (phy_status == IXGBE_ERR_SFP_NOT_SUPPORTED)
+ if (phy_status == -EOPNOTSUPP)
return phy_status;
- if (phy_status == IXGBE_ERR_SFP_NOT_PRE
Re: [Intel-wired-lan] [PATCH v2 iwl-next] ice: Fix some null pointer dereference issues in ice_ptp.c
> -Original Message-
> From: Intel-wired-lan On Behalf Of Kunwu
> Chan
> Sent: Tuesday, December 12, 2023 8:10 AM
> To: Brandeburg, Jesse ; Nguyen, Anthony L
> ; da...@davemloft.net; eduma...@google.com;
> k...@kernel.org; pab...@redhat.com; richardcoch...@gmail.com; Keller, Jacob E
>
> Cc: Michalik, Michal ; Kunwu Chan
> ; Kunwu Chan ;
> net...@vger.kernel.org; linux-ker...@vger.kernel.org; Kolacinski, Karol
> ; intel-wired-...@lists.osuosl.org; Kitszel,
> Przemyslaw
> Subject: [Intel-wired-lan] [PATCH v2 iwl-next] ice: Fix some null pointer
> dereference issues in ice_ptp.c
>
> devm_kasprintf() returns a pointer to dynamically allocated memory
> which can be NULL upon failure.
>
> Fixes: d938a8cca88a ("ice: Auxbus devices & driver for E822 TS")
> Cc: Kunwu Chan
> Suggested-by: Przemek Kitszel
> Signed-off-by: Kunwu Chan
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 4
> 1 file changed, 4 insertions(+)
>
Tested-by: Pucha Himasekhar Reddy (A
Contingent worker at Intel)
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-net] idpf: avoid compiler introduced padding in virtchnl2_rss_key struct
On Fri, Dec 15, 2023 at 03:48:07PM -0800, Pavan Kumar Linga wrote:
> Size of the virtchnl2_rss_key struct should be 7 bytes but the
> compiler introduces a padding byte for the structure alignment.
> This results in idpf sending an additional byte of memory to the device
> control plane than the expected buffer size. As the control plane
> enforces virtchnl message size checks to validate the message,
> set RSS key message fails resulting in the driver load failure.
>
> Remove implicit compiler padding by using "__packed" structure
> attribute for the virtchnl2_rss_key struct.
>
> Also there is no need to use __DECLARE_FLEX_ARRAY macro for the
> 'key_flex' struct field. So drop it.
>
> Fixes: 0d7502a9b4a7 ("virtchnl: add virtchnl version 2 ops")
> Reviewed-by: Larysa Zaremba
> Signed-off-by: Pavan Kumar Linga
Reviewed-by: Simon Horman
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v6] ice: Add support for packet mirroring using hardware in switchdev mode
> -Original Message- > From: Intel-wired-lan On Behalf Of > Andrii Staikov > Sent: Tuesday, December 12, 2023 6:21 PM > To: intel-wired-...@lists.osuosl.org > Cc: net...@vger.kernel.org; Szycik, Marcin ; > Drewek, Wojciech ; linux- > ker...@vger.kernel.org; Staikov, Andrii > Subject: [Intel-wired-lan] [PATCH iwl-next v6] ice: Add support for packet > mirroring using hardware in switchdev mode > > Switchdev mode allows to add mirroring rules to mirror incoming and > outgoing packets to the interface's port representor. Previously, this was > available only using software functionality. Add possibility to offload this > functionality to the NIC hardware. > > Introduce ICE_MIRROR_PACKET filter action to the ice_sw_fwd_act_type > enum to identify the desired action and pass it to the hardware as well as the > VSI to mirror. > > Example of tc mirror command using hardware: > tc filter add dev ens1f0np0 ingress protocol ip prio 1 flower src_mac > b4:96:91:a5:c7:a7 skip_sw action mirred egress mirror dev eth1 > > ens1f0np0 - PF > b4:96:91:a5:c7:a7 - source MAC address > eth1 - PR of a VF to mirror to > > Co-developed-by: Marcin Szycik > Signed-off-by: Marcin Szycik > Reviewed-by: Wojciech Drewek > Signed-off-by: Andrii Staikov > --- > v1 -> v2: no need for changes in ice_add_tc_flower_adv_fltr() > v2 -> v3: add another if branch for netif_is_ice(act->dev) || > ice_is_tunnel_supported(act->dev) for FLOW_ACTION_MIRRED action and > add direction rules for filters > v3 -> v4: move setting mirroring into dedicated function > ice_tc_setup_mirror_action() > v4 -> v5: Fix packets not mirroring from VF to VF by changing > ICE_ESWITCH_FLTR_INGRESS to ICE_ESWITCH_FLTR_EGRESS where needed > v5 -> v6: Additionally fix some tags > --- > drivers/net/ethernet/intel/ice/ice_switch.c | 25 + > drivers/net/ethernet/intel/ice/ice_tc_lib.c | 41 + > drivers/net/ethernet/intel/ice/ice_type.h | 1 + > 3 files changed, 60 insertions(+), 7 deletions(-) > Tested-by: Sujai Buvaneswaran ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/7] Add PFCP filter support
From: Marcin Szycik
Date: Mon, 18 Dec 2023 11:04:01 +0100
>
>
> On 15.12.2023 17:49, Jakub Kicinski wrote:
>> On Fri, 15 Dec 2023 11:11:23 +0100 Alexander Lobakin wrote:
>>> Ping? :s
>>> Or should we resubmit?
>>
>> Can you wait for next merge window instead?
>> We're getting flooded with patches as everyone seemingly tries to get
>> their own (i.e. the most important!) work merged before the end of
>> the year. The set of PRs from the bitmap tree which Linus decided
>> not to pull is not empty. So we'd have to go figure out what's exactly
>> is in that branch we're supposed to pull, and whether it's fine.
>> It probably is, but you see, this is a problem which can be solved by
>> waiting, and letting Linus pull it himself. While the 150 patches we're
>> getting a day now have to be looked at.
>
> Let's wait to the next window then.
Hey Yury,
Given that PFCP will be resent in the next window...
Your "boys" tree is in fact self-contained -- those are mostly
optimizations and cleanups, and for the new API -- bitmap_{read,write}()
-- it has internal users (after "bitmap: make bitmap_{get,set}_value8()
use bitmap_{read,write}()"). IOW, I don't see a reason for not merging
it into your main for-next tree (this week :p).
What do you think?
>
> Thanks,
> Marcin
Thanks,
Olek
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [PATCH iwl-net v2] ice: dpll: fix phase offset value
Stop dividing the phase_offset value received from firmware. This fault
is present since the initial implementation.
The phase_offset value received from firmware is in 0.01ps resolution.
Dpll subsystem is using the value in 0.001ps, raw value is adjusted
before providing it to the user.
The user can observe the value of phase offset with response to
`pin-get` netlink message of dpll subsystem for an active pin:
$ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
--do pin-get --json '{"id":2}'
Where example of correct response would be:
{'board-label': 'C827_0-RCLKA',
'capabilities': 6,
'clock-id': 4658613174691613800,
'frequency': 1953125,
'id': 2,
'module-name': 'ice',
'parent-device': [{'direction': 'input',
'parent-id': 6,
'phase-offset': -216839550,
'prio': 9,
'state': 'connected'},
{'direction': 'input',
'parent-id': 7,
'phase-offset': -42930,
'prio': 8,
'state': 'connected'}],
'phase-adjust': 0,
'phase-adjust-max': 16723,
'phase-adjust-min': -16723,
'type': 'mux'}
Provided phase-offset value (-42930) shall be divided by the user with
DPLL_PHASE_OFFSET_DIVIDER to get actual value of -42.930 ps.
Before the fix, the response was not correct:
{'board-label': 'C827_0-RCLKA',
'capabilities': 6,
'clock-id': 4658613174691613800,
'frequency': 1953125,
'id': 2,
'module-name': 'ice',
'parent-device': [{'direction': 'input',
'parent-id': 6,
'phase-offset': -216839,
'prio': 9,
'state': 'connected'},
{'direction': 'input',
'parent-id': 7,
'phase-offset': -42,
'prio': 8,
'state': 'connected'}],
'phase-adjust': 0,
'phase-adjust-max': 16723,
'phase-adjust-min': -16723,
'type': 'mux'}
Where phase-offset value (-42), after division
(DPLL_PHASE_OFFSET_DIVIDER) would be: -0.042 ps.
Fixes: 8a3a565ff210 ("ice: add admin commands to access cgu configuration")
Fixes: 90e1c90750d7 ("ice: dpll: implement phase related callbacks")
Reviewed-by: Aleksandr Loktionov
Reviewed-by: Przemek Kitszel
Signed-off-by: Arkadiusz Kubalewski
---
drivers/net/ethernet/intel/ice/ice_common.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index 9a6c25f98632..edac34c796ce 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -5332,7 +5332,6 @@ ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8
dpll_num, u8 *ref_state,
u8 *eec_mode)
{
struct ice_aqc_get_cgu_dpll_status *cmd;
- const s64 nsec_per_psec = 1000LL;
struct ice_aq_desc desc;
int status;
@@ -5348,8 +5347,7 @@ ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8
dpll_num, u8 *ref_state,
*phase_offset = le32_to_cpu(cmd->phase_offset_h);
*phase_offset <<= 32;
*phase_offset += le32_to_cpu(cmd->phase_offset_l);
- *phase_offset = div64_s64(sign_extend64(*phase_offset, 47),
- nsec_per_psec);
+ *phase_offset = sign_extend64(*phase_offset, 47);
*eec_mode = cmd->eec_mode;
}
--
2.38.1
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-net v2] ice: dpll: fix phase offset value
Dear Arkadiusz,
Am 18.12.23 um 15:58 schrieb Arkadiusz Kubalewski:
Stop dividing the phase_offset value received from firmware. This fault
is present since the initial implementation.
The phase_offset value received from firmware is in 0.01ps resolution.
Dpll subsystem is using the value in 0.001ps, raw value is adjusted
before providing it to the user.
The user can observe the value of phase offset with response to
`pin-get` netlink message of dpll subsystem for an active pin:
$ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
--do pin-get --json '{"id":2}'
Where example of correct response would be:
{'board-label': 'C827_0-RCLKA',
'capabilities': 6,
'clock-id': 4658613174691613800,
'frequency': 1953125,
'id': 2,
'module-name': 'ice',
'parent-device': [{'direction': 'input',
'parent-id': 6,
'phase-offset': -216839550,
'prio': 9,
'state': 'connected'},
{'direction': 'input',
'parent-id': 7,
'phase-offset': -42930,
'prio': 8,
'state': 'connected'}],
'phase-adjust': 0,
'phase-adjust-max': 16723,
'phase-adjust-min': -16723,
'type': 'mux'}
Provided phase-offset value (-42930) shall be divided by the user with
DPLL_PHASE_OFFSET_DIVIDER to get actual value of -42.930 ps.
Before the fix, the response was not correct:
{'board-label': 'C827_0-RCLKA',
'capabilities': 6,
'clock-id': 4658613174691613800,
'frequency': 1953125,
'id': 2,
'module-name': 'ice',
'parent-device': [{'direction': 'input',
'parent-id': 6,
'phase-offset': -216839,
'prio': 9,
'state': 'connected'},
{'direction': 'input',
'parent-id': 7,
'phase-offset': -42,
'prio': 8,
'state': 'connected'}],
'phase-adjust': 0,
'phase-adjust-max': 16723,
'phase-adjust-min': -16723,
'type': 'mux'}
Where phase-offset value (-42), after division
(DPLL_PHASE_OFFSET_DIVIDER) would be: -0.042 ps.
Thank you for the detailed description.
Fixes: 8a3a565ff210 ("ice: add admin commands to access cgu configuration")
Fixes: 90e1c90750d7 ("ice: dpll: implement phase related callbacks")
Reviewed-by: Aleksandr Loktionov
Reviewed-by: Przemek Kitszel
Signed-off-by: Arkadiusz Kubalewski
---
drivers/net/ethernet/intel/ice/ice_common.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index 9a6c25f98632..edac34c796ce 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -5332,7 +5332,6 @@ ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8
dpll_num, u8 *ref_state,
u8 *eec_mode)
{
struct ice_aqc_get_cgu_dpll_status *cmd;
- const s64 nsec_per_psec = 1000LL;
struct ice_aq_desc desc;
int status;
@@ -5348,8 +5347,7 @@ ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state,
*phase_offset = le32_to_cpu(cmd->phase_offset_h);
*phase_offset <<= 32;
*phase_offset += le32_to_cpu(cmd->phase_offset_l);
- *phase_offset = div64_s64(sign_extend64(*phase_offset, 47),
- nsec_per_psec);
+ *phase_offset = sign_extend64(*phase_offset, 47);
*eec_mode = cmd->eec_mode;
}
Reviewed-by: Paul Menzel
Kind regards,
Paul
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/7] Add PFCP filter support
+ Alexander Potapenko
On Mon, Dec 18, 2023 at 01:47:01PM +0100, Alexander Lobakin wrote:
> From: Marcin Szycik
> Date: Mon, 18 Dec 2023 11:04:01 +0100
>
> >
> >
> > On 15.12.2023 17:49, Jakub Kicinski wrote:
> >> On Fri, 15 Dec 2023 11:11:23 +0100 Alexander Lobakin wrote:
> >>> Ping? :s
> >>> Or should we resubmit?
> >>
> >> Can you wait for next merge window instead?
> >> We're getting flooded with patches as everyone seemingly tries to get
> >> their own (i.e. the most important!) work merged before the end of
> >> the year. The set of PRs from the bitmap tree which Linus decided
> >> not to pull is not empty. So we'd have to go figure out what's exactly
> >> is in that branch we're supposed to pull, and whether it's fine.
> >> It probably is, but you see, this is a problem which can be solved by
> >> waiting, and letting Linus pull it himself. While the 150 patches we're
> >> getting a day now have to be looked at.
> >
> > Let's wait to the next window then.
>
> Hey Yury,
>
> Given that PFCP will be resent in the next window...
>
> Your "boys" tree is in fact self-contained -- those are mostly
> optimizations and cleanups, and for the new API -- bitmap_{read,write}()
> -- it has internal users (after "bitmap: make bitmap_{get,set}_value8()
> use bitmap_{read,write}()"). IOW, I don't see a reason for not merging
> it into your main for-next tree (this week :p).
> What do you think?
I think that there's already enough mess with this patch. Alexander
submitted new version of his MTE series together with the patch.
https://lore.kernel.org/lkml/ZXtciaxTKFBiui%2FX@yury-ThinkPad/T/
Now you're asking me to merge it separately. I don't want to undercut
arm64 folks.
Can you guys decide what you want? If you want to move
bitmap_read/write() with my branch, I need to send it in -next for
testing ASAP. And for that, as I already said, I need at least one
active user in current kernel tree. (Yes, bitmap_get_value8() counts.)
If you want to move it this way, please resend all the patches
together.
Thanks,
Yury
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [net PATCH] i40e: fix use-after-free in i40e_aqc_add_filters()
On 2023/12/16 1:16, Brett Creeley wrote:
On 12/13/2023 2:49 AM, Ke Xiao wrote:
Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 1ab8dbe2d880..16b574d69843 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -108,11 +108,17 @@ static void netdev_hw_addr_refcnt(struct
i40e_mac_filter *f,
struct net_device *netdev, int delta)
{
struct netdev_hw_addr *ha;
+ struct netdev_hw_addr_list *ha_list;
Nit, needs to be in Reverse Christmas Tree (RCT) order.
Thanks, I will send the V2 to follow the rule.
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [net PATCH v2] i40e: fix use-after-free in i40e_aqc_add_filters()
Commit 3116f59c12bd ("i40e: fix use-after-free in
i40e_sync_filters_subtask()") avoided use-after-free issues,
by increasing refcount during update the VSI filter list to
the HW. However, it missed the unicast situation.
When deleting an unicast FDB entry, the i40e driver will release
the mac_filter, and i40e_service_task will concurrently request
firmware to add the mac_filter, which will lead to the following
use-after-free issue.
Fix again for both netdev->uc and netdev->mc.
BUG: KASAN: use-after-free in i40e_aqc_add_filters+0x55c/0x5b0 [i40e]
Read of size 2 at addr 888eb3452d60 by task kworker/8:7/6379
CPU: 8 PID: 6379 Comm: kworker/8:7 Kdump: loaded Tainted: G
Workqueue: i40e i40e_service_task [i40e]
Call Trace:
dump_stack+0x71/0xab
print_address_description+0x6b/0x290
kasan_report+0x14a/0x2b0
i40e_aqc_add_filters+0x55c/0x5b0 [i40e]
i40e_sync_vsi_filters+0x1676/0x39c0 [i40e]
i40e_service_task+0x1397/0x2bb0 [i40e]
process_one_work+0x56a/0x11f0
worker_thread+0x8f/0xf40
kthread+0x2a0/0x390
ret_from_fork+0x1f/0x40
Allocated by task 21948:
kasan_kmalloc+0xa6/0xd0
kmem_cache_alloc_trace+0xdb/0x1c0
i40e_add_filter+0x11e/0x520 [i40e]
i40e_addr_sync+0x37/0x60 [i40e]
__hw_addr_sync_dev+0x1f5/0x2f0
i40e_set_rx_mode+0x61/0x1e0 [i40e]
dev_uc_add_excl+0x137/0x190
i40e_ndo_fdb_add+0x161/0x260 [i40e]
rtnl_fdb_add+0x567/0x950
rtnetlink_rcv_msg+0x5db/0x880
netlink_rcv_skb+0x254/0x380
netlink_unicast+0x454/0x610
netlink_sendmsg+0x747/0xb00
sock_sendmsg+0xe2/0x120
__sys_sendto+0x1ae/0x290
__x64_sys_sendto+0xdd/0x1b0
do_syscall_64+0xa0/0x370
entry_SYSCALL_64_after_hwframe+0x65/0xca
Freed by task 21948:
__kasan_slab_free+0x137/0x190
kfree+0x8b/0x1b0
__i40e_del_filter+0x116/0x1e0 [i40e]
i40e_del_mac_filter+0x16c/0x300 [i40e]
i40e_addr_unsync+0x134/0x1b0 [i40e]
__hw_addr_sync_dev+0xff/0x2f0
i40e_set_rx_mode+0x61/0x1e0 [i40e]
dev_uc_del+0x77/0x90
rtnl_fdb_del+0x6a5/0x860
rtnetlink_rcv_msg+0x5db/0x880
netlink_rcv_skb+0x254/0x380
netlink_unicast+0x454/0x610
netlink_sendmsg+0x747/0xb00
sock_sendmsg+0xe2/0x120
__sys_sendto+0x1ae/0x290
__x64_sys_sendto+0xdd/0x1b0
do_syscall_64+0xa0/0x370
entry_SYSCALL_64_after_hwframe+0x65/0xca
Fixes: 3116f59c12bd ("i40e: fix use-after-free in i40e_sync_filters_subtask()")
Fixes: 41c445ff0f48 ("i40e: main driver core")
Signed-off-by: Ke Xiao
Signed-off-by: Ding Hui
Cc: Di Zhu
Reviewed-by: Jan Sokolowski
Reviewed-by: Simon Horman
---
v2:
- Order local variable declarations in Reverse Christmas Tree (RCT)
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 1ab8dbe2d880..d5633a440cca 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -107,12 +107,18 @@ static struct workqueue_struct *i40e_wq;
static void netdev_hw_addr_refcnt(struct i40e_mac_filter *f,
struct net_device *netdev, int delta)
{
+ struct netdev_hw_addr_list *ha_list;
struct netdev_hw_addr *ha;
if (!f || !netdev)
return;
- netdev_for_each_mc_addr(ha, netdev) {
+ if (is_unicast_ether_addr(f->macaddr) ||
is_link_local_ether_addr(f->macaddr))
+ ha_list = &netdev->uc;
+ else
+ ha_list = &netdev->mc;
+
+ netdev_hw_addr_list_for_each(ha, ha_list) {
if (ether_addr_equal(ha->addr, f->macaddr)) {
ha->refcount += delta;
if (ha->refcount <= 0)
--
2.17.1
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next] i40e: Avoid unnecessary use of comma operator
On Sun, Dec 17, 2023 at 1:45 AM Simon Horman wrote: > > Although it does not seem to have any untoward side-effects, > the use of ';' to separate to assignments seems more appropriate than ','. > > Flagged by clang-17 -Wcomma Yikes! This kind of example is why I hate the comma operator! Reviewed-by: Nick Desaulniers (Is -Wcomma enabled by -Wall?) Is there a fixes tag we can add? > > No functional change intended. > Compile tested only. > > Signed-off-by: Simon Horman > --- > drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > index 812d04747bd0..f542f2671957 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > @@ -1917,7 +1917,7 @@ int i40e_get_eeprom(struct net_device *netdev, > len = eeprom->len - (I40E_NVM_SECTOR_SIZE * i); > last = true; > } > - offset = eeprom->offset + (I40E_NVM_SECTOR_SIZE * i), > + offset = eeprom->offset + (I40E_NVM_SECTOR_SIZE * i); > ret_val = i40e_aq_read_nvm(hw, 0x0, offset, len, > (u8 *)eeprom_buff + (I40E_NVM_SECTOR_SIZE * > i), > last, NULL); > > -- Thanks, ~Nick Desaulniers ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/7] Add PFCP filter support
On Mon, Dec 18, 2023 at 4:57 PM Yury Norov wrote:
>
> + Alexander Potapenko
>
> On Mon, Dec 18, 2023 at 01:47:01PM +0100, Alexander Lobakin wrote:
> > From: Marcin Szycik
> > Date: Mon, 18 Dec 2023 11:04:01 +0100
> >
> > >
> > >
> > > On 15.12.2023 17:49, Jakub Kicinski wrote:
> > >> On Fri, 15 Dec 2023 11:11:23 +0100 Alexander Lobakin wrote:
> > >>> Ping? :s
> > >>> Or should we resubmit?
> > >>
> > >> Can you wait for next merge window instead?
> > >> We're getting flooded with patches as everyone seemingly tries to get
> > >> their own (i.e. the most important!) work merged before the end of
> > >> the year. The set of PRs from the bitmap tree which Linus decided
> > >> not to pull is not empty. So we'd have to go figure out what's exactly
> > >> is in that branch we're supposed to pull, and whether it's fine.
> > >> It probably is, but you see, this is a problem which can be solved by
> > >> waiting, and letting Linus pull it himself. While the 150 patches we're
> > >> getting a day now have to be looked at.
> > >
> > > Let's wait to the next window then.
> >
> > Hey Yury,
> >
> > Given that PFCP will be resent in the next window...
> >
> > Your "boys" tree is in fact self-contained -- those are mostly
> > optimizations and cleanups, and for the new API -- bitmap_{read,write}()
> > -- it has internal users (after "bitmap: make bitmap_{get,set}_value8()
> > use bitmap_{read,write}()"). IOW, I don't see a reason for not merging
> > it into your main for-next tree (this week :p).
> > What do you think?
>
> I think that there's already enough mess with this patch. Alexander
> submitted new version of his MTE series together with the patch.
Yeah, sorry about that. Because the MTE part of the patches was still
awaiting review, I thought it would be better to land the bitmap API
separately, but as you pointed out there should be at least one user
for it, which it wouldn't have in that case.
I don't have a strong preference about whether to submit the patches
before or after the end of year - in fact I don't think they are
urgent enough, and we'd better postpone them till January.
So unless Alexander has urgent fixes depending on my bitmap patches,
I'd suggest waiting till they are taken via the arm64 tree.
> https://lore.kernel.org/lkml/ZXtciaxTKFBiui%2FX@yury-ThinkPad/T/
>
> Now you're asking me to merge it separately. I don't want to undercut
> arm64 folks.
>
> Can you guys decide what you want? If you want to move
> bitmap_read/write() with my branch, I need to send it in -next for
> testing ASAP. And for that, as I already said, I need at least one
> active user in current kernel tree. (Yes, bitmap_get_value8() counts.)
>
> If you want to move it this way, please resend all the patches
> together.
>
> Thanks,
> Yury
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [ANN] no call tomorrow
Hi folks, many people are on winter break already, so there will be no netdev call tomorrow. This weeks reviewers are: Intel Happy holidays everyone! ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] linux-next: Tree for Dec 15 (drivers/net/ethernet/intel/ice/ice_base.c)
On 12/15/2023 9:26 PM, Randy Dunlap wrote:
On 12/14/23 20:01, Stephen Rothwell wrote:
Hi all,
Changes since 20231214:
on s390:
# CONFIG_XDP_SOCKETS is not set
../drivers/net/ethernet/intel/ice/ice_base.c: In function
'ice_xsk_pool_fill_cb':
../drivers/net/ethernet/intel/ice/ice_base.c:533:16: error: variable 'desc' has
initializer but incomplete type
533 | struct xsk_cb_desc desc = {};
|^~~
../drivers/net/ethernet/intel/ice/ice_base.c:533:28: error: storage size of
'desc' isn't known
533 | struct xsk_cb_desc desc = {};
|^~~~
Full randconfig file is attached.
Adding bpf as it's from d68d707dcbbf ("ice: Support XDP hints in AF_XDP
ZC mode") which is on bpf-next (and hasn't made it's way to the Intel
trees yet).
Thanks,
Tony
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next] i40e: Avoid unnecessary use of comma operator
On Mon, Dec 18, 2023 at 11:00 AM Nathan Chancellor wrote: > > On Mon, Dec 18, 2023 at 08:32:28AM -0800, Nick Desaulniers wrote: > > (Is -Wcomma enabled by -Wall?) > > No and last time that I looked into enabling it, there were a lot of > instances in the kernel: > > https://lore.kernel.org/20230630192825.GA2745548@dev-arch.thelio-3990X/ > > It is still probably worth pursuing at some point but that is a lot of > instances to clean up (along with potentially having a decent amount of > pushback depending on the changes necessary to eliminate all instances). Filed this todo: https://github.com/ClangBuiltLinux/linux/issues/1968 I'd be happy if Simon keeps poking at getting that warning enabled. -- Thanks, ~Nick Desaulniers ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/5] iavf: Add devlink and devlink rate support'
On Fri, 2023-12-15 at 14:41 -0800, Jakub Kicinski wrote: > I explained before (perhaps on the netdev call) - Qdiscs have two > different offload models. "local" and "switchdev", here we want "local" > AFAIU and TBF only has "switchdev" offload (take a look at the enqueue > method and which drivers support it today). I must admit the above is not yet clear to me. I initially thought you meant that "local" offloads properly reconfigure the S/W datapath so that locally generated traffic would go through the expected processing (e.g. shaping) just once, while with "switchdev" offload locally generated traffic will see shaping done both by the S/W and the H/W[1]. Reading the above I now think you mean that local offloads has only effect for locally generated traffic but not on traffic forwarded via eswitch, and vice versa[2]. The drivers I looked at did not show any clue (to me). FTR, I think that [1] is a bug worth fixing and [2] is evil ;) Could you please clarify which is the difference exactly between them? > "We'll extend TBF" is very much adding a new API. You'll have to add > "local offload" support in TBF and no NIC driver today supports it. > I'm not saying TBF is bad, but I disagree that it's any different > than a new NDO for all practical purposes. > > > ndo_setup_tc() feels like the natural choice for H/W offload and TBF > > is the existing interface IMHO nearest to the requirements here. > > I question whether something as basic as scheduling and ACLs should > follow the "offload SW constructs" mantra. You are exposed to more > diverse users so please don't hesitate to disagree, but AFAICT > the transparent offload (user installs SW constructs and if offload > is available - offload, otherwise use SW is good enough) has not > played out like we have hoped. > > Let's figure out what is the abstract model of scheduling / shaping > within a NIC that we want to target. And then come up with a way of > representing it in SW. Not which uAPI we can shoehorn into the use > case. I thought the model was quite well defined since the initial submission from Intel, and is quite simple: expose TX shaping on per tx queue basis, with min rate, max rate (in bps) and burst (in bytes). I think that making it more complex (e.g. with nesting, pkt overhead, etc) we will still not cover every possible use case and will add considerable complexity. > Cheers, Paolo ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [net PATCH v2] i40e: fix use-after-free in i40e_aqc_add_filters()
On 12/17/2023 11:08 PM, Ke Xiao wrote:
> Commit 3116f59c12bd ("i40e: fix use-after-free in
> i40e_sync_filters_subtask()") avoided use-after-free issues,
> by increasing refcount during update the VSI filter list to
> the HW. However, it missed the unicast situation.
>
> When deleting an unicast FDB entry, the i40e driver will release
> the mac_filter, and i40e_service_task will concurrently request
> firmware to add the mac_filter, which will lead to the following
> use-after-free issue.
>
> Fix again for both netdev->uc and netdev->mc.
>
Thanks for fixing this!
Reviewed-by: Jacob Keller
> BUG: KASAN: use-after-free in i40e_aqc_add_filters+0x55c/0x5b0 [i40e]
> Read of size 2 at addr 888eb3452d60 by task kworker/8:7/6379
>
> CPU: 8 PID: 6379 Comm: kworker/8:7 Kdump: loaded Tainted: G
> Workqueue: i40e i40e_service_task [i40e]
> Call Trace:
> dump_stack+0x71/0xab
> print_address_description+0x6b/0x290
> kasan_report+0x14a/0x2b0
> i40e_aqc_add_filters+0x55c/0x5b0 [i40e]
> i40e_sync_vsi_filters+0x1676/0x39c0 [i40e]
> i40e_service_task+0x1397/0x2bb0 [i40e]
> process_one_work+0x56a/0x11f0
> worker_thread+0x8f/0xf40
> kthread+0x2a0/0x390
> ret_from_fork+0x1f/0x40
>
> Allocated by task 21948:
> kasan_kmalloc+0xa6/0xd0
> kmem_cache_alloc_trace+0xdb/0x1c0
> i40e_add_filter+0x11e/0x520 [i40e]
> i40e_addr_sync+0x37/0x60 [i40e]
> __hw_addr_sync_dev+0x1f5/0x2f0
> i40e_set_rx_mode+0x61/0x1e0 [i40e]
> dev_uc_add_excl+0x137/0x190
> i40e_ndo_fdb_add+0x161/0x260 [i40e]
> rtnl_fdb_add+0x567/0x950
> rtnetlink_rcv_msg+0x5db/0x880
> netlink_rcv_skb+0x254/0x380
> netlink_unicast+0x454/0x610
> netlink_sendmsg+0x747/0xb00
> sock_sendmsg+0xe2/0x120
> __sys_sendto+0x1ae/0x290
> __x64_sys_sendto+0xdd/0x1b0
> do_syscall_64+0xa0/0x370
> entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> Freed by task 21948:
> __kasan_slab_free+0x137/0x190
> kfree+0x8b/0x1b0
> __i40e_del_filter+0x116/0x1e0 [i40e]
> i40e_del_mac_filter+0x16c/0x300 [i40e]
> i40e_addr_unsync+0x134/0x1b0 [i40e]
> __hw_addr_sync_dev+0xff/0x2f0
> i40e_set_rx_mode+0x61/0x1e0 [i40e]
> dev_uc_del+0x77/0x90
> rtnl_fdb_del+0x6a5/0x860
> rtnetlink_rcv_msg+0x5db/0x880
> netlink_rcv_skb+0x254/0x380
> netlink_unicast+0x454/0x610
> netlink_sendmsg+0x747/0xb00
> sock_sendmsg+0xe2/0x120
> __sys_sendto+0x1ae/0x290
> __x64_sys_sendto+0xdd/0x1b0
> do_syscall_64+0xa0/0x370
> entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> Fixes: 3116f59c12bd ("i40e: fix use-after-free in
> i40e_sync_filters_subtask()")
> Fixes: 41c445ff0f48 ("i40e: main driver core")
> Signed-off-by: Ke Xiao
> Signed-off-by: Ding Hui
> Cc: Di Zhu
> Reviewed-by: Jan Sokolowski
> Reviewed-by: Simon Horman
> ---
> v2:
> - Order local variable declarations in Reverse Christmas Tree (RCT)
>
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 1ab8dbe2d880..d5633a440cca 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -107,12 +107,18 @@ static struct workqueue_struct *i40e_wq;
> static void netdev_hw_addr_refcnt(struct i40e_mac_filter *f,
> struct net_device *netdev, int delta)
> {
> + struct netdev_hw_addr_list *ha_list;
> struct netdev_hw_addr *ha;
>
> if (!f || !netdev)
> return;
>
> - netdev_for_each_mc_addr(ha, netdev) {
> + if (is_unicast_ether_addr(f->macaddr) ||
> is_link_local_ether_addr(f->macaddr))
> + ha_list = &netdev->uc;
> + else
> + ha_list = &netdev->mc;
> +
> + netdev_hw_addr_list_for_each(ha, ha_list) {
> if (ether_addr_equal(ha->addr, f->macaddr)) {
> ha->refcount += delta;
> if (ha->refcount <= 0)
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v4 0/5] iavf: Add devlink and devlink rate support'
On Mon, 18 Dec 2023 21:12:35 +0100 Paolo Abeni wrote: > On Fri, 2023-12-15 at 14:41 -0800, Jakub Kicinski wrote: > > I explained before (perhaps on the netdev call) - Qdiscs have two > > different offload models. "local" and "switchdev", here we want "local" > > AFAIU and TBF only has "switchdev" offload (take a look at the enqueue > > method and which drivers support it today). > > I must admit the above is not yet clear to me. > > I initially thought you meant that "local" offloads properly > reconfigure the S/W datapath so that locally generated traffic would go > through the expected processing (e.g. shaping) just once, while with > "switchdev" offload locally generated traffic will see shaping done > both by the S/W and the H/W[1]. > > Reading the above I now think you mean that local offloads has only > effect for locally generated traffic but not on traffic forwarded via > eswitch, and vice versa[2]. > > The drivers I looked at did not show any clue (to me). > > FTR, I think that [1] is a bug worth fixing and [2] is evil ;) > > Could you please clarify which is the difference exactly between them? The practical difference which you can see in the code is that "locally offloaded" qdiscs will act like a FIFO in the SW path (at least to some extent). While "switchdev" offload qdiscs act exactly the same regardless of the offload. Neither is wrong, they are offloading different things. Qdisc offload on a representor (switchdev) offloads from the switch perspective, i.e. "ingress to host". Only fallback goes thru SW path, and should be negligible. "Local" offload can be implemented as admission control (and is sometimes work conserving), it's on the "real" interface, it's egress, and doesn't take part in forwarding. > > I question whether something as basic as scheduling and ACLs should > > follow the "offload SW constructs" mantra. You are exposed to more > > diverse users so please don't hesitate to disagree, but AFAICT > > the transparent offload (user installs SW constructs and if offload > > is available - offload, otherwise use SW is good enough) has not > > played out like we have hoped. > > > > Let's figure out what is the abstract model of scheduling / shaping > > within a NIC that we want to target. And then come up with a way of > > representing it in SW. Not which uAPI we can shoehorn into the use > > case. > > I thought the model was quite well defined since the initial submission > from Intel, and is quite simple: expose TX shaping on per tx queue > basis, with min rate, max rate (in bps) and burst (in bytes). For some definition of a model, I guess. Given the confusion about switchdev vs local (ingress vs egress) - I can't agree that the model is well defined :( What I mean is - given piece of functionality like "Tx queue shaping" you can come up with a reasonable uAPI that you can hijack and it makes sense to you. But someone else (switchdev ingress) can chose the same API to implement a different offload. Not to mention that yet another person will chose a different API to implement the same things as you :( Off the top of my head we have at least: - Tx DMA admission control / scheduling (which Tx DMA queue will NIC pull from) - Rx DMA scheduling (which Rx queue will NIC push to) - buffer/queue configuration (how to deal with buildup of packets in NIC SRAM, usually mostly for ingress) - NIC buffer configuration (how the SRAM is allocated to queues) - policers in the NIC forwarding logic Let's extend this list so that it covers all reasonable NIC designs, and them work on mapping how each of them is configured? > I think that making it more complex (e.g. with nesting, pkt overhead, > etc) we will still not cover every possible use case and will add > considerable complexity. ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next] i40e: Avoid unnecessary use of comma operator
On Mon, Dec 18, 2023 at 08:32:28AM -0800, Nick Desaulniers wrote: > On Sun, Dec 17, 2023 at 1:45 AM Simon Horman wrote: > > > > Although it does not seem to have any untoward side-effects, > > the use of ';' to separate to assignments seems more appropriate than ','. > > > > Flagged by clang-17 -Wcomma > > Yikes! This kind of example is why I hate the comma operator! > > Reviewed-by: Nick Desaulniers > > (Is -Wcomma enabled by -Wall?) No and last time that I looked into enabling it, there were a lot of instances in the kernel: https://lore.kernel.org/20230630192825.GA2745548@dev-arch.thelio-3990X/ It is still probably worth pursuing at some point but that is a lot of instances to clean up (along with potentially having a decent amount of pushback depending on the changes necessary to eliminate all instances). > Is there a fixes tag we can add? > > > > > No functional change intended. > > Compile tested only. > > > > Signed-off-by: Simon Horman > > --- > > drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > > b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > > index 812d04747bd0..f542f2671957 100644 > > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > > @@ -1917,7 +1917,7 @@ int i40e_get_eeprom(struct net_device *netdev, > > len = eeprom->len - (I40E_NVM_SECTOR_SIZE * i); > > last = true; > > } > > - offset = eeprom->offset + (I40E_NVM_SECTOR_SIZE * i), > > + offset = eeprom->offset + (I40E_NVM_SECTOR_SIZE * i); > > ret_val = i40e_aq_read_nvm(hw, 0x0, offset, len, > > (u8 *)eeprom_buff + (I40E_NVM_SECTOR_SIZE * > > i), > > last, NULL); > > > > > > > -- > Thanks, > ~Nick Desaulniers > ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: Reset VF on Tx MDD event
On 12/14/2023 8:51 AM, Pawel Chmielewski wrote: On Thu, Dec 14, 2023 at 09:37:32AM +0100, Michal Schmidt wrote: On Thu, Nov 2, 2023 at 4:56 PM Pawel Chmielewski wrote: From: Liang-Min Wang ... When Malicious Driver Detection event occurs, perform graceful VF reset to quickly bring VF back to operational state. Add a log message to notify about the cause of the reset. Sorry for bringing this up so late, but I have just now realized this: Wasn't freezing of the queue originally the intended behavior, as a penalty for being malicious? Shouldn't these resets at least be guarded by ICE_FLAG_MDD_AUTO_RESET_VF? Michal In some cases, the MDD can be caused also by a regular software error (like the one mentioned in commit message), and not the actual malicious action. There was decision to change the default behavior to avoid denial of service. Michal brings up some valid questions. I'd like to clarify the expectations between how the two should work together before moving forward with this. Thanks, Tony ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next] i40e: add trace events related to SFP module IOCTLs
On 12/14/2023 6:10 AM, Aleksandr Loktionov wrote: Add trace events related to SFP module IOCTLs for troubleshooting. Riewed-by: Przemek Kitszel Signed-off-by: Aleksandr Loktionov --- src/CORE/i40e_ethtool.c | 5 + src/CORE/i40e_trace.h | 18 ++ 2 files changed, 23 insertions(+) This doesn't apply. Thanks, Tony ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v5 1/2] ixgbe: Refactor overtemp event handling
On 12/18/2023 2:39 AM, Jedrzej Jagielski wrote: > Currently ixgbe driver is notified of overheating events > via internal IXGBE_ERR_OVERTEMP error code. > > Change the approach for handle_lasi() to use freshly introduced > is_overtemp function parameter which set when such event occurs. > Change check_overtemp() to bool and return true if overtemp > event occurs. > > Reviewed-by: Przemek Kitszel > Signed-off-by: Jedrzej Jagielski > --- > v2: change aproach to use additional function parameter to notify when > overheat > v4: change check_overtemp to bool > v5: adress Simon's comments > > https://lore.kernel.org/netdev/20231217093800.gx6...@kernel.org/ Reviewed-by: Jacob Keller ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH net-next 20/24] net: intel: Use nested-BH locking for XDP redirect.
On Sat, Dec 16, 2023 at 12:53:43PM +0800, kernel test robot wrote: > Hi Sebastian, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on net-next/main] > > url: > https://github.com/intel-lab-lkp/linux/commits/Sebastian-Andrzej-Siewior/locking-local_lock-Introduce-guard-definition-for-local_lock/20231216-011911 > base: net-next/main > patch link: > https://lore.kernel.org/r/20231215171020.687342-21-bigeasy%40linutronix.de > patch subject: [PATCH net-next 20/24] net: intel: Use nested-BH locking for > XDP redirect. > config: arm-defconfig > (https://download.01.org/0day-ci/archive/20231216/202312161212.d5tju5i6-...@intel.com/config) > compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git > f28c006a5895fc0e329fe15fead81e37457cb1d1) > reproduce (this is a W=1 build): > (https://download.01.org/0day-ci/archive/20231216/202312161212.d5tju5i6-...@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version > of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot > | Closes: > https://lore.kernel.org/oe-kbuild-all/202312161212.d5tju5i6-...@intel.com/ > > All errors (new ones prefixed by >>): > > >> drivers/net/ethernet/intel/igb/igb_main.c:8620:3: error: cannot jump from > >> this goto statement to its label >goto xdp_out; >^ >drivers/net/ethernet/intel/igb/igb_main.c:8624:2: note: jump bypasses > initialization of variable with __attribute__((cleanup)) >guard(local_lock_nested_bh)(&bpf_run_lock.redirect_lock); >^ >include/linux/cleanup.h:142:15: note: expanded from macro 'guard' >CLASS(_name, __UNIQUE_ID(guard)) > ^ >include/linux/compiler.h:180:29: note: expanded from macro '__UNIQUE_ID' >#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), > __COUNTER__) >^ >include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE' >#define __PASTE(a,b) ___PASTE(a,b) > ^ >include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE' >#define ___PASTE(a,b) a##b > ^ >:52:1: note: expanded from here >__UNIQUE_ID_guard753 >^ >1 error generated. I initially thought that this may have been https://github.com/ClangBuiltLinux/linux/issues/1886 but asm goto is not involved here. This error occurs because jumping over the initialization of a variable declared with __attribute__((__cleanup__(...))) does not prevent the clean up function from running as one may expect it to, but could instead result in the clean up function getting run on uninitialized memory. A contrived example (see the bottom of the "Output" tabs for the execution output): https://godbolt.org/z/9bvGboxvc While there is a warning from GCC in that example, I don't see one in the kernel's case. I see there is an open GCC issue around this problem: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951 While it is possible that there may not actually be a problem with how the kernel uses __attribute__((__cleanup__(...))) and gotos, I think clang's behavior is reasonable given the potential footguns that this construct has. Cheers, Nathan ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v2 01/15] e1000e: make lost bits explicit
On 12/6/2023 03:01, Jesse Brandeburg wrote: For more than 15 years this code has passed in a request for a page and masked off that page when read/writing. This code has been here forever, but FIELD_PREP finds the bug when converted to use it. Change the code to do exactly the same thing but allow the conversion to FIELD_PREP in a later patch. To make it clear what we lost when making this change I left a comment, but there is no point to change the code to generate a correct sequence at this point. This is not a Fixes tagged patch on purpose because it doesn't change the binary output. Reviewed-by: Marcin Szycik Signed-off-by: Jesse Brandeburg --- drivers/net/ethernet/intel/e1000e/80003es2lan.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) Tested-by: Naama Meir ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
