Re: [Intel-wired-lan] [PATCH] ice: Use common error handling code in two functions
On 9/19/24 19:15, Markus Elfring wrote: From: Markus Elfring Date: Thu, 19 Sep 2024 19:00:25 +0200 Add jump targets so that a bit of exception handling can be better reused at the end of two function implementations. Thank you for contribution, the change is fine, but not as a bugfix. Please send as a [iwl-next], when the submission window opens. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring @@ -3168,6 +3164,10 @@ static int ice_ptp_init_owner(struct ice_pf *pf) pf->ptp.clock = NULL; err_exit: return err; + +err_unlock: + ice_ptp_unlock(hw); + return err; } You kept the current label naming scheme, that's good.
Re: [Intel-wired-lan] [PATCH net 2/2] page_pool: fix IOMMU crash when driver has already unbound
On 19/09/2024 13.15, Yunsheng Lin wrote: On 2024/9/19 17:42, Jesper Dangaard Brouer wrote: On 18/09/2024 19.06, Ilias Apalodimas wrote: In order not to do the dma unmmapping after driver has already unbound and stall the unloading of the networking driver, add the pool->items array to record all the pages including the ones which are handed over to network stack, so the page_pool can do the dma unmmapping for those pages when page_pool_destroy() is called. So, I was thinking of a very similar idea. But what do you mean by "all"? The pages that are still in caches (slow or fast) of the pool will be unmapped during page_pool_destroy(). I really dislike this idea of having to keep track of all outstanding pages. I liked Jakub's idea of keeping the netdev around for longer. This is all related to destroying the struct device that have points to the DMA engine, right? Yes, the problem seems to be that when device_del() is called, there is no guarantee hw behind the 'struct device ' will be usable even if we call get_device() on it. Why don't we add an API that allow netdev to "give" struct device to page_pool. And then the page_poll will take over when we can safely free the stuct device? By 'allow netdev to "give" struct device to page_pool', does it mean page_pool become the driver for the device? If yes, it seems that is similar to jakub's idea, as both seems to stall the calling of device_del() by not returning when the driver unloading. Yes, this is what I mean. (That is why I mentioned Jakub's idea). If no, it seems that the problem is still existed when the driver for the device has unbound after device_del() is called.
Re: [Intel-wired-lan] [PATCH net 2/2] page_pool: fix IOMMU crash when driver has already unbound
Hi Jesper, On Fri, 20 Sept 2024 at 00:04, Jesper Dangaard Brouer wrote: > > > > On 19/09/2024 13.15, Yunsheng Lin wrote: > > On 2024/9/19 17:42, Jesper Dangaard Brouer wrote: > >> > >> On 18/09/2024 19.06, Ilias Apalodimas wrote: > In order not to do the dma unmmapping after driver has already > unbound and stall the unloading of the networking driver, add > the pool->items array to record all the pages including the ones > which are handed over to network stack, so the page_pool can > do the dma unmmapping for those pages when page_pool_destroy() > is called. > >>> > >>> So, I was thinking of a very similar idea. But what do you mean by > >>> "all"? The pages that are still in caches (slow or fast) of the pool > >>> will be unmapped during page_pool_destroy(). > >> > >> I really dislike this idea of having to keep track of all outstanding > >> pages. > >> > >> I liked Jakub's idea of keeping the netdev around for longer. > >> > >> This is all related to destroying the struct device that have points to > >> the DMA engine, right? > > > > Yes, the problem seems to be that when device_del() is called, there is > > no guarantee hw behind the 'struct device ' will be usable even if we > > call get_device() on it. > > > >> > >> Why don't we add an API that allow netdev to "give" struct device to > >> page_pool. And then the page_poll will take over when we can safely > >> free the stuct device? > > > > By 'allow netdev to "give" struct device to page_pool', does it mean > > page_pool become the driver for the device? > > If yes, it seems that is similar to jakub's idea, as both seems to stall > > the calling of device_del() by not returning when the driver unloading. > > Yes, this is what I mean. (That is why I mentioned Jakub's idea). Keeping track of inflight packets that need to be unmapped is certainly more complex. Delaying the netdevice destruction certainly solves the problem but there's a huge cost IMHO. Those devices might stay there forever and we have zero guarantees that the network stack will eventually release (and unmap) those packets. What happens in that case? The user basically has to reboot the entire machine, just because he tries to bring an interface down and up again. Thanks /Ilias > > > > If no, it seems that the problem is still existed when the driver for > > the device has unbound after device_del() is called.
[Intel-wired-lan] [tnguy-next-queue:dev-queue] BUILD SUCCESS 2f0bdf7f38208cd8ddac5c4aa261829a8056c192
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue branch HEAD: 2f0bdf7f38208cd8ddac5c4aa261829a8056c192 ice: Implement PTP support for E830 devices elapsed time: 2582m configs tested: 105 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alpha allnoconfiggcc-13.3.0 alphaallyesconfiggcc-13.3.0 arc allmodconfiggcc-13.2.0 arc allnoconfiggcc-13.2.0 arc allyesconfiggcc-13.2.0 arc randconfig-001-20240919gcc-13.2.0 arc randconfig-002-20240919gcc-13.2.0 arm allmodconfiggcc-14.1.0 arm allnoconfigclang-20 arm allyesconfiggcc-14.1.0 arm exynos_defconfigclang-17 arm randconfig-001-20240919clang-20 arm randconfig-002-20240919clang-20 arm randconfig-003-20240919clang-20 arm randconfig-004-20240919clang-20 arm wpcm450_defconfiggcc-14.1.0 arm64allmodconfigclang-20 arm64 allnoconfiggcc-14.1.0 arm64 randconfig-001-20240919gcc-14.1.0 arm64 randconfig-002-20240919clang-15 arm64 randconfig-003-20240919clang-20 arm64 randconfig-004-20240919gcc-14.1.0 csky allnoconfiggcc-14.1.0 csky randconfig-001-20240919gcc-14.1.0 csky randconfig-002-20240919gcc-14.1.0 hexagon allnoconfigclang-20 hexagon randconfig-001-20240919clang-20 hexagon randconfig-002-20240919clang-20 i386 allmodconfiggcc-12 i386 allnoconfiggcc-12 i386 allyesconfiggcc-12 i386buildonly-randconfig-001-20240919clang-18 i386buildonly-randconfig-002-20240919gcc-12 i386buildonly-randconfig-003-20240919clang-18 i386buildonly-randconfig-004-20240919gcc-12 i386buildonly-randconfig-005-20240919clang-18 i386buildonly-randconfig-006-20240919clang-18 i386defconfigclang-18 i386 randconfig-001-20240919clang-18 i386 randconfig-002-20240919clang-18 i386 randconfig-003-20240919gcc-12 i386 randconfig-004-20240919clang-18 i386 randconfig-005-20240919clang-18 i386 randconfig-006-20240919clang-18 i386 randconfig-011-20240919gcc-12 i386 randconfig-012-20240919gcc-12 i386 randconfig-013-20240919clang-18 i386 randconfig-014-20240919gcc-12 i386 randconfig-015-20240919gcc-11 i386 randconfig-016-20240919clang-18 loongarchallmodconfiggcc-14.1.0 loongarch allnoconfiggcc-14.1.0 loongarch randconfig-001-20240919gcc-14.1.0 loongarch randconfig-002-20240919gcc-14.1.0 m68k allmodconfiggcc-14.1.0 m68k allnoconfiggcc-14.1.0 m68k allyesconfiggcc-14.1.0 microblazeallnoconfiggcc-14.1.0 mips allnoconfiggcc-14.1.0 mips ip22_defconfiggcc-14.1.0 mips loongson2k_defconfiggcc-13.2.0 mips maltaaprp_defconfigclang-14 nios2 allnoconfiggcc-14.1.0 nios2 randconfig-001-20240919gcc-14.1.0 nios2 randconfig-002-20240919gcc-14.1.0 openrisc allnoconfiggcc-14.1.0 openrisc allyesconfiggcc-14.1.0 parisc alldefconfiggcc-14.1.0 parisc allmodconfiggcc-14.1.0 pariscallnoconfiggcc-14.1.0 parisc allyesconfiggcc-14.1.0 pariscrandconfig-001-20240919gcc-14.1.0 pariscrandconfig-002-20240919gcc-14.1.0 powerpc allmodconfiggcc-14.1.0 powerpc allnoconfiggcc-14.1.0 powerpc allyesconfigclang-20 powerpc arches_defconfiggcc-14.1.0 powerpc ps3_defconfiggcc-14.1.0 powerpc randconfig-001
Re: [Intel-wired-lan] [PATCH iwl-net v2] ice: disallow DPLL_PIN_STATE_SELECTABLE for dpll output pins
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Arkadiusz Kubalewski
> Sent: Thursday, September 12, 2024 2:24 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: Loktionov, Aleksandr ; Paul Menzel
> ; Kubalewski, Arkadiusz
>
> Subject: [Intel-wired-lan] [PATCH iwl-net v2] ice: disallow
> DPLL_PIN_STATE_SELECTABLE for dpll output pins
>
> Currently the user may request DPLL_PIN_STATE_SELECTABLE for an output pin,
> and this would actually set the DISCONNECTED state instead.
>
> It doesn't make any sense. SELECTABLE is valid only in case of input pins (on
> AUTOMATIC type dpll), where dpll itself would select best valid input.
> For the output pin only CONNECTED/DISCONNECTED are expected.
>
> Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
> Reviewed-by: Aleksandr Loktionov
> Reviewed-by: Paul Menzel
> Signed-off-by: Arkadiusz Kubalewski
> ---
> v2:
> - use more explicit commit title
> - add empty line between commit message paragraphs
> ---
> drivers/net/ethernet/intel/ice/ice_dpll.c | 2 ++
> 1 file changed, 2 insertions(+)
>
Tested-by: Pucha Himasekhar Reddy (A
Contingent worker at Intel)
Re: [Intel-wired-lan] [PATCH iwl-net v2] ice: disallow DPLL_PIN_STATE_SELECTABLE for dpll output pins
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Arkadiusz Kubalewski
> Sent: Thursday, September 12, 2024 2:24 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: Loktionov, Aleksandr ; Paul Menzel
> ; Kubalewski, Arkadiusz
>
> Subject: [Intel-wired-lan] [PATCH iwl-net v2] ice: disallow
> DPLL_PIN_STATE_SELECTABLE for dpll output pins
>
> Currently the user may request DPLL_PIN_STATE_SELECTABLE for an output pin,
> and this would actually set the DISCONNECTED state instead.
>
> It doesn't make any sense. SELECTABLE is valid only in case of input pins (on
> AUTOMATIC type dpll), where dpll itself would select best valid input.
> For the output pin only CONNECTED/DISCONNECTED are expected.
>
> Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
> Reviewed-by: Aleksandr Loktionov
> Reviewed-by: Paul Menzel
> Signed-off-by: Arkadiusz Kubalewski
> ---
> v2:
> - use more explicit commit title
> - add empty line between commit message paragraphs
> ---
> drivers/net/ethernet/intel/ice/ice_dpll.c | 2 ++
> 1 file changed, 2 insertions(+)
>
Tested-by: Pucha Himasekhar Reddy (A
Contingent worker at Intel)
Re: [Intel-wired-lan] [PATCH net 2/2] page_pool: fix IOMMU crash when driver has already unbound
On 2024/9/19 1:06, Ilias Apalodimas wrote: > Hi Yunsheng, > > Thanks for looking into this! > > On Wed, 18 Sept 2024 at 14:24, Yunsheng Lin wrote: >> >> Networking driver with page_pool support may hand over page >> still with dma mapping to network stack and try to reuse that >> page after network stack is done with it and passes it back >> to page_pool to avoid the penalty of dma mapping/unmapping. > > I think you can shorten this to "If recycling and DMA mapping are > enabled during the pool creation" I am not sure if I understand the 'recycling' part here. Is the 'recycling' part referring to whether skb_mark_for_recycle() is called to enable recycling for the skb? Is there still any driver with page_pool support but doesn't call skb_mark_for_recycle() when handing over page to network stack? For the 'DMA mapping' part, as there is no space in 'struct page' to track the inflight pages, so 'pp' in 'struct page' is renamed to 'pp_item' to enable the tracking of inflight page. I tried shortening this for 'pool->dma_map being false' when coding, but it seems differentiating the same field in 'struct page' doesn't make much sense according to 'pool->dma_map' as it means we might need to add an union in 'struct page' for that to work and add additional checking to decide if it is 'pp' or 'pp_item'. > >> With all the caching in the network stack, some pages may be >> held in the network stack without returning to the page_pool >> soon enough, and with VF disable causing the driver unbound, >> the page_pool does not stop the driver from doing it's >> unbounding work, instead page_pool uses workqueue to check >> if there is some pages coming back from the network stack >> periodically, if there is any, it will do the dma unmmapping >> related cleanup work. >> >> As mentioned in [1], attempting DMA unmaps after the driver >> has already unbound may leak resources or at worst corrupt >> memory. Fundamentally, the page pool code cannot allow DMA >> mappings to outlive the driver they belong to. >> >> Currently it seems there are at least two cases that the page >> is not released fast enough causing dma unmmapping done after >> driver has already unbound: >> 1. ipv4 packet defragmentation timeout: this seems to cause >>delay up to 30 secs: >> >> 2. skb_defer_free_flush(): this may cause infinite delay if >>there is no triggering for net_rx_action(). >> >> In order not to do the dma unmmapping after driver has already >> unbound and stall the unloading of the networking driver, add >> the pool->items array to record all the pages including the ones >> which are handed over to network stack, so the page_pool can >> do the dma unmmapping for those pages when page_pool_destroy() >> is called. > > So, I was thinking of a very similar idea. But what do you mean by > "all"? The pages that are still in caches (slow or fast) of the pool > will be unmapped during page_pool_destroy(). Yes, it includes the one in pool->alloc and pool->ring. > Don't we 'just' need a list of the inflight packets and their pages or > fragments? What we could do is go through that list and unmap these > pages during page_pool_destroy(). The main reason for that is to avoid the overhead of page_pool_item_del() and page_pool_item_add() when allocing/freeing page from/to pool->alloc and pool->ring. Yes, including the pages in pool->ring seems to make the pool->ring somewhat duplicated, maybe we can remove pool->ring if we can make and prove 'pool->items' is performing better than pool->ring in the future? > > I'll have a closer look at the patch tomorrow Thanks for the reviewing. > > Thanks! > /Ilias >
Re: [Intel-wired-lan] [PATCH net 2/2] page_pool: fix IOMMU crash when driver has already unbound
On 2024/9/19 17:42, Jesper Dangaard Brouer wrote: > > On 18/09/2024 19.06, Ilias Apalodimas wrote: >>> In order not to do the dma unmmapping after driver has already >>> unbound and stall the unloading of the networking driver, add >>> the pool->items array to record all the pages including the ones >>> which are handed over to network stack, so the page_pool can >>> do the dma unmmapping for those pages when page_pool_destroy() >>> is called. >> >> So, I was thinking of a very similar idea. But what do you mean by >> "all"? The pages that are still in caches (slow or fast) of the pool >> will be unmapped during page_pool_destroy(). > > I really dislike this idea of having to keep track of all outstanding pages. > > I liked Jakub's idea of keeping the netdev around for longer. > > This is all related to destroying the struct device that have points to > the DMA engine, right? Yes, the problem seems to be that when device_del() is called, there is no guarantee hw behind the 'struct device ' will be usable even if we call get_device() on it. > > Why don't we add an API that allow netdev to "give" struct device to > page_pool. And then the page_poll will take over when we can safely > free the stuct device? By 'allow netdev to "give" struct device to page_pool', does it mean page_pool become the driver for the device? If yes, it seems that is similar to jakub's idea, as both seems to stall the calling of device_del() by not returning when the driver unloading. If no, it seems that the problem is still existed when the driver for the device has unbound after device_del() is called. > > --Jesper
Re: [Intel-wired-lan] [PATCH v7 net-next 11/15] testing: net-drv: add basic shaper test
On 9/10/24 23:41, Stanislav Fomichev wrote:
On 09/10, Paolo Abeni wrote:
Leverage a basic/dummy netdevsim implementation to do functional
coverage for NL interface.
Signed-off-by: Paolo Abeni
---
v5 -> v6:
- additional test-cases for delegation and queue reconf
v4 -> v5:
- updated to new driver API
- more consistent indentation
rfc v1 -> v2:
- added more test-cases WRT nesting and grouping
---
drivers/net/Kconfig | 1 +
drivers/net/netdevsim/ethtool.c | 2 +
drivers/net/netdevsim/netdev.c| 39 ++
tools/testing/selftests/drivers/net/Makefile | 1 +
tools/testing/selftests/drivers/net/shaper.py | 457 ++
.../testing/selftests/net/lib/py/__init__.py | 1 +
tools/testing/selftests/net/lib/py/ynl.py | 5 +
7 files changed, 506 insertions(+)
create mode 100755 tools/testing/selftests/drivers/net/shaper.py
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 9920b3a68ed1..1fd5acdc73c6 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -641,6 +641,7 @@ config NETDEVSIM
depends on PTP_1588_CLOCK_MOCK || PTP_1588_CLOCK_MOCK=n
select NET_DEVLINK
select PAGE_POOL
+ select NET_SHAPER
help
This driver is a developer testing tool and software model that can
be used to test various control path networking APIs, especially
diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index 1436905bc106..5fe1eaef99b5 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -103,8 +103,10 @@ nsim_set_channels(struct net_device *dev, struct
ethtool_channels *ch)
struct netdevsim *ns = netdev_priv(dev);
int err;
+ mutex_lock(&dev->lock);
err = netif_set_real_num_queues(dev, ch->combined_count,
ch->combined_count);
+ mutex_unlock(&dev->lock);
if (err)
return err;
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 017a6102be0a..cad85bb0cf54 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -475,6 +476,43 @@ static int nsim_stop(struct net_device *dev)
return 0;
}
+static int nsim_shaper_set(struct net_shaper_binding *binding,
+ const struct net_shaper *shaper,
+ struct netlink_ext_ack *extack)
+{
+ return 0;
+}
+
+static int nsim_shaper_del(struct net_shaper_binding *binding,
+ const struct net_shaper_handle *handle,
+ struct netlink_ext_ack *extack)
+{
+ return 0;
+}
+
+static int nsim_shaper_group(struct net_shaper_binding *binding,
+int leaves_count,
+const struct net_shaper *leaves,
+const struct net_shaper *root,
+struct netlink_ext_ack *extack)
+{
+ return 0;
+}
+
+static void nsim_shaper_cap(struct net_shaper_binding *binding,
+ enum net_shaper_scope scope,
+ unsigned long *flags)
+{
+ *flags = ULONG_MAX;
+}
+
+static const struct net_shaper_ops nsim_shaper_ops = {
+ .set= nsim_shaper_set,
+ .delete = nsim_shaper_del,
+ .group = nsim_shaper_group,
+ .capabilities = nsim_shaper_cap,
+};
+
static const struct net_device_ops nsim_netdev_ops = {
.ndo_start_xmit = nsim_start_xmit,
.ndo_set_rx_mode= nsim_set_rx_mode,
@@ -496,6 +534,7 @@ static const struct net_device_ops nsim_netdev_ops = {
.ndo_bpf= nsim_bpf,
.ndo_open = nsim_open,
.ndo_stop = nsim_stop,
+ .net_shaper_ops = &nsim_shaper_ops,
};
static const struct net_device_ops nsim_vf_netdev_ops = {
diff --git a/tools/testing/selftests/drivers/net/Makefile
b/tools/testing/selftests/drivers/net/Makefile
index 39fb97a8c1df..25aec5c081df 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -9,6 +9,7 @@ TEST_PROGS := \
ping.py \
queues.py \
stats.py \
+ shaper.py
# end of TEST_PROGS
include ../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/shaper.py
b/tools/testing/selftests/drivers/net/shaper.py
new file mode 100755
index ..3504d51985bc
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/shaper.py
@@ -0,0 +1,457 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_true, KsftSkipEx
+from lib.py import EthtoolFamily, NetshaperFamily
+from lib.py import NetDrvEnv
+from lib.py import N
Re: [Intel-wired-lan] [PATCH net 2/2] page_pool: fix IOMMU crash when driver has already unbound
On 18/09/2024 19.06, Ilias Apalodimas wrote: In order not to do the dma unmmapping after driver has already unbound and stall the unloading of the networking driver, add the pool->items array to record all the pages including the ones which are handed over to network stack, so the page_pool can do the dma unmmapping for those pages when page_pool_destroy() is called. So, I was thinking of a very similar idea. But what do you mean by "all"? The pages that are still in caches (slow or fast) of the pool will be unmapped during page_pool_destroy(). I really dislike this idea of having to keep track of all outstanding pages. I liked Jakub's idea of keeping the netdev around for longer. This is all related to destroying the struct device that have points to the DMA engine, right? Why don't we add an API that allow netdev to "give" struct device to page_pool. And then the page_poll will take over when we can safely free the stuct device? --Jesper
Re: [Intel-wired-lan] [PATCH v7 net-next 14/15] iavf: Add net_shaper_ops support
On 9/11/24 00:03, Jakub Kicinski wrote:
On Tue, 10 Sep 2024 00:10:08 +0200 Paolo Abeni wrote:
+ if (adapter->netdev->reg_state == NETREG_REGISTERED) {
+ mutex_lock(&adapter->netdev->lock);
+ devlock = true;
+ }
This leads to a false positive in cocci.
Any concerns about moving the mutex_init() / _destroy() into
alloc_netdev_mqs() / free_netdev()? I guess one could argue
that narrower scope of the lock being valid may help catching
errors, but I think we'll instead end up with more checks like
the above sprinkled around than bugs caught?
I considered moving the locking initialization and shutdown, but I kept
there for symmetry with the rss_lock. I'll move the init/destroy in the
next revision.
Thanks,
Paolo
Re: [Intel-wired-lan] [PATCH v7 net-next 02/15] netlink: spec: add shaper YAML spec
Hi, On 9/10/24 14:08, Paul Menzel wrote>> +config NET_SHAPER + bool It’d be great if you added a help text/description. […] Thank you for the feedback. The lack of description here is intentional: we don't want user to enable the knob explicitly, only via 'select'. I'll handle the other comments in the next revision. Cheers, Paolo
[Intel-wired-lan] [PATCH] ice: Use common error handling code in two functions
From: Markus Elfring
Date: Thu, 19 Sep 2024 19:00:25 +0200
Add jump targets so that a bit of exception handling can be better reused
at the end of two function implementations.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index ef2e858f49bb..c445ae80094b 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2813,10 +2813,8 @@ static int ice_ptp_rebuild_owner(struct ice_pf *pf)
/* Write the increment time value to PHY and LAN */
err = ice_ptp_write_incval(hw, ice_base_incval(pf));
- if (err) {
- ice_ptp_unlock(hw);
- return err;
- }
+ if (err)
+ goto err_unlock;
/* Write the initial Time value to PHY and LAN using the cached PHC
* time before the reset and time difference between stopping and
@@ -2829,10 +2827,8 @@ static int ice_ptp_rebuild_owner(struct ice_pf *pf)
ts = ktime_to_timespec64(ktime_get_real());
}
err = ice_ptp_write_init(pf, &ts);
- if (err) {
- ice_ptp_unlock(hw);
- return err;
- }
+ if (err)
+ goto err_unlock;
/* Release the global hardware lock */
ice_ptp_unlock(hw);
@@ -2856,6 +2852,10 @@ static int ice_ptp_rebuild_owner(struct ice_pf *pf)
ice_ptp_enable_all_extts(pf);
return 0;
+
+err_unlock:
+ ice_ptp_unlock(hw);
+ return err;
}
/**
@@ -3129,18 +3129,14 @@ static int ice_ptp_init_owner(struct ice_pf *pf)
/* Write the increment time value to PHY and LAN */
err = ice_ptp_write_incval(hw, ice_base_incval(pf));
- if (err) {
- ice_ptp_unlock(hw);
- goto err_exit;
- }
+ if (err)
+ goto err_unlock;
ts = ktime_to_timespec64(ktime_get_real());
/* Write the initial Time value to PHY and LAN */
err = ice_ptp_write_init(pf, &ts);
- if (err) {
- ice_ptp_unlock(hw);
- goto err_exit;
- }
+ if (err)
+ goto err_unlock;
/* Release the global hardware lock */
ice_ptp_unlock(hw);
@@ -3168,6 +3164,10 @@ static int ice_ptp_init_owner(struct ice_pf *pf)
pf->ptp.clock = NULL;
err_exit:
return err;
+
+err_unlock:
+ ice_ptp_unlock(hw);
+ return err;
}
/**
--
2.46.0
Re: [Intel-wired-lan] [PATCH] ice: Use common error handling code in two functions
> -Original Message- > From: Markus Elfring > Sent: Thursday, September 19, 2024 10:15 AM > To: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; David S. Miller > ; Eric Dumazet ; Keller, Jacob E > ; Jakub Kicinski ; Kolacinski, > Karol > ; Paolo Abeni ; Kitszel, > Przemyslaw ; Richard Cochran > ; Nguyen, Anthony L > Cc: LKML > Subject: [PATCH] ice: Use common error handling code in two functions > > From: Markus Elfring > Date: Thu, 19 Sep 2024 19:00:25 +0200 > > Add jump targets so that a bit of exception handling can be better reused > at the end of two function implementations. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring > --- Seems fine. Reviewed-by: Jacob Keller
