Hi, Qi
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zh...@intel.com>
> Sent: Thursday, October 15, 2020 1:11 PM
> To: Su, Simei <simei...@intel.com>; Yang, Qiming <qiming.y...@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.w...@intel.com>; Xing, Beilei
> <beilei.x...@intel.com>; Ding, Xuan <xuan.d...@intel.com>
> Subject: RE: [PATCH v3 3/3] net/ice: support ACL filter in DCF
>
> some minor captures
>
> > -----Original Message-----
> > From: Su, Simei <simei...@intel.com>
> > Sent: Wednesday, October 14, 2020 4:54 PM
> > To: Zhang, Qi Z <qi.z.zh...@intel.com>; Yang, Qiming
> > <qiming.y...@intel.com>
> > Cc: dev@dpdk.org; Wang, Haiyue <haiyue.w...@intel.com>; Xing, Beilei
> > <beilei.x...@intel.com>; Ding, Xuan <xuan.d...@intel.com>; Su, Simei
> > <simei...@intel.com>
> > Subject: [PATCH v3 3/3] net/ice: support ACL filter in DCF
> >
> > Add ice_acl_create_filter to create a rule and ice_acl_destroy_filter
> > to destroy a rule. If a flow is matched by ACL filter, filter rule
> > will be set to HW. Currently IPV4/IPV4_UDP/IPV4_TCP/IPV4_SCTP pattern
> and drop action are supported.
> >
> > Signed-off-by: Simei Su <simei...@intel.com>
> > Signed-off-by: Xuan Ding <xuan.d...@intel.com>
> > ---
> > doc/guides/rel_notes/release_20_11.rst | 6 +
> > drivers/net/ice/ice_acl_filter.c | 1034
> > ++++++++++++++++++++++++++++++++
> > drivers/net/ice/ice_ethdev.h | 17 +
> > drivers/net/ice/ice_generic_flow.c | 2 +
> > drivers/net/ice/meson.build | 3 +-
> > 5 files changed, 1061 insertions(+), 1 deletion(-) create mode
> > 100644 drivers/net/ice/ice_acl_filter.c
> >
> > diff --git a/doc/guides/rel_notes/release_20_11.rst
> > b/doc/guides/rel_notes/release_20_11.rst
> > index e8ae4d4..6cf1ef8 100644
> > --- a/doc/guides/rel_notes/release_20_11.rst
> > +++ b/doc/guides/rel_notes/release_20_11.rst
> ...
>
> > +#define MAX_ACL_SLOTS_ID 2048
> > +
> > +#define ICE_ACL_INSET_ETH_IPV4 ( \
> > +ICE_INSET_SMAC | ICE_INSET_DMAC | \
> > +ICE_INSET_IPV4_SRC | ICE_INSET_IPV4_DST) #define
> > +ICE_ACL_INSET_ETH_IPV4_UDP ( \ ICE_INSET_SMAC | ICE_INSET_DMAC | \
> > +ICE_INSET_IPV4_SRC | ICE_INSET_IPV4_DST | \ ICE_INSET_UDP_SRC_PORT
> |
> > +ICE_INSET_UDP_DST_PORT)
>
>
> Better to reuse ICE_ACL_INSET_ETH_IPV4 in ICE_ACL_INSET_ETH_IPV4_UDP
Ok, will simplify it in v4.
>
> ...
> > +static void
> > +acl_prof_helper_function(struct ice_hw *hw, struct ice_flow_seg_info *seg,
>
> The function is always used before ice_add_prof, so better rename to
> acl_add_prof_prepare.
Ok, will rename it in next version.
>
> > + bool is_l4, uint16_t src_port, uint16_t dst_port) {
> > +uint16_t val_loc, mask_loc;
> > +
> > +ret = ice_acl_parse_action(ad, actions, error, filter);
> > +if (ret)
> > +goto error;
> > +
> > +if (meta)
> > +*meta = filter;
> > +
> > +error:
> > +rte_free(item);
> > +return ret;
> > +}
> > +
> ....
>
> >
> > +struct ice_acl_conf {
> > +struct ice_fdir_fltr input;
> > +uint64_t input_set;
> > +};
> > +
> > +/**
> > + * A structure used to define fields of ACL related info.
> > + */
> > +struct ice_acl_info {
> > +struct ice_acl_conf conf;
> > +};
> > +
> > struct ice_pf {
> > struct ice_adapter *adapter; /* The adapter this PF associate to */
> > struct ice_vsi *main_vsi; /* pointer to main VSI structure */ @@ -421,6
> > +435,7 @@ struct ice_pf {
> > uint16_t fdir_nb_qps; /* The number of queue pairs of Flow Director */
> > uint16_t fdir_qp_offset;
> > struct ice_fdir_info fdir; /* flow director info */
> > +struct ice_acl_info acl; /* ACL info */
> > struct ice_hash_ctx hash_ctx;
> > uint16_t hw_prof_cnt[ICE_FLTR_PTYPE_MAX][ICE_FD_HW_SEG_MAX];
> > uint16_t fdir_fltr_cnt[ICE_FLTR_PTYPE_MAX][ICE_FD_HW_SEG_MAX];
> > @@ -440,6 +455,8 @@ struct ice_pf {
> > uint64_t old_rx_bytes;
> > uint64_t old_tx_bytes;
> > uint64_t supported_rxdid; /* bitmap for supported RXDID */
> > +struct rte_bitmap *slots;
> > +uint64_t hw_entry_id[MAX_ACL_ENTRIES];
>
> Can we move above 2 fields into ice_acl_info?
>
> > };
Yes, we can.
> >
> >
>
