Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe
On 2/2/2024 9:01 PM, Alexander Lobakin wrote: From: Maciej Fijalkowski Date: Fri, 2 Feb 2024 14:00:03 +0100 On Fri, Feb 02, 2024 at 01:40:18PM +0100, Alexander Lobakin wrote: From: Alexander Lobakin Date: Fri, 2 Feb 2024 13:39:28 +0100 From: Michal Schmidt Date: Thu, 1 Feb 2024 19:40:17 +0100 On 1/31/24 17:59, Alexander Lobakin wrote: From: Jiri Pirko Date: Wed, 31 Jan 2024 13:17:44 +0100 Wed, Jan 31, 2024 at 12:58:23PM CET, mschm...@redhat.com wrote: diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c index 2a25323105e5..d4848f6fe919 100644 --- a/drivers/net/ethernet/intel/ice/ice_lag.c +++ b/drivers/net/ethernet/intel/ice/ice_lag.c @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw *hw, u16 *rid, new_rcp->content.act_ctrl_fwd_priority = prio; new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT; new_rcp->recipe_indx = *rid; - bitmap_zero((unsigned long *)new_rcp->recipe_bitmap, - ICE_MAX_NUM_RECIPES); - set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap); + put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap); Looks like there might be another incorrect bitmap usage for this in ice_add_sw_recipe(). Care to fix it there as well? Those are already fixed in one switchdev series and will be sent to IWL soon. I believe this patch would also make no sense after it's sent. Hi Alexander, When will the series be sent? The bug causes a kernel panic. Will the series target net.git? The global fix is here: [0] It's targeting net-next. I don't know what the best way here would be. Target net instead or pick your fix to net and then fix it properly in net-next? Sorry, forgot to paste the link :clownface: IMHO 1/2 should go to net. Then you would have to wait for it to got accepted and get merged to -next and then you come back with 2/2. You know the deal. Agree! Hi Steve, Could you please send the first patch from your series to net instead of net-next? (and add "Fixes:" tag with the blamed commit) Hi Olek, Sure, I will do it soon. [0] https://lore.kernel.org/intel-wired-lan/20240130025146.30265-2-steven@intel.com Thanks, Olek Thanks, Olek -- Best Regards, Steven
Re: [Intel-wired-lan] [PATCH net] ice: fix unaligned access in ice_create_lag_recipe
On 2/7/2024 8:44 AM, Zou, Steven wrote: On 2/2/2024 9:01 PM, Alexander Lobakin wrote: From: Maciej Fijalkowski Date: Fri, 2 Feb 2024 14:00:03 +0100 On Fri, Feb 02, 2024 at 01:40:18PM +0100, Alexander Lobakin wrote: From: Alexander Lobakin Date: Fri, 2 Feb 2024 13:39:28 +0100 From: Michal Schmidt Date: Thu, 1 Feb 2024 19:40:17 +0100 On 1/31/24 17:59, Alexander Lobakin wrote: From: Jiri Pirko Date: Wed, 31 Jan 2024 13:17:44 +0100 Wed, Jan 31, 2024 at 12:58:23PM CET, mschm...@redhat.com wrote: diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c index 2a25323105e5..d4848f6fe919 100644 --- a/drivers/net/ethernet/intel/ice/ice_lag.c +++ b/drivers/net/ethernet/intel/ice/ice_lag.c @@ -1829,9 +1829,7 @@ static int ice_create_lag_recipe(struct ice_hw *hw, u16 *rid, new_rcp->content.act_ctrl_fwd_priority = prio; new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT; new_rcp->recipe_indx = *rid; - bitmap_zero((unsigned long *)new_rcp->recipe_bitmap, - ICE_MAX_NUM_RECIPES); - set_bit(*rid, (unsigned long *)new_rcp->recipe_bitmap); + put_unaligned_le64(BIT_ULL(*rid), new_rcp->recipe_bitmap); Looks like there might be another incorrect bitmap usage for this in ice_add_sw_recipe(). Care to fix it there as well? Those are already fixed in one switchdev series and will be sent to IWL soon. I believe this patch would also make no sense after it's sent. Hi Alexander, When will the series be sent? The bug causes a kernel panic. Will the series target net.git? The global fix is here: [0] It's targeting net-next. I don't know what the best way here would be. Target net instead or pick your fix to net and then fix it properly in net-next? Sorry, forgot to paste the link :clownface: IMHO 1/2 should go to net. Then you would have to wait for it to got accepted and get merged to -next and then you come back with 2/2. You know the deal. Agree! Hi Steve, Could you please send the first patch from your series to net instead of net-next? (and add "Fixes:" tag with the blamed commit) Hi Olek, Sure, I will do it soon. Hi team, The patch has been rebased and submitted on: https://lore.kernel.org/intel-wired-lan/20240207014959.24012-1-steven@intel.com/ https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20240207014959.24012-1-steven@intel.com/ Thanks, Steven [0] https://lore.kernel.org/intel-wired-lan/20240130025146.30265-2-steven@intel.com Thanks, Olek Thanks, Olek
Re: [Intel-wired-lan] [PATCH iwl-next] ice: Add switch recipe reusing feature
On 3/6/2024 9:48 PM, Sharma, Mayank wrote: -Original Message- From: Intel-wired-lan On Behalf Of Steven Zou Sent: Thursday, February 8, 2024 8:49 AM To: intel-wired-...@lists.osuosl.org Cc: net...@vger.kernel.org; Zou, Steven ; Staikov, Andrii ; Lobakin, Aleksander ; Nguyen, Anthony L ; Simon Horman ; Kitszel, Przemyslaw Subject: [Intel-wired-lan] [PATCH iwl-next] ice: Add switch recipe reusing feature New E810 firmware supports the corresponding functionality, so the driver allows PFs to subscribe the same switch recipes. Then when the PF is done with a switch recipes, the PF can ask firmware to free that switch recipe. When users configure a rule to PFn into E810 switch component, if there is no existing recipe matching this rule's pattern, the driver will request firmware to allocate and return a new recipe resource for the rule by calling ice_add_sw_recipe() and ice_alloc_recipe(). If there is an existing recipe matching this rule's pattern with different key value, or this is a same second rule to PFm into switch component, the driver checks out this recipe by calling ice_find_recp(), the driver will tell firmware to share using this same recipe resource by calling ice_subscribable_recp_shared() and ice_subscribe_recipe(). When firmware detects that all subscribing PFs have freed the switch recipe, firmware will free the switch recipe so that it can be reused. This feature also fixes a problem where all switch recipes would eventually be exhausted because switch recipes could not be freed, as freeing a shared recipe could potentially break other PFs that were using it. Reviewed-by: Przemek Kitszel Reviewed-by: Andrii Staikov Reviewed-by: Simon Horman Signed-off-by: Steven Zou --- .../net/ethernet/intel/ice/ice_adminq_cmd.h | 2 + drivers/net/ethernet/intel/ice/ice_common.c | 2 + drivers/net/ethernet/intel/ice/ice_switch.c | 187 -- drivers/net/ethernet/intel/ice/ice_switch.h | 1 + drivers/net/ethernet/intel/ice/ice_type.h | 2 + 5 files changed, 177 insertions(+), 17 deletions(-) We are seeing following kernel compilation error while compiling next kernel: "error: dereferencing pointer to incomplete type 'struct dpll_pin'" Thanks Mayank! The patch does not touch 'stuct dpll_pin', I do not think this is caused by the patch changes. And the patch is based on the base-commit: ce1833c065c8c9aec8b147dd682b0ddca8c30071 that I built and loaded the ice.ko successfully before I submitted the patch. Hi Tony, Do you have any suggestion about this compilation issue? Regards, Mayank Sharma Best Regards, Steven
Re: [Intel-wired-lan] [PATCH iwl-next v6] ice: Add support for switch recipe reusing feature
On 9/26/2023 4:11 PM, Przemek Kitszel wrote: On 9/26/23 05:12, Steven Zou wrote: If E810 firmware supports the corresponding functionality, the driver allows PFs to subscribe the same switch recipes. Then when the PF is done with a switch recipes, the PF can ask firmware to free that switch recipe. When users configure a rule to PFn into E810 switch component, if there is no existing recipe matching this rule's pattern, the driver will request firmware to allocate and return a new recipe resource for the rule by calling ice_add_sw_recipe() and ice_alloc_recipe(). If there is an existing recipe matching this rule's pattern with different key value, or this is a same second rule to PFm into switch component, the driver checks out this recipe by calling ice_find_recp(), the driver will tell firmware to share using this same recipe resource by calling ice_subscribable_recp_shared() and ice_subscribe_recipe(). When firmware detects that all subscribing PFs have freed the switch recipe, firmware will free the switch recipe so that it can be reused. This feature also fixes a problem where all switch recipes would eventually be exhausted because switch recipes could not be freed, as freeing a shared recipe could potentially break other PFs that were using it. Signed-off-by: Steven Zou --- v2: - fix nvm version detection for the feature (Thanks to Przemek) - fix more small commit message Reviewed-by Przemek v3: refactor ice_alloc_recipe to support both legacy and new methods v4: - a small change in ice_subscribe_recipe, sw_resp instead of flu_resp - move ice_subscribe_recipe() and ice_subscribable_recp_shared() definitions down to before ice_add_adv_recipe() - add commit message for new function of ice_subscribe_recipe() v5: - refactor ice_init_chk_subscribable_recipe_support() and replace it to ice_init_chk_recipe_reuse_support() - add a newline before 'return' in ice_init_hw() - remove redundant cast of u8 - make u8 and u16 to u32 in the subfunctions v6: - fix AdminQ data type from/to FW and casting for bitmap type issue - fix hole padding with adding new member in ice_hw structure - fix setting the flag of recp_created issue - fix redundant lable - change bit traversal method --- .../net/ethernet/intel/ice/ice_adminq_cmd.h | 4 +- drivers/net/ethernet/intel/ice/ice_common.c | 2 + drivers/net/ethernet/intel/ice/ice_lag.c | 4 +- drivers/net/ethernet/intel/ice/ice_switch.c | 247 +++--- drivers/net/ethernet/intel/ice/ice_switch.h | 5 +- drivers/net/ethernet/intel/ice/ice_type.h | 2 + 6 files changed, 229 insertions(+), 35 deletions(-) Hi, we are not yet done with our pre-IWL (e1000) review. General rule is to have Reviewed-by: tag prior to posting here. And public submission versioning should start with v1, without the changelog from e1000. Normally one should also CC netdev, which you have not. Thank you very much! My mistake. Please ignore my submission this time here. ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
