diff mbox series

[net,1/2] iavf: add iavf_schedule_aq_request() helper

Message ID 20230906141411.121142-1-poros@redhat.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net,1/2] iavf: add iavf_schedule_aq_request() helper | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1330 this patch: 1330
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1353 this patch: 1353
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 35 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Petr Oros Sept. 6, 2023, 2:14 p.m. UTC 
Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately
schedule watchdog_task. Helper will be used in cases where it is
necessary to run aq requests asap

Signed-off-by: Petr Oros <poros@redhat.com>
Co-developed-by: Michal Schmidt <mschmidt@redhat.com>
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Co-developed-by: Ivan Vecera <ivecera@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/iavf/iavf.h         |  2 +-
 drivers/net/ethernet/intel/iavf/iavf_ethtool.c |  2 +-
 drivers/net/ethernet/intel/iavf/iavf_main.c    | 10 ++++------
 3 files changed, 6 insertions(+), 8 deletions(-)

Comments

Ahmed Zaki Sept. 6, 2023, 3:32 p.m. UTC | #1 
On 2023-09-06 08:14, Petr Oros wrote:
> Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately
> schedule watchdog_task. Helper will be used in cases where it is
> necessary to run aq requests asap
>
> Signed-off-by: Petr Oros <poros@redhat.com>
> Co-developed-by: Michal Schmidt <mschmidt@redhat.com>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> Co-developed-by: Ivan Vecera <ivecera@redhat.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/net/ethernet/intel/iavf/iavf.h         |  2 +-
>   drivers/net/ethernet/intel/iavf/iavf_ethtool.c |  2 +-
>   drivers/net/ethernet/intel/iavf/iavf_main.c    | 10 ++++------
>   3 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
> index 85fba85fbb232b..e110ba3461857b 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> @@ -521,7 +521,7 @@ void iavf_down(struct iavf_adapter *adapter);
>   int iavf_process_config(struct iavf_adapter *adapter);
>   int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter);
>   void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags);
> -void iavf_schedule_request_stats(struct iavf_adapter *adapter);
> +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 flags);
>   void iavf_schedule_finish_config(struct iavf_adapter *adapter);
>   void iavf_reset(struct iavf_adapter *adapter);
>   void iavf_set_ethtool_ops(struct net_device *netdev);
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> index a34303ad057d00..90397293525f71 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> @@ -362,7 +362,7 @@ static void iavf_get_ethtool_stats(struct net_device *netdev,
>   	unsigned int i;
>   
>   	/* Explicitly request stats refresh */
> -	iavf_schedule_request_stats(adapter);
> +	iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_REQUEST_STATS);
>   
>   	iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
>   
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 7b300c86ceda73..86d472dfdbc10c 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -314,15 +314,13 @@ void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags)
>   }
>   
>   /**
> - * iavf_schedule_request_stats - Set the flags and schedule statistics request
> + * iavf_schedule_aq_request - Set the flags and schedule aq request
>    * @adapter: board private structure
> - *
> - * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task() will explicitly
> - * request and refresh ethtool stats
> + * @flags: requested aq flags
>    **/
> -void iavf_schedule_request_stats(struct iavf_adapter *adapter)
> +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 flags)
>   {
> -	adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS;
> +	adapter->aq_required |= flags;
>   	mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
>   }
>   

There are other places where the helper can be used without functional 
changes, e.g. iavf_add_fdir_ethtool() , iavf_replace_primary_mac() and 
couple of other places. In all of them, mod_delayed_work() is called 
after setting the AQ flag. For the sake of consistency, can you use the 
helper there too?


Thanks,

Ahmed
Simon Horman Sept. 6, 2023, 4:44 p.m. UTC | #2 
On Wed, Sep 06, 2023 at 09:32:59AM -0600, Ahmed Zaki wrote:
> 
> On 2023-09-06 08:14, Petr Oros wrote:
> > Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately

