[Intel-wired-lan] [PATCH net] ice: add profile conflict check for AVF FDIR

[Guo, Junfeng]

> -----Original Message-----
> From: Nguyen, Anthony L <anthony.l.nguyen at intel.com>
> Sent: Saturday, March 11, 2023 02:03
> To: Michal Swiatkowski <michal.swiatkowski at linux.intel.com>; Guo,
> Junfeng <junfeng.guo at intel.com>
> Cc: intel-wired-lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH net] ice: add profile conflict check
> for AVF FDIR
> 
> On 3/10/2023 2:15 AM, Michal Swiatkowski wrote:
> > On Fri, Mar 10, 2023 at 05:16:22AM +0000, Guo, Junfeng wrote:
> 
> [...]
> 
> >>>> +
> >>> 	to_fltr_conf_from_desc(desc);
> >>>> +		struct ice_fdir_fltr *a = &existing_conf->input;
> >>>> +		struct ice_fdir_fltr *b = &conf->input;
> >>>> +
> >>>> +		enum ice_fltr_ptype flow_type_a = a->flow_type;
> >>>> +		enum ice_fltr_ptype flow_type_b = b->flow_type;
> >>> I think You should folow RCT variable declaration here, and remove
> >>> empty
> >>> line.
> >>
> >> Thanks for the advice!
> >>
> >> Do you mean update the code order like this?
> >> 	{
> >>
> > To follow RCT:
> > struct ice_fdir_fltr *a = &existing_conf->input;
> > enum ice_fltr_ptype flow_type_a, flow_type_b;
> > struct ice_fdir_fltr *b = &conf->input;
> >
> >> 	flow_type_a = a->flow_type;
> >> 	flow_type_b = b->flow_type;
> >> 	}
> >> Or like this?
> >> 	{
> >> 	enum ice_fltr_ptype flow_type_a, flow_type_b;
> >> 	struct ice_fdir_fltr *a, *b;
> > This is also fine
> >
> > Also fine will be:
> > struct ice_fdir_fltr *a = &existing_conf->input;
> > enum ice_fltr_ptype flow_type_a = a->flow_type;
> > enum ice_fltr_ptype flow_type_b = b->flow_type;
> > struct ice_fdir_fltr *b = &conf->input;
> >
> > And it's look the best in my opinion, but it is only cosmetic.
> 
> Looks like flow type b has a dependency on fltr b so I don't think this
> will work.
> 
> Either of the suggestions previously mentioned should work:
> 
> 	struct ice_fdir_fltr *a = &existing_conf->input;
> 	enum ice_fltr_ptype flow_type_a, flow_type_b;
> 	struct ice_fdir_fltr *b = &conf->input;
> 
> 	flow_type_a = a->flow_type;
> 	flow_type_b = b->flow_type;
> or:
> 
> 	enum ice_fltr_ptype flow_type_a, flow_type_b;
> 	struct ice_fdir_fltr *a, *b;
> 
> 	a = &existing_conf->input;
> 	b = &conf->input;
> 	flow_type_a = a->flow_type;
> 	flow_type_b = b->flow_type;

LGTM, thank you all for the careful review!
Will update this in the coming version patch.