> -----Original Message-----
> From: McDaniel, Timothy <timothy.mcdan...@intel.com>
> Sent: Friday, September 11, 2020 2:18 PM
> Cc: dev@dpdk.org; Carrillo, Erik G <erik.g.carri...@intel.com>; Eads, Gage
> <gage.e...@intel.com>; Van Haaren, Harry <harry.van.haa...@intel.com>;
> jer...@marvell.com
> Subject: [PATCH v4 03/22] event/dlb: add private data structures and
> constants
>
> Add headers used internally by the PMD. These headers are used
> internally by the PMD. They include constants, macros for device
Safe to drop either sentence #1 or #2 😊
[...]
Looked like some of these structs and fields are unused, so I did a few
test builds (of the whole series) to confirm. More details below.
> +
> +union dlb_port_config {
> + struct dlb_create_ldb_port_args ldb;
> + struct dlb_create_dir_port_args dir;
> +};
> +
> +enum DLB_PORT_STATE {
Nit: inconsistent capitalization (lower-case used in enum names elsewhere)
> + PORT_CLOSED,
> + PORT_STARTED,
> + PORT_STOPPED
> +};
> +
> +enum dlb_configuration_state {
> + /* The resource has not been configured */
> + DLB_NOT_CONFIGURED,
> + /* The resource was configured, but the device was stopped */
> + DLB_PREV_CONFIGURED,
> + /* The resource is currently configured */
> + DLB_CONFIGURED
> +};
> +
> +struct dlb_port {
> + uint32_t id;
> + bool is_directed;
> + bool gen_bit;
> + uint16_t dir_credits;
> + uint32_t dequeue_depth;
> + enum dlb_token_pop_mode token_pop_mode;
> + union dlb_port_config cfg;
This field is set but not used. Looks like it and the union can be dropped.
> + int pp_mmio_base;
> + uint16_t cached_ldb_credits;
> + uint16_t ldb_pushcount_at_credit_expiry;
> + uint16_t ldb_credits;
> + uint16_t cached_dir_credits;
> + uint16_t dir_pushcount_at_credit_expiry;
> + bool int_armed;
> + bool use_rsvd_token_scheme;
> + uint8_t cq_rsvd_token_deficit;
> + uint16_t owed_tokens;
> + int16_t issued_releases;
> + int16_t token_pop_thresh;
> + int cq_depth;
> + uint16_t cq_idx;
> + uint16_t cq_idx_unmasked;
> + uint16_t cq_depth_mask;
> + uint16_t gen_bit_shift;
> + enum DLB_PORT_STATE state;
> + enum dlb_configuration_state config_state;
> + int num_mapped_qids;
> + uint8_t *qid_mappings;
> + struct dlb_enqueue_qe *qe4; /* Cache line's worth of QEs (4) */
> + struct dlb_cq_pop_qe *consume_qe;
> + struct dlb_eventdev *dlb; /* back ptr */
> + struct dlb_eventdev_port *ev_port; /* back ptr */
> +};
> +
> +/* Per-process per-port mmio and memory pointers */
> +struct process_local_port_data {
> + uint64_t *pp_addr;
> + uint16_t *ldb_popcount;
> + uint16_t *dir_popcount;
> + struct dlb_dequeue_qe *cq_base;
> + bool mmaped;
> +};
> +
> +struct dlb_config {
> + int configured;
> + int reserved;
> + uint32_t ldb_credit_pool_id;
> + uint32_t dir_credit_pool_id;
> + uint32_t num_ldb_credits;
> + uint32_t num_dir_credits;
> + struct dlb_create_sched_domain_args resources;
> +};
> +
> +struct dlb_hw_dev {
> + char device_name[DLB_NAME_SIZE];
> + char device_path[DLB_MAX_DEVICE_PATH];
device_name and device_path are used in strings but never set
> + int device_path_id;
Set but never used
> + char domain_device_path[DLB_MAX_DEVICE_PATH];
Used in an xstats print but never set
> + struct dlb_config cfg;
> + struct dlb_hw_resource_info info;
> + void *pf_dev; /* opaque pointer to PF PMD dev (struct dlb_dev) */
> + int device_id;
> + uint32_t domain_id;
> + int domain_id_valid;
> + rte_spinlock_t resource_lock; /* for MP support */
> +}; __rte_cache_aligned
> +
> +/* End HW related defines and structs */
> +
> +/* Begin DLB PMD Eventdev related defines and structs */
> +
> +#define DLB_MAX_NUM_QUEUES \
> + (DLB_MAX_NUM_DIR_QUEUES + DLB_MAX_NUM_LDB_QUEUES)
> +
> +#define DLB_MAX_NUM_PORTS (DLB_MAX_NUM_DIR_PORTS +
> DLB_MAX_NUM_LDB_PORTS)
> +#define DLB_MAX_INPUT_QUEUE_DEPTH 256
> +
> +/* Used for parsing dir ports/queues. */
What does this comment refer to?
> +
> +/** Structure to hold the queue to port link establishment attributes */
> +
> +struct dlb_event_queue_link {
> + uint8_t queue_id;
> + uint8_t priority;
> + bool mapped;
> + bool valid;
> +};
> +
> +struct dlb_traffic_stats {
> + uint64_t rx_ok;
> + uint64_t rx_drop;
> + uint64_t rx_interrupt_wait;
> + uint64_t rx_umonitor_umwait;
> + uint64_t tx_ok;
> + uint64_t total_polls;
> + uint64_t zero_polls;
> + uint64_t tx_nospc_ldb_hw_credits;
> + uint64_t tx_nospc_dir_hw_credits;
> + uint64_t tx_nospc_inflight_max;
> + uint64_t tx_nospc_new_event_limit;
> + uint64_t tx_nospc_inflight_credits;
> +};
> +
> +struct dlb_port_stats {
> + struct dlb_traffic_stats traffic;
> + uint64_t tx_op_cnt[4]; /* indexed by rte_event.op */
> + uint64_t tx_implicit_rel;
> + uint64_t tx_sched_cnt[DLB_NUM_HW_SCHED_TYPES];
> + uint64_t tx_invalid;
> + uint64_t rx_sched_cnt[DLB_NUM_HW_SCHED_TYPES];
> + uint64_t rx_sched_invalid;
> + uint64_t enq_ok[DLB_MAX_NUM_QUEUES]; /* per-queue enq_ok
> */
> +};
> +
> +struct dlb_eventdev_port {
> + struct dlb_port qm_port; /* hw specific data structure */
> + struct rte_event_port_conf conf; /* user-supplied configuration */
> + uint16_t inflight_credits; /* num credits this port has right now */
> + uint16_t credit_update_quanta;
> + struct dlb_eventdev *dlb; /* backlink optimization */
> + struct dlb_port_stats stats __rte_cache_aligned;
> + struct dlb_event_queue_link
> link[DLB_MAX_NUM_QIDS_PER_LDB_CQ];
> + int num_links;
> + uint32_t id;
> + /* num releases yet to be completed on this port.
> + * Only applies to load-balanced ports.
> + */
> + uint16_t outstanding_releases;
> + uint16_t inflight_max; /* app requested max inflights for this port */
> + /* setup_done is set when the event port is setup */
> + bool setup_done;
> + /* enq_configured is set when the qm port is created */
> + bool enq_configured;
> + uint8_t implicit_release; /* release events before dequeueing */
> +} __rte_cache_aligned;
> +
> +struct dlb_queue {
> + uint32_t num_qid_inflights; /* User config */
> + uint32_t num_atm_inflights; /* User config */
> + enum dlb_configuration_state config_state;
> + int sched_type; /* LB queue only */
> + uint32_t id;
> + bool is_directed;
> +};
> +
> +struct dlb_eventdev_queue {
> + struct dlb_queue qm_queue;
> + struct rte_event_queue_conf conf; /* User config */
> + uint64_t enq_ok;
> + uint32_t id;
> + bool setup_done;
> + uint8_t num_links;
> +};
> +
> +enum dlb_run_state {
> + DLB_RUN_STATE_STOPPED = 0,
> + DLB_RUN_STATE_STOPPING,
> + DLB_RUN_STATE_STARTING,
> + DLB_RUN_STATE_STARTED
> +};
> +
> +#define DLB_IS_VDEV true
> +#define DLB_NOT_VDEV false
Both these macros are unused
> +
> +struct dlb_eventdev {
> + struct dlb_eventdev_port ev_ports[DLB_MAX_NUM_PORTS];
> + struct dlb_eventdev_queue
> ev_queues[DLB_MAX_NUM_QUEUES];
> + uint8_t qm_ldb_to_ev_queue_id[DLB_MAX_NUM_QUEUES];
> + uint8_t qm_dir_to_ev_queue_id[DLB_MAX_NUM_QUEUES];
> +
> + /* store num stats and offset of the stats for each queue */
> + uint16_t xstats_count_per_qid[DLB_MAX_NUM_QUEUES];
> + uint16_t xstats_offset_for_qid[DLB_MAX_NUM_QUEUES];
> +
> + /* store num stats and offset of the stats for each port */
> + uint16_t xstats_count_per_port[DLB_MAX_NUM_PORTS];
> + uint16_t xstats_offset_for_port[DLB_MAX_NUM_PORTS];
> + struct dlb_get_num_resources_args hw_rsrc_query_results;
> + uint32_t xstats_count_mode_queue;
> + struct dlb_hw_dev qm_instance; /* strictly hw related */
> + uint64_t global_dequeue_wait_ticks;
> + struct dlb_xstats_entry *xstats;
> + struct rte_eventdev *event_dev; /* backlink to dev */
> + uint32_t xstats_count_mode_port;
> + uint32_t xstats_count_mode_dev;
> + uint32_t xstats_count;
> + uint32_t inflights; /* use __atomic builtins to access */
> + uint32_t new_event_limit;
> + int max_num_events_override;
> + int num_dir_credits_override;
> + volatile enum dlb_run_state run_state;
> + uint16_t num_dir_queues; /* total num of evdev dir queues
> requested */
> + uint16_t num_dir_credits;
> + uint16_t num_ldb_credits;
> + uint16_t num_queues; /* total queues */
> + uint16_t num_ldb_queues; /* total num of evdev ldb queues
> requested */
> + uint16_t num_ports; /* total num of evdev ports requested */
> + uint16_t num_ldb_ports; /* total num of ldb ports requested */
> + uint16_t num_dir_ports; /* total num of dir ports requested */
> + bool is_vdev;
> + bool umwait_allowed;
> + bool global_dequeue_wait; /* Not using per dequeue wait if true */
> + bool defer_sched;
> + unsigned int num_atm_inflights_per_queue;
> + enum dlb_cq_poll_modes poll_mode;
> + uint8_t revision;
> + bool configured;
> +};
> +
> +/* End Eventdev related defines and structs */
> +
> +/* externs */
> +
> +extern struct dlb_dev *dlb_probe(struct rte_pci_device *pdev);
This extern isn't needed, the function is later declared in pf/dlb_main.h
> +extern struct process_local_port_data
> dlb_port[][NUM_DLB_PORT_TYPES];
> +
> +/* Forwards for non-inlined functions */
> +
> +void dlb_free_qe_mem(struct dlb_port *port);
> +
> +int dlb_init_qe_mem(struct dlb_port *port, char *mz_name);
> +
> +int dlb_init_send_qe(struct dlb_port *port, char *mz_name);
> +
> +int dlb_init_partial_qe(struct dlb_port *port, char *mz_name);
> +
> +int dlb_init_fwd_qe(struct dlb_port *port, char *mz_name);
> +
> +int dlb_init_complete_qe(struct dlb_port *port, char *mz_name);
> +
> +int dlb_init_noop_qe(struct dlb_port *port, char *mz_name);
dlb_init_qe_mem -> dlb_init_noop_qe are only used in dlb.c, and
don't need forward declarations (dlb_free_qe_mem() should be
defined 'static' though).
> +
> +int dlb_uninit(const char *name);
This function is never defined
> +
> +void dlb_eventdev_dump(struct rte_eventdev *dev, FILE *f);
> +
> +int dlb_xstats_init(struct dlb_eventdev *dlb);
> +
> +void dlb_xstats_uninit(struct dlb_eventdev *dlb);
> +
> +int dlb_eventdev_xstats_get(const struct rte_eventdev *dev,
> + enum rte_event_dev_xstats_mode mode,
> + uint8_t queue_port_id, const unsigned int ids[],
> + uint64_t values[], unsigned int n);
> +
> +int dlb_eventdev_xstats_get_names(const struct rte_eventdev *dev,
> + enum rte_event_dev_xstats_mode mode,
> + uint8_t queue_port_id,
> + struct rte_event_dev_xstats_name
> *xstat_names,
> + unsigned int *ids, unsigned int size);
> +
> +uint64_t dlb_eventdev_xstats_get_by_name(const struct rte_eventdev
> *dev,
> + const char *name, unsigned int *id);
> +
> +int dlb_eventdev_xstats_reset(struct rte_eventdev *dev,
> + enum rte_event_dev_xstats_mode mode,
> + int16_t queue_port_id,
> + const uint32_t ids[],
> + uint32_t nb_ids);
> +
> +int test_dlb_eventdev(void);
> +
> +int dlb_primary_eventdev_probe(struct rte_eventdev *dev,
> + const char *name,
> + struct dlb_devargs *dlb_args,
> + bool is_vdev);
> +
> +int dlb_secondary_eventdev_probe(struct rte_eventdev *dev,
> + const char *name,
> + bool is_vdev);
> +uint32_t dlb_get_queue_depth(struct dlb_eventdev *dlb,
> + struct dlb_eventdev_queue *queue);
> +
> +int set_numa_node(const char *key __rte_unused, const char *value,
> + void *opaque);
> +
> +int set_dir_ports(const char *key __rte_unused,
> + const char *value __rte_unused,
> + void *opaque __rte_unused);
> +
> +int set_dir_queues(const char *key __rte_unused,
> + const char *value __rte_unused,
> + void *opaque __rte_unused);
The set_dir_* functions are also undefined
> +
> +int set_max_num_events(const char *key __rte_unused, const char
> *value,
> + void *opaque);
> +
> +int set_num_dir_credits(const char *key __rte_unused, const char *value,
> + void *opaque);
> +
> +void dlb_drain(struct rte_eventdev *dev);
Only used in dlb.c, no need for a forward declaration -- should be static
instead.
> +
> +void dlb_entry_points_init(struct rte_eventdev *dev);
> +
> +int dlb_parse_params(const char *params,
> + const char *name,
> + struct dlb_devargs *dlb_args);
> +
> +int dlb_string_to_int(int *result, const char *str);
> +
> +#endif /* _DLB_PRIV_H_ */
> diff --git a/drivers/event/dlb/rte_pmd_dlb.h
> b/drivers/event/dlb/rte_pmd_dlb.h
This file should be introduced in the commit "event/dlb: add PMD's token pop
public interface"
Thanks,
Gage
