Intel-Wired-Lan Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage
@ 2023-03-09  2:49 Lingyu Liu
  2023-03-10 10:19 ` Michal Swiatkowski
  2023-03-27  9:08 ` Paul Menzel
  0 siblings, 2 replies; 6+ messages in thread
From: Lingyu Liu @ 2023-03-09  2:49 UTC (permalink / raw)
  To: intel-wired-lan; +Cc: Junfeng Guo

From: Junfeng Guo <junfeng.guo@intel.com>

This patch added the missing FDIR counters reset process when
FDIR inits. Without this patch, when VF initializes or resets,
all the FDIR counters will not be cleaned, which may cause
unexpected behaviors for future FDIR rule create (e.g., rule
conflict).

Fixes: 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF")
Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
Signed-off-by: Lingyu Liu <lingyu.liu@intel.com>
---
v2: change commit message to apply more to a kernel use
---
 drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
index e6ef6b303222..60c9da4aac1d 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
@@ -541,6 +541,20 @@ static void ice_vc_fdir_rem_prof_all(struct ice_vf *vf)
 	}
 }
 
+/**
+ * ice_vc_fdir_reset_cnt_all - reset all FDIR counters for this VF FDIR
+ * @fdir: pointer to the VF FDIR structure
+ */
+static void ice_vc_fdir_reset_cnt_all(struct ice_vf_fdir *fdir)
+{
+	enum ice_fltr_ptype flow = ICE_FLTR_PTYPE_NONF_NONE;
+
+	for (; flow < ICE_FLTR_PTYPE_MAX; flow++) {
+		fdir->fdir_fltr_cnt[flow][0] = 0;
+		fdir->fdir_fltr_cnt[flow][1] = 0;
+	}
+}
+
 /**
  * ice_vc_fdir_write_flow_prof
  * @vf: pointer to the VF structure
@@ -1924,6 +1938,7 @@ void ice_vf_fdir_init(struct ice_vf *vf)
 	spin_lock_init(&fdir->ctx_lock);
 	fdir->ctx_irq.flags = 0;
 	fdir->ctx_done.flags = 0;
+	ice_vc_fdir_reset_cnt_all(fdir);
 }
 
 /**
-- 
2.25.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage
  2023-03-09  2:49 [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage Lingyu Liu
@ 2023-03-10 10:19 ` Michal Swiatkowski
  2023-03-14  0:39   ` Liu, Lingyu
  2023-03-27  9:08 ` Paul Menzel
  1 sibling, 1 reply; 6+ messages in thread
From: Michal Swiatkowski @ 2023-03-10 10:19 UTC (permalink / raw)
  To: Lingyu Liu; +Cc: intel-wired-lan

On Thu, Mar 09, 2023 at 02:49:34AM +0000, Lingyu Liu wrote:
> From: Junfeng Guo <junfeng.guo@intel.com>
> 
> This patch added the missing FDIR counters reset process when
> FDIR inits. Without this patch, when VF initializes or resets,
> all the FDIR counters will not be cleaned, which may cause
> unexpected behaviors for future FDIR rule create (e.g., rule
> conflict).
> 
> Fixes: 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF")
> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
> Signed-off-by: Lingyu Liu <lingyu.liu@intel.com>
> ---
> v2: change commit message to apply more to a kernel use
> ---
>  drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> index e6ef6b303222..60c9da4aac1d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> @@ -541,6 +541,20 @@ static void ice_vc_fdir_rem_prof_all(struct ice_vf *vf)
>  	}
>  }
>  
> +/**
> + * ice_vc_fdir_reset_cnt_all - reset all FDIR counters for this VF FDIR
> + * @fdir: pointer to the VF FDIR structure
> + */
> +static void ice_vc_fdir_reset_cnt_all(struct ice_vf_fdir *fdir)
> +{
> +	enum ice_fltr_ptype flow = ICE_FLTR_PTYPE_NONF_NONE;
> +
> +	for (; flow < ICE_FLTR_PTYPE_MAX; flow++) {
> +		fdir->fdir_fltr_cnt[flow][0] = 0;
> +		fdir->fdir_fltr_cnt[flow][1] = 0;
> +	}
> +}
> +
>  /**
>   * ice_vc_fdir_write_flow_prof
>   * @vf: pointer to the VF structure
> @@ -1924,6 +1938,7 @@ void ice_vf_fdir_init(struct ice_vf *vf)
>  	spin_lock_init(&fdir->ctx_lock);
>  	fdir->ctx_irq.flags = 0;
>  	fdir->ctx_done.flags = 0;
> +	ice_vc_fdir_reset_cnt_all(fdir);
I am fine with this change, however, maybe the better place for
resetting counters will be when the flows are removed? Or maybe
the flow are removed only by hw?

>  }
>  
>  /**
> -- 
> 2.25.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

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

* Re: [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage
  2023-03-10 10:19 ` Michal Swiatkowski
@ 2023-03-14  0:39   ` Liu, Lingyu
  2023-03-27  8:34     ` Michal Swiatkowski
  0 siblings, 1 reply; 6+ messages in thread
From: Liu, Lingyu @ 2023-03-14  0:39 UTC (permalink / raw)
  To: Michal Swiatkowski; +Cc: intel-wired-lan


[-- Attachment #1.1: Type: text/plain, Size: 2501 bytes --]


On 3/10/2023 6:19 PM, Michal Swiatkowski wrote:
> On Thu, Mar 09, 2023 at 02:49:34AM +0000, Lingyu Liu wrote:
>> From: Junfeng Guo<junfeng.guo@intel.com>
>>
>> This patch added the missing FDIR counters reset process when
>> FDIR inits. Without this patch, when VF initializes or resets,
>> all the FDIR counters will not be cleaned, which may cause
>> unexpected behaviors for future FDIR rule create (e.g., rule
>> conflict).
>>
>> Fixes: 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF")
>> Signed-off-by: Junfeng Guo<junfeng.guo@intel.com>
>> Signed-off-by: Lingyu Liu<lingyu.liu@intel.com>
>> ---
>> v2: change commit message to apply more to a kernel use
>> ---
>>   drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
>> index e6ef6b303222..60c9da4aac1d 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
>> @@ -541,6 +541,20 @@ static void ice_vc_fdir_rem_prof_all(struct ice_vf *vf)
>>   	}
>>   }
>>   
>> +/**
>> + * ice_vc_fdir_reset_cnt_all - reset all FDIR counters for this VF FDIR
>> + * @fdir: pointer to the VF FDIR structure
>> + */
>> +static void ice_vc_fdir_reset_cnt_all(struct ice_vf_fdir *fdir)
>> +{
>> +	enum ice_fltr_ptype flow = ICE_FLTR_PTYPE_NONF_NONE;
>> +
>> +	for (; flow < ICE_FLTR_PTYPE_MAX; flow++) {
>> +		fdir->fdir_fltr_cnt[flow][0] = 0;
>> +		fdir->fdir_fltr_cnt[flow][1] = 0;
>> +	}
>> +}
>> +
>>   /**
>>    * ice_vc_fdir_write_flow_prof
>>    * @vf: pointer to the VF structure
>> @@ -1924,6 +1938,7 @@ void ice_vf_fdir_init(struct ice_vf *vf)
>>   	spin_lock_init(&fdir->ctx_lock);
>>   	fdir->ctx_irq.flags = 0;
>>   	fdir->ctx_done.flags = 0;
>> +	ice_vc_fdir_reset_cnt_all(fdir);
> I am fine with this change, however, maybe the better place for
> resetting counters will be when the flows are removed? Or maybe
> the flow are removed only by hw?

The flows are removed in ice_vf_fdir_exit(). When VF resets,
ice_vf_fdir_init() and ice_vf_fdir_exit() is called in pairs.
While ice_vf_fdir_init() is called when vf entry initializes,
where these counters should be initialized.

>>   }
>>   
>>   /**
>> -- 
>> 2.25.1
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[-- Attachment #1.2: Type: text/html, Size: 3521 bytes --]

[-- Attachment #2: Type: text/plain, Size: 162 bytes --]

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage
  2023-03-14  0:39   ` Liu, Lingyu
@ 2023-03-27  8:34     ` Michal Swiatkowski
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Swiatkowski @ 2023-03-27  8:34 UTC (permalink / raw)
  To: Liu, Lingyu; +Cc: intel-wired-lan

On Tue, Mar 14, 2023 at 08:39:09AM +0800, Liu, Lingyu wrote:
> 
> On 3/10/2023 6:19 PM, Michal Swiatkowski wrote:
> > On Thu, Mar 09, 2023 at 02:49:34AM +0000, Lingyu Liu wrote:
> > > From: Junfeng Guo<junfeng.guo@intel.com>
> > > 
> > > This patch added the missing FDIR counters reset process when
> > > FDIR inits. Without this patch, when VF initializes or resets,
> > > all the FDIR counters will not be cleaned, which may cause
> > > unexpected behaviors for future FDIR rule create (e.g., rule
> > > conflict).
> > > 
> > > Fixes: 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF")
> > > Signed-off-by: Junfeng Guo<junfeng.guo@intel.com>
> > > Signed-off-by: Lingyu Liu<lingyu.liu@intel.com>
> > > ---
> > > v2: change commit message to apply more to a kernel use
> > > ---
> > >   drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 15 +++++++++++++++
> > >   1 file changed, 15 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> > > index e6ef6b303222..60c9da4aac1d 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> > > @@ -541,6 +541,20 @@ static void ice_vc_fdir_rem_prof_all(struct ice_vf *vf)
> > >   	}
> > >   }
> > > +/**
> > > + * ice_vc_fdir_reset_cnt_all - reset all FDIR counters for this VF FDIR
> > > + * @fdir: pointer to the VF FDIR structure
> > > + */
> > > +static void ice_vc_fdir_reset_cnt_all(struct ice_vf_fdir *fdir)
> > > +{
> > > +	enum ice_fltr_ptype flow = ICE_FLTR_PTYPE_NONF_NONE;
> > > +
> > > +	for (; flow < ICE_FLTR_PTYPE_MAX; flow++) {
> > > +		fdir->fdir_fltr_cnt[flow][0] = 0;
> > > +		fdir->fdir_fltr_cnt[flow][1] = 0;
> > > +	}
> > > +}
> > > +
> > >   /**
> > >    * ice_vc_fdir_write_flow_prof
> > >    * @vf: pointer to the VF structure
> > > @@ -1924,6 +1938,7 @@ void ice_vf_fdir_init(struct ice_vf *vf)
> > >   	spin_lock_init(&fdir->ctx_lock);
> > >   	fdir->ctx_irq.flags = 0;
> > >   	fdir->ctx_done.flags = 0;
> > > +	ice_vc_fdir_reset_cnt_all(fdir);
> > I am fine with this change, however, maybe the better place for
> > resetting counters will be when the flows are removed? Or maybe
> > the flow are removed only by hw?
> 
> The flows are removed in ice_vf_fdir_exit(). When VF resets,
> ice_vf_fdir_init() and ice_vf_fdir_exit() is called in pairs.
> While ice_vf_fdir_init() is called when vf entry initializes,
> where these counters should be initialized.
> 

Ok, thanks.

Sorry, I missed the replay :(

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> > >   }
> > >   /**
> > > -- 
> > > 2.25.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

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

* Re: [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage
  2023-03-09  2:49 [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage Lingyu Liu
  2023-03-10 10:19 ` Michal Swiatkowski
@ 2023-03-27  9:08 ` Paul Menzel
  2023-03-28 10:41   ` Liu, Lingyu
  1 sibling, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2023-03-27  9:08 UTC (permalink / raw)
  To: Lingyu Liu, Junfeng Guo; +Cc: intel-wired-lan

Dear Lingyu, dear Junfeng,


Thank you for the patch.

Am 09.03.23 um 03:49 schrieb Lingyu Liu:
> From: Junfeng Guo <junfeng.guo@intel.com>

A shorter commit message summary/title would be:

Reset FDIR counter in FDIR init stage

> This patch added the missing FDIR counters reset process when

What commit do you reference? If it’s about the current change, please use:

Add the missing FDIR counters reset process …

Better:

Reset the FDIR counters when …

> FDIR inits. Without this patch, when VF initializes or resets,
> all the FDIR counters will not be cleaned, which may cause

… are not cleaned …

> unexpected behaviors for future FDIR rule create (e.g., rule
> conflict).
> 
> Fixes: 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF")
> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
> Signed-off-by: Lingyu Liu <lingyu.liu@intel.com>
> ---
> v2: change commit message to apply more to a kernel use
> ---
>   drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> index e6ef6b303222..60c9da4aac1d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
> @@ -541,6 +541,20 @@ static void ice_vc_fdir_rem_prof_all(struct ice_vf *vf)
>   	}
>   }
>   
> +/**
> + * ice_vc_fdir_reset_cnt_all - reset all FDIR counters for this VF FDIR
> + * @fdir: pointer to the VF FDIR structure
> + */
> +static void ice_vc_fdir_reset_cnt_all(struct ice_vf_fdir *fdir)
> +{
> +	enum ice_fltr_ptype flow = ICE_FLTR_PTYPE_NONF_NONE;
> +
> +	for (; flow < ICE_FLTR_PTYPE_MAX; flow++) {
> +		fdir->fdir_fltr_cnt[flow][0] = 0;
> +		fdir->fdir_fltr_cnt[flow][1] = 0;
> +	}

In all other places this construct is written like:

	enum ice_fltr_ptype flow;

	for (flow = ICE_FLTR_PTYPE_NONF_NONE; flow < … ) {
		…
	}

> +}
> +
>   /**
>    * ice_vc_fdir_write_flow_prof
>    * @vf: pointer to the VF structure
> @@ -1924,6 +1938,7 @@ void ice_vf_fdir_init(struct ice_vf *vf)
>   	spin_lock_init(&fdir->ctx_lock);
>   	fdir->ctx_irq.flags = 0;
>   	fdir->ctx_done.flags = 0;
> +	ice_vc_fdir_reset_cnt_all(fdir);
>   }
>   
>   /**


Kind regards,

Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

* Re: [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage
  2023-03-27  9:08 ` Paul Menzel
@ 2023-03-28 10:41   ` Liu, Lingyu
  0 siblings, 0 replies; 6+ messages in thread
From: Liu, Lingyu @ 2023-03-28 10:41 UTC (permalink / raw)
  To: Paul Menzel, Junfeng Guo; +Cc: intel-wired-lan


On 3/27/2023 5:08 PM, Paul Menzel wrote:
> Dear Lingyu, dear Junfeng,
>
>
> Thank you for the patch.
>
> Am 09.03.23 um 03:49 schrieb Lingyu Liu:
>> From: Junfeng Guo <junfeng.guo@intel.com>
>
> A shorter commit message summary/title would be:
>
> Reset FDIR counter in FDIR init stage


Hi Paul, thanks for the review. Updated in V3.


>
>> This patch added the missing FDIR counters reset process when
>
> What commit do you reference? If it’s about the current change, please 
> use:
>
> Add the missing FDIR counters reset process …
>
> Better:
>
> Reset the FDIR counters when …
>
>> FDIR inits. Without this patch, when VF initializes or resets,
>> all the FDIR counters will not be cleaned, which may cause
>
> … are not cleaned …
>
>> unexpected behaviors for future FDIR rule create (e.g., rule
>> conflict).
>>
>> Fixes: 1f7ea1cd6a37 ("ice: Enable FDIR Configure for AVF")
>> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
>> Signed-off-by: Lingyu Liu <lingyu.liu@intel.com>
>> ---
>> v2: change commit message to apply more to a kernel use
>> ---
>>   drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 15 
>> +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c 
>> b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
>> index e6ef6b303222..60c9da4aac1d 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
>> @@ -541,6 +541,20 @@ static void ice_vc_fdir_rem_prof_all(struct 
>> ice_vf *vf)
>>       }
>>   }
>>   +/**
>> + * ice_vc_fdir_reset_cnt_all - reset all FDIR counters for this VF FDIR
>> + * @fdir: pointer to the VF FDIR structure
>> + */
>> +static void ice_vc_fdir_reset_cnt_all(struct ice_vf_fdir *fdir)
>> +{
>> +    enum ice_fltr_ptype flow = ICE_FLTR_PTYPE_NONF_NONE;
>> +
>> +    for (; flow < ICE_FLTR_PTYPE_MAX; flow++) {
>> +        fdir->fdir_fltr_cnt[flow][0] = 0;
>> +        fdir->fdir_fltr_cnt[flow][1] = 0;
>> +    }
>
> In all other places this construct is written like:
>
>     enum ice_fltr_ptype flow;
>
>     for (flow = ICE_FLTR_PTYPE_NONF_NONE; flow < … ) {
>         …
>     }

Thanks for reminding. This is better. Updated in V3.


>
>> +}
>> +
>>   /**
>>    * ice_vc_fdir_write_flow_prof
>>    * @vf: pointer to the VF structure
>> @@ -1924,6 +1938,7 @@ void ice_vf_fdir_init(struct ice_vf *vf)
>>       spin_lock_init(&fdir->ctx_lock);
>>       fdir->ctx_irq.flags = 0;
>>       fdir->ctx_done.flags = 0;
>> +    ice_vc_fdir_reset_cnt_all(fdir);
>>   }
>>     /**
>
>
> Kind regards,
>
> Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

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

end of thread, other threads:[~2023-03-28 10:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-09  2:49 [Intel-wired-lan] [PATCH net v2] ice: add FDIR counter reset in FDIR init stage Lingyu Liu
2023-03-10 10:19 ` Michal Swiatkowski
2023-03-14  0:39   ` Liu, Lingyu
2023-03-27  8:34     ` Michal Swiatkowski
2023-03-27  9:08 ` Paul Menzel
2023-03-28 10:41   ` Liu, Lingyu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox