RE: [PATCH] net/ice: revert mark action in switch filter

["Zeng, ZhichaoX" <zhichaox.zeng@intel.com>]

Hi Zhirun,

> -----Original Message-----
> From: Yan, Zhirun <zhirun.yan@intel.com>
> Sent: Sunday, June 25, 2023 10:20 AM
> To: Zeng, ZhichaoX <zhichaox.zeng@intel.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>
> Subject: RE: [PATCH] net/ice: revert mark action in switch filter
> 
> 
> 
> > -----Original Message-----
> > From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> > Sent: Wednesday, June 21, 2023 5:41 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zeng, ZhichaoX
> > <zhichaox.zeng@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Yan,
> > Zhirun <zhirun.yan@intel.com>
> > Subject: [PATCH] net/ice: revert mark action in switch filter
> >
> > This implementation has some problem, the function did not work
> > properly, and it broke the mark action in FDir filter, so revert this commit.
> >
> Hi Zhichao,
> 
> Actually, I think it is better to describe a functional impact in commit message.
> It could make it easier to understand the issue.
> 
> It could be:
> Revert this part to disable mark action in switch and recover the mark action
> hit in FDIR by default. For current impl, switch filter missed the consumption
> part of mark id for app.
> And switch filter capture the rules first rather than FDIR which makes FDIR
> rules lost.
> 
> Does it sound OK to you?
>

Thanks for your comments, I will submit v2 with detailed commit log as your advice.
 
> > Fixes: e24f43f2c824 ("net/ice: support flow mark action in switch")
> > Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> > ---
> >  drivers/net/ice/ice_switch_filter.c | 9 ---------
> >  1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_switch_filter.c
> > b/drivers/net/ice/ice_switch_filter.c
> > index 1ff4b1afda..110d8895fe 100644
> > --- a/drivers/net/ice/ice_switch_filter.c
> > +++ b/drivers/net/ice/ice_switch_filter.c
> > @@ -1715,7 +1715,6 @@ ice_switch_parse_action(struct ice_pf *pf,
> >  	struct rte_eth_dev_data *dev_data = pf->adapter->pf.dev_data;
> >  	const struct rte_flow_action_queue *act_q;
> >  	const struct rte_flow_action_rss *act_qgrop;
> > -	const struct rte_flow_action_mark *act_mark;
> >  	uint16_t base_queue, i;
> >  	const struct rte_flow_action *action;
> >  	enum rte_flow_action_type action_type; @@ -1768,13 +1767,6 @@
> > ice_switch_parse_action(struct ice_pf *pf,
> >  				ICE_DROP_PACKET;
> >  			break;
> >
> > -		case RTE_FLOW_ACTION_TYPE_MARK:
> > -			act_mark = action->conf;
> > -			rule_info->sw_act.fltr_act =
> > -				ICE_SET_MARK;
> > -			rule_info->sw_act.markid = act_mark->id;
> > -			break;
> > -
> >  		case RTE_FLOW_ACTION_TYPE_VOID:
> >  			break;
> >
> > @@ -1830,7 +1822,6 @@ ice_switch_check_action(const struct
> > rte_flow_action *actions,
> >  		case RTE_FLOW_ACTION_TYPE_RSS:
> >  		case RTE_FLOW_ACTION_TYPE_QUEUE:
> >  		case RTE_FLOW_ACTION_TYPE_DROP:
> > -		case RTE_FLOW_ACTION_TYPE_MARK:
> >  		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
> >  		case RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR:
> >  			actions_num++;
> > --
> > 2.34.1