nit: immediately
Petr Oros Sept. 7, 2023, 7:01 a.m. UTC | #3 
Ahmed Zaki píše v St 06. 09. 2023 v 09:32 -0600:
> 
> On 2023-09-06 08:14, Petr Oros wrote:
> > Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately
> > schedule watchdog_task. Helper will be used in cases where it is
> > necessary to run aq requests asap
> > 
> > Signed-off-by: Petr Oros <poros@redhat.com>
> > Co-developed-by: Michal Schmidt <mschmidt@redhat.com>
> > Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> > Co-developed-by: Ivan Vecera <ivecera@redhat.com>
> > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > ---
> >   drivers/net/ethernet/intel/iavf/iavf.h         |  2 +-
> >   drivers/net/ethernet/intel/iavf/iavf_ethtool.c |  2 +-
> >   drivers/net/ethernet/intel/iavf/iavf_main.c    | 10 ++++------
> >   3 files changed, 6 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf.h
> > b/drivers/net/ethernet/intel/iavf/iavf.h
> > index 85fba85fbb232b..e110ba3461857b 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf.h
> > +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> > @@ -521,7 +521,7 @@ void iavf_down(struct iavf_adapter *adapter);
> >   int iavf_process_config(struct iavf_adapter *adapter);
> >   int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter);
> >   void iavf_schedule_reset(struct iavf_adapter *adapter, u64
> > flags);
> > -void iavf_schedule_request_stats(struct iavf_adapter *adapter);
> > +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64
> > flags);
> >   void iavf_schedule_finish_config(struct iavf_adapter *adapter);
> >   void iavf_reset(struct iavf_adapter *adapter);
> >   void iavf_set_ethtool_ops(struct net_device *netdev);
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > index a34303ad057d00..90397293525f71 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > @@ -362,7 +362,7 @@ static void iavf_get_ethtool_stats(struct
> > net_device *netdev,
> >         unsigned int i;
> >   
> >         /* Explicitly request stats refresh */
> > -       iavf_schedule_request_stats(adapter);
> > +       iavf_schedule_aq_request(adapter,
> > IAVF_FLAG_AQ_REQUEST_STATS);
> >   
> >         iavf_add_ethtool_stats(&data, adapter,
> > iavf_gstrings_stats);
> >   
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > index 7b300c86ceda73..86d472dfdbc10c 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > @@ -314,15 +314,13 @@ void iavf_schedule_reset(struct iavf_adapter
> > *adapter, u64 flags)
> >   }
> >   
> >   /**
> > - * iavf_schedule_request_stats - Set the flags and schedule
> > statistics request
> > + * iavf_schedule_aq_request - Set the flags and schedule aq
> > request
> >    * @adapter: board private structure
> > - *
> > - * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task()
> > will explicitly
> > - * request and refresh ethtool stats
> > + * @flags: requested aq flags
> >    **/
> > -void iavf_schedule_request_stats(struct iavf_adapter *adapter)
> > +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64
> > flags)
> >   {
> > -       adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS;
> > +       adapter->aq_required |= flags;
> >         mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
> >   }
> >   
> 
> There are other places where the helper can be used without
> functional 
> changes, e.g. iavf_add_fdir_ethtool() , iavf_replace_primary_mac()
> and 
> couple of other places. In all of them, mod_delayed_work() is called 
> after setting the AQ flag. For the sake of consistency, can you use
> the 
> helper there too?

These two commits is fixes for issue -> net. But on
iavf_add_fdir_ethtool and iavf_replace_primary_mac is mod_delayed_work
called after spin_unlock_bh ->
looks like no functional chages but i would like be sure and better
will send this to net-next. Are you ok with this?

