From: "Zhao1, Wei" <wei.zhao1@intel.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>,
"Yigit, Ferruh" <ferruh.yigit@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
Subject: Re: [dpdk-dev] [PATCH 15/18] net/ixgbe: parse flow director filter
Date: Tue, 27 Dec 2016 03:31:28 +0000 [thread overview]
Message-ID: <A2573D2ACFCADC41BB3BE09C6DE313CA020190A1@PGSMSX103.gar.corp.intel.com> (raw)
In-Reply-To: <20161223081310.GH10340@6wind.com>
Hi, Adrien
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Friday, December 23, 2016 4:13 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 15/18] net/ixgbe: parse flow director filter
>
> Hi,
>
> On Thu, Dec 22, 2016 at 10:44:32AM +0000, Ferruh Yigit wrote:
> > On 12/22/2016 9:19 AM, Zhao1, Wei wrote:
> > > Hi, Yigit
> > >
> > >> -----Original Message-----
> > >> From: Yigit, Ferruh
> > >> Sent: Wednesday, December 21, 2016 1:01 AM
> > >> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> > >> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> > >> Subject: Re: [dpdk-dev] [PATCH 15/18] net/ixgbe: parse flow
> > >> director filter
> > >>
> > >> On 12/2/2016 10:43 AM, Wei Zhao wrote:
> > >>> From: wei zhao1 <wei.zhao1@intel.com>
> > >>>
> > >>> check if the rule is a flow director rule, and get the flow director info.
> > >>>
> > >>> Signed-off-by: wei zhao1 <wei.zhao1@intel.com>
> > >>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > >>> ---
> > >>
> > >> <...>
> > >>
> > >>> + PATTERN_SKIP_VOID(rule, struct ixgbe_fdir_rule,
> > >>> + RTE_FLOW_ERROR_TYPE_ITEM_NUM);
> > >>> + if (item->type != RTE_FLOW_ITEM_TYPE_ETH &&
> > >>> + item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
> > >>> + item->type != RTE_FLOW_ITEM_TYPE_IPV6 &&
> > >>> + item->type != RTE_FLOW_ITEM_TYPE_UDP &&
> > >>> + item->type != RTE_FLOW_ITEM_TYPE_VXLAN &&
> > >>> + item->type != RTE_FLOW_ITEM_TYPE_NVGRE) {
> > >>
> > >> This gives build error [1], there are a few more same usage:
> > >>
> > >> .../drivers/net/ixgbe/ixgbe_ethdev.c:9238:17: error: comparison of
> > >> constant
> > >> 241 with expression of type 'const enum rte_flow_item_type' is
> > >> always true [-Werror,-Wtautological-constant-out-of-range-compare]
> > >> item->type != RTE_FLOW_ITEM_TYPE_NVGRE) {
> > >>
> > >>
> > >>
> > >
> > > Ok, I will add two type definition RTE_FLOW_ITEM_TYPE_NVGRE and
> RTE_FLOW_ITEM_TYPE_E_TAG into const enum rte_flow_item_type to
> eliminate this problem.
> > > Thank you.
> > >
> >
> > CC: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>
> Thanks, the original message did not catch my attention.
>
> > Yes, that is what right thing to do, since rte_flow patchset not
> > merged yet, perhaps Adrien may want to include this as next version of
> > his patchset?
> >
> > What do you think Adrien?
>
> I think by now the rte_flow series is automatically categorized as spam by
> half the community already, and that new items such as these can be added
> subsequently on their own, ideally before the entire ixgbe/i40e series.
>
> I have a few comments regarding these new items if we want to make them
> part of rte_flow.h (see definitions below).
>
oK , I will add these type definition by my patch in v2
> Unfortunately, even though they are super convenient to use and expose in
> the testpmd flow command, we need to avoid C-style bit-field definitions
> such as these for protocol header matching because they are not endian-
> safe, particularly multi-byte fields. Only meta-data that should be interpreted
> with host endianness can use them (e.g. struct rte_flow_attr, struct
> rte_flow_action_vf, etc):
>
> struct rte_flow_item_nvgre {
> uint32_t flags0:1; /**< 0 */
> uint32_t rsvd1:1; /**< 1 bit not defined */
> uint32_t flags1:2; /**< 2 bits, 1 0 */
> uint32_t rsvd0:9; /**< Reserved0 */
> uint32_t ver:3; /**< version */
> uint32_t protocol:16; /**< protocol type, 0x6558 */
> uint32_t tni:24; /**< tenant network ID or virtual subnet ID */
> uint32_t flow_id:8; /**< flow ID or Reserved */ };
>
> For an example how to avoid them, see struct ipv6_hdr definition in rte_ip.h,
> where field vtc_flow is 32 bit but covers three protocol fields and is
> considered big-endian (Nelio's endianness series [1] would be really handy to
> eliminate confusion here). Also see struct rte_flow_item_vxlan, which
> covers 24-bit fields using uint8_t arrays.
>
> struct rte_flow_item_e_tag {
> struct ether_addr dst; /**< Destination MAC. */
> struct ether_addr src; /**< Source MAC. */
> uint16_t e_tag_ethertype; /**< E-tag EtherType, 0x893F. */
> uint16_t e_pcp:3; /**< E-PCP */
> uint16_t dei:1; /**< DEI */
> uint16_t in_e_cid_base:12; /**< Ingress E-CID base */
> uint16_t rsv:2; /**< reserved */
> uint16_t grp:2; /**< GRP */
> uint16_t e_cid_base:12; /**< E-CID base */
> uint16_t in_e_cid_ext:8; /**< Ingress E-CID extend */
> uint16_t e_cid_ext:8; /**< E-CID extend */
> uint16_t type; /**< MAC type. */
> unsigned int tags; /**< Number of 802.1Q/ad tags defined. */
> struct {
> uint16_t tpid; /**< Tag protocol identifier. */
> uint16_t tci; /**< Tag control information. */
> } tag[]; /**< 802.1Q/ad tag definitions, outermost first. */ };
>
> Besides the bit-field issue for this one, looks like it should be split, at least the
> initial part is already covered by rte_flow_item_eth.
>
> I do not know much about E-Tag (IEEE 802.1BR right?) but it sort of looks like a
> 2.5 layer protocol reminiscent of VLAN.
>
> tags and tag[] fields seem based on the VLAN definition of the original
> rte_flow RFC that has since been replaced with stacked rte_flow_item_vlan
> items, much easier to program for. Perhaps this can be relied on instead of
> having e_tag implement its own variant.
>
> As a protocol-matching item and E-Tag TCI being 6 bytes according to IEEE
> 802.1BR, sizeof(struct rte_flow_item_e_tag) should ideally return 6 as well
> and would likely comprise three big-endian uint16_t fields. See how
> PCP/DEI/VID VLAN fields are interpreted in testpmd [2].
>
> Again, these concerns only stand if you intend to include these definitions
> into rte_flow.h.
>
> [1] http://dpdk.org/ml/archives/dev/2016-November/050060.html
> [2] http://dpdk.org/ml/archives/dev/2016-December/052976.html
>
> --
> Adrien Mazarguil
> 6WIND
next prev parent reply other threads:[~2016-12-27 3:31 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-02 10:42 [dpdk-dev] [PATCH 00/18] net/ixgbe: Consistent filter API Wei Zhao
2016-12-02 10:42 ` [dpdk-dev] [PATCH 01/18] net/ixgbe: store SYN filter Wei Zhao
2016-12-20 16:55 ` Ferruh Yigit
2016-12-22 9:48 ` Zhao1, Wei
2017-01-06 16:26 ` Ferruh Yigit
2017-01-10 6:48 ` Zhao1, Wei
2016-12-26 1:47 ` Zhao1, Wei
2016-12-02 10:42 ` [dpdk-dev] [PATCH 02/18] net/ixgbe: store flow director filter Wei Zhao
2016-12-20 16:58 ` Ferruh Yigit
2016-12-26 2:50 ` Zhao1, Wei
2016-12-02 10:42 ` [dpdk-dev] [PATCH 03/18] net/ixgbe: store L2 tunnel filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 04/18] net/ixgbe: restore n-tuple filter Wei Zhao
2016-12-20 16:58 ` Ferruh Yigit
2016-12-26 3:32 ` Zhao1, Wei
2016-12-02 10:43 ` [dpdk-dev] [PATCH 05/18] net/ixgbe: restore ether type filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 06/18] net/ixgbe: restore SYN filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 07/18] net/ixgbe: restore flow director filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 08/18] net/ixgbe: restore L2 tunnel filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 09/18] net/ixgbe: store and restore L2 tunnel configuration Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 10/18] net/ixgbe: flush all the filters Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 11/18] net/ixgbe: parse n-tuple filter Wei Zhao
2016-12-20 17:23 ` Ferruh Yigit
2016-12-02 10:43 ` [dpdk-dev] [PATCH 12/18] net/ixgbe: parse ethertype filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 13/18] net/ixgbe: parse SYN filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 14/18] net/ixgbe: parse L2 tunnel filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 15/18] net/ixgbe: parse flow director filter Wei Zhao
2016-12-20 17:00 ` Ferruh Yigit
2016-12-22 9:19 ` Zhao1, Wei
2016-12-22 10:44 ` Ferruh Yigit
2016-12-23 8:13 ` Adrien Mazarguil
2016-12-27 3:31 ` Zhao1, Wei [this message]
2016-12-02 10:43 ` [dpdk-dev] [PATCH 16/18] net/ixgbe: create consistent filter Wei Zhao
2016-12-20 17:25 ` Ferruh Yigit
2016-12-23 6:26 ` Zhao1, Wei
2016-12-02 10:43 ` [dpdk-dev] [PATCH 17/18] net/ixgbe: destroy " Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 18/18] net/ixgbe: flush " Wei Zhao
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=A2573D2ACFCADC41BB3BE09C6DE313CA020190A1@PGSMSX103.gar.corp.intel.com \
--to=wei.zhao1@intel.com \
--cc=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@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).