From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E0553A034F; Mon, 11 Oct 2021 11:20:16 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 99E19410E6; Mon, 11 Oct 2021 11:20:16 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 07E11410DF for ; Mon, 11 Oct 2021 11:20:15 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 6F32F7F508; Mon, 11 Oct 2021 12:20:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 6F32F7F508 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1633944014; bh=aSRmjunNL1vLlGmXOe0QNZlbYrS3BTN6mN+mtgTCPbM=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=EU7/Sg2RpqJlZIUVrvjzWH3EBavNRQFLLhV9LcjW9wVNHWU+es0WXxQxWFTxsGaQs envTO9+yXPppx0xmwLRtmk4Kh4xD9Wf+U8F4VzPTtYkpLj7dQ2Jem1m5OF3VbWCgyx 9x3YSCxsfguQOoxpep1GU+q1xVqE/AYHKySPlOoM= To: Konstantin Ananyev , dev@dpdk.org Cc: xiaoyun.li@intel.com, anoobj@marvell.com, jerinj@marvell.com, ndabilpuram@marvell.com, adwivedi@marvell.com, shepard.siegel@atomicrules.com, ed.czeck@atomicrules.com, john.miller@atomicrules.com, irusskikh@marvell.com, ajit.khaparde@broadcom.com, somnath.kotur@broadcom.com, rahul.lakkireddy@chelsio.com, hemant.agrawal@nxp.com, sachin.saxena@oss.nxp.com, haiyue.wang@intel.com, johndale@cisco.com, hyonkim@cisco.com, qi.z.zhang@intel.com, xiao.w.wang@intel.com, humin29@huawei.com, yisen.zhuang@huawei.com, oulijun@huawei.com, beilei.xing@intel.com, jingjing.wu@intel.com, qiming.yang@intel.com, matan@nvidia.com, viacheslavo@nvidia.com, sthemmin@microsoft.com, longli@microsoft.com, heinrich.kuhn@corigine.com, kirankumark@marvell.com, mczekaj@marvell.com, jiawenwu@trustnetic.com, jianwang@trustnetic.com, maxime.coquelin@redhat.com, chenbo.xia@intel.com, thomas@monjalon.net, ferruh.yigit@intel.com, mdr@ashroe.eu, jay.jayatheerthan@intel.com References: <20211004135603.20593-1-konstantin.ananyev@intel.com> <20211007112750.25526-1-konstantin.ananyev@intel.com> <20211007112750.25526-8-konstantin.ananyev@intel.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <9b125d31-8245-6b7b-d9ab-403c44021c8f@oktetlabs.ru> Date: Mon, 11 Oct 2021 12:20:14 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20211007112750.25526-8-konstantin.ananyev@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v5 7/7] ethdev: hide eth dev related structures X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/7/21 2:27 PM, Konstantin Ananyev wrote: > Move rte_eth_dev, rte_eth_dev_data, rte_eth_rxtx_callback and related > data into private header (ethdev_driver.h). > Few minor changes to keep DPDK building after that. > > Signed-off-by: Konstantin Ananyev [snip] I realize that many notes below correspond to just moved code, but I still think that ti would be nice to fix anyway. > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > index cc2c75261c..a743553d81 100644 > --- a/lib/ethdev/ethdev_driver.h > +++ b/lib/ethdev/ethdev_driver.h > @@ -17,6 +17,154 @@ > > #include > > +/** > + * @internal > + * Structure used to hold information about the callbacks to be called for a > + * queue on RX and TX. RX -> Rx, TX -> Tx > + */ > +struct rte_eth_rxtx_callback { > + struct rte_eth_rxtx_callback *next; > + union{ missing space before { > + rte_rx_callback_fn rx; > + rte_tx_callback_fn tx; > + } fn; > + void *param; > +}; > + > +/** > + * @internal > + * The generic data structure associated with each ethernet device. ethernet -> Ethernet > + * > + * Pointers to burst-oriented packet receive and transmit functions are > + * located at the beginning of the structure, along with the pointer to > + * where all the data elements for the particular device are stored in shared > + * memory. This split allows the function pointer and driver data to be per- > + * process, while the actual configuration data for the device is shared. > + */ > +struct rte_eth_dev { > + eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */ > + eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */ > + eth_tx_prep_t tx_pkt_prepare; > + /**< Pointer to PMD transmit prepare function. */ > + eth_rx_queue_count_t rx_queue_count; > + /**< Get the number of used RX descriptors. */ > + eth_rx_descriptor_status_t rx_descriptor_status; > + /**< Check the status of a Rx descriptor. */ > + eth_tx_descriptor_status_t tx_descriptor_status; > + /**< Check the status of a Tx descriptor. */ Please, move comments to be before documented structure members. > + > + /** > + * points to device data that is shared between > + * primary and secondary processes. > + */ > + struct rte_eth_dev_data *data; > + void *process_private; /**< Pointer to per-process device data. */ > + const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */ > + struct rte_device *device; /**< Backing device */ > + struct rte_intr_handle *intr_handle; /**< Device interrupt handle */ > + /** User application callbacks for NIC interrupts */ > + struct rte_eth_dev_cb_list link_intr_cbs; > + /** > + * User-supplied functions called from rx_burst to post-process > + * received packets before passing them to the user > + */ > + struct rte_eth_rxtx_callback *post_rx_burst_cbs[RTE_MAX_QUEUES_PER_PORT]; > + /** > + * User-supplied functions called from tx_burst to pre-process > + * received packets before passing them to the driver for transmission. > + */ > + struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT]; > + enum rte_eth_dev_state state; /**< Flag indicating the port state */ > + void *security_ctx; /**< Context for security ops */ > + > + uint64_t reserved_64s[4]; /**< Reserved for future fields */ > + void *reserved_ptrs[4]; /**< Reserved for future fields */ Shoudl we remove these reserved fields? > +} __rte_cache_aligned; > + > +struct rte_eth_dev_sriov; > +struct rte_eth_dev_owner; > + > +/** > + * @internal > + * The data part, with no function pointers, associated with each ethernet ethernet -> Ethernet > + * device. This structure is safe to place in shared memory to be common > + * among different processes in a multi-process configuration. > + */ > +struct rte_eth_dev_data { > + char name[RTE_ETH_NAME_MAX_LEN]; /**< Unique identifier name */ > + > + void **rx_queues; /**< Array of pointers to RX queues. */ > + void **tx_queues; /**< Array of pointers to TX queues. */ > + uint16_t nb_rx_queues; /**< Number of RX queues. */ > + uint16_t nb_tx_queues; /**< Number of TX queues. */ RX -> Rx, TX -> Tx > + > + struct rte_eth_dev_sriov sriov; /**< SRIOV data */ > + > + void *dev_private; > + /**< PMD-specific private data. > + * @see rte_eth_dev_release_port() > + */ > + > + struct rte_eth_link dev_link; /**< Link-level information & status. */ > + struct rte_eth_conf dev_conf; /**< Configuration applied to device. */ > + uint16_t mtu; /**< Maximum Transmission Unit. */ > + uint32_t min_rx_buf_size; > + /**< Common RX buffer size handled by all queues. */ RX -> Rx > + > + uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */ RX -> Rx > + struct rte_ether_addr *mac_addrs; > + /**< Device Ethernet link address. > + * @see rte_eth_dev_release_port() > + */ > + uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR]; > + /**< Bitmap associating MAC addresses to pools. */ > + struct rte_ether_addr *hash_mac_addrs; > + /**< Device Ethernet MAC addresses of hash filtering. > + * @see rte_eth_dev_release_port() > + */ Please, move comments to be before. It looks like it will fit in one line in many cases. > + uint16_t port_id; /**< Device [external] port identifier. */ > + > + __extension__ > + uint8_t promiscuous : 1, > + /**< RX promiscuous mode ON(1) / OFF(0). */ > + scattered_rx : 1, > + /**< RX of scattered packets is ON(1) / OFF(0) */ > + all_multicast : 1, > + /**< RX all multicast mode ON(1) / OFF(0). */ > + dev_started : 1, > + /**< Device state: STARTED(1) / STOPPED(0). */ > + lro : 1, > + /**< RX LRO is ON(1) / OFF(0) */ > + dev_configured : 1; > + /**< Indicates whether the device is configured. > + * CONFIGURED(1) / NOT CONFIGURED(0). > + */ > + uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT]; > + /**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */ > + uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT]; > + /**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */ Comments should be before the code. > + uint32_t dev_flags; /**< Capabilities. */ > + int numa_node; /**< NUMA node connection. */ > + struct rte_vlan_filter_conf vlan_filter_conf; > + /**< VLAN filter configuration. */ > + struct rte_eth_dev_owner owner; /**< The port owner. */ > + uint16_t representor_id; > + /**< Switch-specific identifier. > + * Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags. > + */ > + > + pthread_mutex_t flow_ops_mutex; /**< rte_flow ops mutex. */ > + uint64_t reserved_64s[4]; /**< Reserved for future fields */ > + void *reserved_ptrs[4]; /**< Reserved for future fields */ Should we remove these reserved fields? > +} __rte_cache_aligned; > + > +/** > + * @internal > + * The pool of *rte_eth_dev* structures. The size of the pool > + * is configured at compile-time in the file. > + */ > +extern struct rte_eth_dev rte_eth_devices[]; > + > /**< @internal Declaration of the hairpin peer queue information structure. */ > struct rte_hairpin_peer_info; [snip]