DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wang, Haiyue" <haiyue.wang@intel.com>
To: "Guo, Jia" <jia.guo@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"Jiang, JunyuX" <junyux.jiang@intel.com>,
	"Rong, Leyi" <leyi.rong@intel.com>,
	"Yang, Qiming" <qiming.yang@intel.com>,
	"Sun, GuinanX" <guinanx.sun@intel.com>,
	"Guo, Junfeng" <junfeng.guo@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3] net/ice: refactor the Rx FlexiMD handling
Date: Tue, 22 Sep 2020 06:23:57 +0000	[thread overview]
Message-ID: <BN8PR11MB3795FADAEC0DF7017404FDEFF73B0@BN8PR11MB3795.namprd11.prod.outlook.com> (raw)
In-Reply-To: <c2ace59afb41405db51fdc6bda282cba@intel.com>

> -----Original Message-----
> From: Guo, Jia <jia.guo@intel.com>
> Sent: Tuesday, September 22, 2020 14:15
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Jiang, JunyuX <junyux.jiang@intel.com>; Rong, Leyi
> <leyi.rong@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>; Guo,
> Junfeng <junfeng.guo@intel.com>
> Subject: RE: [PATCH v3] net/ice: refactor the Rx FlexiMD handling
> 
> 
> > -----Original Message-----
> > From: Wang, Haiyue <haiyue.wang@intel.com>
> > Sent: Tuesday, September 22, 2020 2:09 PM
> > To: Guo, Jia <jia.guo@intel.com>; dev@dpdk.org
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Jiang, JunyuX
> > <junyux.jiang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>; Guo,
> > Junfeng <junfeng.guo@intel.com>
> > Subject: RE: [PATCH v3] net/ice: refactor the Rx FlexiMD handling
> >
> > > -----Original Message-----
> > > From: Guo, Jia <jia.guo@intel.com>
> > > Sent: Tuesday, September 22, 2020 13:51
> > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> > > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Jiang, JunyuX
> > > <junyux.jiang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Yang,
> > > Qiming <qiming.yang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>;
> > > Guo, Junfeng <junfeng.guo@intel.com>
> > > Subject: RE: [PATCH v3] net/ice: refactor the Rx FlexiMD handling
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wang, Haiyue <haiyue.wang@intel.com>
> > > > Sent: Tuesday, September 22, 2020 1:42 PM
> > > > To: Guo, Jia <jia.guo@intel.com>; dev@dpdk.org
> > > > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Jiang, JunyuX
> > > > <junyux.jiang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Yang,
> > > > Qiming <qiming.yang@intel.com>; Sun, GuinanX
> > > > <guinanx.sun@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>
> > > > Subject: RE: [PATCH v3] net/ice: refactor the Rx FlexiMD handling
> > > >
> > > > Hi Jeff,
> > > >
> > > > > -----Original Message-----
> > > > > From: Guo, Jia <jia.guo@intel.com>
> > > > > Sent: Tuesday, September 22, 2020 13:35
> > > > > To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> > > > > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Jiang, JunyuX
> > > > > <junyux.jiang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Yang,
> > > > > Qiming <qiming.yang@intel.com>; Sun, GuinanX
> > > > > <guinanx.sun@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>
> > > > > Subject: RE: [PATCH v3] net/ice: refactor the Rx FlexiMD handling
> > > > >
> > > > > Hi, haiyue
> > > > >
> >
> > > > > > +   struct rte_mbuf *mb,
> > > > > > +   volatile union ice_rx_flex_desc *rxdp) { volatile struct
> > > > > > +ice_32b_rx_flex_desc_comms_ovs *desc = (volatile struct
> > > > > > +ice_32b_rx_flex_desc_comms_ovs
> > > > > > *)rxdp; #ifndef
> > > > > > +RTE_LIBRTE_ICE_16BYTE_RX_DESC
> > > > > > +uint16_t stat_err;
> > > > > > +#endif
> > > > >
> > > > > This #ifndef could be better combine with below #ifndef.
> > > > >
> > > >
> > > > I changed it to according to the different offsets, like ovs has rss
> > > > hash in Qword 3, which is after flow Id Qword 1, others are
> > > > opposite. So that this handling order can reflect the offset
> > > > difference, although, it MAY looks not so beautiful. What do you
> > > > think ? :)
> > > >
> > >
> > > I am not sure I got you point about the order reason, but I think in
> > > or out #ifndef should be clear show the offset difference,
> >
> > You mean below ? If so, 'uint16_t stat_err;' is not so good in the middle of
> > two code blocks.
> >
> > #ifndef RTE_LIBRTE_ICE_16BYTE_RX_DESC
> > uint16_t stat_err;
> >
> > stat_err = rte_le_to_cpu_16(desc->status_error0);
> > if (likely(stat_err & (1 << ICE_RX_FLEX_DESC_STATUS0_RSS_VALID_S))) {
> > mb->ol_flags |= PKT_RX_RSS_HASH;
> > mb->hash.rss = rte_le_to_cpu_32(desc->rss_hash);
> > }
> > #endif
> >
> 
> Sorry, let me show clear as below
> 
> #ifndef RTE_LIBRTE_ICE_16BYTE_RX_DESC
> uint16_t stat_err;
> 
> stat_err = rte_le_to_cpu_16(desc->status_error0);
> if (likely(stat_err & (1 << ICE_RX_FLEX_DESC_STATUS0_RSS_VALID_S))) {
> mb->ol_flags |= PKT_RX_RSS_HASH;
> mb->hash.rss = rte_le_to_cpu_32(desc->rss_hash);
> }
> #endif
> 
> if (desc->flow_id != 0xFFFFFFFF) {
> mb->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID;
> mb->hash.fdir.hi = rte_le_to_cpu_32(desc->flow_id);
> }
> 

