Re: [dpdk-dev] [PATCH v3 3/3] net/ice: support ACL filter in DCF

["Su, Simei" <simei.su@intel.com>]

Hi, Qi

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Thursday, October 15, 2020 1:11 PM
> To: Su, Simei <simei.su@intel.com>; Yang, Qiming <qiming.yang@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Ding, Xuan <xuan.ding@intel.com>
> Subject: RE: [PATCH v3 3/3] net/ice: support ACL filter in DCF
> 
> some minor captures
> 
> > -----Original Message-----
> > From: Su, Simei <simei.su@intel.com>
> > Sent: Wednesday, October 14, 2020 4:54 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>
> > Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Ding, Xuan <xuan.ding@intel.com>; Su, Simei
> > <simei.su@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.su@intel.com>
> > Signed-off-by: Xuan Ding <xuan.ding@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.

> >
> >
>