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:09:17 +0000
Message-ID: <BN8PR11MB37958BDBBD99C9376CEF8D1CF73B0@BN8PR11MB3795.namprd11.prod.outlook.com> (raw)
In-Reply-To: <be9f8cc8935d4f7fa3440df57e8fef9b@intel.com>

> -----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


I am insist about that but if other also agree. And I still have 2
> comments assume you are default agree and could I expect a new version or not?
> 

Oh, I missed you other two comments. Your reply is embedded into my original many
code lines. You can remove the unnecessary codes, so that people can get your comments
quickly without moving mouse down to check line by line. ;-)


> > > > +if (rxq->xtr_ol_flag) {
> > > > +uint32_t metadata = 0;
> > > > +
> > > > +if (desc->flex_ts.flex.aux0 != 0xFFFF) metadata =
> > > > +rte_le_to_cpu_16(desc-
> > > > >flex_ts.flex.aux0);
> > > > +else if (desc->flex_ts.flex.aux1 != 0xFFFF) metadata =
> > > > +rte_le_to_cpu_16(desc-
> > > > >flex_ts.flex.aux1);
> > >
> > > So you mean the ICE_RX_FLEX_DESC_STATUS1_XTRMD4_VALID_S and
> > > ICE_RX_FLEX_DESC_STATUS1_XTRMD5_VALID_S
> > > could not use to identify the IPv4 hdr offset and IPv6 hdr offset here in
> > rxdid # 25?
> > > And if yes they can and they will not set at the same time, is that separate
> > this v2 from v1 necessary?

XTRMD4/5_VALID_S can't detect the value of IP offset available, non 0xffffffff
is the right check condition.

> > > > +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: /* .... */ ?

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


  reply	other threads:[~2020-09-22  6:09 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 [this message]
2020-09-22  6:14           ` Guo, Jia
2020-09-22  6:23             ` Wang, Haiyue
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=BN8PR11MB37958BDBBD99C9376CEF8D1CF73B0@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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git