That's what I said about the order, you can check "ice_rxd_to_pkt_fields_by_comms_ovs"
vs "ice_rxd_to_pkt_fields_by_comms_aux_v1" about "desc->flow_id" and "desc->rss_hash"
handling order: offset first, handle it firstly.

> > > > > > +uint64_t xtr_ol_flag; /* Protocol extraction offload flag */
> > > > > > +ice_rxd_to_pkt_fields_t rxd_to_pkt_fields;
> > > > >
> > > > > If create a function pointer here in .h, it is better add some doc.
> >
> > C comments: /* .... */ ?
> >
> 
> Yes.

OK.

> 
> > > > >
> > > > > >  ice_rx_release_mbufs_t rx_rel_mbufs;  };
> > > > > >
> > > > > > --
> > > > > > 2.28.0
> > > > >
> > > >
> > >
> >
> 


  reply	other threads:[~2020-09-22  6:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-17 11:53 [dpdk-dev] [PATCH v1] net/ice: enhance the " Haiyue Wang
2020-09-17 12:49 ` [dpdk-dev] [PATCH v2] " Haiyue Wang
2020-09-18  1:05 ` [dpdk-dev] [PATCH v3] net/ice: refactor the Rx " Haiyue Wang
2020-09-22  5:35   ` Guo, Jia
2020-09-22  5:42     ` Wang, Haiyue
2020-09-22  5:50       ` Guo, Jia
2020-09-22  6:09         ` Wang, Haiyue
2020-09-22  6:14           ` Guo, Jia
2020-09-22  6:23             ` Wang, Haiyue [this message]
2020-09-22  6:33               ` Guo, Jia
2020-09-22  6:40 ` [dpdk-dev] [PATCH v4] " Haiyue Wang
2020-09-22  7:24   ` Guo, Jia
2020-09-22 12:58     ` Zhang, Qi Z

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BN8PR11MB3795FADAEC0DF7017404FDEFF73B0@BN8PR11MB3795.namprd11.prod.outlook.com \
    --to=haiyue.wang@intel.com \
    --cc=dev@dpdk.org \
    --cc=guinanx.sun@intel.com \
    --cc=jia.guo@intel.com \
    --cc=junfeng.guo@intel.com \
    --cc=junyux.jiang@intel.com \
    --cc=leyi.rong@intel.com \
    --cc=qi.z.zhang@intel.com \
    --cc=qiming.yang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).