* [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported
@ 2015-04-08 20:58 Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 1/4] Rename of device types to be generic device names Keith Wiles
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Keith Wiles @ 2015-04-08 20:58 UTC (permalink / raw)
To: dev
Hi All,
Here is a set of RFC patches to update DPDK to support a generic set of
devices. The patches that follow create two files eal_common_device.h
and rte_common_device.c and then a few items in rte_ethdev.[ch] were moved
to cleanup those files by moving the device generic values and functions
into a common set of device generic files.
In the rte_common_device.h file are a couple of macros to abstract a few
common items from rte_ethdev.h which are not ethernet specific. These
generic device specific structure members are place into macros to be
placed at the top of each new device type crypto, hardware offload, dpi,
compression and possible others to be defined later.
Note: could have used nested structures, but required a larger change set.
I did not pull the Rx/Tx Routines into a common function, but it could be
done if everyone agrees. It does not mean every device will use a common
Rx/Tx routine. Without pulling Rx/Tx routines into a common file allows
the device writer to have similar or different set of routines.
These patches do not contain any new devices as these are being work on
today.
More cleanup of ethdev could be done to remove NIC specific features not
supported in all devices and someone needs to do that cleanup IMO.
The code is untested and I wanted to get something our for others to poke at
today and more could be pulled out of ethdev as well. I/We will be looking
at testing the code as we get more completed.
I have not finished up the crypto APIs yet, but I am planning to work on
those additions today. The crypto code we are using is the Quick Assist code
found on 01.org, but we need to update the code to be move DPDK friendly.
The QAT code does have a modified like API similar to Linux Kernel crypto API
and I want to review that code first.
Regards,
++Keith
Keith Wiles (4):
Rename of device types to be generic device names.
Add the new common device header and C file.
Update files to build new device generic common files and headers.
Update the rte_ethdev.[ch] files for the new device generic changes.
app/test/test_link_bonding.c | 10 +-
app/test/virtual_pmd.c | 4 +-
examples/link_status_interrupt/main.c | 6 +-
lib/librte_eal/bsdapp/eal/Makefile | 1 +
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/eal_common_device.c | 185 +++++++
lib/librte_eal/common/include/rte_common_device.h | 617 ++++++++++++++++++++++
lib/librte_eal/common/include/rte_log.h | 1 +
lib/librte_eal/linuxapp/eal/Makefile | 1 +
lib/librte_ether/rte_ethdev.c | 290 ++--------
lib/librte_ether/rte_ethdev.h | 225 +++-----
lib/librte_pmd_af_packet/rte_eth_af_packet.c | 2 +-
lib/librte_pmd_bond/rte_eth_bond_api.c | 6 +-
lib/librte_pmd_bond/rte_eth_bond_pmd.c | 12 +-
lib/librte_pmd_bond/rte_eth_bond_private.h | 2 +-
lib/librte_pmd_e1000/em_ethdev.c | 8 +-
lib/librte_pmd_e1000/em_rxtx.c | 4 +-
lib/librte_pmd_e1000/igb_ethdev.c | 2 +-
lib/librte_pmd_i40e/i40e_ethdev.c | 4 +-
lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 2 +-
lib/librte_pmd_mlx4/mlx4.c | 2 +-
lib/librte_pmd_null/rte_eth_null.c | 2 +-
lib/librte_pmd_pcap/rte_eth_pcap.c | 2 +-
lib/librte_pmd_ring/rte_eth_ring.c | 2 +-
lib/librte_pmd_virtio/virtio_ethdev.c | 2 +-
lib/librte_pmd_xenvirt/rte_eth_xenvirt.c | 2 +-
26 files changed, 969 insertions(+), 426 deletions(-)
create mode 100644 lib/librte_eal/common/eal_common_device.c
create mode 100644 lib/librte_eal/common/include/rte_common_device.h
--
2.3.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [RFC PATCH 1/4] Rename of device types to be generic device names.
2015-04-08 20:58 [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Keith Wiles
@ 2015-04-08 20:58 ` Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file Keith Wiles
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Keith Wiles @ 2015-04-08 20:58 UTC (permalink / raw)
To: dev
Rename the RTE_ETH_PCI, VIRTUAL, ... to be RTE_DEV_PCI, ... names.
Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
app/test/test_link_bonding.c | 10 +++++-----
app/test/virtual_pmd.c | 4 ++--
examples/link_status_interrupt/main.c | 6 +++---
lib/librte_pmd_af_packet/rte_eth_af_packet.c | 2 +-
lib/librte_pmd_bond/rte_eth_bond_api.c | 6 +++---
lib/librte_pmd_bond/rte_eth_bond_pmd.c | 12 ++++++------
lib/librte_pmd_bond/rte_eth_bond_private.h | 2 +-
lib/librte_pmd_e1000/em_ethdev.c | 8 ++++----
lib/librte_pmd_e1000/em_rxtx.c | 4 ++--
lib/librte_pmd_e1000/igb_ethdev.c | 2 +-
lib/librte_pmd_i40e/i40e_ethdev.c | 4 ++--
lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 2 +-
lib/librte_pmd_mlx4/mlx4.c | 2 +-
lib/librte_pmd_null/rte_eth_null.c | 2 +-
lib/librte_pmd_pcap/rte_eth_pcap.c | 2 +-
lib/librte_pmd_ring/rte_eth_ring.c | 2 +-
lib/librte_pmd_virtio/virtio_ethdev.c | 2 +-
lib/librte_pmd_xenvirt/rte_eth_xenvirt.c | 2 +-
18 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 8c24314..1e1bc01 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -1179,7 +1179,7 @@ int test_lsc_interrupt_count;
static void
test_bonding_lsc_event_callback(uint8_t port_id __rte_unused,
- enum rte_eth_event_type type __rte_unused, void *param __rte_unused)
+ enum rte_dev_event_type type __rte_unused, void *param __rte_unused)
{
pthread_mutex_lock(&mutex);
test_lsc_interrupt_count++;
@@ -1231,7 +1231,7 @@ test_status_interrupt(void)
/* register link status change interrupt callback */
rte_eth_dev_callback_register(test_params->bonded_port_id,
- RTE_ETH_EVENT_INTR_LSC, test_bonding_lsc_event_callback,
+ RTE_DEV_EVENT_INTR_LSC, test_bonding_lsc_event_callback,
&test_params->bonded_port_id);
slave_count = rte_eth_bond_active_slaves_get(test_params->bonded_port_id,
@@ -1298,7 +1298,7 @@ test_status_interrupt(void)
/* unregister lsc callback before exiting */
rte_eth_dev_callback_unregister(test_params->bonded_port_id,
- RTE_ETH_EVENT_INTR_LSC, test_bonding_lsc_event_callback,
+ RTE_DEV_EVENT_INTR_LSC, test_bonding_lsc_event_callback,
&test_params->bonded_port_id);
/* Clean up and remove slaves from bonded device */
@@ -2031,7 +2031,7 @@ test_roundrobin_verfiy_polling_slave_link_status_change(void)
/* Register link status change interrupt callback */
rte_eth_dev_callback_register(test_params->bonded_port_id,
- RTE_ETH_EVENT_INTR_LSC, test_bonding_lsc_event_callback,
+ RTE_DEV_EVENT_INTR_LSC, test_bonding_lsc_event_callback,
&test_params->bonded_port_id);
/* link status change callback for first slave link up */
@@ -2059,7 +2059,7 @@ test_roundrobin_verfiy_polling_slave_link_status_change(void)
/* Un-Register link status change interrupt callback */
rte_eth_dev_callback_unregister(test_params->bonded_port_id,
- RTE_ETH_EVENT_INTR_LSC, test_bonding_lsc_event_callback,
+ RTE_DEV_EVENT_INTR_LSC, test_bonding_lsc_event_callback,
&test_params->bonded_port_id);
diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c
index f163562..6143bfb 100644
--- a/app/test/virtual_pmd.c
+++ b/app/test/virtual_pmd.c
@@ -478,7 +478,7 @@ virtual_ethdev_simulate_link_status_interrupt(uint8_t port_id,
vrtl_eth_dev->data->dev_link.link_status = link_status;
- _rte_eth_dev_callback_process(vrtl_eth_dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(vrtl_eth_dev, RTE_DEV_EVENT_INTR_LSC);
}
int
@@ -580,7 +580,7 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
goto err;
/* reserve an ethdev entry */
- eth_dev = rte_eth_dev_allocate(name, RTE_ETH_DEV_PCI);
+ eth_dev = rte_eth_dev_allocate(name, RTE_DEV_PCI);
if (eth_dev == NULL)
goto err;
diff --git a/examples/link_status_interrupt/main.c b/examples/link_status_interrupt/main.c
index e6fb218..9e31028 100644
--- a/examples/link_status_interrupt/main.c
+++ b/examples/link_status_interrupt/main.c
@@ -516,14 +516,14 @@ lsi_parse_args(int argc, char **argv)
* void.
*/
static void
-lsi_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+lsi_event_callback(uint8_t port_id, enum rte_dev_event_type type, void *param)
{
struct rte_eth_link link;
RTE_SET_USED(param);
printf("\n\nIn registered callback...\n");
- printf("Event type: %s\n", type == RTE_ETH_EVENT_INTR_LSC ? "LSC interrupt" : "unknown event");
+ printf("Event type: %s\n", type == RTE_DEV_EVENT_INTR_LSC ? "LSC interrupt" : "unknown event");
rte_eth_link_get_nowait(port_id, &link);
if (link.link_status) {
printf("Port %d Link Up - speed %u Mbps - %s\n\n",
@@ -703,7 +703,7 @@ main(int argc, char **argv)
* be registered will never be called.
*/
rte_eth_dev_callback_register(portid,
- RTE_ETH_EVENT_INTR_LSC, lsi_event_callback, NULL);
+ RTE_DEV_EVENT_INTR_LSC, lsi_event_callback, NULL);
rte_eth_macaddr_get(portid,
&lsi_ports_eth_addr[portid]);
diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
index 2ac50ba..48059fd 100644
--- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
+++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
@@ -650,7 +650,7 @@ rte_pmd_init_internals(const char *name,
}
/* reserve an ethdev entry */
- *eth_dev = rte_eth_dev_allocate(name, RTE_ETH_DEV_VIRTUAL);
+ *eth_dev = rte_eth_dev_allocate(name, RTE_DEV_VIRTUAL);
if (*eth_dev == NULL)
goto error;
diff --git a/lib/librte_pmd_bond/rte_eth_bond_api.c b/lib/librte_pmd_bond/rte_eth_bond_api.c
index 13f3941..8318d09 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_api.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_api.c
@@ -254,7 +254,7 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
}
/* reserve an ethdev entry */
- eth_dev = rte_eth_dev_allocate(name, RTE_ETH_DEV_VIRTUAL);
+ eth_dev = rte_eth_dev_allocate(name, RTE_DEV_VIRTUAL);
if (eth_dev == NULL) {
RTE_BOND_LOG(ERR, "Unable to allocate rte_eth_dev");
goto err;
@@ -422,7 +422,7 @@ __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
/* Register link status change callback with bonded device pointer as
* argument*/
- rte_eth_dev_callback_register(slave_port_id, RTE_ETH_EVENT_INTR_LSC,
+ rte_eth_dev_callback_register(slave_port_id, RTE_DEV_EVENT_INTR_LSC,
bond_ethdev_lsc_event_callback, &bonded_eth_dev->data->port_id);
/* If bonded device is started then we can add the slave to our active
@@ -498,7 +498,7 @@ __eth_bond_slave_remove_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
/* Un-register link status change callback with bonded device pointer as
* argument*/
- rte_eth_dev_callback_unregister(slave_port_id, RTE_ETH_EVENT_INTR_LSC,
+ rte_eth_dev_callback_unregister(slave_port_id, RTE_DEV_EVENT_INTR_LSC,
bond_ethdev_lsc_event_callback,
&rte_eth_devices[bonded_port_id].data->port_id);
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index c937e6b..14f90cf 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -1681,7 +1681,7 @@ bond_ethdev_slave_link_status_change_monitor(void *cb_arg)
slave_ethdev->data->dev_link.link_status;
bond_ethdev_lsc_event_callback(internals->slaves[i].port_id,
- RTE_ETH_EVENT_INTR_LSC,
+ RTE_DEV_EVENT_INTR_LSC,
&bonded_ethdev->data->port_id);
}
}
@@ -1829,11 +1829,11 @@ bond_ethdev_delayed_lsc_propagation(void *arg)
return;
_rte_eth_dev_callback_process((struct rte_eth_dev *)arg,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_DEV_EVENT_INTR_LSC);
}
void
-bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
+bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_dev_event_type type,
void *param)
{
struct rte_eth_dev *bonded_eth_dev, *slave_eth_dev;
@@ -1844,7 +1844,7 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
uint8_t active_pos;
uint8_t lsc_flag = 0;
- if (type != RTE_ETH_EVENT_INTR_LSC || param == NULL)
+ if (type != RTE_DEV_EVENT_INTR_LSC || param == NULL)
return;
bonded_eth_dev = &rte_eth_devices[*(uint8_t *)param];
@@ -1940,7 +1940,7 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
(void *)bonded_eth_dev);
else
_rte_eth_dev_callback_process(bonded_eth_dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_DEV_EVENT_INTR_LSC);
} else {
if (internals->link_down_delay_ms > 0)
@@ -1949,7 +1949,7 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
(void *)bonded_eth_dev);
else
_rte_eth_dev_callback_process(bonded_eth_dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_DEV_EVENT_INTR_LSC);
}
}
}
diff --git a/lib/librte_pmd_bond/rte_eth_bond_private.h b/lib/librte_pmd_bond/rte_eth_bond_private.h
index 45e5c65..f285518 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_private.h
+++ b/lib/librte_pmd_bond/rte_eth_bond_private.h
@@ -244,7 +244,7 @@ bond_ethdev_primary_set(struct bond_dev_private *internals,
uint8_t slave_port_id);
void
-bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
+bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_dev_event_type type,
void *param);
int
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 76f45c9..ffdb888 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -226,8 +226,8 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
eth_dev->dev_ops = ð_em_ops;
- eth_dev->rx_pkt_burst = (eth_rx_burst_t)ð_em_recv_pkts;
- eth_dev->tx_pkt_burst = (eth_tx_burst_t)ð_em_xmit_pkts;
+ eth_dev->rx_pkt_burst = (dev_rx_burst_t)ð_em_recv_pkts;
+ eth_dev->tx_pkt_burst = (dev_tx_burst_t)ð_em_xmit_pkts;
/* for secondary processes, we don't initialise any further as primary
* has already done this work. Only check we don't need a different
@@ -235,7 +235,7 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
if (rte_eal_process_type() != RTE_PROC_PRIMARY){
if (eth_dev->data->scattered_rx)
eth_dev->rx_pkt_burst =
- (eth_rx_burst_t)ð_em_recv_scattered_pkts;
+ (dev_rx_burst_t)ð_em_recv_scattered_pkts;
return 0;
}
@@ -1340,7 +1340,7 @@ eth_em_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
eth_em_interrupt_get_status(dev);
eth_em_interrupt_action(dev);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_DEV_EVENT_INTR_LSC);
}
static int
diff --git a/lib/librte_pmd_e1000/em_rxtx.c b/lib/librte_pmd_e1000/em_rxtx.c
index 8e20920..09dae30 100644
--- a/lib/librte_pmd_e1000/em_rxtx.c
+++ b/lib/librte_pmd_e1000/em_rxtx.c
@@ -1663,7 +1663,7 @@ eth_em_rx_init(struct rte_eth_dev *dev)
if (hw->mac.type == e1000_82573)
E1000_WRITE_REG(hw, E1000_RDTR, 0x20);
- dev->rx_pkt_burst = (eth_rx_burst_t)eth_em_recv_pkts;
+ dev->rx_pkt_burst = (dev_rx_burst_t)eth_em_recv_pkts;
/* Determine RX bufsize. */
rctl_bsize = EM_MAX_BUF_SIZE;
@@ -1732,7 +1732,7 @@ eth_em_rx_init(struct rte_eth_dev *dev)
if (!dev->data->scattered_rx)
PMD_INIT_LOG(DEBUG, "forcing scatter mode");
dev->rx_pkt_burst =
- (eth_rx_burst_t)eth_em_recv_scattered_pkts;
+ (dev_rx_burst_t)eth_em_recv_scattered_pkts;
dev->data->scattered_rx = 1;
}
}
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index b3892a5..736035d 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -1935,7 +1935,7 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev)
E1000_WRITE_REG(hw, E1000_TCTL, tctl);
E1000_WRITE_REG(hw, E1000_RCTL, rctl);
E1000_WRITE_FLUSH(hw);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_DEV_EVENT_INTR_LSC);
}
return 0;
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 6b8f96e..6e765f6 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -3947,7 +3947,7 @@ i40e_dev_interrupt_delayed_handler(void *param)
/* handle the link up interrupt in an alarm callback */
i40e_dev_link_update(dev, 0);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_DEV_EVENT_INTR_LSC);
i40e_pf_enable_irq0(hw);
rte_intr_enable(&(dev->pci_dev->intr_handle));
@@ -4031,7 +4031,7 @@ i40e_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
return;
else
_rte_eth_dev_callback_process(dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_DEV_EVENT_INTR_LSC);
}
done:
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 5caee22..21e91c8 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -2406,7 +2406,7 @@ ixgbe_dev_interrupt_delayed_handler(void *param)
ixgbe_dev_link_update(dev, 0);
intr->flags &= ~IXGBE_FLAG_NEED_LINK_UPDATE;
ixgbe_dev_link_status_print(dev);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_DEV_EVENT_INTR_LSC);
}
PMD_DRV_LOG(DEBUG, "enable intr in delayed handler S[%08x]", eicr);
diff --git a/lib/librte_pmd_mlx4/mlx4.c b/lib/librte_pmd_mlx4/mlx4.c
index fa749f4..7a3ccae 100644
--- a/lib/librte_pmd_mlx4/mlx4.c
+++ b/lib/librte_pmd_mlx4/mlx4.c
@@ -4578,7 +4578,7 @@ mlx4_pci_devinit(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
snprintf(name, sizeof(name), "%s port %u",
ibv_get_device_name(ibv_dev), port);
- eth_dev = rte_eth_dev_allocate(name, RTE_ETH_DEV_PCI);
+ eth_dev = rte_eth_dev_allocate(name, RTE_DEV_PCI);
}
if (eth_dev == NULL) {
ERROR("can not allocate rte ethdev");
diff --git a/lib/librte_pmd_null/rte_eth_null.c b/lib/librte_pmd_null/rte_eth_null.c
index 0e18502..61382d2 100644
--- a/lib/librte_pmd_null/rte_eth_null.c
+++ b/lib/librte_pmd_null/rte_eth_null.c
@@ -412,7 +412,7 @@ eth_dev_null_create(const char *name,
goto error;
/* reserve an ethdev entry */
- eth_dev = rte_eth_dev_allocate(name, RTE_ETH_DEV_VIRTUAL);
+ eth_dev = rte_eth_dev_allocate(name, RTE_DEV_VIRTUAL);
if (eth_dev == NULL)
goto error;
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
index 204ae68..e295510 100644
--- a/lib/librte_pmd_pcap/rte_eth_pcap.c
+++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
@@ -716,7 +716,7 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
goto error;
/* reserve an ethdev entry */
- *eth_dev = rte_eth_dev_allocate(name, RTE_ETH_DEV_VIRTUAL);
+ *eth_dev = rte_eth_dev_allocate(name, RTE_DEV_VIRTUAL);
if (*eth_dev == NULL)
goto error;
diff --git a/lib/librte_pmd_ring/rte_eth_ring.c b/lib/librte_pmd_ring/rte_eth_ring.c
index 1e66d4e..914792a 100644
--- a/lib/librte_pmd_ring/rte_eth_ring.c
+++ b/lib/librte_pmd_ring/rte_eth_ring.c
@@ -297,7 +297,7 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
goto error;
/* reserve an ethdev entry */
- eth_dev = rte_eth_dev_allocate(name, RTE_ETH_DEV_VIRTUAL);
+ eth_dev = rte_eth_dev_allocate(name, RTE_DEV_VIRTUAL);
if (eth_dev == NULL)
goto error;
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 7b83d9b..a71dbf6 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -1092,7 +1092,7 @@ virtio_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
if (isr & VIRTIO_PCI_ISR_CONFIG) {
if (virtio_dev_link_update(dev, 0) == 0)
_rte_eth_dev_callback_process(dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_DEV_EVENT_INTR_LSC);
}
}
diff --git a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
index bc403d6..a63e041 100644
--- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
+++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
@@ -648,7 +648,7 @@ eth_dev_xenvirt_create(const char *name, const char *params,
goto err;
/* reserve an ethdev entry */
- eth_dev = rte_eth_dev_allocate(name, RTE_ETH_DEV_VIRTUAL);
+ eth_dev = rte_eth_dev_allocate(name, RTE_DEV_VIRTUAL);
if (eth_dev == NULL)
goto err;
--
2.3.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file.
2015-04-08 20:58 [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 1/4] Rename of device types to be generic device names Keith Wiles
@ 2015-04-08 20:58 ` Keith Wiles
2015-04-09 11:53 ` Neil Horman
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 3/4] Update files to build new device generic common files and headers Keith Wiles
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Keith Wiles @ 2015-04-08 20:58 UTC (permalink / raw)
To: dev
Move a number of device specific define, structures and functions
into a generic device base set of files for all device not just Ethernet.
Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
lib/librte_eal/common/eal_common_device.c | 185 +++++++
lib/librte_eal/common/include/rte_common_device.h | 617 ++++++++++++++++++++++
2 files changed, 802 insertions(+)
create mode 100644 lib/librte_eal/common/eal_common_device.c
create mode 100644 lib/librte_eal/common/include/rte_common_device.h
diff --git a/lib/librte_eal/common/eal_common_device.c b/lib/librte_eal/common/eal_common_device.c
new file mode 100644
index 0000000..a9ef925
--- /dev/null
+++ b/lib/librte_eal/common/eal_common_device.c
@@ -0,0 +1,185 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ * Copyright(c) 2014 6WIND S.A.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <sys/queue.h>
+
+#include <rte_dev.h>
+#include <rte_devargs.h>
+#include <rte_debug.h>
+#include <rte_devargs.h>
+#include <rte_log.h>
+#include <rte_malloc.h>
+#include <rte_errno.h>
+
+#include "rte_common_device.h"
+
+void *
+rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
+ void * fn, void *user_param)
+{
+ struct rte_dev_rxtx_callback *cb;
+
+ cb = rte_zmalloc(NULL, sizeof(*cb), 0);
+
+ if (cb == NULL) {
+ rte_errno = ENOMEM;
+ return NULL;
+ }
+
+ cb->fn.vp = fn;
+ cb->param = user_param;
+ cb->next = *cbp;
+ *cbp = cb;
+ return cb;
+}
+
+int
+rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
+ struct rte_dev_rxtx_callback *user_cb)
+{
+ struct rte_dev_rxtx_callback *cb = *cbp;
+ struct rte_dev_rxtx_callback *prev_cb;
+
+ /* Reset head pointer and remove user cb if first in the list. */
+ if (cb == user_cb) {
+ *cbp = user_cb->next;
+ return 0;
+ }
+
+ /* Remove the user cb from the callback list. */
+ do {
+ prev_cb = cb;
+ cb = cb->next;
+
+ if (cb == user_cb) {
+ prev_cb->next = user_cb->next;
+ return 0;
+ }
+ } while (cb != NULL);
+
+ /* Callback wasn't found. */
+ return (-EINVAL);
+}
+
+int
+rte_dev_callback_register(struct rte_dev_cb_list * cb_list,
+ rte_spinlock_t * lock,
+ enum rte_dev_event_type event,
+ rte_dev_cb_fn cb_fn, void *cb_arg)
+{
+ struct rte_dev_callback *cb;
+
+ rte_spinlock_lock(lock);
+
+ TAILQ_FOREACH(cb, cb_list, next) {
+ if (cb->cb_fn == cb_fn &&
+ cb->cb_arg == cb_arg &&
+ cb->event == event) {
+ break;
+ }
+ }
+
+ /* create a new callback. */
+ if (cb == NULL && (cb = rte_zmalloc("INTR_USER_CALLBACK",
+ sizeof(struct rte_dev_callback), 0)) != NULL) {
+ cb->cb_fn = cb_fn;
+ cb->cb_arg = cb_arg;
+ cb->event = event;
+ TAILQ_INSERT_TAIL(cb_list, cb, next);
+ }
+
+ rte_spinlock_unlock(lock);
+ return ((cb == NULL) ? -ENOMEM : 0);
+}
+
+int
+rte_dev_callback_unregister(struct rte_dev_cb_list * cb_list,
+ rte_spinlock_t * lock,
+ enum rte_dev_event_type event,
+ rte_dev_cb_fn cb_fn, void *cb_arg)
+{
+ int ret;
+ struct rte_dev_callback *cb, *next;
+
+ rte_spinlock_lock(lock);
+
+ ret = 0;
+ for (cb = TAILQ_FIRST(cb_list); cb != NULL; cb = next) {
+
+ next = TAILQ_NEXT(cb, next);
+
+ if (cb->cb_fn != cb_fn || cb->event != event ||
+ (cb->cb_arg != (void *)-1 &&
+ cb->cb_arg != cb_arg))
+ continue;
+
+ /*
+ * if this callback is not executing right now,
+ * then remove it.
+ */
+ if (cb->active == 0) {
+ TAILQ_REMOVE(cb_list, cb, next);
+ rte_free(cb);
+ } else {
+ ret = -EAGAIN;
+ }
+ }
+
+ rte_spinlock_unlock(lock);
+ return (ret);
+}
+
+void
+rte_dev_callback_process(struct rte_dev_cb_list * cb_list, uint8_t port_id,
+ enum rte_dev_event_type event, rte_spinlock_t *lock)
+{
+ struct rte_dev_callback *cb;
+ struct rte_dev_callback dev_cb;
+
+ rte_spinlock_lock(lock);
+ TAILQ_FOREACH(cb, cb_list, next) {
+ if (cb->cb_fn == NULL || cb->event != event)
+ continue;
+ dev_cb = *cb;
+ cb->active = 1;
+ rte_spinlock_unlock(lock);
+ dev_cb.cb_fn(port_id, dev_cb.event, dev_cb.cb_arg);
+ rte_spinlock_lock(lock);
+ cb->active = 0;
+ }
+ rte_spinlock_unlock(lock);
+}
diff --git a/lib/librte_eal/common/include/rte_common_device.h b/lib/librte_eal/common/include/rte_common_device.h
new file mode 100644
index 0000000..5baefb6
--- /dev/null
+++ b/lib/librte_eal/common/include/rte_common_device.h
@@ -0,0 +1,617 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHPDF IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_COMMON_DEVICE_H_
+#define _RTE_COMMON_DEVICE_H_
+
+/**
+ * @file
+ *
+ * Common Device Helpers in RTE
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <rte_spinlock.h>
+
+/* Macros for checking for restricting functions to primary instance only */
+#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+ PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+ return (retval); \
+ } \
+} while(0)
+#define PROC_PRIMARY_OR_RET() do { \
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+ PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+ return; \
+ } \
+} while(0)
+
+/* Macros to check for invalid function pointers in dev_ops structure */
+#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
+ if ((func) == NULL) { \
+ PMD_DEBUG_TRACE("Function not supported\n"); \
+ return (retval); \
+ } \
+} while(0)
+#define FUNC_PTR_OR_RET(func) do { \
+ if ((func) == NULL) { \
+ PMD_DEBUG_TRACE("Function not supported\n"); \
+ return; \
+ } \
+} while(0)
+
+enum {
+ DEV_DETACHED = 0,
+ DEV_ATTACHED
+};
+
+/*
+ * The device type
+ */
+enum rte_dev_type {
+ RTE_DEV_UNKNOWN, /**< unknown device type */
+ RTE_DEV_PCI,
+ /**< Physical function and Virtual function of PCI devices */
+ RTE_DEV_VIRTUAL, /**< non hardware device */
+ RTE_DEV_MAX /**< max value of this enum */
+};
+
+/**
+ * The device event type for interrupt, and maybe others in the future.
+ */
+enum rte_dev_event_type {
+ RTE_DEV_EVENT_UNKNOWN, /**< unknown event type */
+ RTE_DEV_EVENT_INTR_LSC, /**< lsc interrupt event */
+ RTE_DEV_EVENT_MAX /**< max value of this enum */
+};
+
+struct rte_dev_callback;
+
+/** @internal Structure to keep track of registered callback */
+TAILQ_HEAD(rte_dev_cb_list, rte_dev_callback);
+
+struct rte_mbuf;
+
+typedef uint16_t (*dev_rx_burst_t)(void *rxq,
+ struct rte_mbuf **rx_pkts,
+ uint16_t nb_pkts);
+/**< @internal Retrieve input packets from a receive queue of a device. */
+
+typedef uint16_t (*dev_tx_burst_t)(void *txq,
+ struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
+/**< @internal Send output packets on a transmit queue of a device. */
+
+typedef void (*rte_dev_cb_fn)(uint8_t port_id, \
+ enum rte_dev_event_type event, void *cb_arg);
+/**< user application callback to be registered for interrupts */
+
+#define RTE_DEV_NAME_MAX_LEN (32)
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_dev'
+ */
+#define RTE_COMMON_DEV(_t) \
+ dev_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */ \
+ dev_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */ \
+ struct rte_##_t##dev_data *data; /**< Pointer to device data */ \
+ struct rte_##_t##global *gbl_data; /**< Pointer to device global data */ \
+ const struct _t##driver *driver; /**< Driver for this device */ \
+ struct _t##dev_ops *dev_ops; /**< Functions exported by PMD */ \
+ struct rte_pci_device *pci_dev; /**< PCI info. supplied by probing */ \
+ /** User application callback for interrupts if present */ \
+ struct rte_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_dev_rxtx_callback **post_rx_burst_cbs; \
+ /** \
+ * User-supplied functions called from tx_burst to pre-process \
+ * received packets before passing them to the driver for transmission. \
+ */ \
+ struct rte_dev_rxtx_callback **pre_tx_burst_cbs; \
+ enum rte_dev_type dev_type; /**< Flag indicating the device type */ \
+ uint8_t attached /**< Flag indicating the port is attached */
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_dev_data'
+ */
+#define RTE_COMMON_DEV_DATA \
+ char name[RTE_DEV_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. */ \
+ \
+ uint16_t mtu; /**< Maximum Transmission Unit. */ \
+ uint8_t unit_id; /**< Unit/Port ID for this instance */ \
+ uint8_t _pad0; /* alignment filler */ \
+ \
+ /* 64bit alignment starts here */ \
+ void *dev_private; /**< PMD-specific private data */ \
+ uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */ \
+ uint32_t min_rx_buf_size; /**< Common rx buffer size handled by all queues */ \
+ uint32_t _pad1 /**< Align to a 64bit boundary */
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_dev_info'
+ */
+#define RTE_COMMON_DEV_INFO \
+ struct rte_pci_device *pci_dev; /**< Device PCI information. */ \
+ const char *driver_name; /**< Device Driver name. */ \
+ unsigned int if_index; /**< Index to bound host interface, or 0 if none. */ \
+ /* Use if_indextoname() to translate into an interface name. */ \
+ uint32_t _pad0
+
+#define port_id unit_id
+
+/**
+ * @internal
+ * Common set of members for all devices included at the top of 'struct rte_xyz_global'
+ */
+#define RTE_COMMON_GLOBAL(_t) \
+ struct rte_##_t##dev * devs;/**< Device information array */ \
+ struct rte_##_t##dev_data * data; /**< Device private data */ \
+ uint8_t nb_ports; /**< Number of ports/units found */ \
+ uint8_t max_ports; /**< Max number of ports or units */ \
+ uint16_t dflt_mtu; /**< Default MTU if needed by device */\
+ uint16_t dev_size; /**< Internal size of device struct */ \
+ uint16_t data_size; /**< Internal size of data structure */\
+ const char * mz_dev_data /**< String constant for device data */
+
+/**
+ * @internal
+ * Common overlay type structures with all devices.
+ */
+struct rte_dev_info {
+ RTE_COMMON_DEV_INFO;
+};
+
+/**
+ * @internal
+ * The generic data structure associated with each 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_dev {
+ RTE_COMMON_DEV();
+};
+
+/**
+ * @internal
+ * The data part, with no function pointers, associated with each device.
+ *
+ * This structure is safe to place in shared memory to be common among different
+ * processes in a multi-process configuration.
+ */
+struct rte_dev_data {
+ RTE_COMMON_DEV_DATA;
+};
+
+/**
+ * @internal
+ * This structure is attempting to mirror the rte_xyz_global structures.
+ *
+ * Be careful as this structure is over layered on top of the xyzdev structures.
+ */
+struct rte_dev_global {
+ RTE_COMMON_GLOBAL();
+};
+
+/**
+ * Get the rte_pkt_dev structure device pointer for the device.
+ *
+ * @param gbl pointer to device specific global structure.
+ * @param port_id Port ID value to select the device structure.
+ *
+ * @return
+ * - The rte_pkt_dev structure pointer for the given port ID.
+ */
+static inline void *
+rte_get_dev(void * _gbl, uint8_t port_id)
+{
+ struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+
+ return (void *)((uint8_t *)gbl->devs + (port_id * gbl->dev_size));
+}
+
+/**
+ * Get the number of device ports in the system.
+ *
+ * @param gbl
+ * The pointer to the current global data structure for xyzdev.
+ *
+ * @return
+ * - Number of ports found in the system.
+ */
+static inline uint8_t
+rte_pkt_dev_count(void * _gbl)
+{
+ struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+ return gbl->nb_ports;
+}
+
+/**
+ * Validate if the port number is valid
+ *
+ * @param gbl The pointer to the current global data structure for xyzdev.
+ * @param port_id Port ID value to select the device.
+ *
+ * @return
+ * - Number of ports found in the system.
+ */
+static inline int
+rte_dev_is_valid_port(void * _gbl, uint8_t port_id)
+{
+ struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+ struct rte_dev * dev;
+
+ if (port_id >= gbl->nb_ports)
+ return 0;
+
+ dev = rte_get_dev(gbl, port_id);
+ if (dev->attached != DEV_ATTACHED)
+ return 0;
+ else
+ return 1;
+}
+
+/**
+ * Get the number of device ports in the system.
+ *
+ * @param gbl
+ * The pointer to the current global data structure for xyzdev.
+ *
+ * @return
+ * - Number of ports found in the system.
+ */
+static inline uint8_t
+rte_dev_count(void * _gbl)
+{
+ struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+ return gbl->nb_ports;
+}
+
+/**
+ * Get the rte_pkt_dev structure device pointer for the device by name.
+ *
+ * @param gbl pointer to device specific global structure.
+ * @param name Name of device.
+ *
+ * @return
+ * - The rte_dev structure pointer for the given name.
+ */
+static inline struct rte_dev *
+rte_get_named_dev(void * _gbl, const char *name)
+{
+ struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+ struct rte_dev *dev;
+ int i;
+
+ if (name == NULL)
+ return NULL;
+
+ for (i = 0; i < gbl->nb_ports; i++) {
+ dev = (struct rte_dev *)((uint8_t *)gbl->devs +
+ (i * gbl->dev_size));
+
+ if (strcmp(dev->data->name, name) == 0)
+ return dev;
+ }
+
+ return NULL;
+}
+
+/**
+ * Get the rte_pkt_dev structure device pointer for the PCI name.
+ *
+ * @param gbl pointer to device specific global structure.
+ * @param name Name of PCI device.
+ *
+ * @return
+ * - The rte_dev structure pointer for the given PCIname.
+ */
+static inline struct rte_dev *
+rte_get_pci_named_dev(void * _gbl, struct rte_pci_addr *pci_addr)
+{
+ struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+ struct rte_dev *dev;
+ int i;
+
+ if (pci_addr == NULL)
+ return NULL;
+
+ for (i = 0; i < gbl->nb_ports; i++) {
+ dev = (struct rte_dev *)((uint8_t *)gbl->devs +
+ (i * gbl->dev_size));
+
+ if (rte_eal_compare_pci_addr(&dev->pci_dev->addr, pci_addr) == 0)
+ return dev;
+ }
+
+ return NULL;
+}
+
+/*
+ * Return the NUMA socket to which a device is connected
+ *
+ * @param gbl
+ * The global structure pointer for a given xyzdev.
+ * @param port_id
+ * The port identifier of the device
+ * @return
+ * The NUMA socket id to which the device is connected or
+ * a default of zero if the socket could not be determined.
+ * -1 is returned is the port_id value is out of range.
+ */
+static inline int
+rte_dev_socket_id(void * _gbl, uint8_t port_id)
+{
+ struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
+ struct rte_dev * dev;
+
+ if (!rte_dev_is_valid_port(gbl, port_id))
+ return -1;
+
+ dev = rte_get_dev(gbl, port_id);
+
+ if ( dev->pci_dev )
+ return dev->pci_dev->numa_node;
+ else
+ return 0;
+}
+/**
+ * Function type used for RX packet processing packet a callback.
+ *
+ * The callback function is called on RX with a burst of packets that have
+ * been received on the given port and queue.
+ *
+ * @param port
+ * The Ethernet port on which RX is being performed.
+ * @param queue
+ * The queue on the Ethernet port which is being used to receive the packets.
+ * @param pkts
+ * The burst of packets that have just been received.
+ * @param nb_pkts
+ * The number of packets in the burst pointed to by "pkts".
+ * @param max_pkts
+ * The max number of packets that can be stored in the "pkts" array.
+ * @param user_param
+ * The arbitrary user parameter passed in by the application when the callback
+ * was originally configured.
+ * @return
+ * The number of packets returned to the user.
+ */
+typedef uint16_t (*rte_rx_callback_fn)(uint8_t port, uint16_t queue,
+ struct rte_mbuf *pkts[], uint16_t nb_pkts, uint16_t max_pkts,
+ void *user_param);
+
+/**
+ * Function type used for TX packet processing packet a callback.
+ *
+ * The callback function is called on TX with a burst of packets immediately
+ * before the packets are put onto the hardware queue for transmission.
+ *
+ * @param port
+ * The Ethernet port on which TX is being performed.
+ * @param queue
+ * The queue on the Ethernet port which is being used to transmit the packets.
+ * @param pkts
+ * The burst of packets that are about to be transmitted.
+ * @param nb_pkts
+ * The number of packets in the burst pointed to by "pkts".
+ * @param user_param
+ * The arbitrary user parameter passed in by the application when the callback
+ * was originally configured.
+ * @return
+ * The number of packets to be written to the NIC.
+ */
+typedef uint16_t (*rte_tx_callback_fn)(uint8_t port, uint16_t queue,
+ struct rte_mbuf *pkts[], uint16_t nb_pkts, void *user_param);
+
+/**
+ * @internal
+ * Union used to hold the callback function pointers for RX and TX callback.
+ */
+union rte_dev_callback_fn {
+ void * vp; /**< Allow for a generic function pointer to avoid casting in common routines */
+ rte_rx_callback_fn rx; /**< Ethernet or packet based callback function for RX packets */
+ rte_tx_callback_fn tx; /**< Ethernet or packet based callback function for TX packets */
+ /**< Add other device callback prototypes here */
+};
+
+/**
+ * @internal
+ * Structure used to hold information about a callback to be called for a
+ * queue on RX and TX.
+ */
+struct rte_dev_rxtx_callback {
+ struct rte_dev_rxtx_callback *next;
+ union rte_dev_callback_fn fn;
+ void *param;
+};
+
+/**
+ * The user application callback description.
+ *
+ * It contains callback address to be registered by user application,
+ * the pointer to the parameters for callback, and the event type.
+ */
+struct rte_dev_callback {
+ TAILQ_ENTRY(rte_dev_callback) next; /**< Callback list */
+ rte_dev_cb_fn cb_fn; /**< Callback address */
+ void *cb_arg; /**< Parameter for callback */
+ enum rte_dev_event_type event; /**< Interrupt event type */
+ uint32_t active; /**< Callback is executing */
+};
+
+/**
+ * Register a callback function for specific port id.
+ *
+ * @param cbp
+ * Pointer to the ret_dev_cb_list structure.
+ * @param lock
+ * Pointer to the rte_spinlock_t structure.
+ * @param event
+ * Event interested.
+ * @param cb_fn
+ * User supplied callback function to be called.
+ * @param cb_arg
+ * Pointer to the parameters for the registered callback.
+ *
+ * @return
+ * - On success, zero.
+ * - On failure, a negative value.
+ */
+int rte_dev_callback_register(struct rte_dev_cb_list * cbp, rte_spinlock_t * lock,
+ enum rte_dev_event_type event,
+ rte_dev_cb_fn cb_fn, void *cb_arg);
+
+/**
+ * Unregister a callback function for specific port id.
+ *
+ * @param cb_list
+ * Pointer to the ret_dev_cb_list structure.
+ * @param lock
+ * Pointer to the rte_spinlock_t structure.
+ * @param event
+ * Event interested.
+ * @param cb_fn
+ * User supplied callback function to be called.
+ * @param cb_arg
+ * Pointer to the parameters for the registered callback. -1 means to
+ * remove all for the same callback address and same event.
+ *
+ * @return
+ * - On success, zero.
+ * - On failure, a negative value.
+ */
+int rte_dev_callback_unregister(struct rte_dev_cb_list * cb_list, rte_spinlock_t * lock,
+ enum rte_dev_event_type event,
+ rte_dev_cb_fn cb_fn, void *cb_arg);
+
+/**
+ * @internal Executes all the user application registered callbacks for
+ * the specific device. It is for DPDK internal user only. User
+ * application should not call it directly.
+ *
+ * @param cb_list
+ * Pointer to struct rte_dev_cb_list.
+ * @param port_id
+ * Port_id or unit_id value.
+ * @param event
+ * Eth device interrupt event type.
+ * @param lock
+ * Pointer to the lock structure.
+ *
+ * @return
+ * void
+ */
+void rte_dev_callback_process(struct rte_dev_cb_list * cb_list, uint8_t port_id,
+ enum rte_dev_event_type event, rte_spinlock_t *lock);
+
+/**
+ * Add a callback to be called on packet RX or TX on a given port and queue.
+ *
+ * This API configures a function to be called for each burst of
+ * packets received on a given NIC port queue. The return value is a pointer
+ * that can be used to later remove the callback using
+ * rte_dev_remove_callback().
+ *
+ * @param cbp
+ * Pointer to a pointer of the ret_dev_cb_list structure.
+ * @param fn
+ * The callback function
+ * @param user_param
+ * A generic pointer parameter which will be passed to each invocation of the
+ * callback function on this port and queue.
+ *
+ * @return
+ * NULL on error.
+ * On success, a pointer value which can later be used to remove the callback.
+ */
+void *rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
+ void * fn, void *user_param);
+
+/**
+ * Remove an RX or TX packet callback from a given port and queue.
+ *
+ * This function is used to removed a callback that were added to a NIC port
+ * queue using rte_dev_add_callback().
+ *
+ * Note: the callback is removed from the callback list but it isn't freed
+ * since the it may still be in use. The memory for the callback can be
+ * subsequently freed back by the application by calling rte_free():
+ *
+ * - Immediately - if the port is stopped, or the user knows that no
+ * callback's are in flight e.g. if called from the thread doing RX/TX
+ * on that queue.
+ *
+ * - After a short delay - where the delay is sufficient to allow any
+ * in-flight callback's to complete.
+ *
+ * @param cbp
+ * Pointer to a pointer to use as the head of the list.
+ * @param user_cb
+ * User supplied callback created via rte_dev_add_callback().
+ *
+ * @return
+ * - 0: Success. Callback was removed.
+ * - -ENOTSUP: Callback support is not available.
+ * - -EINVAL: The port_id or the queue_id is out of range, or the callback
+ * is NULL or not found for the port/queue.
+ */
+int rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
+ struct rte_dev_rxtx_callback *user_cb);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_COMMON_DEVICE_H_ */
--
2.3.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [RFC PATCH 3/4] Update files to build new device generic common files and headers.
2015-04-08 20:58 [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 1/4] Rename of device types to be generic device names Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file Keith Wiles
@ 2015-04-08 20:58 ` Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 4/4] Update the rte_ethdev.[ch] files for the new device generic changes Keith Wiles
2015-04-10 18:47 ` [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Wiles, Keith
4 siblings, 0 replies; 9+ messages in thread
From: Keith Wiles @ 2015-04-08 20:58 UTC (permalink / raw)
To: dev
Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
lib/librte_eal/bsdapp/eal/Makefile | 1 +
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/include/rte_log.h | 1 +
lib/librte_eal/linuxapp/eal/Makefile | 1 +
4 files changed, 4 insertions(+)
diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index 2357cfa..7bb2689 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_devargs.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_dev.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_options.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_thread.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_device.c
CFLAGS_eal.o := -D_GNU_SOURCE
#CFLAGS_eal_thread.o := -D_GNU_SOURCE
diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index 3ea3bbf..c4bf805 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -40,6 +40,7 @@ INC += rte_string_fns.h rte_version.h
INC += rte_eal_memconfig.h rte_malloc_heap.h
INC += rte_hexdump.h rte_devargs.h rte_dev.h
INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
+INC += rte_common_device.h
ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)
INC += rte_warnings.h
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index f83a0d9..543deb7 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -77,6 +77,7 @@ extern struct rte_logs rte_logs;
#define RTE_LOGTYPE_PORT 0x00002000 /**< Log related to port. */
#define RTE_LOGTYPE_TABLE 0x00004000 /**< Log related to table. */
#define RTE_LOGTYPE_PIPELINE 0x00008000 /**< Log related to pipeline. */
+#define RTE_LOGTYPE_DEV 0x00010000 /**< Log related to device. */
/* these log types can be used in an application */
#define RTE_LOGTYPE_USER1 0x01000000 /**< User-defined log type 1. */
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 01f7b70..935720a 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -90,6 +90,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_devargs.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_dev.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_options.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_thread.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_device.c
CFLAGS_eal.o := -D_GNU_SOURCE
CFLAGS_eal_interrupts.o := -D_GNU_SOURCE
--
2.3.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [RFC PATCH 4/4] Update the rte_ethdev.[ch] files for the new device generic changes.
2015-04-08 20:58 [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Keith Wiles
` (2 preceding siblings ...)
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 3/4] Update files to build new device generic common files and headers Keith Wiles
@ 2015-04-08 20:58 ` Keith Wiles
2015-04-10 18:47 ` [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Wiles, Keith
4 siblings, 0 replies; 9+ messages in thread
From: Keith Wiles @ 2015-04-08 20:58 UTC (permalink / raw)
To: dev
rte_ethdev.c (main points):
- Collect up the globals and static variables in the file into a new rte_eth_globals structure.
- Update the global references to use the new global structure
- Move parts of the callback routines into eal_common_device.c to all other device a common routine
- Moved the debug macros PROC_PRIMARY_OR_ERR_RET, PROC_PRIMARY_OR_RET, FUNC_PTR_OR_ERR_RET and
FUNC_PTR_OR_RET into the rte_common_device.h as a common macro between devices.
rte_ethdev.h (main points):
- Replace the first couple members in rte_eth_dev_info to commmon macro
- Remove rte_eth_dev_cb_list define an use common rte_dev_cb_list instead
- Move eth_[rt]x_burst_t to dev_[rt]x_burst_t in eal_common_device.h
- Move rte_[rt]x_callback typedefs to eal_common_device.h as they are generic
- Move rte_eth_dev_type to eal_common_device.h and rename to rte_dev_type
- Move rte_eth_event_type to eal_common_device.h and rename to rte_dev_event_type
- Replace the content of rte_eth_dev with macro RTE_COMMON_DEV in eal_common_device.h
Replace part of the content of rte_eth_dev_data with macro RTE_COMMON_DEV_DATA
Replace part of the content of rte_eth_dev_info with macro RTE_COMMON_DEV_INFO
- Added the new global device structure to hold device local data instead of globals.
- Some of the simple functions rte_eth_dev_count and others now call rte_dev_count() via macro.
The eal_common_device.c and rte_common_device.h could merged into eal_common_dev.c and
rte_common_dev.h, which is not done here as to not effect those files.
Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
lib/librte_ether/rte_ethdev.c | 290 +++++++++---------------------------------
lib/librte_ether/rte_ethdev.h | 225 ++++++++++----------------------
2 files changed, 126 insertions(+), 389 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index e20cca5..84cef16 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -77,38 +77,20 @@
#define PMD_DEBUG_TRACE(fmt, args...)
#endif
-/* Macros for checking for restricting functions to primary instance only */
-#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
- if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
- PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
- return (retval); \
- } \
-} while(0)
-#define PROC_PRIMARY_OR_RET() do { \
- if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
- PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
- return; \
- } \
-} while(0)
-
-/* Macros to check for invlaid function pointers in dev_ops structure */
-#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
- if ((func) == NULL) { \
- PMD_DEBUG_TRACE("Function not supported\n"); \
- return (retval); \
- } \
-} while(0)
-#define FUNC_PTR_OR_RET(func) do { \
- if ((func) == NULL) { \
- PMD_DEBUG_TRACE("Function not supported\n"); \
- return; \
- } \
-} while(0)
-
-static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
-static struct rte_eth_dev_data *rte_eth_dev_data = NULL;
-static uint8_t nb_ports = 0;
+
+static struct rte_eth_global eth_globals = {
+ .devs = &rte_eth_devices[0],
+ .data = NULL,
+ .nb_ports = 0,
+ .max_ports = RTE_MAX_ETHPORTS,
+ .dflt_mtu = ETHER_MTU,
+ .dev_size = sizeof(struct rte_eth_dev),
+ .data_size = sizeof(struct rte_eth_dev_data),
+ .mz_dev_data = "rte_eth_dev_data"
+};
+
+struct rte_eth_global * rte_eth_globals = ð_globals;
/* spinlock for eth device callbacks */
static rte_spinlock_t rte_eth_dev_cb_lock = RTE_SPINLOCK_INITIALIZER;
@@ -155,30 +137,11 @@ static struct rte_eth_xstats_name_off rte_txq_stats_strings[] = {
sizeof(rte_txq_stats_strings[0]))
-/**
- * The user application callback description.
- *
- * It contains callback address to be registered by user application,
- * the pointer to the parameters for callback, and the event type.
- */
-struct rte_eth_dev_callback {
- TAILQ_ENTRY(rte_eth_dev_callback) next; /**< Callbacks list */
- rte_eth_dev_cb_fn cb_fn; /**< Callback address */
- void *cb_arg; /**< Parameter for callback */
- enum rte_eth_event_type event; /**< Interrupt event type */
- uint32_t active; /**< Callback is executing */
-};
-
enum {
STAT_QMAP_TX = 0,
STAT_QMAP_RX
};
-enum {
- DEV_DETACHED = 0,
- DEV_ATTACHED
-};
-
static inline void
rte_eth_dev_data_alloc(void)
{
@@ -186,18 +149,18 @@ rte_eth_dev_data_alloc(void)
const struct rte_memzone *mz;
if (rte_eal_process_type() == RTE_PROC_PRIMARY){
- mz = rte_memzone_reserve(MZ_RTE_ETH_DEV_DATA,
- RTE_MAX_ETHPORTS * sizeof(*rte_eth_dev_data),
+ mz = rte_memzone_reserve(eth_globals.mz_dev_data,
+ eth_globals.max_ports * eth_globals.data_size,
rte_socket_id(), flags);
} else
- mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
+ mz = rte_memzone_lookup(eth_globals.mz_dev_data);
if (mz == NULL)
rte_panic("Cannot allocate memzone for ethernet port data\n");
- rte_eth_dev_data = mz->addr;
+ eth_globals.data = mz->addr;
if (rte_eal_process_type() == RTE_PROC_PRIMARY)
- memset(rte_eth_dev_data, 0,
- RTE_MAX_ETHPORTS * sizeof(*rte_eth_dev_data));
+ memset(eth_globals.data, 0,
+ eth_globals.max_ports * eth_globals.data_size);
}
struct rte_eth_dev *
@@ -205,7 +168,7 @@ rte_eth_dev_allocated(const char *name)
{
unsigned i;
- for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+ for (i = 0; i < eth_globals.max_ports; i++) {
if ((rte_eth_devices[i].attached == DEV_ATTACHED) &&
strcmp(rte_eth_devices[i].data->name, name) == 0)
return &rte_eth_devices[i];
@@ -218,26 +181,26 @@ rte_eth_dev_find_free_port(void)
{
unsigned i;
- for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+ for (i = 0; i < eth_globals.max_ports; i++) {
if (rte_eth_devices[i].attached == DEV_DETACHED)
return i;
}
- return RTE_MAX_ETHPORTS;
+ return eth_globals.max_ports;
}
struct rte_eth_dev *
-rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type)
+rte_eth_dev_allocate(const char *name, enum rte_dev_type type)
{
uint8_t port_id;
struct rte_eth_dev *eth_dev;
port_id = rte_eth_dev_find_free_port();
- if (port_id == RTE_MAX_ETHPORTS) {
+ if (port_id == eth_globals.max_ports) {
PMD_DEBUG_TRACE("Reached maximum number of Ethernet ports\n");
return NULL;
}
- if (rte_eth_dev_data == NULL)
+ if (eth_globals.data == NULL)
rte_eth_dev_data_alloc();
if (rte_eth_dev_allocated(name) != NULL) {
@@ -246,12 +209,12 @@ rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type)
}
eth_dev = &rte_eth_devices[port_id];
- eth_dev->data = &rte_eth_dev_data[port_id];
+ eth_dev->data = ð_globals.data[port_id];
snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
eth_dev->data->port_id = port_id;
eth_dev->attached = DEV_ATTACHED;
eth_dev->dev_type = type;
- nb_ports++;
+ eth_globals.nb_ports++;
return eth_dev;
}
@@ -279,7 +242,7 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
return -EINVAL;
eth_dev->attached = 0;
- nb_ports--;
+ eth_globals.nb_ports--;
return 0;
}
@@ -299,7 +262,7 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv,
rte_eth_dev_create_unique_device_name(ethdev_name,
sizeof(ethdev_name), pci_dev);
- eth_dev = rte_eth_dev_allocate(ethdev_name, RTE_ETH_DEV_PCI);
+ eth_dev = rte_eth_dev_allocate(ethdev_name, RTE_DEV_PCI);
if (eth_dev == NULL)
return -ENOMEM;
@@ -334,7 +297,7 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv,
if (rte_eal_process_type() == RTE_PROC_PRIMARY)
rte_free(eth_dev->data->dev_private);
eth_dev->attached = DEV_DETACHED;
- nb_ports--;
+ eth_globals.nb_ports--;
return diag;
}
@@ -401,16 +364,6 @@ rte_eth_driver_register(struct eth_driver *eth_drv)
rte_eal_pci_register(ð_drv->pci_drv);
}
-static int
-rte_eth_dev_is_valid_port(uint8_t port_id)
-{
- if (port_id >= RTE_MAX_ETHPORTS ||
- rte_eth_devices[port_id].attached != DEV_ATTACHED)
- return 0;
- else
- return 1;
-}
-
int
rte_eth_dev_socket_id(uint8_t port_id)
{
@@ -419,20 +372,14 @@ rte_eth_dev_socket_id(uint8_t port_id)
return rte_eth_devices[port_id].pci_dev->numa_node;
}
-uint8_t
-rte_eth_dev_count(void)
-{
- return (nb_ports);
-}
-
/* So far, DPDK hotplug function only supports linux */
#ifdef RTE_LIBRTE_EAL_HOTPLUG
-static enum rte_eth_dev_type
+static enum rte_dev_type
rte_eth_dev_get_device_type(uint8_t port_id)
{
if (!rte_eth_dev_is_valid_port(port_id))
- return RTE_ETH_DEV_UNKNOWN;
+ return RTE_DEV_UNKNOWN;
return rte_eth_devices[port_id].dev_type;
}
@@ -440,7 +387,7 @@ static int
rte_eth_dev_save(struct rte_eth_dev *devs, size_t size)
{
if ((devs == NULL) ||
- (size != sizeof(struct rte_eth_dev) * RTE_MAX_ETHPORTS))
+ (size != sizeof(struct rte_eth_dev) * eth_globals.max_ports))
return -EINVAL;
/* save current rte_eth_devices */
@@ -455,7 +402,7 @@ rte_eth_dev_get_changed_port(struct rte_eth_dev *devs, uint8_t *port_id)
return -EINVAL;
/* check which port was attached or detached */
- for (*port_id = 0; *port_id < RTE_MAX_ETHPORTS; (*port_id)++, devs++) {
+ for (*port_id = 0; *port_id < eth_globals.max_ports; (*port_id)++, devs++) {
if (rte_eth_devices[*port_id].attached ^ devs->attached)
return 0;
}
@@ -496,7 +443,7 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
/* shouldn't check 'rte_eth_devices[i].data',
* because it might be overwritten by VDEV PMD */
- tmp = rte_eth_dev_data[port_id].name;
+ tmp = eth_globals.data[port_id].name;
strcpy(name, tmp);
return 0;
}
@@ -506,12 +453,12 @@ rte_eth_dev_is_detachable(uint8_t port_id)
{
uint32_t drv_flags;
- if (port_id >= RTE_MAX_ETHPORTS) {
+ if (port_id >= eth_globals.max_ports) {
PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
return -EINVAL;
}
- if (rte_eth_devices[port_id].dev_type == RTE_ETH_DEV_PCI) {
+ if (rte_eth_devices[port_id].dev_type == RTE_DEV_PCI) {
switch (rte_eth_devices[port_id].pci_dev->kdrv) {
case RTE_KDRV_IGB_UIO:
case RTE_KDRV_UIO_GENERIC:
@@ -691,7 +638,7 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
if (name == NULL)
return -EINVAL;
- if (rte_eth_dev_get_device_type(port_id) == RTE_ETH_DEV_PCI) {
+ if (rte_eth_dev_get_device_type(port_id) == RTE_DEV_PCI) {
ret = rte_eth_dev_get_addr_by_port(port_id, &addr);
if (ret < 0)
return ret;
@@ -2507,7 +2454,7 @@ rte_eth_dev_rss_reta_query(uint8_t port_id,
struct rte_eth_dev *dev;
int ret;
- if (port_id >= nb_ports) {
+ if (port_id >= eth_globals.nb_ports) {
PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
return -ENODEV;
}
@@ -3178,11 +3125,10 @@ rte_eth_rx_descriptor_done(uint8_t port_id, uint16_t queue_id, uint16_t offset)
int
rte_eth_dev_callback_register(uint8_t port_id,
- enum rte_eth_event_type event,
- rte_eth_dev_cb_fn cb_fn, void *cb_arg)
+ enum rte_dev_event_type event,
+ rte_dev_cb_fn cb_fn, void *cb_arg)
{
struct rte_eth_dev *dev;
- struct rte_eth_dev_callback *user_cb;
if (!cb_fn)
return (-EINVAL);
@@ -3193,37 +3139,16 @@ rte_eth_dev_callback_register(uint8_t port_id,
}
dev = &rte_eth_devices[port_id];
- rte_spinlock_lock(&rte_eth_dev_cb_lock);
-
- TAILQ_FOREACH(user_cb, &(dev->link_intr_cbs), next) {
- if (user_cb->cb_fn == cb_fn &&
- user_cb->cb_arg == cb_arg &&
- user_cb->event == event) {
- break;
- }
- }
-
- /* create a new callback. */
- if (user_cb == NULL && (user_cb = rte_zmalloc("INTR_USER_CALLBACK",
- sizeof(struct rte_eth_dev_callback), 0)) != NULL) {
- user_cb->cb_fn = cb_fn;
- user_cb->cb_arg = cb_arg;
- user_cb->event = event;
- TAILQ_INSERT_TAIL(&(dev->link_intr_cbs), user_cb, next);
- }
-
- rte_spinlock_unlock(&rte_eth_dev_cb_lock);
- return ((user_cb == NULL) ? -ENOMEM : 0);
+ return rte_dev_callback_register(&dev->link_intr_cbs,
+ &rte_eth_dev_cb_lock, event, cb_fn, cb_arg);
}
int
rte_eth_dev_callback_unregister(uint8_t port_id,
- enum rte_eth_event_type event,
- rte_eth_dev_cb_fn cb_fn, void *cb_arg)
+ enum rte_dev_event_type event,
+ rte_dev_cb_fn cb_fn, void *cb_arg)
{
- int ret;
struct rte_eth_dev *dev;
- struct rte_eth_dev_callback *cb, *next;
if (!cb_fn)
return (-EINVAL);
@@ -3234,55 +3159,18 @@ rte_eth_dev_callback_unregister(uint8_t port_id,
}
dev = &rte_eth_devices[port_id];
- rte_spinlock_lock(&rte_eth_dev_cb_lock);
-
- ret = 0;
- for (cb = TAILQ_FIRST(&dev->link_intr_cbs); cb != NULL; cb = next) {
-
- next = TAILQ_NEXT(cb, next);
-
- if (cb->cb_fn != cb_fn || cb->event != event ||
- (cb->cb_arg != (void *)-1 &&
- cb->cb_arg != cb_arg))
- continue;
-
- /*
- * if this callback is not executing right now,
- * then remove it.
- */
- if (cb->active == 0) {
- TAILQ_REMOVE(&(dev->link_intr_cbs), cb, next);
- rte_free(cb);
- } else {
- ret = -EAGAIN;
- }
- }
-
- rte_spinlock_unlock(&rte_eth_dev_cb_lock);
- return (ret);
+ return rte_dev_callback_unregister(&dev->link_intr_cbs,
+ &rte_eth_dev_cb_lock, event, cb_fn, cb_arg);
}
void
_rte_eth_dev_callback_process(struct rte_eth_dev *dev,
- enum rte_eth_event_type event)
+ enum rte_dev_event_type event)
{
- struct rte_eth_dev_callback *cb_lst;
- struct rte_eth_dev_callback dev_cb;
-
- rte_spinlock_lock(&rte_eth_dev_cb_lock);
- TAILQ_FOREACH(cb_lst, &(dev->link_intr_cbs), next) {
- if (cb_lst->cb_fn == NULL || cb_lst->event != event)
- continue;
- dev_cb = *cb_lst;
- cb_lst->active = 1;
- rte_spinlock_unlock(&rte_eth_dev_cb_lock);
- dev_cb.cb_fn(dev->data->port_id, dev_cb.event,
- dev_cb.cb_arg);
- rte_spinlock_lock(&rte_eth_dev_cb_lock);
- cb_lst->active = 0;
- }
- rte_spinlock_unlock(&rte_eth_dev_cb_lock);
+ rte_dev_callback_process(&dev->link_intr_cbs, dev->data->port_id,
+ event, &rte_eth_dev_cb_lock);
}
+
#ifdef RTE_NIC_BYPASS
int rte_eth_dev_bypass_init(uint8_t port_id)
{
@@ -3510,18 +3398,7 @@ rte_eth_add_rx_callback(uint8_t port_id, uint16_t queue_id,
return NULL;
}
- struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);
-
- if (cb == NULL) {
- rte_errno = ENOMEM;
- return NULL;
- }
-
- cb->fn.rx = fn;
- cb->param = user_param;
- cb->next = rte_eth_devices[port_id].post_rx_burst_cbs[queue_id];
- rte_eth_devices[port_id].post_rx_burst_cbs[queue_id] = cb;
- return cb;
+ return rte_dev_add_callback(&rte_eth_devices[port_id].post_rx_burst_cbs[queue_id], fn, user_param);
}
void *
@@ -3539,23 +3416,12 @@ rte_eth_add_tx_callback(uint8_t port_id, uint16_t queue_id,
return NULL;
}
- struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);
-
- if (cb == NULL) {
- rte_errno = ENOMEM;
- return NULL;
- }
-
- cb->fn.tx = fn;
- cb->param = user_param;
- cb->next = rte_eth_devices[port_id].pre_tx_burst_cbs[queue_id];
- rte_eth_devices[port_id].pre_tx_burst_cbs[queue_id] = cb;
- return cb;
+ return rte_dev_add_callback(&rte_eth_devices[port_id].pre_tx_burst_cbs[queue_id], fn, user_param);
}
int
rte_eth_remove_rx_callback(uint8_t port_id, uint16_t queue_id,
- struct rte_eth_rxtx_callback *user_cb)
+ struct rte_dev_rxtx_callback *user_cb)
{
#ifndef RTE_ETHDEV_RXTX_CALLBACKS
return (-ENOTSUP);
@@ -3567,34 +3433,13 @@ rte_eth_remove_rx_callback(uint8_t port_id, uint16_t queue_id,
}
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
- struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
- struct rte_eth_rxtx_callback *prev_cb;
- /* Reset head pointer and remove user cb if first in the list. */
- if (cb == user_cb) {
- dev->post_rx_burst_cbs[queue_id] = user_cb->next;
- return 0;
- }
-
- /* Remove the user cb from the callback list. */
- do {
- prev_cb = cb;
- cb = cb->next;
-
- if (cb == user_cb) {
- prev_cb->next = user_cb->next;
- return 0;
- }
-
- } while (cb != NULL);
-
- /* Callback wasn't found. */
- return (-EINVAL);
+ return rte_dev_remove_callback(&dev->post_rx_burst_cbs[queue_id], user_cb);
}
int
rte_eth_remove_tx_callback(uint8_t port_id, uint16_t queue_id,
- struct rte_eth_rxtx_callback *user_cb)
+ struct rte_dev_rxtx_callback *user_cb)
{
#ifndef RTE_ETHDEV_RXTX_CALLBACKS
return (-ENOTSUP);
@@ -3606,27 +3451,6 @@ rte_eth_remove_tx_callback(uint8_t port_id, uint16_t queue_id,
}
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
- struct rte_eth_rxtx_callback *cb = dev->pre_tx_burst_cbs[queue_id];
- struct rte_eth_rxtx_callback *prev_cb;
-
- /* Reset head pointer and remove user cb if first in the list. */
- if (cb == user_cb) {
- dev->pre_tx_burst_cbs[queue_id] = user_cb->next;
- return 0;
- }
-
- /* Remove the user cb from the callback list. */
- do {
- prev_cb = cb;
- cb = cb->next;
-
- if (cb == user_cb) {
- prev_cb->next = user_cb->next;
- return 0;
- }
-
- } while (cb != NULL);
- /* Callback wasn't found. */
- return (-EINVAL);
+ return rte_dev_remove_callback(&dev->pre_tx_burst_cbs[queue_id], user_cb);
}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index e8df027..991e8a5 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -178,6 +178,7 @@ extern "C" {
#include <rte_dev.h>
#include <rte_devargs.h>
#include <rte_mbuf.h>
+#include <rte_common_device.h>
#include "rte_ether.h"
#include "rte_eth_ctrl.h"
@@ -896,10 +897,8 @@ struct rte_eth_conf {
#define DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000080 /**< Used for tunneling packet. */
struct rte_eth_dev_info {
- struct rte_pci_device *pci_dev; /**< Device PCI information. */
- const char *driver_name; /**< Device Driver name. */
- unsigned int if_index; /**< Index to bound host interface, or 0 if none.
- Use if_indextoname() to translate into an interface name. */
+ RTE_COMMON_DEV_INFO;
+
uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
uint32_t max_rx_pktlen; /**< Maximum configurable length of RX pkt. */
uint16_t max_rx_queues; /**< Maximum number of RX queues. */
@@ -939,10 +938,6 @@ struct rte_eth_xstats {
struct rte_eth_dev;
-struct rte_eth_dev_callback;
-/** @internal Structure to keep track of registered callbacks */
-TAILQ_HEAD(rte_eth_dev_cb_list, rte_eth_dev_callback);
-
/*
* Definitions of all functions exported by an Ethernet driver through the
* the generic structure of type *eth_dev_ops* supplied in the *rte_eth_dev*
@@ -1065,16 +1060,6 @@ typedef void (*vlan_strip_queue_set_t)(struct rte_eth_dev *dev,
int on);
/**< @internal VLAN stripping enable/disable by an queue of Ethernet device. */
-typedef uint16_t (*eth_rx_burst_t)(void *rxq,
- struct rte_mbuf **rx_pkts,
- uint16_t nb_pkts);
-/**< @internal Retrieve input packets from a receive queue of an Ethernet device. */
-
-typedef uint16_t (*eth_tx_burst_t)(void *txq,
- struct rte_mbuf **tx_pkts,
- uint16_t nb_pkts);
-/**< @internal Send output packets on a transmit queue of an Ethernet device. */
-
typedef int (*fdir_add_signature_filter_t)(struct rte_eth_dev *dev,
struct rte_fdir_filter *fdir_ftr,
uint8_t rx_queue);
@@ -1384,80 +1369,6 @@ struct eth_dev_ops {
};
/**
- * Function type used for RX packet processing packet callbacks.
- *
- * The callback function is called on RX with a burst of packets that have
- * been received on the given port and queue.
- *
- * @param port
- * The Ethernet port on which RX is being performed.
- * @param queue
- * The queue on the Ethernet port which is being used to receive the packets.
- * @param pkts
- * The burst of packets that have just been received.
- * @param nb_pkts
- * The number of packets in the burst pointed to by "pkts".
- * @param max_pkts
- * The max number of packets that can be stored in the "pkts" array.
- * @param user_param
- * The arbitrary user parameter passed in by the application when the callback
- * was originally configured.
- * @return
- * The number of packets returned to the user.
- */
-typedef uint16_t (*rte_rx_callback_fn)(uint8_t port, uint16_t queue,
- struct rte_mbuf *pkts[], uint16_t nb_pkts, uint16_t max_pkts,
- void *user_param);
-
-/**
- * Function type used for TX packet processing packet callbacks.
- *
- * The callback function is called on TX with a burst of packets immediately
- * before the packets are put onto the hardware queue for transmission.
- *
- * @param port
- * The Ethernet port on which TX is being performed.
- * @param queue
- * The queue on the Ethernet port which is being used to transmit the packets.
- * @param pkts
- * The burst of packets that are about to be transmitted.
- * @param nb_pkts
- * The number of packets in the burst pointed to by "pkts".
- * @param user_param
- * The arbitrary user parameter passed in by the application when the callback
- * was originally configured.
- * @return
- * The number of packets to be written to the NIC.
- */
-typedef uint16_t (*rte_tx_callback_fn)(uint8_t port, uint16_t queue,
- struct rte_mbuf *pkts[], uint16_t nb_pkts, void *user_param);
-
-/**
- * @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;
-};
-
-/*
- * The eth device type
- */
-enum rte_eth_dev_type {
- RTE_ETH_DEV_UNKNOWN, /**< unknown device type */
- RTE_ETH_DEV_PCI,
- /**< Physical function and Virtual function of PCI devices */
- RTE_ETH_DEV_VIRTUAL, /**< non hardware device */
- RTE_ETH_DEV_MAX /**< max value of this enum */
-};
-
-/**
* @internal
* The generic data structure associated with each ethernet device.
*
@@ -1468,26 +1379,7 @@ enum rte_eth_dev_type {
* 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. */
- struct rte_eth_dev_data *data; /**< Pointer to device data */
- const struct eth_driver *driver;/**< Driver for this device */
- struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
- struct rte_pci_device *pci_dev; /**< PCI info. supplied by probing */
- /** 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];
- uint8_t attached; /**< Flag indicating the port is attached */
- enum rte_eth_dev_type dev_type; /**< Flag indicating the device type */
+ RTE_COMMON_DEV(eth_);
};
struct rte_eth_dev_sriov {
@@ -1498,7 +1390,7 @@ struct rte_eth_dev_sriov {
};
#define RTE_ETH_DEV_SRIOV(dev) ((dev)->data->sriov)
-#define RTE_ETH_NAME_MAX_LEN (32)
+#define RTE_ETH_NAME_MAX_LEN RTE_DEV_NAME_MAX_LEN
/**
* @internal
@@ -1508,33 +1400,20 @@ struct rte_eth_dev_sriov {
* 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. */
+ RTE_COMMON_DEV_DATA;
struct rte_eth_dev_sriov sriov; /**< SRIOV data */
- void *dev_private; /**< PMD-specific private data */
-
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 */
-
- uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
- struct ether_addr* mac_addrs;/**< Device Ethernet Link address. */
+ struct ether_addr* mac_addrs; /**< Device Ethernet Link address. */
uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
/** bitmap array of associating Ethernet MAC addresses to pools */
struct ether_addr* hash_mac_addrs;
+
/** Device Ethernet MAC addresses of hash filtering. */
- uint8_t port_id; /**< Device [external] port identifier. */
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). */
@@ -1549,6 +1428,41 @@ struct rte_eth_dev_data {
extern struct rte_eth_dev rte_eth_devices[];
/**
+ * @internal
+ * A global structure to hold global values per device type.
+ */
+struct rte_eth_global {
+ RTE_COMMON_GLOBAL(eth_);
+};
+
+/**
+ * @internal
+ * The Ethernet device data structure. Look in <rte_pktdev.c> file.
+ */
+extern struct rte_eth_global *rte_eth_globals;
+
+/**
+ * Return the global structure pointer.
+ *
+ * @return
+ * Return the global structure pointer.
+ */
+static inline struct rte_eth_global * rte_eth_dev_global(void) {
+ return rte_eth_globals;
+}
+
+/**
+ * Validate if the port number is valid
+ *
+ * @param port_id Port ID value to select the device.
+ *
+ * @return
+ * - Number of ports found in the system.
+ */
+#define rte_eth_dev_is_valid_port(port_id) \
+ rte_dev_is_valid_port((struct rte_pkt_global *)rte_eth_globals, port_id)
+
+/**
* Get the total number of Ethernet devices that have been successfully
* initialized by the [matching] Ethernet driver during the PCI probing phase.
* All devices whose port identifier is in the range
@@ -1561,7 +1475,20 @@ extern struct rte_eth_dev rte_eth_devices[];
* @return
* - The total number of usable Ethernet devices.
*/
-extern uint8_t rte_eth_dev_count(void);
+#define rte_eth_dev_count() \
+ rte_dev_count((struct rte_dev_global *)rte_eth_globals)
+
+/**
+ * Get the rte_eth_dev structure device pointer for the device.
+ *
+ * @param pid
+ * Port ID value to select the device structure.
+ *
+ * @return
+ * - The rte_eth_dev structure pointer for the given port ID.
+ */
+#define rte_eth_get_dev(pid) \
+ (struct rte_eth_dev *)rte_get_dev((struct rte_dev_global *)rte_eth_globals, pid)
/**
* Function for internal use by port hotplug functions.
@@ -1585,7 +1512,7 @@ extern struct rte_eth_dev *rte_eth_dev_allocated(const char *name);
* - Slot in the rte_dev_devices array for a new device;
*/
struct rte_eth_dev *rte_eth_dev_allocate(const char *name,
- enum rte_eth_dev_type type);
+ enum rte_dev_type type);
/**
* Function for internal use by dummy drivers primarily, e.g. ring-based
@@ -2418,7 +2345,7 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
rx_pkts, nb_pkts);
#ifdef RTE_ETHDEV_RXTX_CALLBACKS
- struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
+ struct rte_dev_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
if (unlikely(cb != NULL)) {
do {
@@ -2558,7 +2485,7 @@ rte_eth_tx_burst(uint8_t port_id, uint16_t queue_id,
dev = &rte_eth_devices[port_id];
#ifdef RTE_ETHDEV_RXTX_CALLBACKS
- struct rte_eth_rxtx_callback *cb = dev->pre_tx_burst_cbs[queue_id];
+ struct rte_dev_rxtx_callback *cb = dev->pre_tx_burst_cbs[queue_id];
if (unlikely(cb != NULL)) {
do {
@@ -2789,20 +2716,6 @@ int rte_eth_dev_fdir_remove_perfect_filter(uint8_t port_id,
int rte_eth_dev_fdir_set_masks(uint8_t port_id,
struct rte_fdir_masks *fdir_mask);
-/**
- * The eth device event type for interrupt, and maybe others in the future.
- */
-enum rte_eth_event_type {
- RTE_ETH_EVENT_UNKNOWN, /**< unknown event type */
- RTE_ETH_EVENT_INTR_LSC, /**< lsc interrupt event */
- RTE_ETH_EVENT_MAX /**< max value of this enum */
-};
-
-typedef void (*rte_eth_dev_cb_fn)(uint8_t port_id, \
- enum rte_eth_event_type event, void *cb_arg);
-/**< user application callback to be registered for interrupts */
-
-
/**
* Register a callback function for specific port id.
@@ -2821,8 +2734,8 @@ typedef void (*rte_eth_dev_cb_fn)(uint8_t port_id, \
* - On failure, a negative value.
*/
int rte_eth_dev_callback_register(uint8_t port_id,
- enum rte_eth_event_type event,
- rte_eth_dev_cb_fn cb_fn, void *cb_arg);
+ enum rte_dev_event_type event,
+ rte_dev_cb_fn cb_fn, void *cb_arg);
/**
* Unregister a callback function for specific port id.
@@ -2842,8 +2755,8 @@ int rte_eth_dev_callback_register(uint8_t port_id,
* - On failure, a negative value.
*/
int rte_eth_dev_callback_unregister(uint8_t port_id,
- enum rte_eth_event_type event,
- rte_eth_dev_cb_fn cb_fn, void *cb_arg);
+ enum rte_dev_event_type event,
+ rte_dev_cb_fn cb_fn, void *cb_arg);
/**
* @internal Executes all the user application registered callbacks for
@@ -2859,7 +2772,7 @@ int rte_eth_dev_callback_unregister(uint8_t port_id,
* void
*/
void _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
- enum rte_eth_event_type event);
+ enum rte_dev_event_type event);
/**
* Turn on the LED on the Ethernet device.
@@ -3512,7 +3425,7 @@ int rte_eth_dev_filter_ctrl(uint8_t port_id, enum rte_filter_type filter_type,
* On success, a pointer value which can later be used to remove the callback.
*/
void *rte_eth_add_rx_callback(uint8_t port_id, uint16_t queue_id,
- rte_rx_callback_fn fn, void *user_param);
+ rte_rx_callback_fn fn, void *user_param);
/**
* Add a callback to be called on packet TX on a given port and queue.
@@ -3537,7 +3450,7 @@ void *rte_eth_add_rx_callback(uint8_t port_id, uint16_t queue_id,
* On success, a pointer value which can later be used to remove the callback.
*/
void *rte_eth_add_tx_callback(uint8_t port_id, uint16_t queue_id,
- rte_tx_callback_fn fn, void *user_param);
+ rte_tx_callback_fn fn, void *user_param);
/**
* Remove an RX packet callback from a given port and queue.
@@ -3570,7 +3483,7 @@ void *rte_eth_add_tx_callback(uint8_t port_id, uint16_t queue_id,
* is NULL or not found for the port/queue.
*/
int rte_eth_remove_rx_callback(uint8_t port_id, uint16_t queue_id,
- struct rte_eth_rxtx_callback *user_cb);
+ struct rte_dev_rxtx_callback *user_cb);
/**
* Remove a TX packet callback from a given port and queue.
@@ -3603,7 +3516,7 @@ int rte_eth_remove_rx_callback(uint8_t port_id, uint16_t queue_id,
* is NULL or not found for the port/queue.
*/
int rte_eth_remove_tx_callback(uint8_t port_id, uint16_t queue_id,
- struct rte_eth_rxtx_callback *user_cb);
+ struct rte_dev_rxtx_callback *user_cb);
#ifdef __cplusplus
}
--
2.3.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file.
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file Keith Wiles
@ 2015-04-09 11:53 ` Neil Horman
2015-04-10 19:25 ` Wiles, Keith
0 siblings, 1 reply; 9+ messages in thread
From: Neil Horman @ 2015-04-09 11:53 UTC (permalink / raw)
To: Keith Wiles; +Cc: dev
On Wed, Apr 08, 2015 at 03:58:38PM -0500, Keith Wiles wrote:
> Move a number of device specific define, structures and functions
> into a generic device base set of files for all device not just Ethernet.
>
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
> lib/librte_eal/common/eal_common_device.c | 185 +++++++
> lib/librte_eal/common/include/rte_common_device.h | 617 ++++++++++++++++++++++
> 2 files changed, 802 insertions(+)
> create mode 100644 lib/librte_eal/common/eal_common_device.c
> create mode 100644 lib/librte_eal/common/include/rte_common_device.h
>
> diff --git a/lib/librte_eal/common/eal_common_device.c b/lib/librte_eal/common/eal_common_device.c
> new file mode 100644
> index 0000000..a9ef925
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common_device.c
> @@ -0,0 +1,185 @@
> +/*-
> + * BSD LICENSE
> + *
> + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + * Copyright(c) 2014 6WIND S.A.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in
> + * the documentation and/or other materials provided with the
> + * distribution.
> + * * Neither the name of Intel Corporation nor the names of its
> + * contributors may be used to endorse or promote products derived
> + * from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <inttypes.h>
> +#include <sys/queue.h>
> +
> +#include <rte_dev.h>
> +#include <rte_devargs.h>
> +#include <rte_debug.h>
> +#include <rte_devargs.h>
> +#include <rte_log.h>
> +#include <rte_malloc.h>
> +#include <rte_errno.h>
> +
> +#include "rte_common_device.h"
> +
> +void *
> +rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
> + void * fn, void *user_param)
> +{
> + struct rte_dev_rxtx_callback *cb;
> +
> + cb = rte_zmalloc(NULL, sizeof(*cb), 0);
> +
> + if (cb == NULL) {
> + rte_errno = ENOMEM;
> + return NULL;
> + }
> +
> + cb->fn.vp = fn;
> + cb->param = user_param;
> + cb->next = *cbp;
> + *cbp = cb;
> + return cb;
> +}
> +
> +int
> +rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
> + struct rte_dev_rxtx_callback *user_cb)
> +{
> + struct rte_dev_rxtx_callback *cb = *cbp;
> + struct rte_dev_rxtx_callback *prev_cb;
> +
> + /* Reset head pointer and remove user cb if first in the list. */
> + if (cb == user_cb) {
> + *cbp = user_cb->next;
> + return 0;
> + }
> +
> + /* Remove the user cb from the callback list. */
> + do {
> + prev_cb = cb;
> + cb = cb->next;
> +
> + if (cb == user_cb) {
> + prev_cb->next = user_cb->next;
> + return 0;
> + }
> + } while (cb != NULL);
> +
> + /* Callback wasn't found. */
> + return (-EINVAL);
> +}
> +
> +int
> +rte_dev_callback_register(struct rte_dev_cb_list * cb_list,
> + rte_spinlock_t * lock,
> + enum rte_dev_event_type event,
> + rte_dev_cb_fn cb_fn, void *cb_arg)
> +{
> + struct rte_dev_callback *cb;
> +
> + rte_spinlock_lock(lock);
> +
> + TAILQ_FOREACH(cb, cb_list, next) {
> + if (cb->cb_fn == cb_fn &&
> + cb->cb_arg == cb_arg &&
> + cb->event == event) {
> + break;
> + }
> + }
> +
> + /* create a new callback. */
> + if (cb == NULL && (cb = rte_zmalloc("INTR_USER_CALLBACK",
> + sizeof(struct rte_dev_callback), 0)) != NULL) {
> + cb->cb_fn = cb_fn;
> + cb->cb_arg = cb_arg;
> + cb->event = event;
> + TAILQ_INSERT_TAIL(cb_list, cb, next);
> + }
> +
> + rte_spinlock_unlock(lock);
> + return ((cb == NULL) ? -ENOMEM : 0);
> +}
> +
> +int
> +rte_dev_callback_unregister(struct rte_dev_cb_list * cb_list,
> + rte_spinlock_t * lock,
> + enum rte_dev_event_type event,
> + rte_dev_cb_fn cb_fn, void *cb_arg)
> +{
> + int ret;
> + struct rte_dev_callback *cb, *next;
> +
> + rte_spinlock_lock(lock);
> +
> + ret = 0;
> + for (cb = TAILQ_FIRST(cb_list); cb != NULL; cb = next) {
> +
> + next = TAILQ_NEXT(cb, next);
> +
> + if (cb->cb_fn != cb_fn || cb->event != event ||
> + (cb->cb_arg != (void *)-1 &&
> + cb->cb_arg != cb_arg))
> + continue;
> +
> + /*
> + * if this callback is not executing right now,
> + * then remove it.
> + */
> + if (cb->active == 0) {
> + TAILQ_REMOVE(cb_list, cb, next);
> + rte_free(cb);
> + } else {
> + ret = -EAGAIN;
> + }
> + }
> +
> + rte_spinlock_unlock(lock);
> + return (ret);
> +}
> +
> +void
> +rte_dev_callback_process(struct rte_dev_cb_list * cb_list, uint8_t port_id,
> + enum rte_dev_event_type event, rte_spinlock_t *lock)
> +{
> + struct rte_dev_callback *cb;
> + struct rte_dev_callback dev_cb;
> +
> + rte_spinlock_lock(lock);
> + TAILQ_FOREACH(cb, cb_list, next) {
> + if (cb->cb_fn == NULL || cb->event != event)
> + continue;
> + dev_cb = *cb;
> + cb->active = 1;
> + rte_spinlock_unlock(lock);
> + dev_cb.cb_fn(port_id, dev_cb.event, dev_cb.cb_arg);
> + rte_spinlock_lock(lock);
> + cb->active = 0;
> + }
> + rte_spinlock_unlock(lock);
> +}
This is a bit...odd. The rte_eth callbacks had some context because you knew
when the callback was going to be issued (at the end of an rx and tx operation).
Here, by making the process routine generic, you're removed that context, and so
the caller has no guarantee as to when their callbacks will be called. If its
to be done at the end of the generic transmit and routine functions, that would
be fine, but I don't see that happening here.
> diff --git a/lib/librte_eal/common/include/rte_common_device.h b/lib/librte_eal/common/include/rte_common_device.h
> new file mode 100644
> index 0000000..5baefb6
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_common_device.h
> @@ -0,0 +1,617 @@
> +/*-
> + * BSD LICENSE
> + *
> + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in
> + * the documentation and/or other materials provided with the
> + * distribution.
> + * * Neither the name of Intel Corporation nor the names of its
> + * contributors may be used to endorse or promote products derived
> + * from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHPDF IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _RTE_COMMON_DEVICE_H_
> +#define _RTE_COMMON_DEVICE_H_
> +
> +/**
> + * @file
> + *
> + * Common Device Helpers in RTE
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <rte_spinlock.h>
> +
> +/* Macros for checking for restricting functions to primary instance only */
> +#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> + PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> + return (retval); \
> + } \
> +} while(0)
> +#define PROC_PRIMARY_OR_RET() do { \
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> + PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> + return; \
> + } \
> +} while(0)
> +
> +/* Macros to check for invalid function pointers in dev_ops structure */
> +#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
> + if ((func) == NULL) { \
> + PMD_DEBUG_TRACE("Function not supported\n"); \
> + return (retval); \
> + } \
> +} while(0)
> +#define FUNC_PTR_OR_RET(func) do { \
> + if ((func) == NULL) { \
> + PMD_DEBUG_TRACE("Function not supported\n"); \
> + return; \
> + } \
> +} while(0)
> +
> +enum {
> + DEV_DETACHED = 0,
> + DEV_ATTACHED
> +};
> +
> +/*
> + * The device type
> + */
> +enum rte_dev_type {
> + RTE_DEV_UNKNOWN, /**< unknown device type */
> + RTE_DEV_PCI,
> + /**< Physical function and Virtual function of PCI devices */
> + RTE_DEV_VIRTUAL, /**< non hardware device */
> + RTE_DEV_MAX /**< max value of this enum */
> +};
> +
> +/**
> + * The device event type for interrupt, and maybe others in the future.
> + */
> +enum rte_dev_event_type {
> + RTE_DEV_EVENT_UNKNOWN, /**< unknown event type */
> + RTE_DEV_EVENT_INTR_LSC, /**< lsc interrupt event */
> + RTE_DEV_EVENT_MAX /**< max value of this enum */
> +};
> +
> +struct rte_dev_callback;
> +
> +/** @internal Structure to keep track of registered callback */
> +TAILQ_HEAD(rte_dev_cb_list, rte_dev_callback);
> +
> +struct rte_mbuf;
> +
> +typedef uint16_t (*dev_rx_burst_t)(void *rxq,
> + struct rte_mbuf **rx_pkts,
> + uint16_t nb_pkts);
> +/**< @internal Retrieve input packets from a receive queue of a device. */
> +
> +typedef uint16_t (*dev_tx_burst_t)(void *txq,
> + struct rte_mbuf **tx_pkts,
> + uint16_t nb_pkts);
> +/**< @internal Send output packets on a transmit queue of a device. */
> +
> +typedef void (*rte_dev_cb_fn)(uint8_t port_id, \
> + enum rte_dev_event_type event, void *cb_arg);
> +/**< user application callback to be registered for interrupts */
> +
> +#define RTE_DEV_NAME_MAX_LEN (32)
> +
> +/**
> + * @internal
> + * Common set of members for all devices included at the top of 'struct rte_xyz_dev'
> + */
> +#define RTE_COMMON_DEV(_t) \
> + dev_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */ \
> + dev_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */ \
> + struct rte_##_t##dev_data *data; /**< Pointer to device data */ \
> + struct rte_##_t##global *gbl_data; /**< Pointer to device global data */ \
> + const struct _t##driver *driver; /**< Driver for this device */ \
> + struct _t##dev_ops *dev_ops; /**< Functions exported by PMD */ \
This doesn't make any sense to me. You've made a generic macro that creates
what is ostensibly supposed to be common code point, but then you include some
string concatenation so that the common parts are actually unique to a given
device class. Why would you do that? At that point the common parts aren't
common by definition, and should go in a structure specific to that device class
> + struct rte_pci_device *pci_dev; /**< PCI info. supplied by probing */ \
> + /** User application callback for interrupts if present */ \
> + struct rte_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_dev_rxtx_callback **post_rx_burst_cbs; \
> + /** \
> + * User-supplied functions called from tx_burst to pre-process \
> + * received packets before passing them to the driver for transmission. \
> + */ \
> + struct rte_dev_rxtx_callback **pre_tx_burst_cbs; \
> + enum rte_dev_type dev_type; /**< Flag indicating the device type */ \
> + uint8_t attached /**< Flag indicating the port is attached */
> +
I'm also confused here. Looking at this, this is effectively a complete
renaming of the rte_eth_dev device structure. Once again, why not just leave
rte_eth_dev alone, and develop the crypto device from scratch? If your intent
is for it to have so much in common with an ethernet device that you can
effectively just rename the existing structure, why not just call it an ethernet
device?
> +/**
> + * @internal
> + * Common set of members for all devices included at the top of 'struct rte_xyz_dev_data'
> + */
> +#define RTE_COMMON_DEV_DATA \
> + char name[RTE_DEV_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. */ \
> + \
> + uint16_t mtu; /**< Maximum Transmission Unit. */ \
> + uint8_t unit_id; /**< Unit/Port ID for this instance */ \
> + uint8_t _pad0; /* alignment filler */ \
> + \
> + /* 64bit alignment starts here */ \
> + void *dev_private; /**< PMD-specific private data */ \
> + uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */ \
> + uint32_t min_rx_buf_size; /**< Common rx buffer size handled by all queues */ \
> + uint32_t _pad1 /**< Align to a 64bit boundary */
> +
> +/**
> + * @internal
> + * Common set of members for all devices included at the top of 'struct rte_xyz_dev_info'
> + */
> +#define RTE_COMMON_DEV_INFO \
> + struct rte_pci_device *pci_dev; /**< Device PCI information. */ \
> + const char *driver_name; /**< Device Driver name. */ \
> + unsigned int if_index; /**< Index to bound host interface, or 0 if none. */ \
> + /* Use if_indextoname() to translate into an interface name. */ \
> + uint32_t _pad0
> +
> +#define port_id unit_id
> +
> +/**
> + * @internal
> + * Common set of members for all devices included at the top of 'struct rte_xyz_global'
> + */
> +#define RTE_COMMON_GLOBAL(_t) \
> + struct rte_##_t##dev * devs;/**< Device information array */ \
> + struct rte_##_t##dev_data * data; /**< Device private data */ \
Again, if you're going to make something common, It doesn't make sense to then
give it a name that is going to be unique to the device class instantiating the
macro.
> + uint8_t nb_ports; /**< Number of ports/units found */ \
> + uint8_t max_ports; /**< Max number of ports or units */ \
> + uint16_t dflt_mtu; /**< Default MTU if needed by device */\
> + uint16_t dev_size; /**< Internal size of device struct */ \
> + uint16_t data_size; /**< Internal size of data structure */\
> + const char * mz_dev_data /**< String constant for device data */
> +
> +/**
> + * @internal
> + * Common overlay type structures with all devices.
> + */
> +struct rte_dev_info {
> + RTE_COMMON_DEV_INFO;
> +};
> +
> +/**
> + * @internal
> + * The generic data structure associated with each 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_dev {
> + RTE_COMMON_DEV();
> +};
> +
> +/**
> + * @internal
> + * The data part, with no function pointers, associated with each device.
> + *
> + * This structure is safe to place in shared memory to be common among different
> + * processes in a multi-process configuration.
> + */
> +struct rte_dev_data {
> + RTE_COMMON_DEV_DATA;
> +};
> +
> +/**
> + * @internal
> + * This structure is attempting to mirror the rte_xyz_global structures.
> + *
> + * Be careful as this structure is over layered on top of the xyzdev structures.
> + */
> +struct rte_dev_global {
> + RTE_COMMON_GLOBAL();
> +};
> +
I don't understand, you created macros that allow for naming specifics, then
don't use it? If your intent was for that data to be generically named (which
would make sense), then you should remove the parameter from the macro so that
can't change.
> +/**
> + * Get the rte_pkt_dev structure device pointer for the device.
> + *
> + * @param gbl pointer to device specific global structure.
> + * @param port_id Port ID value to select the device structure.
> + *
> + * @return
> + * - The rte_pkt_dev structure pointer for the given port ID.
> + */
> +static inline void *
> +rte_get_dev(void * _gbl, uint8_t port_id)
> +{
> + struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
> +
> + return (void *)((uint8_t *)gbl->devs + (port_id * gbl->dev_size));
> +}
> +
How will this be helpful? the caller will need to know what type of struct to
cast this to (since it can be named different for different dev classes).
doesn't that sort of thing undo the notion of commonality?
Neil
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported
2015-04-08 20:58 [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Keith Wiles
` (3 preceding siblings ...)
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 4/4] Update the rte_ethdev.[ch] files for the new device generic changes Keith Wiles
@ 2015-04-10 18:47 ` Wiles, Keith
4 siblings, 0 replies; 9+ messages in thread
From: Wiles, Keith @ 2015-04-10 18:47 UTC (permalink / raw)
To: Wiles, Keith, dev
Any comments on this RFC Patch? Must be perfect right :-)
On 4/8/15, 3:58 PM, "Keith Wiles" <keith.wiles@intel.com> wrote:
>Hi All,
>
>Here is a set of RFC patches to update DPDK to support a generic set of
>devices. The patches that follow create two files eal_common_device.h
>and rte_common_device.c and then a few items in rte_ethdev.[ch] were moved
>to cleanup those files by moving the device generic values and functions
>into a common set of device generic files.
>
>In the rte_common_device.h file are a couple of macros to abstract a few
>common items from rte_ethdev.h which are not ethernet specific. These
>generic device specific structure members are place into macros to be
>placed at the top of each new device type crypto, hardware offload, dpi,
>compression and possible others to be defined later.
> Note: could have used nested structures, but required a larger change
>set.
>
>I did not pull the Rx/Tx Routines into a common function, but it could be
>done if everyone agrees. It does not mean every device will use a common
>Rx/Tx routine. Without pulling Rx/Tx routines into a common file allows
>the device writer to have similar or different set of routines.
>
>These patches do not contain any new devices as these are being work on
>today.
>
>More cleanup of ethdev could be done to remove NIC specific features not
>supported in all devices and someone needs to do that cleanup IMO.
>
>The code is untested and I wanted to get something our for others to poke
>at
>today and more could be pulled out of ethdev as well. I/We will be looking
>at testing the code as we get more completed.
>
>I have not finished up the crypto APIs yet, but I am planning to work on
>those additions today. The crypto code we are using is the Quick Assist
>code
>found on 01.org, but we need to update the code to be move DPDK friendly.
>
>The QAT code does have a modified like API similar to Linux Kernel crypto
>API
>and I want to review that code first.
>
>Regards,
>++Keith
>
>Keith Wiles (4):
> Rename of device types to be generic device names.
> Add the new common device header and C file.
> Update files to build new device generic common files and headers.
> Update the rte_ethdev.[ch] files for the new device generic changes.
>
> app/test/test_link_bonding.c | 10 +-
> app/test/virtual_pmd.c | 4 +-
> examples/link_status_interrupt/main.c | 6 +-
> lib/librte_eal/bsdapp/eal/Makefile | 1 +
> lib/librte_eal/common/Makefile | 1 +
> lib/librte_eal/common/eal_common_device.c | 185 +++++++
> lib/librte_eal/common/include/rte_common_device.h | 617
>++++++++++++++++++++++
> lib/librte_eal/common/include/rte_log.h | 1 +
> lib/librte_eal/linuxapp/eal/Makefile | 1 +
> lib/librte_ether/rte_ethdev.c | 290 ++--------
> lib/librte_ether/rte_ethdev.h | 225 +++-----
> lib/librte_pmd_af_packet/rte_eth_af_packet.c | 2 +-
> lib/librte_pmd_bond/rte_eth_bond_api.c | 6 +-
> lib/librte_pmd_bond/rte_eth_bond_pmd.c | 12 +-
> lib/librte_pmd_bond/rte_eth_bond_private.h | 2 +-
> lib/librte_pmd_e1000/em_ethdev.c | 8 +-
> lib/librte_pmd_e1000/em_rxtx.c | 4 +-
> lib/librte_pmd_e1000/igb_ethdev.c | 2 +-
> lib/librte_pmd_i40e/i40e_ethdev.c | 4 +-
> lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 2 +-
> lib/librte_pmd_mlx4/mlx4.c | 2 +-
> lib/librte_pmd_null/rte_eth_null.c | 2 +-
> lib/librte_pmd_pcap/rte_eth_pcap.c | 2 +-
> lib/librte_pmd_ring/rte_eth_ring.c | 2 +-
> lib/librte_pmd_virtio/virtio_ethdev.c | 2 +-
> lib/librte_pmd_xenvirt/rte_eth_xenvirt.c | 2 +-
> 26 files changed, 969 insertions(+), 426 deletions(-)
> create mode 100644 lib/librte_eal/common/eal_common_device.c
> create mode 100644 lib/librte_eal/common/include/rte_common_device.h
>
>--
>2.3.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file.
2015-04-09 11:53 ` Neil Horman
@ 2015-04-10 19:25 ` Wiles, Keith
2015-04-12 13:08 ` Neil Horman
0 siblings, 1 reply; 9+ messages in thread
From: Wiles, Keith @ 2015-04-10 19:25 UTC (permalink / raw)
To: Neil Horman; +Cc: dev
On 4/9/15, 6:53 AM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
>On Wed, Apr 08, 2015 at 03:58:38PM -0500, Keith Wiles wrote:
>> Move a number of device specific define, structures and functions
>> into a generic device base set of files for all device not just
>>Ethernet.
>>
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> ---
>> lib/librte_eal/common/eal_common_device.c | 185 +++++++
>> lib/librte_eal/common/include/rte_common_device.h | 617
>>++++++++++++++++++++++
>> 2 files changed, 802 insertions(+)
>> create mode 100644 lib/librte_eal/common/eal_common_device.c
>> create mode 100644 lib/librte_eal/common/include/rte_common_device.h
>>
>> diff --git a/lib/librte_eal/common/eal_common_device.c
>>b/lib/librte_eal/common/eal_common_device.c
>> new file mode 100644
>> index 0000000..a9ef925
>> --- /dev/null
>> +++ b/lib/librte_eal/common/eal_common_device.c
>> @@ -0,0 +1,185 @@
>> +/*-
>> + * BSD LICENSE
>> + *
>> + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
>> + * Copyright(c) 2014 6WIND S.A.
>> + * All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * * Redistributions of source code must retain the above copyright
>> + * notice, this list of conditions and the following disclaimer.
>> + * * Redistributions in binary form must reproduce the above
>>copyright
>> + * notice, this list of conditions and the following disclaimer
>>in
>> + * the documentation and/or other materials provided with the
>> + * distribution.
>> + * * Neither the name of Intel Corporation nor the names of its
>> + * contributors may be used to endorse or promote products
>>derived
>> + * from this software without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
>>CONTRIBUTORS
>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
>>FOR
>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
>>COPYRIGHT
>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>>INCIDENTAL,
>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>>USE,
>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
>>ANY
>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
>>TORT
>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
>>USE
>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>>DAMAGE.
>> + */
>> +
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <inttypes.h>
>> +#include <sys/queue.h>
>> +
>> +#include <rte_dev.h>
>> +#include <rte_devargs.h>
>> +#include <rte_debug.h>
>> +#include <rte_devargs.h>
>> +#include <rte_log.h>
>> +#include <rte_malloc.h>
>> +#include <rte_errno.h>
>> +
>> +#include "rte_common_device.h"
>> +
>> +void *
>> +rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
>> + void * fn, void *user_param)
>> +{
>> + struct rte_dev_rxtx_callback *cb;
>> +
>> + cb = rte_zmalloc(NULL, sizeof(*cb), 0);
>> +
>> + if (cb == NULL) {
>> + rte_errno = ENOMEM;
>> + return NULL;
>> + }
>> +
>> + cb->fn.vp = fn;
>> + cb->param = user_param;
>> + cb->next = *cbp;
>> + *cbp = cb;
>> + return cb;
>> +}
>> +
>> +int
>> +rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
>> + struct rte_dev_rxtx_callback *user_cb)
>> +{
>> + struct rte_dev_rxtx_callback *cb = *cbp;
>> + struct rte_dev_rxtx_callback *prev_cb;
>> +
>> + /* Reset head pointer and remove user cb if first in the list. */
>> + if (cb == user_cb) {
>> + *cbp = user_cb->next;
>> + return 0;
>> + }
>> +
>> + /* Remove the user cb from the callback list. */
>> + do {
>> + prev_cb = cb;
>> + cb = cb->next;
>> +
>> + if (cb == user_cb) {
>> + prev_cb->next = user_cb->next;
>> + return 0;
>> + }
>> + } while (cb != NULL);
>> +
>> + /* Callback wasn't found. */
>> + return (-EINVAL);
>> +}
>> +
>> +int
>> +rte_dev_callback_register(struct rte_dev_cb_list * cb_list,
>> + rte_spinlock_t * lock,
>> + enum rte_dev_event_type event,
>> + rte_dev_cb_fn cb_fn, void *cb_arg)
>> +{
>> + struct rte_dev_callback *cb;
>> +
>> + rte_spinlock_lock(lock);
>> +
>> + TAILQ_FOREACH(cb, cb_list, next) {
>> + if (cb->cb_fn == cb_fn &&
>> + cb->cb_arg == cb_arg &&
>> + cb->event == event) {
>> + break;
>> + }
>> + }
>> +
>> + /* create a new callback. */
>> + if (cb == NULL && (cb = rte_zmalloc("INTR_USER_CALLBACK",
>> + sizeof(struct rte_dev_callback), 0)) != NULL) {
>> + cb->cb_fn = cb_fn;
>> + cb->cb_arg = cb_arg;
>> + cb->event = event;
>> + TAILQ_INSERT_TAIL(cb_list, cb, next);
>> + }
>> +
>> + rte_spinlock_unlock(lock);
>> + return ((cb == NULL) ? -ENOMEM : 0);
>> +}
>> +
>> +int
>> +rte_dev_callback_unregister(struct rte_dev_cb_list * cb_list,
>> + rte_spinlock_t * lock,
>> + enum rte_dev_event_type event,
>> + rte_dev_cb_fn cb_fn, void *cb_arg)
>> +{
>> + int ret;
>> + struct rte_dev_callback *cb, *next;
>> +
>> + rte_spinlock_lock(lock);
>> +
>> + ret = 0;
>> + for (cb = TAILQ_FIRST(cb_list); cb != NULL; cb = next) {
>> +
>> + next = TAILQ_NEXT(cb, next);
>> +
>> + if (cb->cb_fn != cb_fn || cb->event != event ||
>> + (cb->cb_arg != (void *)-1 &&
>> + cb->cb_arg != cb_arg))
>> + continue;
>> +
>> + /*
>> + * if this callback is not executing right now,
>> + * then remove it.
>> + */
>> + if (cb->active == 0) {
>> + TAILQ_REMOVE(cb_list, cb, next);
>> + rte_free(cb);
>> + } else {
>> + ret = -EAGAIN;
>> + }
>> + }
>> +
>> + rte_spinlock_unlock(lock);
>> + return (ret);
>> +}
>> +
>> +void
>> +rte_dev_callback_process(struct rte_dev_cb_list * cb_list, uint8_t
>>port_id,
>> + enum rte_dev_event_type event, rte_spinlock_t *lock)
>> +{
>> + struct rte_dev_callback *cb;
>> + struct rte_dev_callback dev_cb;
>> +
>> + rte_spinlock_lock(lock);
>> + TAILQ_FOREACH(cb, cb_list, next) {
>> + if (cb->cb_fn == NULL || cb->event != event)
>> + continue;
>> + dev_cb = *cb;
>> + cb->active = 1;
>> + rte_spinlock_unlock(lock);
>> + dev_cb.cb_fn(port_id, dev_cb.event, dev_cb.cb_arg);
>> + rte_spinlock_lock(lock);
>> + cb->active = 0;
>> + }
>> + rte_spinlock_unlock(lock);
>> +}
>
>
>This is a bit...odd. The rte_eth callbacks had some context because you
>knew
>when the callback was going to be issued (at the end of an rx and tx
>operation).
>Here, by making the process routine generic, you're removed that context,
>and so
>the caller has no guarantee as to when their callbacks will be called.
>If its
>to be done at the end of the generic transmit and routine functions, that
>would
>be fine, but I don't see that happening here.
No able to follow you here as this routine is called from the rte_ethdev.c
_rte_eth_dev_callback_process(). We could remove the
_rte_eth_dev_callback_process function and have the rest of the code call
this routine instead. No context is lost here and the callbacks are called
in the same location as before.
>
>> diff --git a/lib/librte_eal/common/include/rte_common_device.h
>>b/lib/librte_eal/common/include/rte_common_device.h
>> new file mode 100644
>> index 0000000..5baefb6
>> --- /dev/null
>> +++ b/lib/librte_eal/common/include/rte_common_device.h
>> @@ -0,0 +1,617 @@
>> +/*-
>> + * BSD LICENSE
>> + *
>> + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
>> + * All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * * Redistributions of source code must retain the above copyright
>> + * notice, this list of conditions and the following disclaimer.
>> + * * Redistributions in binary form must reproduce the above
>>copyright
>> + * notice, this list of conditions and the following disclaimer
>>in
>> + * the documentation and/or other materials provided with the
>> + * distribution.
>> + * * Neither the name of Intel Corporation nor the names of its
>> + * contributors may be used to endorse or promote products
>>derived
>> + * from this software without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
>>CONTRIBUTORS
>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
>>FOR
>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
>>COPYRIGHT
>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>>INCIDENTAL,
>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>>USE,
>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
>>ANY
>> + * THEORY OF LIABILITY, WHPDF IN CONTRACT, STRICT LIABILITY, OR TORT
>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
>>USE
>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>>DAMAGE.
>> + */
>> +
>> +#ifndef _RTE_COMMON_DEVICE_H_
>> +#define _RTE_COMMON_DEVICE_H_
>> +
>> +/**
>> + * @file
>> + *
>> + * Common Device Helpers in RTE
>> + */
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <rte_spinlock.h>
>> +
>> +/* Macros for checking for restricting functions to primary instance
>>only */
>> +#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
>> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
>> + PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
>> + return (retval); \
>> + } \
>> +} while(0)
>> +#define PROC_PRIMARY_OR_RET() do { \
>> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
>> + PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
>> + return; \
>> + } \
>> +} while(0)
>> +
>> +/* Macros to check for invalid function pointers in dev_ops structure
>>*/
>> +#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
>> + if ((func) == NULL) { \
>> + PMD_DEBUG_TRACE("Function not supported\n"); \
>> + return (retval); \
>> + } \
>> +} while(0)
>> +#define FUNC_PTR_OR_RET(func) do { \
>> + if ((func) == NULL) { \
>> + PMD_DEBUG_TRACE("Function not supported\n"); \
>> + return; \
>> + } \
>> +} while(0)
>> +
>> +enum {
>> + DEV_DETACHED = 0,
>> + DEV_ATTACHED
>> +};
>> +
>> +/*
>> + * The device type
>> + */
>> +enum rte_dev_type {
>> + RTE_DEV_UNKNOWN, /**< unknown device type */
>> + RTE_DEV_PCI,
>> + /**< Physical function and Virtual function of PCI devices */
>> + RTE_DEV_VIRTUAL, /**< non hardware device */
>> + RTE_DEV_MAX /**< max value of this enum */
>> +};
>> +
>> +/**
>> + * The device event type for interrupt, and maybe others in the future.
>> + */
>> +enum rte_dev_event_type {
>> + RTE_DEV_EVENT_UNKNOWN, /**< unknown event type */
>> + RTE_DEV_EVENT_INTR_LSC, /**< lsc interrupt event */
>> + RTE_DEV_EVENT_MAX /**< max value of this enum */
>> +};
>> +
>> +struct rte_dev_callback;
>> +
>> +/** @internal Structure to keep track of registered callback */
>> +TAILQ_HEAD(rte_dev_cb_list, rte_dev_callback);
>> +
>> +struct rte_mbuf;
>> +
>> +typedef uint16_t (*dev_rx_burst_t)(void *rxq,
>> + struct rte_mbuf **rx_pkts,
>> + uint16_t nb_pkts);
>> +/**< @internal Retrieve input packets from a receive queue of a
>>device. */
>> +
>> +typedef uint16_t (*dev_tx_burst_t)(void *txq,
>> + struct rte_mbuf **tx_pkts,
>> + uint16_t nb_pkts);
>> +/**< @internal Send output packets on a transmit queue of a device. */
>> +
>> +typedef void (*rte_dev_cb_fn)(uint8_t port_id, \
>> + enum rte_dev_event_type event, void *cb_arg);
>> +/**< user application callback to be registered for interrupts */
>> +
>> +#define RTE_DEV_NAME_MAX_LEN (32)
>> +
>> +/**
>> + * @internal
>> + * Common set of members for all devices included at the top of
>>'struct rte_xyz_dev'
>> + */
>> +#define RTE_COMMON_DEV(_t)
>> \
>> + dev_rx_burst_t rx_pkt_burst; /**< Pointer to PMD
>>receive function. */ \
>> + dev_tx_burst_t tx_pkt_burst; /**< Pointer to PMD
>>transmit function. */ \
>> + struct rte_##_t##dev_data *data; /**< Pointer to device
>>data */ \
>> + struct rte_##_t##global *gbl_data; /**< Pointer to device
>>global data */ \
>> + const struct _t##driver *driver; /**< Driver for this
>>device */ \
>> + struct _t##dev_ops *dev_ops; /**< Functions
>>exported by PMD */ \
>
>This doesn't make any sense to me. You've made a generic macro that
>creates
>what is ostensibly supposed to be common code point, but then you include
>some
>string concatenation so that the common parts are actually unique to a
>given
>device class. Why would you do that? At that point the common parts
>aren't
>common by definition, and should go in a structure specific to that
>device class
The reason for these being specific to the device is the device may have
extra members private to the device. I could have added a private pointer
to the structure to make then generic to allow the device to have its own
private members.
>
>> + struct rte_pci_device *pci_dev; /**< PCI info.
>>supplied by probing */ \
>> + /** User application callback for interrupts if present */
>> \
>> + struct rte_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_dev_rxtx_callback **post_rx_burst_cbs;
>> \
>> + /**
>> \
>> + * User-supplied functions called from tx_burst to pre-process
>> \
>> + * received packets before passing them to the driver for
>>transmission. \
>> + */
>> \
>> + struct rte_dev_rxtx_callback **pre_tx_burst_cbs;
>> \
>> + enum rte_dev_type dev_type; /**< Flag indicating the
>>device type */ \
>> + uint8_t attached /**< Flag indicating
>>the port is attached */
>> +
>
>I'm also confused here. Looking at this, this is effectively a complete
>renaming of the rte_eth_dev device structure. Once again, why not just
>leave
>rte_eth_dev alone, and develop the crypto device from scratch? If your
>intent
>is for it to have so much in common with an ethernet device that you can
>effectively just rename the existing structure, why not just call it an
>ethernet
>device?
My goal was not to have everything common to a ethdev, but to move items
out of the ethdev we can have in common. I feel you are not understanding
something and I can not figure out what it is. If I remove the concat part
maybe this will get my point across better.
>
>> +/**
>> + * @internal
>> + * Common set of members for all devices included at the top of
>>'struct rte_xyz_dev_data'
>> + */
>> +#define RTE_COMMON_DEV_DATA
>> \
>> + char name[RTE_DEV_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. */
>> \
>> +
>> \
>> + uint16_t mtu; /**< Maximum Transmission Unit. */
>> \
>> + uint8_t unit_id; /**< Unit/Port ID for this
>>instance */ \
>> + uint8_t _pad0; /* alignment filler */
>> \
>> +
>> \
>> + /* 64bit alignment starts here */
>> \
>> + void *dev_private; /**< PMD-specific private data */
>> \
>> + uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation
>>failures. */ \
>> + uint32_t min_rx_buf_size; /**< Common rx buffer size handled
>>by all queues */ \
>> + uint32_t _pad1 /**< Align to a 64bit boundary */
>> +
>> +/**
>> + * @internal
>> + * Common set of members for all devices included at the top of
>>'struct rte_xyz_dev_info'
>> + */
>> +#define RTE_COMMON_DEV_INFO
>> \
>> + struct rte_pci_device *pci_dev; /**< Device PCI
>>information. */ \
>> + const char *driver_name; /**< Device Driver name.
>>*/ \
>> + unsigned int if_index; /**< Index to bound host
>>interface, or 0 if none. */ \
>> + /* Use if_indextoname() to translate into an interface name.
>>*/ \
>> + uint32_t _pad0
>> +
>> +#define port_id unit_id
>> +
>> +/**
>> + * @internal
>> + * Common set of members for all devices included at the top of
>>'struct rte_xyz_global'
>> + */
>> +#define RTE_COMMON_GLOBAL(_t)
>> \
>> + struct rte_##_t##dev * devs;/**< Device information array */
>>\
>> + struct rte_##_t##dev_data * data; /**< Device private data */
>>\
>Again, if you're going to make something common, It doesn't make sense to
>then
>give it a name that is going to be unique to the device class
>instantiating the
>macro.
OK, will try to update the code to be non-specific to a given device.
>
>> + uint8_t nb_ports; /**< Number of ports/units found */
>> \
>> + uint8_t max_ports; /**< Max number of ports or units */
>> \
>> + uint16_t dflt_mtu; /**< Default MTU if needed by device
>>*/\
>> + uint16_t dev_size; /**< Internal size of device struct
>>*/ \
>> + uint16_t data_size; /**< Internal size of data structure
>>*/\
>> + const char * mz_dev_data /**< String constant for device data
>>*/
>> +
>> +/**
>> + * @internal
>> + * Common overlay type structures with all devices.
>> + */
>> +struct rte_dev_info {
>> + RTE_COMMON_DEV_INFO;
>> +};
>> +
>> +/**
>> + * @internal
>> + * The generic data structure associated with each 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_dev {
>> + RTE_COMMON_DEV();
>> +};
>> +
>> +/**
>> + * @internal
>> + * The data part, with no function pointers, associated with each
>>device.
>> + *
>> + * This structure is safe to place in shared memory to be common among
>>different
>> + * processes in a multi-process configuration.
>> + */
>> +struct rte_dev_data {
>> + RTE_COMMON_DEV_DATA;
>> +};
>> +
>> +/**
>> + * @internal
>> + * This structure is attempting to mirror the rte_xyz_global
>>structures.
>> + *
>> + * Be careful as this structure is over layered on top of the xyzdev
>>structures.
>> + */
>> +struct rte_dev_global {
>> + RTE_COMMON_GLOBAL();
>> +};
>> +
>I don't understand, you created macros that allow for naming specifics,
>then
>don't use it? If your intent was for that data to be generically named
>(which
>would make sense), then you should remove the parameter from the macro so
>that
>can't change.
>
>> +/**
>> + * Get the rte_pkt_dev structure device pointer for the device.
>> + *
>> + * @param gbl pointer to device specific global structure.
>> + * @param port_id Port ID value to select the device structure.
>> + *
>> + * @return
>> + * - The rte_pkt_dev structure pointer for the given port ID.
>> + */
>> +static inline void *
>> +rte_get_dev(void * _gbl, uint8_t port_id)
>> +{
>> + struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
>> +
>> + return (void *)((uint8_t *)gbl->devs + (port_id * gbl->dev_size));
>> +}
>> +
>How will this be helpful? the caller will need to know what type of
>struct to
>cast this to (since it can be named different for different dev classes).
>doesn't that sort of thing undo the notion of commonality?
Not really the point was to have a routine able to return the correct
device structure pointer and the developer should understand which global
structure he is passing into the routine and what type is being returned.
>
>Neil
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file.
2015-04-10 19:25 ` Wiles, Keith
@ 2015-04-12 13:08 ` Neil Horman
0 siblings, 0 replies; 9+ messages in thread
From: Neil Horman @ 2015-04-12 13:08 UTC (permalink / raw)
To: Wiles, Keith; +Cc: dev
On Fri, Apr 10, 2015 at 07:25:32PM +0000, Wiles, Keith wrote:
>
>
> On 4/9/15, 6:53 AM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
>
> >On Wed, Apr 08, 2015 at 03:58:38PM -0500, Keith Wiles wrote:
> >> Move a number of device specific define, structures and functions
> >> into a generic device base set of files for all device not just
> >>Ethernet.
> >>
> >> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> >> ---
> >> lib/librte_eal/common/eal_common_device.c | 185 +++++++
> >> lib/librte_eal/common/include/rte_common_device.h | 617
> >>++++++++++++++++++++++
> >> 2 files changed, 802 insertions(+)
> >> create mode 100644 lib/librte_eal/common/eal_common_device.c
> >> create mode 100644 lib/librte_eal/common/include/rte_common_device.h
> >>
> >> diff --git a/lib/librte_eal/common/eal_common_device.c
> >>b/lib/librte_eal/common/eal_common_device.c
> >> new file mode 100644
> >> index 0000000..a9ef925
> >> --- /dev/null
> >> +++ b/lib/librte_eal/common/eal_common_device.c
> >> @@ -0,0 +1,185 @@
> >> +/*-
> >> + * BSD LICENSE
> >> + *
> >> + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> >> + * Copyright(c) 2014 6WIND S.A.
> >> + * All rights reserved.
> >> + *
> >> + * Redistribution and use in source and binary forms, with or without
> >> + * modification, are permitted provided that the following conditions
> >> + * are met:
> >> + *
> >> + * * Redistributions of source code must retain the above copyright
> >> + * notice, this list of conditions and the following disclaimer.
> >> + * * Redistributions in binary form must reproduce the above
> >>copyright
> >> + * notice, this list of conditions and the following disclaimer
> >>in
> >> + * the documentation and/or other materials provided with the
> >> + * distribution.
> >> + * * Neither the name of Intel Corporation nor the names of its
> >> + * contributors may be used to endorse or promote products
> >>derived
> >> + * from this software without specific prior written permission.
> >> + *
> >> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> >>CONTRIBUTORS
> >> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> >>FOR
> >> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> >>COPYRIGHT
> >> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> >>INCIDENTAL,
> >> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> >>USE,
> >> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> >>ANY
> >> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> >>TORT
> >> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> >>USE
> >> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> >>DAMAGE.
> >> + */
> >> +
> >> +#include <stdio.h>
> >> +#include <string.h>
> >> +#include <inttypes.h>
> >> +#include <sys/queue.h>
> >> +
> >> +#include <rte_dev.h>
> >> +#include <rte_devargs.h>
> >> +#include <rte_debug.h>
> >> +#include <rte_devargs.h>
> >> +#include <rte_log.h>
> >> +#include <rte_malloc.h>
> >> +#include <rte_errno.h>
> >> +
> >> +#include "rte_common_device.h"
> >> +
> >> +void *
> >> +rte_dev_add_callback(struct rte_dev_rxtx_callback ** cbp,
> >> + void * fn, void *user_param)
> >> +{
> >> + struct rte_dev_rxtx_callback *cb;
> >> +
> >> + cb = rte_zmalloc(NULL, sizeof(*cb), 0);
> >> +
> >> + if (cb == NULL) {
> >> + rte_errno = ENOMEM;
> >> + return NULL;
> >> + }
> >> +
> >> + cb->fn.vp = fn;
> >> + cb->param = user_param;
> >> + cb->next = *cbp;
> >> + *cbp = cb;
> >> + return cb;
> >> +}
> >> +
> >> +int
> >> +rte_dev_remove_callback(struct rte_dev_rxtx_callback ** cbp,
> >> + struct rte_dev_rxtx_callback *user_cb)
> >> +{
> >> + struct rte_dev_rxtx_callback *cb = *cbp;
> >> + struct rte_dev_rxtx_callback *prev_cb;
> >> +
> >> + /* Reset head pointer and remove user cb if first in the list. */
> >> + if (cb == user_cb) {
> >> + *cbp = user_cb->next;
> >> + return 0;
> >> + }
> >> +
> >> + /* Remove the user cb from the callback list. */
> >> + do {
> >> + prev_cb = cb;
> >> + cb = cb->next;
> >> +
> >> + if (cb == user_cb) {
> >> + prev_cb->next = user_cb->next;
> >> + return 0;
> >> + }
> >> + } while (cb != NULL);
> >> +
> >> + /* Callback wasn't found. */
> >> + return (-EINVAL);
> >> +}
> >> +
> >> +int
> >> +rte_dev_callback_register(struct rte_dev_cb_list * cb_list,
> >> + rte_spinlock_t * lock,
> >> + enum rte_dev_event_type event,
> >> + rte_dev_cb_fn cb_fn, void *cb_arg)
> >> +{
> >> + struct rte_dev_callback *cb;
> >> +
> >> + rte_spinlock_lock(lock);
> >> +
> >> + TAILQ_FOREACH(cb, cb_list, next) {
> >> + if (cb->cb_fn == cb_fn &&
> >> + cb->cb_arg == cb_arg &&
> >> + cb->event == event) {
> >> + break;
> >> + }
> >> + }
> >> +
> >> + /* create a new callback. */
> >> + if (cb == NULL && (cb = rte_zmalloc("INTR_USER_CALLBACK",
> >> + sizeof(struct rte_dev_callback), 0)) != NULL) {
> >> + cb->cb_fn = cb_fn;
> >> + cb->cb_arg = cb_arg;
> >> + cb->event = event;
> >> + TAILQ_INSERT_TAIL(cb_list, cb, next);
> >> + }
> >> +
> >> + rte_spinlock_unlock(lock);
> >> + return ((cb == NULL) ? -ENOMEM : 0);
> >> +}
> >> +
> >> +int
> >> +rte_dev_callback_unregister(struct rte_dev_cb_list * cb_list,
> >> + rte_spinlock_t * lock,
> >> + enum rte_dev_event_type event,
> >> + rte_dev_cb_fn cb_fn, void *cb_arg)
> >> +{
> >> + int ret;
> >> + struct rte_dev_callback *cb, *next;
> >> +
> >> + rte_spinlock_lock(lock);
> >> +
> >> + ret = 0;
> >> + for (cb = TAILQ_FIRST(cb_list); cb != NULL; cb = next) {
> >> +
> >> + next = TAILQ_NEXT(cb, next);
> >> +
> >> + if (cb->cb_fn != cb_fn || cb->event != event ||
> >> + (cb->cb_arg != (void *)-1 &&
> >> + cb->cb_arg != cb_arg))
> >> + continue;
> >> +
> >> + /*
> >> + * if this callback is not executing right now,
> >> + * then remove it.
> >> + */
> >> + if (cb->active == 0) {
> >> + TAILQ_REMOVE(cb_list, cb, next);
> >> + rte_free(cb);
> >> + } else {
> >> + ret = -EAGAIN;
> >> + }
> >> + }
> >> +
> >> + rte_spinlock_unlock(lock);
> >> + return (ret);
> >> +}
> >> +
> >> +void
> >> +rte_dev_callback_process(struct rte_dev_cb_list * cb_list, uint8_t
> >>port_id,
> >> + enum rte_dev_event_type event, rte_spinlock_t *lock)
> >> +{
> >> + struct rte_dev_callback *cb;
> >> + struct rte_dev_callback dev_cb;
> >> +
> >> + rte_spinlock_lock(lock);
> >> + TAILQ_FOREACH(cb, cb_list, next) {
> >> + if (cb->cb_fn == NULL || cb->event != event)
> >> + continue;
> >> + dev_cb = *cb;
> >> + cb->active = 1;
> >> + rte_spinlock_unlock(lock);
> >> + dev_cb.cb_fn(port_id, dev_cb.event, dev_cb.cb_arg);
> >> + rte_spinlock_lock(lock);
> >> + cb->active = 0;
> >> + }
> >> + rte_spinlock_unlock(lock);
> >> +}
> >
> >
> >This is a bit...odd. The rte_eth callbacks had some context because you
> >knew
> >when the callback was going to be issued (at the end of an rx and tx
> >operation).
> >Here, by making the process routine generic, you're removed that context,
> >and so
> >the caller has no guarantee as to when their callbacks will be called.
> >If its
> >to be done at the end of the generic transmit and routine functions, that
> >would
> >be fine, but I don't see that happening here.
>
> No able to follow you here as this routine is called from the rte_ethdev.c
> _rte_eth_dev_callback_process(). We could remove the
> _rte_eth_dev_callback_process function and have the rest of the code call
> this routine instead. No context is lost here and the callbacks are called
> in the same location as before.
>
Its ok, ignore me here. I hadn't looked ahead to how you used this in the
ethdev case, that makes sense. Though I'm still a bit concerned about how this
might be used in other device cases, but that can wait until you post the QAT
RFC patches.
> >
> >> diff --git a/lib/librte_eal/common/include/rte_common_device.h
> >>b/lib/librte_eal/common/include/rte_common_device.h
> >> new file mode 100644
> >> index 0000000..5baefb6
> >> --- /dev/null
> >> +++ b/lib/librte_eal/common/include/rte_common_device.h
> >> @@ -0,0 +1,617 @@
> >> +/*-
> >> + * BSD LICENSE
> >> + *
> >> + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> >> + * All rights reserved.
> >> + *
> >> + * Redistribution and use in source and binary forms, with or without
> >> + * modification, are permitted provided that the following conditions
> >> + * are met:
> >> + *
> >> + * * Redistributions of source code must retain the above copyright
> >> + * notice, this list of conditions and the following disclaimer.
> >> + * * Redistributions in binary form must reproduce the above
> >>copyright
> >> + * notice, this list of conditions and the following disclaimer
> >>in
> >> + * the documentation and/or other materials provided with the
> >> + * distribution.
> >> + * * Neither the name of Intel Corporation nor the names of its
> >> + * contributors may be used to endorse or promote products
> >>derived
> >> + * from this software without specific prior written permission.
> >> + *
> >> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> >>CONTRIBUTORS
> >> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> >>FOR
> >> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> >>COPYRIGHT
> >> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> >>INCIDENTAL,
> >> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> >>USE,
> >> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> >>ANY
> >> + * THEORY OF LIABILITY, WHPDF IN CONTRACT, STRICT LIABILITY, OR TORT
> >> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> >>USE
> >> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> >>DAMAGE.
> >> + */
> >> +
> >> +#ifndef _RTE_COMMON_DEVICE_H_
> >> +#define _RTE_COMMON_DEVICE_H_
> >> +
> >> +/**
> >> + * @file
> >> + *
> >> + * Common Device Helpers in RTE
> >> + */
> >> +
> >> +#ifdef __cplusplus
> >> +extern "C" {
> >> +#endif
> >> +
> >> +#include <stdint.h>
> >> +#include <stdio.h>
> >> +#include <string.h>
> >> +#include <rte_spinlock.h>
> >> +
> >> +/* Macros for checking for restricting functions to primary instance
> >>only */
> >> +#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
> >> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> >> + PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> >> + return (retval); \
> >> + } \
> >> +} while(0)
> >> +#define PROC_PRIMARY_OR_RET() do { \
> >> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> >> + PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> >> + return; \
> >> + } \
> >> +} while(0)
> >> +
> >> +/* Macros to check for invalid function pointers in dev_ops structure
> >>*/
> >> +#define FUNC_PTR_OR_ERR_RET(func, retval) do { \
> >> + if ((func) == NULL) { \
> >> + PMD_DEBUG_TRACE("Function not supported\n"); \
> >> + return (retval); \
> >> + } \
> >> +} while(0)
> >> +#define FUNC_PTR_OR_RET(func) do { \
> >> + if ((func) == NULL) { \
> >> + PMD_DEBUG_TRACE("Function not supported\n"); \
> >> + return; \
> >> + } \
> >> +} while(0)
> >> +
> >> +enum {
> >> + DEV_DETACHED = 0,
> >> + DEV_ATTACHED
> >> +};
> >> +
> >> +/*
> >> + * The device type
> >> + */
> >> +enum rte_dev_type {
> >> + RTE_DEV_UNKNOWN, /**< unknown device type */
> >> + RTE_DEV_PCI,
> >> + /**< Physical function and Virtual function of PCI devices */
> >> + RTE_DEV_VIRTUAL, /**< non hardware device */
> >> + RTE_DEV_MAX /**< max value of this enum */
> >> +};
> >> +
> >> +/**
> >> + * The device event type for interrupt, and maybe others in the future.
> >> + */
> >> +enum rte_dev_event_type {
> >> + RTE_DEV_EVENT_UNKNOWN, /**< unknown event type */
> >> + RTE_DEV_EVENT_INTR_LSC, /**< lsc interrupt event */
> >> + RTE_DEV_EVENT_MAX /**< max value of this enum */
> >> +};
> >> +
> >> +struct rte_dev_callback;
> >> +
> >> +/** @internal Structure to keep track of registered callback */
> >> +TAILQ_HEAD(rte_dev_cb_list, rte_dev_callback);
> >> +
> >> +struct rte_mbuf;
> >> +
> >> +typedef uint16_t (*dev_rx_burst_t)(void *rxq,
> >> + struct rte_mbuf **rx_pkts,
> >> + uint16_t nb_pkts);
> >> +/**< @internal Retrieve input packets from a receive queue of a
> >>device. */
> >> +
> >> +typedef uint16_t (*dev_tx_burst_t)(void *txq,
> >> + struct rte_mbuf **tx_pkts,
> >> + uint16_t nb_pkts);
> >> +/**< @internal Send output packets on a transmit queue of a device. */
> >> +
> >> +typedef void (*rte_dev_cb_fn)(uint8_t port_id, \
> >> + enum rte_dev_event_type event, void *cb_arg);
> >> +/**< user application callback to be registered for interrupts */
> >> +
> >> +#define RTE_DEV_NAME_MAX_LEN (32)
> >> +
> >> +/**
> >> + * @internal
> >> + * Common set of members for all devices included at the top of
> >>'struct rte_xyz_dev'
> >> + */
> >> +#define RTE_COMMON_DEV(_t)
> >> \
> >> + dev_rx_burst_t rx_pkt_burst; /**< Pointer to PMD
> >>receive function. */ \
> >> + dev_tx_burst_t tx_pkt_burst; /**< Pointer to PMD
> >>transmit function. */ \
> >> + struct rte_##_t##dev_data *data; /**< Pointer to device
> >>data */ \
> >> + struct rte_##_t##global *gbl_data; /**< Pointer to device
> >>global data */ \
> >> + const struct _t##driver *driver; /**< Driver for this
> >>device */ \
> >> + struct _t##dev_ops *dev_ops; /**< Functions
> >>exported by PMD */ \
> >
> >This doesn't make any sense to me. You've made a generic macro that
> >creates
> >what is ostensibly supposed to be common code point, but then you include
> >some
> >string concatenation so that the common parts are actually unique to a
> >given
> >device class. Why would you do that? At that point the common parts
> >aren't
> >common by definition, and should go in a structure specific to that
> >device class
>
> The reason for these being specific to the device is the device may have
> extra members private to the device. I could have added a private pointer
> to the structure to make then generic to allow the device to have its own
> private members.
>
Well, yes, that would make more sense to me. The macro is called
RTE_COMMON_DEV. It seems like it should only create members that are common to
all devices, with common names.
> >
> >> + struct rte_pci_device *pci_dev; /**< PCI info.
> >>supplied by probing */ \
> >> + /** User application callback for interrupts if present */
> >> \
> >> + struct rte_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_dev_rxtx_callback **post_rx_burst_cbs;
> >> \
> >> + /**
> >> \
> >> + * User-supplied functions called from tx_burst to pre-process
> >> \
> >> + * received packets before passing them to the driver for
> >>transmission. \
> >> + */
> >> \
> >> + struct rte_dev_rxtx_callback **pre_tx_burst_cbs;
> >> \
> >> + enum rte_dev_type dev_type; /**< Flag indicating the
> >>device type */ \
> >> + uint8_t attached /**< Flag indicating
> >>the port is attached */
> >> +
> >
> >I'm also confused here. Looking at this, this is effectively a complete
> >renaming of the rte_eth_dev device structure. Once again, why not just
> >leave
> >rte_eth_dev alone, and develop the crypto device from scratch? If your
> >intent
> >is for it to have so much in common with an ethernet device that you can
> >effectively just rename the existing structure, why not just call it an
> >ethernet
> >device?
>
> My goal was not to have everything common to a ethdev, but to move items
> out of the ethdev we can have in common. I feel you are not understanding
> something and I can not figure out what it is. If I remove the concat part
> maybe this will get my point across better.
>
I know what your goal is. All I'm saying is that, in pursuing that goal,
you seem to have decided that all the data in the rte_ethdev is common to all
devices.
> >
> >> +/**
> >> + * @internal
> >> + * Common set of members for all devices included at the top of
> >>'struct rte_xyz_dev_data'
> >> + */
> >> +#define RTE_COMMON_DEV_DATA
> >> \
> >> + char name[RTE_DEV_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. */
> >> \
> >> +
> >> \
> >> + uint16_t mtu; /**< Maximum Transmission Unit. */
> >> \
> >> + uint8_t unit_id; /**< Unit/Port ID for this
> >>instance */ \
> >> + uint8_t _pad0; /* alignment filler */
> >> \
> >> +
> >> \
> >> + /* 64bit alignment starts here */
> >> \
> >> + void *dev_private; /**< PMD-specific private data */
> >> \
> >> + uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation
> >>failures. */ \
> >> + uint32_t min_rx_buf_size; /**< Common rx buffer size handled
> >>by all queues */ \
> >> + uint32_t _pad1 /**< Align to a 64bit boundary */
> >> +
> >> +/**
> >> + * @internal
> >> + * Common set of members for all devices included at the top of
> >>'struct rte_xyz_dev_info'
> >> + */
> >> +#define RTE_COMMON_DEV_INFO
> >> \
> >> + struct rte_pci_device *pci_dev; /**< Device PCI
> >>information. */ \
> >> + const char *driver_name; /**< Device Driver name.
> >>*/ \
> >> + unsigned int if_index; /**< Index to bound host
> >>interface, or 0 if none. */ \
> >> + /* Use if_indextoname() to translate into an interface name.
> >>*/ \
> >> + uint32_t _pad0
> >> +
> >> +#define port_id unit_id
> >> +
> >> +/**
> >> + * @internal
> >> + * Common set of members for all devices included at the top of
> >>'struct rte_xyz_global'
> >> + */
> >> +#define RTE_COMMON_GLOBAL(_t)
> >> \
> >> + struct rte_##_t##dev * devs;/**< Device information array */
> >>\
> >> + struct rte_##_t##dev_data * data; /**< Device private data */
> >>\
> >Again, if you're going to make something common, It doesn't make sense to
> >then
> >give it a name that is going to be unique to the device class
> >instantiating the
> >macro.
>
> OK, will try to update the code to be non-specific to a given device.
>
Thank you.
> >
> >> + uint8_t nb_ports; /**< Number of ports/units found */
> >> \
> >> + uint8_t max_ports; /**< Max number of ports or units */
> >> \
> >> + uint16_t dflt_mtu; /**< Default MTU if needed by device
> >>*/\
> >> + uint16_t dev_size; /**< Internal size of device struct
> >>*/ \
> >> + uint16_t data_size; /**< Internal size of data structure
> >>*/\
> >> + const char * mz_dev_data /**< String constant for device data
> >>*/
> >> +
> >> +/**
> >> + * @internal
> >> + * Common overlay type structures with all devices.
> >> + */
> >> +struct rte_dev_info {
> >> + RTE_COMMON_DEV_INFO;
> >> +};
> >> +
> >> +/**
> >> + * @internal
> >> + * The generic data structure associated with each 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_dev {
> >> + RTE_COMMON_DEV();
> >> +};
> >> +
> >> +/**
> >> + * @internal
> >> + * The data part, with no function pointers, associated with each
> >>device.
> >> + *
> >> + * This structure is safe to place in shared memory to be common among
> >>different
> >> + * processes in a multi-process configuration.
> >> + */
> >> +struct rte_dev_data {
> >> + RTE_COMMON_DEV_DATA;
> >> +};
> >> +
> >> +/**
> >> + * @internal
> >> + * This structure is attempting to mirror the rte_xyz_global
> >>structures.
> >> + *
> >> + * Be careful as this structure is over layered on top of the xyzdev
> >>structures.
> >> + */
> >> +struct rte_dev_global {
> >> + RTE_COMMON_GLOBAL();
> >> +};
> >> +
> >I don't understand, you created macros that allow for naming specifics,
> >then
> >don't use it? If your intent was for that data to be generically named
> >(which
> >would make sense), then you should remove the parameter from the macro so
> >that
> >can't change.
> >
> >> +/**
> >> + * Get the rte_pkt_dev structure device pointer for the device.
> >> + *
> >> + * @param gbl pointer to device specific global structure.
> >> + * @param port_id Port ID value to select the device structure.
> >> + *
> >> + * @return
> >> + * - The rte_pkt_dev structure pointer for the given port ID.
> >> + */
> >> +static inline void *
> >> +rte_get_dev(void * _gbl, uint8_t port_id)
> >> +{
> >> + struct rte_dev_global * gbl = (struct rte_dev_global *)_gbl;
> >> +
> >> + return (void *)((uint8_t *)gbl->devs + (port_id * gbl->dev_size));
> >> +}
> >> +
> >How will this be helpful? the caller will need to know what type of
> >struct to
> >cast this to (since it can be named different for different dev classes).
> >doesn't that sort of thing undo the notion of commonality?
>
> Not really the point was to have a routine able to return the correct
> device structure pointer and the developer should understand which global
> structure he is passing into the routine and what type is being returned.
>
Ok, I get that, but if the caller has to know how to cast this to an appropriate
type, then they will need to know the type of device they are communicating
with, and understand all its member data and methods, which defeats the ability
for a user of the device to write truly generic code to access devices, which,
when we spoke offline (and I think earlier in this thread), you indicated was a
goal of yours.
Neil
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-12 13:09 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-08 20:58 [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 1/4] Rename of device types to be generic device names Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 2/4] Add the new common device header and C file Keith Wiles
2015-04-09 11:53 ` Neil Horman
2015-04-10 19:25 ` Wiles, Keith
2015-04-12 13:08 ` Neil Horman
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 3/4] Update files to build new device generic common files and headers Keith Wiles
2015-04-08 20:58 ` [dpdk-dev] [RFC PATCH 4/4] Update the rte_ethdev.[ch] files for the new device generic changes Keith Wiles
2015-04-10 18:47 ` [dpdk-dev] [RFC PATCH 0/4] Extending DPDK to have more devices supported Wiles, Keith
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).