05/10/2021 18:19, Ananyev, Konstantin:
> > 04/10/2021 15:56, Konstantin Ananyev:
> > > Move rte_eth_dev, rte_eth_dev_data, rte_eth_rxtx_callback and related
> > > data into private header (ethdev_driver.h).
> > [...]
> > > +/**
> > > + * @internal
> > > + * Structure used to hold information about the callbacks to be called
> > > for a
> > > + * queue on RX and TX.
> > > + */
> > > +struct rte_eth_rxtx_callback {
> > > + struct rte_eth_rxtx_callback *next;
> > > + union{
> > > + rte_rx_callback_fn rx;
> > > + rte_tx_callback_fn tx;
> > > + } fn;
> > > + void *param;
> > > +};
> > > +
> > > +/**
> > > + * @internal
> > > + * The generic data structure associated with each ethernet device.
> > > + *
> > > + * 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. */
> >
> > Why not using the new struct rte_eth_fp_ops?
>
> We don't want to change each and every driver for this change.
> The idea beyond it:
> 1. PMDs keep to setup fast-path function pointers and related data
> inside rte_eth_dev struct in the same way they did it before.
> 2. Inside rte_eth_dev_start() and inside rte_eth_dev_probing_finish()
> (for secondary process) we call eth_dev_fp_ops_setup, which
> copies these function and data pointers into rte_eth_fp_ops[port_id].
> 3. Inside rte_eth_dev_stop() and inside rte_eth_dev_release_port()
> we call eth_dev_fp_ops_reset(), which resets rte_eth_fp_ops[port_id]
> into some dummy values.
OK please add this explanation in the commit log.
> > > +
> > > + /**
> > > + * Next two fields are per-device data but *data is shared between
> > > + * primary and secondary processes and *process_private is per-process
> > > + * private. The second one is managed by PMDs if necessary.
> > > + */
> > > + struct rte_eth_dev_data *data; /**< Pointer to device data. */
> >
> > We should mention that "data" is shared between processes.
>
> I think the comment above states exactly that.
> In fact, it is just cut and paste from lib/ethdev/rte_ethdev_core.h to
> lib/ethdev/ethdev_driver.h.
True, but it is confusing, and we cannot have 2 comments for the same field.
The sentence "Next two fields are per-device data" is useless.
Let's comment each field separately.
> > > + 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 */
> > > +} __rte_cache_aligned;
[...]
> > > +extern struct rte_eth_dev rte_eth_devices[];
> >
> > Later we should add a function to configure the size of this array
> > dynamically
> > in the early DPDK init stage.
>
> After we will hide rte_eth_devices[] and friends, we should be able to do
> with them whatever we want.
> But I suppose, that should be a subject of separate patch/discussion,
> Probably not in 21.11 timeframe.
Yes
