Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: Disable Cage Max Power override
> -Original Message- > From: Intel-wired-lan On Behalf Of Ido > Schimmel > Sent: Thursday, September 21, 2023 2:09 PM > To: Drewek, Wojciech > Cc: Jakub Kicinski ; ido...@nvidia.com; intel-wired- > l...@lists.osuosl.org; Kitszel, Przemyslaw ; > net...@vger.kernel.org > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: Disable Cage Max Power > override > > On Fri, Sep 15, 2023 at 01:15:01PM +, Drewek, Wojciech wrote: > > In ice driver port split works per device not per port. According to > > /Documentation/networking/devlink/ice.rst, section "Port split": > > The "ice" driver supports port splitting only for port 0, as the FW has > > a predefined set of available port split options for the whole device. > > And if you look at available port options (same file) you'll see that in > > case of > "Split count" 1 > > only quad 1 is working. And in case of "Split count" 2 the second quad might > be used. So, if we > > increase max_pwr in the first quad in case of "Split count" 1 and then > > switch > to "Split count" 2, > > the second quad might end up with no link (because it will have decreased > max_pwr). > > But there's also an option where the second cage isn't actually used. > Anyway, my suggestion is to allow user space to set / get the max power > using ethtool and give user space visibility about link down reason via > the ethtool extended state. Thanks for discussion Ido, we will follow your suggestion. > ___ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ___ 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 1/5] iavf: fix comments about old bit locks
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Michal Schmidt
> Sent: Tuesday, October 10, 2023 2:25 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: Nguyen, Anthony L ; Radoslaw Tyl
> ; Brandeburg, Jesse
> Subject: [Intel-wired-lan] [PATCH net-next 1/5] iavf: fix comments about old
> bit locks
>
> Bit lock __IAVF_IN_CRITICAL_TASK does not exist anymore since commit
> 5ac49f3c2702 ("iavf: use mutexes for locking of critical sections").
> Adjust the comments accordingly.
>
> Signed-off-by: Michal Schmidt
Thanks for your contribution, Michal!
Reviewed-by: Wojciech Drewek
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 43c47c633162..98ecf5d5a2f2 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1276,7 +1276,7 @@ static void iavf_configure(struct iavf_adapter
> *adapter)
> * iavf_up_complete - Finish the last steps of bringing up a connection
> * @adapter: board private structure
> *
> - * Expects to be called while holding the __IAVF_IN_CRITICAL_TASK bit lock.
> + * Expects to be called while holding crit_lock.
> **/
> static void iavf_up_complete(struct iavf_adapter *adapter)
> {
> @@ -1400,7 +1400,7 @@ static void iavf_clear_adv_rss_conf(struct
> iavf_adapter *adapter)
> * iavf_down - Shutdown the connection processing
> * @adapter: board private structure
> *
> - * Expects to be called while holding the __IAVF_IN_CRITICAL_TASK bit lock.
> + * Expects to be called while holding crit_lock.
> **/
> void iavf_down(struct iavf_adapter *adapter)
> {
> --
> 2.41.0
>
> ___
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
___
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 2/5] iavf: simplify mutex_trylock+sleep loops
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Michal Schmidt
> Sent: Tuesday, October 10, 2023 2:25 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: Nguyen, Anthony L ; Radoslaw Tyl
> ; Brandeburg, Jesse
> Subject: [Intel-wired-lan] [PATCH net-next 2/5] iavf: simplify
> mutex_trylock+sleep loops
>
> This pattern appears in two places in the iavf source code:
> while (!mutex_trylock(...))
> usleep_range(...);
I found a few other places with similar pattern (iavf_configure_clsflower e.g).
Do you think we can fix them as well?
>
> That's just mutex_lock with extra steps.
> The pattern is a leftover from when iavf used bit flags instead of
> mutexes for locking. Commit 5ac49f3c2702 ("iavf: use mutexes for locking
> of critical sections") replaced test_and_set_bit with !mutex_trylock,
> preserving the pattern.
>
> Simplify it to mutex_lock.
>
> Signed-off-by: Michal Schmidt
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 98ecf5d5a2f2..03156ca523fe 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -3016,8 +3016,7 @@ static void iavf_reset_task(struct work_struct
> *work)
> return;
> }
>
> - while (!mutex_trylock(&adapter->client_lock))
> - usleep_range(500, 1000);
> + mutex_lock(&adapter->client_lock);
> if (CLIENT_ENABLED(adapter)) {
> adapter->flags &= ~(IAVF_FLAG_CLIENT_NEEDS_OPEN |
> IAVF_FLAG_CLIENT_NEEDS_CLOSE |
> @@ -5069,8 +5068,7 @@ static int __maybe_unused iavf_suspend(struct
> device *dev_d)
>
> netif_device_detach(netdev);
>
> - while (!mutex_trylock(&adapter->crit_lock))
> - usleep_range(500, 1000);
> + mutex_lock(&adapter->crit_lock);
>
> if (netif_running(netdev)) {
> rtnl_lock();
> --
> 2.41.0
>
> ___
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
___
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 3/5] iavf: in iavf_down, don't queue watchdog_task if comms failed
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Michal Schmidt
> Sent: Tuesday, October 10, 2023 2:25 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: Nguyen, Anthony L ; Radoslaw Tyl
> ; Brandeburg, Jesse
> Subject: [Intel-wired-lan] [PATCH net-next 3/5] iavf: in iavf_down, don't
> queue watchdog_task if comms failed
>
> The reason for queueing watchdog_task is to have it process the
> aq_required flags that are being set here. If comms failed, there's
> nothing to do, so return early.
>
> Signed-off-by: Michal Schmidt
Reviewed-by: Wojciech Drewek
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 03156ca523fe..0b808fa34801 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1420,8 +1420,10 @@ void iavf_down(struct iavf_adapter *adapter)
> iavf_clear_fdir_filters(adapter);
> iavf_clear_adv_rss_conf(adapter);
>
> - if (!(adapter->flags & IAVF_FLAG_PF_COMMS_FAILED) &&
> - !(test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section))) {
> + if (adapter->flags & IAVF_FLAG_PF_COMMS_FAILED)
> + return;
> +
> + if (!test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) {
> /* cancel any current operation */
> adapter->current_op = VIRTCHNL_OP_UNKNOWN;
> /* Schedule operations to close down the HW. Don't wait
> --
> 2.41.0
>
> ___
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
___
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 4/5] iavf: in iavf_down, disable queues when removing the driver
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Michal Schmidt
> Sent: Tuesday, October 10, 2023 2:25 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: Nguyen, Anthony L ; Radoslaw Tyl
> ; Brandeburg, Jesse
> Subject: [Intel-wired-lan] [PATCH net-next 4/5] iavf: in iavf_down, disable
> queues when removing the driver
>
> In iavf_down, we're skipping the scheduling of certain operations if
> the driver is being removed. However, the IAVF_FLAG_AQ_DISABLE_QUEUES
> request must not be skipped in this case, because iavf_close waits
> for the transition to the __IAVF_DOWN state, which happens in
> iavf_virtchnl_completion after the queues are released.
>
> Without this fix, "rmmod iavf" takes half a second per interface that's
> up and prints the "Device resources not yet released" warning.
>
> Fixes: c8de44b577eb ("iavf: do not process adminq tasks when
> __IAVF_IN_REMOVE_TASK is set")
> Signed-off-by: Michal Schmidt
Looks like a fix, could be a separate patch with net as a target.
Reviewed-by: Wojciech Drewek
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 0b808fa34801..2ab08b015b85 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1440,9 +1440,9 @@ void iavf_down(struct iavf_adapter *adapter)
> adapter->aq_required |=
> IAVF_FLAG_AQ_DEL_FDIR_FILTER;
> if (!list_empty(&adapter->adv_rss_list_head))
> adapter->aq_required |=
> IAVF_FLAG_AQ_DEL_ADV_RSS_CFG;
> - adapter->aq_required |= IAVF_FLAG_AQ_DISABLE_QUEUES;
> }
>
> + adapter->aq_required |= IAVF_FLAG_AQ_DISABLE_QUEUES;
> mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
> }
>
> --
> 2.41.0
>
> ___
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
___
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 5/5] iavf: fix the waiting time for initial reset
> -Original Message- > From: Intel-wired-lan On Behalf Of > Michal Schmidt > Sent: Tuesday, October 10, 2023 2:25 AM > To: intel-wired-...@lists.osuosl.org > Cc: Nguyen, Anthony L ; Radoslaw Tyl > ; Brandeburg, Jesse > Subject: [Intel-wired-lan] [PATCH net-next 5/5] iavf: fix the waiting time for > initial reset > > Every time I create VFs on ice, I receive at least one "Device is still > in reset (-16), retrying" message per VF. It recovers fine, but typical > usecases should not trigger scary-looking messages. > > The waiting for reset is too short. It makes no sense to check every 10 > microseconds. Typical reset waiting times are at least tens of > milliseconds and can be several seconds. I suspect the polling interval > was meant to be 10 milliseconds all along. > > IAVF_RESET_WAIT_COMPLETE_COUNT is defined as 2000, so the total waiting > time could be over 20 seconds. I have seen resets take 5 seconds (with > 128 VFs on ice). > > The added benefit of not triggering the "Device is still in reset" path > is that we avoid going through the __IAVF_INIT_FAILED state, which would > take a full second before retrying. > > Signed-off-by: Michal Schmidt Reviewed-by: Wojciech Drewek > --- > drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c > b/drivers/net/ethernet/intel/iavf/iavf_main.c > index 2ab08b015b85..f35d74566faa 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c > @@ -4791,7 +4791,7 @@ static int iavf_check_reset_complete(struct > iavf_hw *hw) > if ((rstat == VIRTCHNL_VFR_VFACTIVE) || > (rstat == VIRTCHNL_VFR_COMPLETED)) > return 0; > - usleep_range(10, 20); > + msleep(IAVF_RESET_WAIT_MS); > } > return -EBUSY; > } > -- > 2.41.0 > > ___ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ___ 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 2/5] iavf: simplify mutex_trylock+sleep loops
> -Original Message-
> From: Michal Schmidt
> Sent: Tuesday, October 10, 2023 1:57 PM
> To: Drewek, Wojciech ; intel-wired-
> l...@lists.osuosl.org
> Cc: Nguyen, Anthony L ; Radoslaw Tyl
> ; Brandeburg, Jesse
> Subject: Re: [Intel-wired-lan] [PATCH net-next 2/5] iavf: simplify
> mutex_trylock+sleep loops
>
> Dne 10. 10. 23 v 13:30 Drewek, Wojciech napsal:
> >> -Original Message-
> >> From: Intel-wired-lan On Behalf Of
> >> Michal Schmidt
> >> Sent: Tuesday, October 10, 2023 2:25 AM
> >> To: intel-wired-...@lists.osuosl.org
> >> Cc: Nguyen, Anthony L ; Radoslaw Tyl
> >> ; Brandeburg, Jesse
>
> >> Subject: [Intel-wired-lan] [PATCH net-next 2/5] iavf: simplify
> >> mutex_trylock+sleep loops
> >>
> >> This pattern appears in two places in the iavf source code:
> >>while (!mutex_trylock(...))
> >>usleep_range(...);
> >
> > I found a few other places with similar pattern (iavf_configure_clsflower
> > e.g).
> > Do you think we can fix them as well?
>
> I think so. But those are with some sort of a timeout, so replacing them
> would be slightly less obviously correct than these here. Later.
> Michal
Makes sense.
Reviewed-by: Wojciech Drewek
>
> >> That's just mutex_lock with extra steps.
> >> The pattern is a leftover from when iavf used bit flags instead of
> >> mutexes for locking. Commit 5ac49f3c2702 ("iavf: use mutexes for locking
> >> of critical sections") replaced test_and_set_bit with !mutex_trylock,
> >> preserving the pattern.
> >>
> >> Simplify it to mutex_lock.
> >>
> >> Signed-off-by: Michal Schmidt
> >> ---
> >> drivers/net/ethernet/intel/iavf/iavf_main.c | 6 ++
> >> 1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> index 98ecf5d5a2f2..03156ca523fe 100644
> >> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> @@ -3016,8 +3016,7 @@ static void iavf_reset_task(struct work_struct
> >> *work)
> >>return;
> >>}
> >>
> >> - while (!mutex_trylock(&adapter->client_lock))
> >> - usleep_range(500, 1000);
> >> + mutex_lock(&adapter->client_lock);
> >>if (CLIENT_ENABLED(adapter)) {
> >>adapter->flags &= ~(IAVF_FLAG_CLIENT_NEEDS_OPEN |
> >>IAVF_FLAG_CLIENT_NEEDS_CLOSE |
> >> @@ -5069,8 +5068,7 @@ static int __maybe_unused iavf_suspend(struct
> >> device *dev_d)
> >>
> >>netif_device_detach(netdev);
> >>
> >> - while (!mutex_trylock(&adapter->crit_lock))
> >> - usleep_range(500, 1000);
> >> + mutex_lock(&adapter->crit_lock);
> >>
> >>if (netif_running(netdev)) {
> >>rtnl_lock();
> >> --
> >> 2.41.0
> >>
> >> ___
> >> Intel-wired-lan mailing list
> >> Intel-wired-lan@osuosl.org
> >> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> >
___
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 4/5] iavf: in iavf_down, disable queues when removing the driver
> -Original Message-
> From: Michal Schmidt
> Sent: Tuesday, October 10, 2023 2:22 PM
> To: Drewek, Wojciech ; intel-wired-
> l...@lists.osuosl.org
> Cc: Nguyen, Anthony L ; Radoslaw Tyl
> ; Brandeburg, Jesse
> Subject: Re: [Intel-wired-lan] [PATCH net-next 4/5] iavf: in iavf_down,
> disable
> queues when removing the driver
>
> Dne 10. 10. 23 v 13:39 Drewek, Wojciech napsal:
> >
> >
> >> -Original Message-
> >> From: Intel-wired-lan On Behalf Of
> >> Michal Schmidt
> >> Sent: Tuesday, October 10, 2023 2:25 AM
> >> To: intel-wired-...@lists.osuosl.org
> >> Cc: Nguyen, Anthony L ; Radoslaw Tyl
> >> ; Brandeburg, Jesse
>
> >> Subject: [Intel-wired-lan] [PATCH net-next 4/5] iavf: in iavf_down, disable
> >> queues when removing the driver
> >>
> >> In iavf_down, we're skipping the scheduling of certain operations if
> >> the driver is being removed. However, the
> IAVF_FLAG_AQ_DISABLE_QUEUES
> >> request must not be skipped in this case, because iavf_close waits
> >> for the transition to the __IAVF_DOWN state, which happens in
> >> iavf_virtchnl_completion after the queues are released.
> >>
> >> Without this fix, "rmmod iavf" takes half a second per interface that's
> >> up and prints the "Device resources not yet released" warning.
> >>
> >> Fixes: c8de44b577eb ("iavf: do not process adminq tasks when
> >> __IAVF_IN_REMOVE_TASK is set")
> >> Signed-off-by: Michal Schmidt
> >
> > Looks like a fix, could be a separate patch with net as a target.
>
> I did not want to separate it from patch 3/5, because it changes the
> logic when IAVF_FLAG_PF_COMMS_FAILED is set. But on second thought, it
> should still work fine in that case too. aq_required would just get
> reset to zero in iavf_watchdog_task in the __IAVF_COMM_FAILED state.
I see, so it's up to you :)
>
> Michal
>
> > Reviewed-by: Wojciech Drewek
> >
> >> ---
> >> drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> index 0b808fa34801..2ab08b015b85 100644
> >> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> >> @@ -1440,9 +1440,9 @@ void iavf_down(struct iavf_adapter *adapter)
> >>adapter->aq_required |=
> >> IAVF_FLAG_AQ_DEL_FDIR_FILTER;
> >>if (!list_empty(&adapter->adv_rss_list_head))
> >>adapter->aq_required |=
> >> IAVF_FLAG_AQ_DEL_ADV_RSS_CFG;
> >> - adapter->aq_required |= IAVF_FLAG_AQ_DISABLE_QUEUES;
> >>}
> >>
> >> + adapter->aq_required |= IAVF_FLAG_AQ_DISABLE_QUEUES;
> >>mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
> >> }
> >>
> >> --
> >> 2.41.0
> >>
> >> ___
> >> Intel-wired-lan mailing list
> >> Intel-wired-lan@osuosl.org
> >> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> >
___
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: Fix SRIOV LAG disable on non-compliant aggreagate
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Dave Ertman
> Sent: Tuesday, October 10, 2023 7:32 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: net...@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH iwl-next v2] ice: Fix SRIOV LAG disable on
> non-compliant aggreagate
>
> If an attribute of an aggregate interface disqualifies it from supporting
> SRIOV, the driver will unwind the SRIOV support. Currently the driver is
> clearing the feature bit for all interfaces in the aggregate, but this is
> not allowing the other interfaces to unwind successfully on driver unload.
>
> Only clear the feature bit for the interface that is currently unwinding.
>
> Fixes: bf65da2eb279 ("ice: enforce interface eligibility and add messaging for
> SRIOV LAG")
> Signed-off-by: Dave Ertman
Thanks Dave!
Reviewed-by: Wojciech Drewek
> ---
> drivers/net/ethernet/intel/ice/ice_lag.c | 12 +++-
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
> b/drivers/net/ethernet/intel/ice/ice_lag.c
> index 2c96d1883e19..f405c07410a7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
> @@ -1513,18 +1513,12 @@ static void ice_lag_chk_disabled_bond(struct
> ice_lag *lag, void *ptr)
> */
> static void ice_lag_disable_sriov_bond(struct ice_lag *lag)
> {
> - struct ice_lag_netdev_list *entry;
> struct ice_netdev_priv *np;
> - struct net_device *netdev;
> struct ice_pf *pf;
>
> - list_for_each_entry(entry, lag->netdev_head, node) {
> - netdev = entry->netdev;
> - np = netdev_priv(netdev);
> - pf = np->vsi->back;
> -
> - ice_clear_feature_support(pf, ICE_F_SRIOV_LAG);
> - }
> + np = netdev_priv(lag->netdev);
> + pf = np->vsi->back;
> + ice_clear_feature_support(pf, ICE_F_SRIOV_LAG);
> }
>
> /**
> --
> 2.40.1
>
> ___
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
___
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 1/4] iavf: rely on netdev's own registered state
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Michal Schmidt
> Sent: Monday, October 16, 2023 6:49 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: net...@vger.kernel.org; Nguyen, Anthony L
> ; Brandeburg, Jesse
>
> Subject: [Intel-wired-lan] [PATCH iwl-next 1/4] iavf: rely on netdev's own
> registered state
>
> The information whether a netdev has been registered is already present
> in the netdev itself. There's no need for a driver flag with the same
> meaning.
>
> Signed-off-by: Michal Schmidt
Thanks Michal, nice cleanup!
Reviewed-by: Wojciech Drewek
> ---
> drivers/net/ethernet/intel/iavf/iavf.h | 1 -
> drivers/net/ethernet/intel/iavf/iavf_main.c | 9 +++--
> 2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h
> b/drivers/net/ethernet/intel/iavf/iavf.h
> index 44daf335e8c5..f026d0670338 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> @@ -377,7 +377,6 @@ struct iavf_adapter {
> unsigned long crit_section;
>
> struct delayed_work watchdog_task;
> - bool netdev_registered;
> bool link_up;
> enum virtchnl_link_speed link_speed;
> /* This is only populated if the VIRTCHNL_VF_CAP_ADV_LINK_SPEED
> is set
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index f35d74566faa..d2f4648a6156 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -2021,7 +2021,7 @@ static void iavf_finish_config(struct work_struct
> *work)
> mutex_lock(&adapter->crit_lock);
>
> if ((adapter->flags & IAVF_FLAG_SETUP_NETDEV_FEATURES) &&
> - adapter->netdev_registered &&
> + adapter->netdev->reg_state == NETREG_REGISTERED &&
> !test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) {
> netdev_update_features(adapter->netdev);
> adapter->flags &= ~IAVF_FLAG_SETUP_NETDEV_FEATURES;
> @@ -2029,7 +2029,7 @@ static void iavf_finish_config(struct work_struct
> *work)
>
> switch (adapter->state) {
> case __IAVF_DOWN:
> - if (!adapter->netdev_registered) {
> + if (adapter->netdev->reg_state != NETREG_REGISTERED) {
> err = register_netdevice(adapter->netdev);
> if (err) {
> dev_err(&adapter->pdev->dev, "Unable to
> register netdev (%d)\n",
> @@ -2043,7 +2043,6 @@ static void iavf_finish_config(struct work_struct
> *work)
>
> __IAVF_INIT_CONFIG_ADAPTER);
> goto out;
> }
> - adapter->netdev_registered = true;
> }
>
> /* Set the real number of queues when reset occurs while
> @@ -5173,10 +5172,8 @@ static void iavf_remove(struct pci_dev *pdev)
> cancel_work_sync(&adapter->finish_config);
>
> rtnl_lock();
> - if (adapter->netdev_registered) {
> + if (netdev->reg_state == NETREG_REGISTERED)
> unregister_netdevice(netdev);
> - adapter->netdev_registered = false;
> - }
> rtnl_unlock();
>
> if (CLIENT_ALLOWED(adapter)) {
> --
> 2.41.0
>
> ___
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
___
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 2/4] iavf: use unregister_netdev
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Michal Schmidt
> Sent: Monday, October 16, 2023 6:49 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: net...@vger.kernel.org; Nguyen, Anthony L
> ; Brandeburg, Jesse
>
> Subject: [Intel-wired-lan] [PATCH iwl-next 2/4] iavf: use unregister_netdev
>
> Use unregister_netdev, which takes rtnl_lock for us. We don't have to
> check the reg_state under rtnl_lock. There's nothing to race with. We
> have just cancelled the finish_config work.
>
> Signed-off-by: Michal Schmidt
Reviewed-by: Wojciech Drewek
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index d2f4648a6156..6036a4582196 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -5171,10 +5171,8 @@ static void iavf_remove(struct pci_dev *pdev)
> cancel_delayed_work_sync(&adapter->watchdog_task);
> cancel_work_sync(&adapter->finish_config);
>
> - rtnl_lock();
> if (netdev->reg_state == NETREG_REGISTERED)
> - unregister_netdevice(netdev);
> - rtnl_unlock();
> + unregister_netdev(netdev);
>
> if (CLIENT_ALLOWED(adapter)) {
> err = iavf_lan_del_device(adapter);
> --
> 2.41.0
>
> ___
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
___
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 3/4] iavf: add a common function for undoing the interrupt scheme
> -Original Message-
> From: Intel-wired-lan On Behalf Of
> Michal Schmidt
> Sent: Monday, October 16, 2023 6:49 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: net...@vger.kernel.org; Nguyen, Anthony L
> ; Brandeburg, Jesse
>
> Subject: [Intel-wired-lan] [PATCH iwl-next 3/4] iavf: add a common function
> for undoing the interrupt scheme
>
> Add a new function iavf_free_interrupt_scheme that does the inverse of
> iavf_init_interrupt_scheme. Symmetry is nice. And there will be three
> callers already.
>
> Signed-off-by: Michal Schmidt
I like symmetry :)
Reviewed-by: Wojciech Drewek
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 26 -
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 6036a4582196..791517cafc3c 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1954,6 +1954,17 @@ static int iavf_init_interrupt_scheme(struct
> iavf_adapter *adapter)
> return err;
> }
>
> +/**
> + * iavf_free_interrupt_scheme - Undo what iavf_init_interrupt_scheme does
> + * @adapter: board private structure
> + **/
> +static void iavf_free_interrupt_scheme(struct iavf_adapter *adapter)
> +{
> + iavf_free_q_vectors(adapter);
> + iavf_reset_interrupt_capability(adapter);
> + iavf_free_queues(adapter);
> +}
> +
> /**
> * iavf_free_rss - Free memory used by RSS structs
> * @adapter: board private structure
> @@ -1982,11 +1993,9 @@ static int iavf_reinit_interrupt_scheme(struct
> iavf_adapter *adapter, bool runni
> if (running)
> iavf_free_traffic_irqs(adapter);
> iavf_free_misc_irq(adapter);
> - iavf_reset_interrupt_capability(adapter);
> - iavf_free_q_vectors(adapter);
> - iavf_free_queues(adapter);
> + iavf_free_interrupt_scheme(adapter);
>
> - err = iavf_init_interrupt_scheme(adapter);
> + err = iavf_init_interrupt_scheme(adapter);
> if (err)
> goto err;
>
> @@ -2973,9 +2982,7 @@ static void iavf_disable_vf(struct iavf_adapter
> *adapter)
> spin_unlock_bh(&adapter->cloud_filter_list_lock);
>
> iavf_free_misc_irq(adapter);
> - iavf_reset_interrupt_capability(adapter);
> - iavf_free_q_vectors(adapter);
> - iavf_free_queues(adapter);
> + iavf_free_interrupt_scheme(adapter);
> memset(adapter->vf_res, 0, IAVF_VIRTCHNL_VF_RESOURCE_SIZE);
> iavf_shutdown_adminq(&adapter->hw);
> adapter->flags &= ~IAVF_FLAG_RESET_PENDING;
> @@ -5206,9 +5213,7 @@ static void iavf_remove(struct pci_dev *pdev)
> iavf_free_all_tx_resources(adapter);
> iavf_free_all_rx_resources(adapter);
> iavf_free_misc_irq(adapter);
> -
> - iavf_reset_interrupt_capability(adapter);
> - iavf_free_q_vectors(adapter);
> + iavf_free_interrupt_scheme(adapter);
>
> iavf_free_rss(adapter);
>
> @@ -5224,7 +5229,6 @@ static void iavf_remove(struct pci_dev *pdev)
>
> iounmap(hw->hw_addr);
> pci_release_regions(pdev);
> - iavf_free_queues(adapter);
> kfree(adapter->vf_res);
> spin_lock_bh(&adapter->mac_vlan_list_lock);
> /* If we got removed before an up/down sequence, we've got a filter
> --
> 2.41.0
>
> ___
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
___
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] iavf: delete unused iavf_mac_info fields
> -Original Message-
> From: Michal Schmidt
> Sent: Wednesday, October 18, 2023 1:15 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: Keller, Jacob E ; Drewek, Wojciech
> ; Brandeburg, Jesse
> ; Nguyen, Anthony L
> ; net...@vger.kernel.org
> Subject: [PATCH net-next] iavf: delete unused iavf_mac_info fields
>
> 'san_addr' and 'mac_fcoeq' members of struct iavf_mac_info are unused.
> 'type' is write-only. Delete all three.
>
> The function iavf_set_mac_type that sets 'type' also checks if the PCI
> vendor ID is Intel. This is unnecessary. Delete the whole function.
>
> If in the future there's a need for the MAC type (or other PCI
> ID-dependent data), I would prefer to use .driver_data in iavf_pci_tbl[]
> for this purpose.
>
> Signed-off-by: Michal Schmidt
Reviewed-by: Wojciech Drewek
Nice cleanup, I've seen similar unused fields in i40e as well.
Any plans for i40e cleanup?
> ---
> drivers/net/ethernet/intel/iavf/iavf_common.c | 32 ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 5 ---
> .../net/ethernet/intel/iavf/iavf_prototype.h | 2 --
> drivers/net/ethernet/intel/iavf/iavf_type.h | 12 ---
> 4 files changed, 51 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_common.c
> b/drivers/net/ethernet/intel/iavf/iavf_common.c
> index 1afd761d8052..8091e6feca01 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_common.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_common.c
> @@ -6,38 +6,6 @@
> #include "iavf_prototype.h"
> #include
>
> -/**
> - * iavf_set_mac_type - Sets MAC type
> - * @hw: pointer to the HW structure
> - *
> - * This function sets the mac type of the adapter based on the
> - * vendor ID and device ID stored in the hw structure.
> - **/
> -enum iavf_status iavf_set_mac_type(struct iavf_hw *hw)
> -{
> - enum iavf_status status = 0;
> -
> - if (hw->vendor_id == PCI_VENDOR_ID_INTEL) {
> - switch (hw->device_id) {
> - case IAVF_DEV_ID_X722_VF:
> - hw->mac.type = IAVF_MAC_X722_VF;
> - break;
> - case IAVF_DEV_ID_VF:
> - case IAVF_DEV_ID_VF_HV:
> - case IAVF_DEV_ID_ADAPTIVE_VF:
> - hw->mac.type = IAVF_MAC_VF;
> - break;
> - default:
> - hw->mac.type = IAVF_MAC_GENERIC;
> - break;
> - }
> - } else {
> - status = IAVF_ERR_DEVICE_NOT_SUPPORTED;
> - }
> -
> - return status;
> -}
> -
> /**
> * iavf_aq_str - convert AQ err code to a string
> * @hw: pointer to the HW structure
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 768bec67825a..c862ebcd2e39 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -2363,11 +2363,6 @@ static void iavf_startup(struct iavf_adapter
> *adapter)
> /* driver loaded, probe complete */
> adapter->flags &= ~IAVF_FLAG_PF_COMMS_FAILED;
> adapter->flags &= ~IAVF_FLAG_RESET_PENDING;
> - status = iavf_set_mac_type(hw);
> - if (status) {
> - dev_err(&pdev->dev, "Failed to set MAC type (%d)\n",
> status);
> - goto err;
> - }
>
> ret = iavf_check_reset_complete(hw);
> if (ret) {
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_prototype.h
> b/drivers/net/ethernet/intel/iavf/iavf_prototype.h
> index 940cb4203fbe..4a48e6171405 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_prototype.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf_prototype.h
> @@ -45,8 +45,6 @@ enum iavf_status iavf_aq_set_rss_lut(struct iavf_hw
> *hw, u16 seid,
> enum iavf_status iavf_aq_set_rss_key(struct iavf_hw *hw, u16 seid,
>struct iavf_aqc_get_set_rss_key_data *key);
>
> -enum iavf_status iavf_set_mac_type(struct iavf_hw *hw);
> -
> extern struct iavf_rx_ptype_decoded iavf_ptype_lookup[];
>
> static inline struct iavf_rx_ptype_decoded decode_rx_desc_ptype(u8 ptype)
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_type.h
> b/drivers/net/ethernet/intel/iavf/iavf_type.h
> index 9f1f523807c4..2b6a207fa441 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_type.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf_type.h
> @@ -69,15 +69,6 @@ enum iavf_debug_mask {
> * the Firmware and AdminQ are intended to insulate the driver from most of
> the
> * future changes, but these structures will also do part of the job.
Re: [Intel-wired-lan] [PATCH net] ice: lag: in RCU, use atomic allocation
> -Original Message-
> From: Michal Schmidt
> Sent: Monday, October 23, 2023 1:00 PM
> To: net...@vger.kernel.org
> Cc: Ertman, David M ; Daniel Machon
> ; Nguyen, Anthony L
> ; Brandeburg, Jesse ;
> intel-wired-...@lists.osuosl.org
> Subject: [PATCH net] ice: lag: in RCU, use atomic allocation
>
> Sleeping is not allowed in RCU read-side critical sections.
> Use atomic allocations under rcu_read_lock.
>
> Fixes: 1e0f9881ef79 ("ice: Flesh out implementation of support for SRIOV on
> bonded interface")
> Fixes: 41ccedf5ca8f ("ice: implement lag netdev event handler")
> Fixes: 3579aa86fb40 ("ice: update reset path for SRIOV LAG support")
> Signed-off-by: Michal Schmidt
Thanks Michal
Reviewed-by: Wojciech Drewek
> ---
> drivers/net/ethernet/intel/ice/ice_lag.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
> b/drivers/net/ethernet/intel/ice/ice_lag.c
> index 7b1256992dcf..33f01420eece 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
> @@ -595,7 +595,7 @@ void ice_lag_move_new_vf_nodes(struct ice_vf *vf)
> INIT_LIST_HEAD(&ndlist.node);
> rcu_read_lock();
> for_each_netdev_in_bond_rcu(lag->upper_netdev, tmp_nd) {
> - nl = kzalloc(sizeof(*nl), GFP_KERNEL);
> + nl = kzalloc(sizeof(*nl), GFP_ATOMIC);
> if (!nl)
> break;
>
> @@ -1672,7 +1672,7 @@ ice_lag_event_handler(struct notifier_block *notif_blk,
> unsigned long event,
>
> rcu_read_lock();
> for_each_netdev_in_bond_rcu(upper_netdev, tmp_nd) {
> - nd_list = kzalloc(sizeof(*nd_list), GFP_KERNEL);
> + nd_list = kzalloc(sizeof(*nd_list), GFP_ATOMIC);
> if (!nd_list)
> break;
>
> @@ -2046,7 +2046,7 @@ void ice_lag_rebuild(struct ice_pf *pf)
> INIT_LIST_HEAD(&ndlist.node);
> rcu_read_lock();
> for_each_netdev_in_bond_rcu(lag->upper_netdev, tmp_nd) {
> - nl = kzalloc(sizeof(*nl), GFP_KERNEL);
> + nl = kzalloc(sizeof(*nl), GFP_ATOMIC);
> if (!nl)
> break;
>
> --
> 2.41.0
>
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
