netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper
@ 2023-09-06 14:14 Petr Oros
  2023-09-06 14:14 ` [PATCH net 2/2] iavf: schedule a request immediately after add/delete vlan Petr Oros
  2023-09-06 15:32 ` [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper Ahmed Zaki
  0 siblings, 2 replies; 7+ messages in thread
From: Petr Oros @ 2023-09-06 14:14 UTC (permalink / raw)
  To: netdev
  Cc: jesse.brandeburg, anthony.l.nguyen, davem, edumazet, kuba, pabeni,
	intel-wired-lan, linux-kernel, mschmidt, ivecera

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);
 }
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net 2/2] iavf: schedule a request immediately after add/delete vlan
  2023-09-06 14:14 [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper Petr Oros
@ 2023-09-06 14:14 ` Petr Oros
  2023-09-06 15:33   ` [Intel-wired-lan] " Ahmed Zaki
  2023-09-06 15:32 ` [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper Ahmed Zaki
  1 sibling, 1 reply; 7+ messages in thread
From: Petr Oros @ 2023-09-06 14:14 UTC (permalink / raw)
  To: netdev
  Cc: jesse.brandeburg, anthony.l.nguyen, davem, edumazet, kuba, pabeni,
	intel-wired-lan, linux-kernel, mschmidt, ivecera

When the iavf driver wants to reconfigure the VLAN filters
(iavf_add_vlan, iavf_del_vlan), it sets a flag in
aq_required:
  adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
or:
  adapter->aq_required |= IAVF_FLAG_AQ_DEL_VLAN_FILTER;

This is later processed by the watchdog_task, but it runs periodically
every 2 seconds, so it can be a long time before it processes the request.

In the worst case, the interface is unable to receive traffic for more
than 2 seconds for no objective reason.

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_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 86d472dfdbc10c..d9f8ac1d57fd62 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -821,7 +821,7 @@ iavf_vlan_filter *iavf_add_vlan(struct iavf_adapter *adapter,
 		list_add_tail(&f->list, &adapter->vlan_filter_list);
 		f->state = IAVF_VLAN_ADD;
 		adapter->num_vlan_filters++;
-		adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
+		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_ADD_VLAN_FILTER);
 	}
 
 clearout:
@@ -843,7 +843,7 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, struct iavf_vlan vlan)
 	f = iavf_find_vlan(adapter, vlan);
 	if (f) {
 		f->state = IAVF_VLAN_REMOVE;
-		adapter->aq_required |= IAVF_FLAG_AQ_DEL_VLAN_FILTER;
+		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_VLAN_FILTER);
 	}
 
 	spin_unlock_bh(&adapter->mac_vlan_list_lock);
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper
  2023-09-06 14:14 [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper Petr Oros
  2023-09-06 14:14 ` [PATCH net 2/2] iavf: schedule a request immediately after add/delete vlan Petr Oros
@ 2023-09-06 15:32 ` Ahmed Zaki
  2023-09-06 16:44   ` Simon Horman
  2023-09-07  7:01   ` Petr Oros
  1 sibling, 2 replies; 7+ messages in thread
From: Ahmed Zaki @ 2023-09-06 15:32 UTC (permalink / raw)
  To: Petr Oros, netdev
  Cc: ivecera, intel-wired-lan, jesse.brandeburg, linux-kernel,
	edumazet, anthony.l.nguyen, kuba, pabeni, davem


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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-wired-lan] [PATCH net 2/2] iavf: schedule a request immediately after add/delete vlan
  2023-09-06 14:14 ` [PATCH net 2/2] iavf: schedule a request immediately after add/delete vlan Petr Oros
@ 2023-09-06 15:33   ` Ahmed Zaki
  0 siblings, 0 replies; 7+ messages in thread
From: Ahmed Zaki @ 2023-09-06 15:33 UTC (permalink / raw)
  To: Petr Oros, netdev
  Cc: ivecera, intel-wired-lan, jesse.brandeburg, linux-kernel,
	edumazet, anthony.l.nguyen, kuba, pabeni, davem


On 2023-09-06 08:14, Petr Oros wrote:
> When the iavf driver wants to reconfigure the VLAN filters
> (iavf_add_vlan, iavf_del_vlan), it sets a flag in
> aq_required:
>    adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
> or:
>    adapter->aq_required |= IAVF_FLAG_AQ_DEL_VLAN_FILTER;
>
> This is later processed by the watchdog_task, but it runs periodically
> every 2 seconds, so it can be a long time before it processes the request.
>
> In the worst case, the interface is unable to receive traffic for more
> than 2 seconds for no objective reason.
>
> 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_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 86d472dfdbc10c..d9f8ac1d57fd62 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -821,7 +821,7 @@ iavf_vlan_filter *iavf_add_vlan(struct iavf_adapter *adapter,
>   		list_add_tail(&f->list, &adapter->vlan_filter_list);
>   		f->state = IAVF_VLAN_ADD;
>   		adapter->num_vlan_filters++;
> -		adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER;
> +		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_ADD_VLAN_FILTER);
>   	}
>   
>   clearout:
> @@ -843,7 +843,7 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, struct iavf_vlan vlan)
>   	f = iavf_find_vlan(adapter, vlan);
>   	if (f) {
>   		f->state = IAVF_VLAN_REMOVE;
> -		adapter->aq_required |= IAVF_FLAG_AQ_DEL_VLAN_FILTER;
> +		iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_DEL_VLAN_FILTER);
>   	}
>   
>   	spin_unlock_bh(&adapter->mac_vlan_list_lock);

Reviewed-by: Ahmed Zaki <ahmed.zaki@intel.com>

Thanks.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper
  2023-09-06 15:32 ` [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper Ahmed Zaki
@ 2023-09-06 16:44   ` Simon Horman
  2023-09-07  7:01   ` Petr Oros
  1 sibling, 0 replies; 7+ messages in thread
From: Simon Horman @ 2023-09-06 16:44 UTC (permalink / raw)
  To: Ahmed Zaki
  Cc: Petr Oros, netdev, ivecera, intel-wired-lan, jesse.brandeburg,
	linux-kernel, edumazet, anthony.l.nguyen, kuba, pabeni, davem

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper
  2023-09-06 15:32 ` [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper Ahmed Zaki
  2023-09-06 16:44   ` Simon Horman
@ 2023-09-07  7:01   ` Petr Oros
  2023-09-07 12:42     ` Ahmed Zaki
  1 sibling, 1 reply; 7+ messages in thread
From: Petr Oros @ 2023-09-07  7:01 UTC (permalink / raw)
  To: Ahmed Zaki, netdev
  Cc: ivecera, intel-wired-lan, jesse.brandeburg, linux-kernel,
	edumazet, anthony.l.nguyen, kuba, pabeni, davem

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
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper
  2023-09-07  7:01   ` Petr Oros
@ 2023-09-07 12:42     ` Ahmed Zaki
  0 siblings, 0 replies; 7+ messages in thread
From: Ahmed Zaki @ 2023-09-07 12:42 UTC (permalink / raw)
  To: poros, netdev
  Cc: ivecera, intel-wired-lan, jesse.brandeburg, linux-kernel,
	edumazet, anthony.l.nguyen, kuba, pabeni, davem


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



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-09-07 17:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-06 14:14 [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper Petr Oros
2023-09-06 14:14 ` [PATCH net 2/2] iavf: schedule a request immediately after add/delete vlan Petr Oros
2023-09-06 15:33   ` [Intel-wired-lan] " Ahmed Zaki
2023-09-06 15:32 ` [Intel-wired-lan] [PATCH net 1/2] iavf: add iavf_schedule_aq_request() helper Ahmed Zaki
2023-09-06 16:44   ` Simon Horman
2023-09-07  7:01   ` Petr Oros
2023-09-07 12:42     ` Ahmed Zaki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).