> 
> 
> Thanks,
> 
> Ahmed
> 
>
Ahmed Zaki Sept. 7, 2023, 12:42 p.m. UTC | #4 
On 2023-09-07 01:01, Petr Oros wrote:
> Ahmed Zaki píše v St 06. 09. 2023 v 09:32 -0600:
>> On 2023-09-06 08:14, Petr Oros wrote:
>>> Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately
>>> schedule watchdog_task. Helper will be used in cases where it is
>>> necessary to run aq requests asap
>>>
>>> Signed-off-by: Petr Oros <poros@redhat.com>
>>> Co-developed-by: Michal Schmidt <mschmidt@redhat.com>
>>> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
>>> Co-developed-by: Ivan Vecera <ivecera@redhat.com>
>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>>> ---
>>>    drivers/net/ethernet/intel/iavf/iavf.h         |  2 +-
>>>    drivers/net/ethernet/intel/iavf/iavf_ethtool.c |  2 +-
>>>    drivers/net/ethernet/intel/iavf/iavf_main.c    | 10 ++++------
>>>    3 files changed, 6 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h
>>> b/drivers/net/ethernet/intel/iavf/iavf.h
>>> index 85fba85fbb232b..e110ba3461857b 100644
>>> --- a/drivers/net/ethernet/intel/iavf/iavf.h
>>> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
>>> @@ -521,7 +521,7 @@ void iavf_down(struct iavf_adapter *adapter);
>>>    int iavf_process_config(struct iavf_adapter *adapter);
>>>    int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter);
>>>    void iavf_schedule_reset(struct iavf_adapter *adapter, u64
>>> flags);
>>> -void iavf_schedule_request_stats(struct iavf_adapter *adapter);
>>> +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64
>>> flags);
>>>    void iavf_schedule_finish_config(struct iavf_adapter *adapter);
>>>    void iavf_reset(struct iavf_adapter *adapter);
>>>    void iavf_set_ethtool_ops(struct net_device *netdev);
>>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
>>> b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
>>> index a34303ad057d00..90397293525f71 100644
>>> --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
>>> +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
>>> @@ -362,7 +362,7 @@ static void iavf_get_ethtool_stats(struct
>>> net_device *netdev,
>>>          unsigned int i;
>>>    
>>>          /* Explicitly request stats refresh */
>>> -       iavf_schedule_request_stats(adapter);
>>> +       iavf_schedule_aq_request(adapter,
>>> IAVF_FLAG_AQ_REQUEST_STATS);
>>>    
>>>          iavf_add_ethtool_stats(&data, adapter,
>>> iavf_gstrings_stats);
>>>    
>>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
>>> b/drivers/net/ethernet/intel/iavf/iavf_main.c
>>> index 7b300c86ceda73..86d472dfdbc10c 100644
>>> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
>>> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
>>> @@ -314,15 +314,13 @@ void iavf_schedule_reset(struct iavf_adapter
>>> *adapter, u64 flags)
>>>    }
>>>    
>>>    /**
>>> - * iavf_schedule_request_stats - Set the flags and schedule
>>> statistics request
>>> + * iavf_schedule_aq_request - Set the flags and schedule aq
>>> request
>>>     * @adapter: board private structure
>>> - *
>>> - * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task()
>>> will explicitly
>>> - * request and refresh ethtool stats
>>> + * @flags: requested aq flags
>>>     **/
>>> -void iavf_schedule_request_stats(struct iavf_adapter *adapter)
>>> +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64
>>> flags)
>>>    {
>>> -       adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS;
>>> +       adapter->aq_required |= flags;
>>>          mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
>>>    }
>>>    
>> There are other places where the helper can be used without
>> functional
>> changes, e.g. iavf_add_fdir_ethtool() , iavf_replace_primary_mac()
>> and
>> couple of other places. In all of them, mod_delayed_work() is called
>> after setting the AQ flag. For the sake of consistency, can you use
>> the
>> helper there too?
> These two commits is fixes for issue -> net. But on
> iavf_add_fdir_ethtool and iavf_replace_primary_mac is mod_delayed_work
> called after spin_unlock_bh ->
> looks like no functional chages but i would like be sure and better
> will send this to net-next. Are you ok with this?
>
It is usually better to use the helper in the same commit that 
introduces it, but no problem. I am OK with sending this later to next.

Thanks,

Ahmed
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index 85fba85fbb232b..e110ba3461857b 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -521,7 +521,7 @@  void iavf_down(struct iavf_adapter *adapter);
 int iavf_process_config(struct iavf_adapter *adapter);
 int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter);
 void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags);
-void iavf_schedule_request_stats(struct iavf_adapter *adapter);
+void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 flags);
 void iavf_schedule_finish_config(struct iavf_adapter *adapter);
 void iavf_reset(struct iavf_adapter *adapter);
 void iavf_set_ethtool_ops(struct net_device *netdev);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
index a34303ad057d00..90397293525f71 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
@@ -362,7 +362,7 @@  static void iavf_get_ethtool_stats(struct net_device *netdev,
 	unsigned int i;
 
 	/* Explicitly request stats refresh */
-	iavf_schedule_request_stats(adapter);
+	iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_REQUEST_STATS);
 
 	iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
 
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 7b300c86ceda73..86d472dfdbc10c 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -314,15 +314,13 @@  void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags)
 }
 
 /**
- * iavf_schedule_request_stats - Set the flags and schedule statistics request
+ * iavf_schedule_aq_request - Set the flags and schedule aq request
  * @adapter: board private structure
- *
- * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task() will explicitly
- * request and refresh ethtool stats
+ * @flags: requested aq flags
  **/
-void iavf_schedule_request_stats(struct iavf_adapter *adapter)
+void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 flags)
 {
-	adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS;
+	adapter->aq_required |= flags;
 	mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
 }