From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f182.google.com (mail-wi0-f182.google.com [209.85.212.182]) by dpdk.org (Postfix) with ESMTP id 48D507F34 for ; Mon, 27 Oct 2014 17:49:32 +0100 (CET) Received: by mail-wi0-f182.google.com with SMTP id d1so3492704wiv.15 for ; Mon, 27 Oct 2014 09:58:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=Y+lbpdF3BGcyGSjuMqo1QHh6IEbYraGspH/sgVdYJvc=; b=euyp1+JrNMUwkEZ6HCxH1AvykSUU1IANtQvUEvIWNGWX85oCwntLNLF7haixTitJCv IGd4EHKNm1NgKcLkvGWYvGytXTi4T+XMsTIhsDgqp1I3vz0SffZvtJZl+obU+tr2WCeM +Ma1lVSeaY7axXDnrchS3VJnPaNsQ1Ai95CTYqd/OEzuljIeITxtd3sGgCkPTZCbkV5w zzGN+/wcIvFy352bqqj7CGvKXwhw0G5QI7rXI2j1dQVTShNkz/eTA50nom93zOJaY0t6 8fnu5cNk7j05cdVuQi8fKZ5+s2E20iwbhWbpYaEpuKh9d2FBv/5hs9UMXOMwcYb7aN2e k3Rg== X-Gm-Message-State: ALoCoQn/LeKfVO5361HfBJLSRr13NepHeM5vBzgGy0VbIZBznTD11VdSrYMTkL+gYySWVWuK0QH1 X-Received: by 10.194.122.104 with SMTP id lr8mr9101128wjb.28.1414429095629; Mon, 27 Oct 2014 09:58:15 -0700 (PDT) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id c15sm10639844wib.3.2014.10.27.09.58.13 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 27 Oct 2014 09:58:14 -0700 (PDT) From: Thomas Monjalon To: Jingjing Wu Date: Mon, 27 Oct 2014 17:57:54 +0100 Message-ID: <3042428.RIRLHjG5pO@xps13> Organization: 6WIND User-Agent: KMail/4.14.2 (Linux/3.17.1-1-ARCH; KDE/4.14.2; x86_64; ; ) In-Reply-To: <1413939687-11177-5-git-send-email-jingjing.wu@intel.com> References: <1411711418-12881-1-git-send-email-jingjing.wu@intel.com> <1413939687-11177-1-git-send-email-jingjing.wu@intel.com> <1413939687-11177-5-git-send-email-jingjing.wu@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v4 04/21] ethdev: define structures for adding/deleting flow director X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Oct 2014 16:49:32 -0000 2014-10-22 09:01, Jingjing Wu: > +/** > + * A structure used to define the input for IPV4 UDP flow > + */ > +struct rte_eth_udpv4_flow { > + uint32_t src_ip; /**< IPv4 source address to match. */ > + uint32_t dst_ip; /**< IPv4 destination address to match. */ > + uint16_t src_port; /**< UDP Source port to match. */ > + uint16_t dst_port; /**< UDP Destination port to match. */ > +}; > + > +/** > + * A structure used to define the input for IPV4 TCP flow > + */ > +struct rte_eth_tcpv4_flow { > + uint32_t src_ip; /**< IPv4 source address to match. */ > + uint32_t dst_ip; /**< IPv4 destination address to match. */ > + uint16_t src_port; /**< TCP Source port to match. */ > + uint16_t dst_port; /**< TCP Destination port to match. */ > +}; > + > +/** > + * A structure used to define the input for IPV4 SCTP flow > + */ > +struct rte_eth_sctpv4_flow { > + uint32_t src_ip; /**< IPv4 source address to match. */ > + uint32_t dst_ip; /**< IPv4 destination address to match. */ > + uint32_t verify_tag; /**< verify tag to match */ > +}; > + > +/** > + * A structure used to define the input for IPV4 flow > + */ > +struct rte_eth_ipv4_flow { > + uint32_t src_ip; /**< IPv4 source address to match. */ > + uint32_t dst_ip; /**< IPv4 destination address to match. */ > +}; Why not defining only 1 structure? struct rte_eth_ipv4_flow { uint32_t src_ip; uint32_t dst_ip; uint16_t src_port; uint16_t dst_port; uint32_t sctp_tag; }; I think the same structure could be used for many filters (not only flow director). > +#define RTE_ETH_FDIR_MAX_FLEXWORD_LEN 8 > +/** > + * A structure used to contain extend input of flow > + */ > +struct rte_eth_fdir_flow_ext { > + uint16_t vlan_tci; > + uint8_t num_flexwords; /**< number of flexwords */ > + uint16_t flexwords[RTE_ETH_FDIR_MAX_FLEXWORD_LEN]; > + uint16_t dest_id; /**< destination vsi or pool id*/ > +}; Flexword should be explained. > +/** > + * A structure used to define the input for an flow director filter entry typo: for *a* flow director > + */ > +struct rte_eth_fdir_input { > + enum rte_eth_flow_type flow_type; /**< type of flow */ > + union rte_eth_fdir_flow flow; /**< specific flow structure */ > + struct rte_eth_fdir_flow_ext flow_ext; /**< specific flow info */ > +}; I don't understand the logic behind flow/flow_ext. Why flow_ext is not merged into flow ? > +/** > + * Flow director report status > + */ > +enum rte_eth_fdir_status { > + RTE_ETH_FDIR_NO_REPORT_STATUS = 0, /**< no report FDIR. */ > + RTE_ETH_FDIR_REPORT_FD_ID, /**< only report FD ID. */ > + RTE_ETH_FDIR_REPORT_FD_ID_FLEX_4, /**< report FD ID and 4 flex bytes. */ > + RTE_ETH_FDIR_REPORT_FLEX_8, /**< report 8 flex bytes. */ > +}; The names and explanations are cryptics. Is FD redundant with FDIR? > +/** > + * A structure used to define an action when match FDIR packet filter. > + */ > +struct rte_eth_fdir_action { > + uint16_t rx_queue; /**< queue assigned to if fdir match. */ > + uint16_t cnt_idx; /**< statistic counter index */ what is the action of "statistic counter index"? > + uint8_t drop; /**< accept or reject */ > + uint8_t flex_off; /**< offset used define words to report */ still difficult to understand the flex logic > + enum rte_eth_fdir_status report_status; /**< status report option */ > +}; > +/** > + * A structure used to define the flow director filter entry by filter_ctl API > + * to support RTE_ETH_FILTER_FDIR with RTE_ETH_FILTER_ADD and > + * RTE_ETH_FILTER_DELETE operations. > + */ > +struct rte_eth_fdir_filter { > + uint32_t soft_id; /**< id */ Should the application handle the id numbering? Why is it soft_id instead of id? > + struct rte_eth_fdir_input input; /**< input set */ > + struct rte_eth_fdir_action action; /**< action taken when match */ > +}; It's really a hard job to define a clear and easy to use API. It would be really interesting to have more people involved in this discussion. Thanks -- Thomas