From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>,
"Zhang, Helin" <helin.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
Subject: Re: [dpdk-dev] [PATCH 2/2] net/i40e: configurable PTYPE mapping
Date: Tue, 7 Mar 2017 02:37:02 +0000 [thread overview]
Message-ID: <039ED4275CED7440929022BC67E7061153063970@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <45a02a73-102d-129a-43da-605d8bdc271e@intel.com>
Hi Ferruh:
Thanks all the good suggestion and help.
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 6, 2017 11:32 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] net/i40e: configurable PTYPE mapping
>
> On 2/27/2017 4:56 AM, Qi Zhang wrote:
> > The patch adds 4 APIs to support configurable PTYPE mapping for i40e
> > device.
> > rte_pmd_i40e_update_ptype_mapping.
> > rte_pmd_i40e_reset_ptype_mapping.
> > rte_pmd_i40e_get_ptype_mapping.
> > rte_pmd_i40e_replace_ptype_mapping.
>
> Hi Qi,
>
> These are added as PMD specific APIs, but not used anywhere, how did you
> test them? Or how others can test it?
>
> Does it make sense to add them into testpmd?
>
Yes, I plan to add
>
>
> And related to API naming, what do you think about following syntax:
> <name_space>_<object>_<action> ?
>
> This helps finding APIs for same object (ptype_mapping for this case):
>
> rte_pmd_i40e_ptype_mapping_update()
> rte_pmd_i40e_ptype_mapping_reset()
> rte_pmd_i40e_ptype_mapping_get()
> rte_pmd_i40e_ptype_mapping_replace()
Agree, that's good.
>
>
> And not directly related to this patch, but it is good idea to extract PMD
> specific APIs into separate file, would you mind taking that task before this
> patch? And add these new APIs to that new file?
>
I like this idea, since we have many changes on this recently, I'd better discuss with
other team member to decide if do the separation before merge or after the merge.
>
> > The mapping from hardware defined packet type to software defined
> > packet type can be updated/reset/read out with these APIs.
> > Also a software ptype with the most significent bit set will be
> > regarded as a custom defined ptype
> > (RTE_PMD_I40E_PTYPE_USER_DEFINE_MASK) so application can use it to
> defined its own PTYPE naming system.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 190
> ++++++++++++++++++++++++++++++++++++++++
> > drivers/net/i40e/i40e_rxtx.c | 1 -
> > drivers/net/i40e/i40e_rxtx.h | 2 +
> > drivers/net/i40e/rte_pmd_i40e.h | 81 +++++++++++++++++
>
> Need to update *version.map file too, for shared library build. It is hard to
> catch these issues since APIs are not used.
>
Will fix.
> <...>
>
> > +
> > +int rte_pmd_i40e_update_ptype_mapping(
>
> DPDK coding convention suggest having return type in separate line:
> int
> rte_pmd_i40e_update_ptype_mapping(...
>
> > + uint8_t port,
> > + struct rte_pmd_i40e_ptype_mapping *mapping_items,
> > + uint16_t count,
> > + uint8_t exclusive)
> > +{
> > + struct rte_eth_dev *dev;
> > + struct i40e_adapter *ad;
> > + int i;
>
> For PMD specific APIs, port_id needs to be verified if it is i40e port or not.
> There is already is_device_supported() function in i40e for this.
Thanks
>
> CC'ed Wenzhuo, he already did this a few times, and may help.
>
> <...>
>
> > +/**
> > + * Update hardware defined ptype to software defined packet type
> > + * mapping table.
> > + *
> > + * @param port
> > + * pointer to port identifier of the device.
> > + * @param mapping_items
> > + * the base address of the mapping items array.
> > + * @param count
> > + * number of mapping items.
> > + * @param exclusive
> > + * the flag indicate different ptype mapping update method.
> > + * -(0) only overwrite refferred PTYPE mapping,
>
> referred
>
> > + * keep other PTYPEs mapping unchanged.
> > + * -(!0) overwrite referred PTYPE mapping,
> > + * set other PTYPEs maps to PTYPE_UNKNOWN.
> > + */
> > +int rte_pmd_i40e_update_ptype_mapping(
> > + uint8_t port,
> > + struct rte_pmd_i40e_ptype_mapping *mapping_items,
> > + uint16_t count,
> > + uint8_t exclusive);
> > +
> > +/**
> > + * Reset hardware defined ptype to software defined ptype
> > + * mapping table to default.
> > + *
> > + * @param port
> > + * pointer to port identifiier of the device
>
> s/identifiier/identifier
>
> <...>
>
> > +/**
> > + * Replace a specific or a group of software defined ptypes
> > + * with a new one
> > + *
> > + * @param port
> > + * pointer to port identifier of the device
> > + * @param target
> > + * the packet type to be replaced
> > + * @param mask
> > + * -(0) target represent a specific software defined ptype.
> > + * -(!0) target is a mask to represent a group of software defined
> ptypes.
> > + * @param pkt_type
> > + * the new packet type to overwrite
> > + */
> > +int rte_pmd_i40e_replace_pkt_type(uint8_t port,
> > + uint32_t target,
> > + uint8_t mask,
> > + uint32_t pkt_type);
>
> API names does not match with one in commit log, and "pkt_type" usage is
> not consistent with other APIs.
Will fix.
>
> > +
> > #endif /* _PMD_I40E_H_ */
> >
next prev parent reply other threads:[~2017-03-07 2:37 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-27 4:56 [dpdk-dev] [PATCH 0/2] " Qi Zhang
2017-02-27 4:56 ` [dpdk-dev] [PATCH 1/2] net/i40e: enable per dev PTYPE mapping table Qi Zhang
2017-02-27 4:56 ` [dpdk-dev] [PATCH 2/2] net/i40e: configurable PTYPE mapping Qi Zhang
2017-03-06 15:32 ` Ferruh Yigit
2017-03-07 2:37 ` Zhang, Qi Z [this message]
2017-03-12 12:08 ` [dpdk-dev] [PATCH v2 0/3] " Qi Zhang
2017-03-12 12:08 ` [dpdk-dev] [PATCH v2 1/3] net/i40e: enable per dev PTYPE mapping table Qi Zhang
2017-03-12 12:08 ` [dpdk-dev] [PATCH v2 2/3] net/i40e: configurable PTYPE mapping Qi Zhang
2017-03-12 12:08 ` [dpdk-dev] [PATCH v2 3/3] app/testpmd: add CL for ptype mapping configure Qi Zhang
2017-03-16 16:27 ` Ferruh Yigit
2017-03-17 9:51 ` Zhang, Qi Z
2017-03-28 3:34 ` Wu, Jingjing
2017-03-28 12:28 ` Ferruh Yigit
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=039ED4275CED7440929022BC67E7061153063970@SHSMSX103.ccr.corp.intel.com \
--to=qi.z.zhang@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=helin.zhang@intel.com \
--cc=jingjing.wu@intel.com \
--cc=wenzhuo.lu@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).