* [dpdk-dev] [PATCH 1/3] net/sfc: carefully cleanup on init failure and shutdown @ 2017-05-17 12:25 Andrew Rybchenko 2017-05-17 12:25 ` [dpdk-dev] [PATCH 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-17 12:25 UTC (permalink / raw) To: dev Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> Reviewed-by: Andy Moreton <amoreton@solarflare.com> --- drivers/net/sfc/sfc_ethdev.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index d5583ec..e4f051a 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -1407,7 +1407,7 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) "Insufficient Hw/FW capabilities to use Rx datapath %s", rx_name); rc = EINVAL; - goto fail_dp_rx; + goto fail_dp_rx_caps; } } else { sa->dp_rx = sfc_dp_find_rx_by_caps(&sfc_dp_head, avail_caps); @@ -1440,7 +1440,7 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) "Insufficient Hw/FW capabilities to use Tx datapath %s", tx_name); rc = EINVAL; - goto fail_dp_tx; + goto fail_dp_tx_caps; } } else { sa->dp_tx = sfc_dp_find_tx_by_caps(&sfc_dp_head, avail_caps); @@ -1460,14 +1460,33 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) return 0; +fail_dp_tx_caps: + sa->dp_tx = NULL; + fail_dp_tx: fail_kvarg_tx_datapath: +fail_dp_rx_caps: + sa->dp_rx = NULL; + fail_dp_rx: fail_kvarg_rx_datapath: return rc; } static void +sfc_eth_dev_clear_ops(struct rte_eth_dev *dev) +{ + struct sfc_adapter *sa = dev->data->dev_private; + + dev->dev_ops = NULL; + dev->rx_pkt_burst = NULL; + dev->tx_pkt_burst = NULL; + + sa->dp_tx = NULL; + sa->dp_rx = NULL; +} + +static void sfc_register_dp(void) { /* Register once */ @@ -1549,6 +1568,8 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) return 0; fail_attach: + sfc_eth_dev_clear_ops(dev); + fail_set_ops: sfc_unprobe(sa); @@ -1577,16 +1598,14 @@ sfc_eth_dev_uninit(struct rte_eth_dev *dev) sfc_adapter_lock(sa); + sfc_eth_dev_clear_ops(dev); + sfc_detach(sa); sfc_unprobe(sa); rte_free(dev->data->mac_addrs); dev->data->mac_addrs = NULL; - dev->dev_ops = NULL; - dev->rx_pkt_burst = NULL; - dev->tx_pkt_burst = NULL; - sfc_kvargs_cleanup(sa); sfc_adapter_unlock(sa); -- 2.9.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-dev] [PATCH 2/3] net/sfc: use locally stored data for logging 2017-05-17 12:25 [dpdk-dev] [PATCH 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko @ 2017-05-17 12:25 ` Andrew Rybchenko 2017-05-18 10:59 ` Ferruh Yigit 2017-05-17 12:25 ` [dpdk-dev] [PATCH 3/3] net/sfc: support multi-process Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko 2 siblings, 1 reply; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-17 12:25 UTC (permalink / raw) To: dev Required to be able to use logging in the secondary process where Ethernet device pointer stored in sfc_adapter is invalid. Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> Reviewed-by: Andy Moreton <amoreton@solarflare.com> --- drivers/net/sfc/sfc.h | 2 ++ drivers/net/sfc/sfc_debug.h | 10 ++++------ drivers/net/sfc/sfc_ethdev.c | 3 +++ drivers/net/sfc/sfc_log.h | 14 ++++++-------- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h index 7927678..772a713 100644 --- a/drivers/net/sfc/sfc.h +++ b/drivers/net/sfc/sfc.h @@ -179,6 +179,8 @@ struct sfc_adapter { */ rte_spinlock_t lock; enum sfc_adapter_state state; + struct rte_pci_addr pci_addr; + uint16_t port_id; struct rte_eth_dev *eth_dev; struct rte_kvargs *kvargs; bool debug_init; diff --git a/drivers/net/sfc/sfc_debug.h b/drivers/net/sfc/sfc_debug.h index f4fe044..92eba9c 100644 --- a/drivers/net/sfc/sfc_debug.h +++ b/drivers/net/sfc/sfc_debug.h @@ -47,14 +47,12 @@ /* Log PMD message, automatically add prefix and \n */ #define sfc_panic(sa, fmt, args...) \ do { \ - const struct rte_eth_dev *_dev = (sa)->eth_dev; \ - const struct rte_pci_device *_pci_dev = \ - RTE_ETH_DEV_TO_PCI(_dev); \ + const struct sfc_adapter *_sa = (sa); \ \ rte_panic("sfc " PCI_PRI_FMT " #%" PRIu8 ": " fmt "\n", \ - _pci_dev->addr.domain, _pci_dev->addr.bus, \ - _pci_dev->addr.devid, _pci_dev->addr.function,\ - _dev->data->port_id, ##args); \ + _sa->pci_addr.domain, _sa->pci_addr.bus, \ + _sa->pci_addr.devid, _sa->pci_addr.function, \ + _sa->port_id, ##args); \ } while (0) #endif /* _SFC_DEBUG_H_ */ diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index e4f051a..d6bba1d 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -1513,6 +1513,9 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) sfc_register_dp(); /* Required for logging */ + sa->pci_addr = pci_dev->addr; + sa->port_id = dev->data->port_id; + sa->eth_dev = dev; /* Copy PCI device info to the dev->data */ diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h index d0f8921..6c43925 100644 --- a/drivers/net/sfc/sfc_log.h +++ b/drivers/net/sfc/sfc_log.h @@ -35,18 +35,16 @@ /* Log PMD message, automatically add prefix and \n */ #define SFC_LOG(sa, level, ...) \ do { \ - const struct rte_eth_dev *_dev = (sa)->eth_dev; \ - const struct rte_pci_device *_pci_dev = \ - RTE_ETH_DEV_TO_PCI(_dev); \ + const struct sfc_adapter *_sa = (sa); \ \ RTE_LOG(level, PMD, \ RTE_FMT("sfc_efx " PCI_PRI_FMT " #%" PRIu8 ": " \ RTE_FMT_HEAD(__VA_ARGS__,) "\n", \ - _pci_dev->addr.domain, \ - _pci_dev->addr.bus, \ - _pci_dev->addr.devid, \ - _pci_dev->addr.function, \ - _dev->data->port_id, \ + _sa->pci_addr.domain, \ + _sa->pci_addr.bus, \ + _sa->pci_addr.devid, \ + _sa->pci_addr.function, \ + _sa->port_id, \ RTE_FMT_TAIL(__VA_ARGS__,))); \ } while (0) -- 2.9.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH 2/3] net/sfc: use locally stored data for logging 2017-05-17 12:25 ` [dpdk-dev] [PATCH 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko @ 2017-05-18 10:59 ` Ferruh Yigit 2017-05-18 14:01 ` Andrew Rybchenko 0 siblings, 1 reply; 15+ messages in thread From: Ferruh Yigit @ 2017-05-18 10:59 UTC (permalink / raw) To: Andrew Rybchenko, dev On 5/17/2017 1:25 PM, Andrew Rybchenko wrote: > Required to be able to use logging in the secondary process > where Ethernet device pointer stored in sfc_adapter is invalid. > > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > Reviewed-by: Andy Moreton <amoreton@solarflare.com> <...> > diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h > index d0f8921..6c43925 100644 > --- a/drivers/net/sfc/sfc_log.h > +++ b/drivers/net/sfc/sfc_log.h > @@ -35,18 +35,16 @@ > /* Log PMD message, automatically add prefix and \n */ > #define SFC_LOG(sa, level, ...) \ > do { \ > - const struct rte_eth_dev *_dev = (sa)->eth_dev; \ > - const struct rte_pci_device *_pci_dev = \ > - RTE_ETH_DEV_TO_PCI(_dev); \ > + const struct sfc_adapter *_sa = (sa); \ Getting following build error with clang and icc [1]. I guess that is because "_sa" declared on both sfc_log_init() and in the macro that function uses (SFC_LOG). [1] .../drivers/net/sfc/sfc_filter.c:121:2: error: variable '_sa' is uninitialized when used within its own initialization [-Werror,-Wuninitialized] sfc_log_init(sa, "failed %d", rc); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ .../drivers/net/sfc/sfc_log.h:68:12: note: expanded from macro 'sfc_log_init' SFC_LOG(_sa, INFO, \ ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ .../drivers/net/sfc/sfc_log.h:38:36: note: expanded from macro 'SFC_LOG' const struct sfc_adapter *_sa = (sa); \ ~~~ ^~ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH 2/3] net/sfc: use locally stored data for logging 2017-05-18 10:59 ` Ferruh Yigit @ 2017-05-18 14:01 ` Andrew Rybchenko 0 siblings, 0 replies; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-18 14:01 UTC (permalink / raw) To: Ferruh Yigit, dev On 05/18/2017 01:59 PM, Ferruh Yigit wrote: > On 5/17/2017 1:25 PM, Andrew Rybchenko wrote: >> Required to be able to use logging in the secondary process >> where Ethernet device pointer stored in sfc_adapter is invalid. >> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >> Reviewed-by: Andy Moreton <amoreton@solarflare.com> > <...> > >> diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h >> index d0f8921..6c43925 100644 >> --- a/drivers/net/sfc/sfc_log.h >> +++ b/drivers/net/sfc/sfc_log.h >> @@ -35,18 +35,16 @@ >> /* Log PMD message, automatically add prefix and \n */ >> #define SFC_LOG(sa, level, ...) \ >> do { \ >> - const struct rte_eth_dev *_dev = (sa)->eth_dev; \ >> - const struct rte_pci_device *_pci_dev = \ >> - RTE_ETH_DEV_TO_PCI(_dev); \ >> + const struct sfc_adapter *_sa = (sa); \ > Getting following build error with clang and icc [1]. I guess that is > because "_sa" declared on both sfc_log_init() and in the macro that > function uses (SFC_LOG). Thanks, I'll fix it in v2. > [1] > .../drivers/net/sfc/sfc_filter.c:121:2: > error: variable '_sa' is uninitialized when used within its own > initialization [-Werror,-Wuninitialized] > sfc_log_init(sa, "failed %d", rc); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > .../drivers/net/sfc/sfc_log.h:68:12: note: expanded from macro > 'sfc_log_init' > SFC_LOG(_sa, INFO, \ > ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > .../drivers/net/sfc/sfc_log.h:38:36: note: expanded from macro 'SFC_LOG' > const struct sfc_adapter *_sa = (sa); \ > ~~~ ^~ ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-dev] [PATCH 3/3] net/sfc: support multi-process 2017-05-17 12:25 [dpdk-dev] [PATCH 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko 2017-05-17 12:25 ` [dpdk-dev] [PATCH 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko @ 2017-05-17 12:25 ` Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko 2 siblings, 0 replies; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-17 12:25 UTC (permalink / raw) To: dev Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> Reviewed-by: Andy Moreton <amoreton@solarflare.com> --- doc/guides/nics/features/sfc_efx.ini | 1 + drivers/net/sfc/sfc.h | 11 +++ drivers/net/sfc/sfc_dp_rx.h | 1 + drivers/net/sfc/sfc_dp_tx.h | 1 + drivers/net/sfc/sfc_ef10_rx.c | 2 +- drivers/net/sfc/sfc_ef10_tx.c | 5 +- drivers/net/sfc/sfc_ethdev.c | 133 ++++++++++++++++++++++++++++++++++- 7 files changed, 148 insertions(+), 6 deletions(-) diff --git a/doc/guides/nics/features/sfc_efx.ini b/doc/guides/nics/features/sfc_efx.ini index 7957b5e..1db7f67 100644 --- a/doc/guides/nics/features/sfc_efx.ini +++ b/doc/guides/nics/features/sfc_efx.ini @@ -28,6 +28,7 @@ Packet type parsing = Y Basic stats = Y Extended stats = Y FW version = Y +Multiprocess aware = Y BSD nic_uio = Y Linux UIO = Y Linux VFIO = Y diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h index 772a713..007ed24 100644 --- a/drivers/net/sfc/sfc.h +++ b/drivers/net/sfc/sfc.h @@ -225,7 +225,18 @@ struct sfc_adapter { uint8_t rss_key[SFC_RSS_KEY_SIZE]; #endif + /* + * Shared memory copy of the Rx datapath name to be used by + * the secondary process to find Rx datapath to be used. + */ + char *dp_rx_name; const struct sfc_dp_rx *dp_rx; + + /* + * Shared memory copy of the Tx datapath name to be used by + * the secondary process to find Rx datapath to be used. + */ + char *dp_tx_name; const struct sfc_dp_tx *dp_tx; }; diff --git a/drivers/net/sfc/sfc_dp_rx.h b/drivers/net/sfc/sfc_dp_rx.h index 9d05a4b..a7b8278 100644 --- a/drivers/net/sfc/sfc_dp_rx.h +++ b/drivers/net/sfc/sfc_dp_rx.h @@ -161,6 +161,7 @@ struct sfc_dp_rx { unsigned int features; #define SFC_DP_RX_FEAT_SCATTER 0x1 +#define SFC_DP_RX_FEAT_MULTI_PROCESS 0x2 sfc_dp_rx_qcreate_t *qcreate; sfc_dp_rx_qdestroy_t *qdestroy; sfc_dp_rx_qstart_t *qstart; diff --git a/drivers/net/sfc/sfc_dp_tx.h b/drivers/net/sfc/sfc_dp_tx.h index 2bb9a2e..c1c3419 100644 --- a/drivers/net/sfc/sfc_dp_tx.h +++ b/drivers/net/sfc/sfc_dp_tx.h @@ -135,6 +135,7 @@ struct sfc_dp_tx { #define SFC_DP_TX_FEAT_VLAN_INSERT 0x1 #define SFC_DP_TX_FEAT_TSO 0x2 #define SFC_DP_TX_FEAT_MULTI_SEG 0x4 +#define SFC_DP_TX_FEAT_MULTI_PROCESS 0x8 sfc_dp_tx_qcreate_t *qcreate; sfc_dp_tx_qdestroy_t *qdestroy; sfc_dp_tx_qstart_t *qstart; diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c index 1484bab..60812cb 100644 --- a/drivers/net/sfc/sfc_ef10_rx.c +++ b/drivers/net/sfc/sfc_ef10_rx.c @@ -699,7 +699,7 @@ struct sfc_dp_rx sfc_ef10_rx = { .type = SFC_DP_RX, .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, }, - .features = 0, + .features = SFC_DP_RX_FEAT_MULTI_PROCESS, .qcreate = sfc_ef10_rx_qcreate, .qdestroy = sfc_ef10_rx_qdestroy, .qstart = sfc_ef10_rx_qstart, diff --git a/drivers/net/sfc/sfc_ef10_tx.c b/drivers/net/sfc/sfc_ef10_tx.c index bac9baa..5482db8 100644 --- a/drivers/net/sfc/sfc_ef10_tx.c +++ b/drivers/net/sfc/sfc_ef10_tx.c @@ -534,7 +534,8 @@ struct sfc_dp_tx sfc_ef10_tx = { .type = SFC_DP_TX, .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, }, - .features = SFC_DP_TX_FEAT_MULTI_SEG, + .features = SFC_DP_TX_FEAT_MULTI_SEG | + SFC_DP_TX_FEAT_MULTI_PROCESS, .qcreate = sfc_ef10_tx_qcreate, .qdestroy = sfc_ef10_tx_qdestroy, .qstart = sfc_ef10_tx_qstart, @@ -549,7 +550,7 @@ struct sfc_dp_tx sfc_ef10_simple_tx = { .name = SFC_KVARG_DATAPATH_EF10_SIMPLE, .type = SFC_DP_TX, }, - .features = 0, + .features = SFC_DP_TX_FEAT_MULTI_PROCESS, .qcreate = sfc_ef10_tx_qcreate, .qdestroy = sfc_ef10_tx_qdestroy, .qstart = sfc_ef10_tx_qstart, diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index d6bba1d..0bd2de4 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -913,6 +913,10 @@ sfc_set_mc_addr_list(struct rte_eth_dev *dev, struct ether_addr *mc_addr_set, return -rc; } +/* + * The function is used by the secondary process as well. It must not + * use any process-local pointers from the adapter data. + */ static void sfc_rx_queue_info_get(struct rte_eth_dev *dev, uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo) @@ -939,6 +943,10 @@ sfc_rx_queue_info_get(struct rte_eth_dev *dev, uint16_t rx_queue_id, sfc_adapter_unlock(sa); } +/* + * The function is used by the secondary process as well. It must not + * use any process-local pointers from the adapter data. + */ static void sfc_tx_queue_info_get(struct rte_eth_dev *dev, uint16_t tx_queue_id, struct rte_eth_txq_info *qinfo) @@ -1372,6 +1380,29 @@ static const struct eth_dev_ops sfc_eth_dev_ops = { .fw_version_get = sfc_fw_version_get, }; +/** + * Duplicate a string in potentially shared memory required for + * multi-process support. + * + * strdup() allocates from process-local heap/memory. + */ +static char * +sfc_strdup(const char *str) +{ + size_t size; + char *copy; + + if (str == NULL) + return NULL; + + size = strlen(str) + 1; + copy = rte_malloc(__func__, size, 0); + if (copy != NULL) + rte_memcpy(copy, str, size); + + return copy; +} + static int sfc_eth_dev_set_ops(struct rte_eth_dev *dev) { @@ -1419,7 +1450,13 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) } } - sfc_info(sa, "use %s Rx datapath", sa->dp_rx->dp.name); + sa->dp_rx_name = sfc_strdup(sa->dp_rx->dp.name); + if (sa->dp_rx_name == NULL) { + rc = ENOMEM; + goto fail_dp_rx_name; + } + + sfc_info(sa, "use %s Rx datapath", sa->dp_rx_name); dev->rx_pkt_burst = sa->dp_rx->pkt_burst; @@ -1452,7 +1489,13 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) } } - sfc_info(sa, "use %s Tx datapath", sa->dp_tx->dp.name); + sa->dp_tx_name = sfc_strdup(sa->dp_tx->dp.name); + if (sa->dp_tx_name == NULL) { + rc = ENOMEM; + goto fail_dp_tx_name; + } + + sfc_info(sa, "use %s Tx datapath", sa->dp_tx_name); dev->tx_pkt_burst = sa->dp_tx->pkt_burst; @@ -1460,11 +1503,16 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) return 0; +fail_dp_tx_name: fail_dp_tx_caps: sa->dp_tx = NULL; fail_dp_tx: fail_kvarg_tx_datapath: + rte_free(sa->dp_rx_name); + sa->dp_rx_name = NULL; + +fail_dp_rx_name: fail_dp_rx_caps: sa->dp_rx = NULL; @@ -1482,10 +1530,80 @@ sfc_eth_dev_clear_ops(struct rte_eth_dev *dev) dev->rx_pkt_burst = NULL; dev->tx_pkt_burst = NULL; + rte_free(sa->dp_tx_name); + sa->dp_tx_name = NULL; sa->dp_tx = NULL; + + rte_free(sa->dp_rx_name); + sa->dp_rx_name = NULL; sa->dp_rx = NULL; } +static const struct eth_dev_ops sfc_eth_dev_secondary_ops = { + .rxq_info_get = sfc_rx_queue_info_get, + .txq_info_get = sfc_tx_queue_info_get, +}; + +static int +sfc_eth_dev_secondary_set_ops(struct rte_eth_dev *dev) +{ + /* + * Device private data has really many process-local pointers. + * Below code should be extremely careful to use data located + * in shared memory only. + */ + struct sfc_adapter *sa = dev->data->dev_private; + const struct sfc_dp_rx *dp_rx; + const struct sfc_dp_tx *dp_tx; + int rc; + + dp_rx = sfc_dp_find_rx_by_name(&sfc_dp_head, sa->dp_rx_name); + if (dp_rx == NULL) { + sfc_err(sa, "cannot find %s Rx datapath", sa->dp_tx_name); + rc = ENOENT; + goto fail_dp_rx; + } + if (~dp_rx->features & SFC_DP_RX_FEAT_MULTI_PROCESS) { + sfc_err(sa, "%s Rx datapath does not support multi-process", + sa->dp_tx_name); + rc = EINVAL; + goto fail_dp_rx_multi_process; + } + + dp_tx = sfc_dp_find_tx_by_name(&sfc_dp_head, sa->dp_tx_name); + if (dp_tx == NULL) { + sfc_err(sa, "cannot find %s Tx datapath", sa->dp_tx_name); + rc = ENOENT; + goto fail_dp_tx; + } + if (~dp_tx->features & SFC_DP_TX_FEAT_MULTI_PROCESS) { + sfc_err(sa, "%s Tx datapath does not support multi-process", + sa->dp_tx_name); + rc = EINVAL; + goto fail_dp_tx_multi_process; + } + + dev->rx_pkt_burst = dp_rx->pkt_burst; + dev->tx_pkt_burst = dp_tx->pkt_burst; + dev->dev_ops = &sfc_eth_dev_secondary_ops; + + return 0; + +fail_dp_tx_multi_process: +fail_dp_tx: +fail_dp_rx_multi_process: +fail_dp_rx: + return rc; +} + +static void +sfc_eth_dev_secondary_clear_ops(struct rte_eth_dev *dev) +{ + dev->dev_ops = NULL; + dev->tx_pkt_burst = NULL; + dev->rx_pkt_burst = NULL; +} + static void sfc_register_dp(void) { @@ -1512,6 +1630,9 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) sfc_register_dp(); + if (rte_eal_process_type() != RTE_PROC_PRIMARY) + return -sfc_eth_dev_secondary_set_ops(dev); + /* Required for logging */ sa->pci_addr = pci_dev->addr; sa->port_id = dev->data->port_id; @@ -1595,8 +1716,14 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) static int sfc_eth_dev_uninit(struct rte_eth_dev *dev) { - struct sfc_adapter *sa = dev->data->dev_private; + struct sfc_adapter *sa; + + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { + sfc_eth_dev_secondary_clear_ops(dev); + return 0; + } + sa = dev->data->dev_private; sfc_log_init(sa, "entry"); sfc_adapter_lock(sa); -- 2.9.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown 2017-05-17 12:25 [dpdk-dev] [PATCH 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko 2017-05-17 12:25 ` [dpdk-dev] [PATCH 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko 2017-05-17 12:25 ` [dpdk-dev] [PATCH 3/3] net/sfc: support multi-process Andrew Rybchenko @ 2017-05-18 14:00 ` Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko ` (2 more replies) 2 siblings, 3 replies; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-18 14:00 UTC (permalink / raw) To: dev; +Cc: Ferruh Yigit Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> Reviewed-by: Andy Moreton <amoreton@solarflare.com> --- v2: - no changes drivers/net/sfc/sfc_ethdev.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index d5583ec..e4f051a 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -1407,7 +1407,7 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) "Insufficient Hw/FW capabilities to use Rx datapath %s", rx_name); rc = EINVAL; - goto fail_dp_rx; + goto fail_dp_rx_caps; } } else { sa->dp_rx = sfc_dp_find_rx_by_caps(&sfc_dp_head, avail_caps); @@ -1440,7 +1440,7 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) "Insufficient Hw/FW capabilities to use Tx datapath %s", tx_name); rc = EINVAL; - goto fail_dp_tx; + goto fail_dp_tx_caps; } } else { sa->dp_tx = sfc_dp_find_tx_by_caps(&sfc_dp_head, avail_caps); @@ -1460,14 +1460,33 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) return 0; +fail_dp_tx_caps: + sa->dp_tx = NULL; + fail_dp_tx: fail_kvarg_tx_datapath: +fail_dp_rx_caps: + sa->dp_rx = NULL; + fail_dp_rx: fail_kvarg_rx_datapath: return rc; } static void +sfc_eth_dev_clear_ops(struct rte_eth_dev *dev) +{ + struct sfc_adapter *sa = dev->data->dev_private; + + dev->dev_ops = NULL; + dev->rx_pkt_burst = NULL; + dev->tx_pkt_burst = NULL; + + sa->dp_tx = NULL; + sa->dp_rx = NULL; +} + +static void sfc_register_dp(void) { /* Register once */ @@ -1549,6 +1568,8 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) return 0; fail_attach: + sfc_eth_dev_clear_ops(dev); + fail_set_ops: sfc_unprobe(sa); @@ -1577,16 +1598,14 @@ sfc_eth_dev_uninit(struct rte_eth_dev *dev) sfc_adapter_lock(sa); + sfc_eth_dev_clear_ops(dev); + sfc_detach(sa); sfc_unprobe(sa); rte_free(dev->data->mac_addrs); dev->data->mac_addrs = NULL; - dev->dev_ops = NULL; - dev->rx_pkt_burst = NULL; - dev->tx_pkt_burst = NULL; - sfc_kvargs_cleanup(sa); sfc_adapter_unlock(sa); -- 2.9.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-dev] [PATCH v2 2/3] net/sfc: use locally stored data for logging 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko @ 2017-05-18 14:00 ` Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process Andrew Rybchenko 2017-05-22 15:42 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Ferruh Yigit 2 siblings, 0 replies; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-18 14:00 UTC (permalink / raw) To: dev; +Cc: Ferruh Yigit Required to be able to use logging in the secondary process where Ethernet device pointer stored in sfc_adapter is invalid. Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> Reviewed-by: Andy Moreton <amoreton@solarflare.com> --- v2: - fix clang/icc build error bacause of local variable initialization by the external variable with the same name drivers/net/sfc/sfc.h | 2 ++ drivers/net/sfc/sfc_debug.h | 10 ++++------ drivers/net/sfc/sfc_ethdev.c | 3 +++ drivers/net/sfc/sfc_log.h | 14 ++++++-------- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h index 7927678..772a713 100644 --- a/drivers/net/sfc/sfc.h +++ b/drivers/net/sfc/sfc.h @@ -179,6 +179,8 @@ struct sfc_adapter { */ rte_spinlock_t lock; enum sfc_adapter_state state; + struct rte_pci_addr pci_addr; + uint16_t port_id; struct rte_eth_dev *eth_dev; struct rte_kvargs *kvargs; bool debug_init; diff --git a/drivers/net/sfc/sfc_debug.h b/drivers/net/sfc/sfc_debug.h index f4fe044..92eba9c 100644 --- a/drivers/net/sfc/sfc_debug.h +++ b/drivers/net/sfc/sfc_debug.h @@ -47,14 +47,12 @@ /* Log PMD message, automatically add prefix and \n */ #define sfc_panic(sa, fmt, args...) \ do { \ - const struct rte_eth_dev *_dev = (sa)->eth_dev; \ - const struct rte_pci_device *_pci_dev = \ - RTE_ETH_DEV_TO_PCI(_dev); \ + const struct sfc_adapter *_sa = (sa); \ \ rte_panic("sfc " PCI_PRI_FMT " #%" PRIu8 ": " fmt "\n", \ - _pci_dev->addr.domain, _pci_dev->addr.bus, \ - _pci_dev->addr.devid, _pci_dev->addr.function,\ - _dev->data->port_id, ##args); \ + _sa->pci_addr.domain, _sa->pci_addr.bus, \ + _sa->pci_addr.devid, _sa->pci_addr.function, \ + _sa->port_id, ##args); \ } while (0) #endif /* _SFC_DEBUG_H_ */ diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index e4f051a..d6bba1d 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -1513,6 +1513,9 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) sfc_register_dp(); /* Required for logging */ + sa->pci_addr = pci_dev->addr; + sa->port_id = dev->data->port_id; + sa->eth_dev = dev; /* Copy PCI device info to the dev->data */ diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h index d0f8921..b1a9df4 100644 --- a/drivers/net/sfc/sfc_log.h +++ b/drivers/net/sfc/sfc_log.h @@ -35,18 +35,16 @@ /* Log PMD message, automatically add prefix and \n */ #define SFC_LOG(sa, level, ...) \ do { \ - const struct rte_eth_dev *_dev = (sa)->eth_dev; \ - const struct rte_pci_device *_pci_dev = \ - RTE_ETH_DEV_TO_PCI(_dev); \ + const struct sfc_adapter *__sa = (sa); \ \ RTE_LOG(level, PMD, \ RTE_FMT("sfc_efx " PCI_PRI_FMT " #%" PRIu8 ": " \ RTE_FMT_HEAD(__VA_ARGS__,) "\n", \ - _pci_dev->addr.domain, \ - _pci_dev->addr.bus, \ - _pci_dev->addr.devid, \ - _pci_dev->addr.function, \ - _dev->data->port_id, \ + __sa->pci_addr.domain, \ + __sa->pci_addr.bus, \ + __sa->pci_addr.devid, \ + __sa->pci_addr.function, \ + __sa->port_id, \ RTE_FMT_TAIL(__VA_ARGS__,))); \ } while (0) -- 2.9.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko @ 2017-05-18 14:00 ` Andrew Rybchenko 2017-05-22 11:29 ` Ferruh Yigit 2017-05-22 15:42 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Ferruh Yigit 2 siblings, 1 reply; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-18 14:00 UTC (permalink / raw) To: dev; +Cc: Ferruh Yigit Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> Reviewed-by: Andy Moreton <amoreton@solarflare.com> --- v2: - no changes doc/guides/nics/features/sfc_efx.ini | 1 + drivers/net/sfc/sfc.h | 11 +++ drivers/net/sfc/sfc_dp_rx.h | 1 + drivers/net/sfc/sfc_dp_tx.h | 1 + drivers/net/sfc/sfc_ef10_rx.c | 2 +- drivers/net/sfc/sfc_ef10_tx.c | 5 +- drivers/net/sfc/sfc_ethdev.c | 133 ++++++++++++++++++++++++++++++++++- 7 files changed, 148 insertions(+), 6 deletions(-) diff --git a/doc/guides/nics/features/sfc_efx.ini b/doc/guides/nics/features/sfc_efx.ini index 7957b5e..1db7f67 100644 --- a/doc/guides/nics/features/sfc_efx.ini +++ b/doc/guides/nics/features/sfc_efx.ini @@ -28,6 +28,7 @@ Packet type parsing = Y Basic stats = Y Extended stats = Y FW version = Y +Multiprocess aware = Y BSD nic_uio = Y Linux UIO = Y Linux VFIO = Y diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h index 772a713..007ed24 100644 --- a/drivers/net/sfc/sfc.h +++ b/drivers/net/sfc/sfc.h @@ -225,7 +225,18 @@ struct sfc_adapter { uint8_t rss_key[SFC_RSS_KEY_SIZE]; #endif + /* + * Shared memory copy of the Rx datapath name to be used by + * the secondary process to find Rx datapath to be used. + */ + char *dp_rx_name; const struct sfc_dp_rx *dp_rx; + + /* + * Shared memory copy of the Tx datapath name to be used by + * the secondary process to find Rx datapath to be used. + */ + char *dp_tx_name; const struct sfc_dp_tx *dp_tx; }; diff --git a/drivers/net/sfc/sfc_dp_rx.h b/drivers/net/sfc/sfc_dp_rx.h index 9d05a4b..a7b8278 100644 --- a/drivers/net/sfc/sfc_dp_rx.h +++ b/drivers/net/sfc/sfc_dp_rx.h @@ -161,6 +161,7 @@ struct sfc_dp_rx { unsigned int features; #define SFC_DP_RX_FEAT_SCATTER 0x1 +#define SFC_DP_RX_FEAT_MULTI_PROCESS 0x2 sfc_dp_rx_qcreate_t *qcreate; sfc_dp_rx_qdestroy_t *qdestroy; sfc_dp_rx_qstart_t *qstart; diff --git a/drivers/net/sfc/sfc_dp_tx.h b/drivers/net/sfc/sfc_dp_tx.h index 2bb9a2e..c1c3419 100644 --- a/drivers/net/sfc/sfc_dp_tx.h +++ b/drivers/net/sfc/sfc_dp_tx.h @@ -135,6 +135,7 @@ struct sfc_dp_tx { #define SFC_DP_TX_FEAT_VLAN_INSERT 0x1 #define SFC_DP_TX_FEAT_TSO 0x2 #define SFC_DP_TX_FEAT_MULTI_SEG 0x4 +#define SFC_DP_TX_FEAT_MULTI_PROCESS 0x8 sfc_dp_tx_qcreate_t *qcreate; sfc_dp_tx_qdestroy_t *qdestroy; sfc_dp_tx_qstart_t *qstart; diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c index 1484bab..60812cb 100644 --- a/drivers/net/sfc/sfc_ef10_rx.c +++ b/drivers/net/sfc/sfc_ef10_rx.c @@ -699,7 +699,7 @@ struct sfc_dp_rx sfc_ef10_rx = { .type = SFC_DP_RX, .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, }, - .features = 0, + .features = SFC_DP_RX_FEAT_MULTI_PROCESS, .qcreate = sfc_ef10_rx_qcreate, .qdestroy = sfc_ef10_rx_qdestroy, .qstart = sfc_ef10_rx_qstart, diff --git a/drivers/net/sfc/sfc_ef10_tx.c b/drivers/net/sfc/sfc_ef10_tx.c index bac9baa..5482db8 100644 --- a/drivers/net/sfc/sfc_ef10_tx.c +++ b/drivers/net/sfc/sfc_ef10_tx.c @@ -534,7 +534,8 @@ struct sfc_dp_tx sfc_ef10_tx = { .type = SFC_DP_TX, .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, }, - .features = SFC_DP_TX_FEAT_MULTI_SEG, + .features = SFC_DP_TX_FEAT_MULTI_SEG | + SFC_DP_TX_FEAT_MULTI_PROCESS, .qcreate = sfc_ef10_tx_qcreate, .qdestroy = sfc_ef10_tx_qdestroy, .qstart = sfc_ef10_tx_qstart, @@ -549,7 +550,7 @@ struct sfc_dp_tx sfc_ef10_simple_tx = { .name = SFC_KVARG_DATAPATH_EF10_SIMPLE, .type = SFC_DP_TX, }, - .features = 0, + .features = SFC_DP_TX_FEAT_MULTI_PROCESS, .qcreate = sfc_ef10_tx_qcreate, .qdestroy = sfc_ef10_tx_qdestroy, .qstart = sfc_ef10_tx_qstart, diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index d6bba1d..0bd2de4 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -913,6 +913,10 @@ sfc_set_mc_addr_list(struct rte_eth_dev *dev, struct ether_addr *mc_addr_set, return -rc; } +/* + * The function is used by the secondary process as well. It must not + * use any process-local pointers from the adapter data. + */ static void sfc_rx_queue_info_get(struct rte_eth_dev *dev, uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo) @@ -939,6 +943,10 @@ sfc_rx_queue_info_get(struct rte_eth_dev *dev, uint16_t rx_queue_id, sfc_adapter_unlock(sa); } +/* + * The function is used by the secondary process as well. It must not + * use any process-local pointers from the adapter data. + */ static void sfc_tx_queue_info_get(struct rte_eth_dev *dev, uint16_t tx_queue_id, struct rte_eth_txq_info *qinfo) @@ -1372,6 +1380,29 @@ static const struct eth_dev_ops sfc_eth_dev_ops = { .fw_version_get = sfc_fw_version_get, }; +/** + * Duplicate a string in potentially shared memory required for + * multi-process support. + * + * strdup() allocates from process-local heap/memory. + */ +static char * +sfc_strdup(const char *str) +{ + size_t size; + char *copy; + + if (str == NULL) + return NULL; + + size = strlen(str) + 1; + copy = rte_malloc(__func__, size, 0); + if (copy != NULL) + rte_memcpy(copy, str, size); + + return copy; +} + static int sfc_eth_dev_set_ops(struct rte_eth_dev *dev) { @@ -1419,7 +1450,13 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) } } - sfc_info(sa, "use %s Rx datapath", sa->dp_rx->dp.name); + sa->dp_rx_name = sfc_strdup(sa->dp_rx->dp.name); + if (sa->dp_rx_name == NULL) { + rc = ENOMEM; + goto fail_dp_rx_name; + } + + sfc_info(sa, "use %s Rx datapath", sa->dp_rx_name); dev->rx_pkt_burst = sa->dp_rx->pkt_burst; @@ -1452,7 +1489,13 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) } } - sfc_info(sa, "use %s Tx datapath", sa->dp_tx->dp.name); + sa->dp_tx_name = sfc_strdup(sa->dp_tx->dp.name); + if (sa->dp_tx_name == NULL) { + rc = ENOMEM; + goto fail_dp_tx_name; + } + + sfc_info(sa, "use %s Tx datapath", sa->dp_tx_name); dev->tx_pkt_burst = sa->dp_tx->pkt_burst; @@ -1460,11 +1503,16 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev) return 0; +fail_dp_tx_name: fail_dp_tx_caps: sa->dp_tx = NULL; fail_dp_tx: fail_kvarg_tx_datapath: + rte_free(sa->dp_rx_name); + sa->dp_rx_name = NULL; + +fail_dp_rx_name: fail_dp_rx_caps: sa->dp_rx = NULL; @@ -1482,10 +1530,80 @@ sfc_eth_dev_clear_ops(struct rte_eth_dev *dev) dev->rx_pkt_burst = NULL; dev->tx_pkt_burst = NULL; + rte_free(sa->dp_tx_name); + sa->dp_tx_name = NULL; sa->dp_tx = NULL; + + rte_free(sa->dp_rx_name); + sa->dp_rx_name = NULL; sa->dp_rx = NULL; } +static const struct eth_dev_ops sfc_eth_dev_secondary_ops = { + .rxq_info_get = sfc_rx_queue_info_get, + .txq_info_get = sfc_tx_queue_info_get, +}; + +static int +sfc_eth_dev_secondary_set_ops(struct rte_eth_dev *dev) +{ + /* + * Device private data has really many process-local pointers. + * Below code should be extremely careful to use data located + * in shared memory only. + */ + struct sfc_adapter *sa = dev->data->dev_private; + const struct sfc_dp_rx *dp_rx; + const struct sfc_dp_tx *dp_tx; + int rc; + + dp_rx = sfc_dp_find_rx_by_name(&sfc_dp_head, sa->dp_rx_name); + if (dp_rx == NULL) { + sfc_err(sa, "cannot find %s Rx datapath", sa->dp_tx_name); + rc = ENOENT; + goto fail_dp_rx; + } + if (~dp_rx->features & SFC_DP_RX_FEAT_MULTI_PROCESS) { + sfc_err(sa, "%s Rx datapath does not support multi-process", + sa->dp_tx_name); + rc = EINVAL; + goto fail_dp_rx_multi_process; + } + + dp_tx = sfc_dp_find_tx_by_name(&sfc_dp_head, sa->dp_tx_name); + if (dp_tx == NULL) { + sfc_err(sa, "cannot find %s Tx datapath", sa->dp_tx_name); + rc = ENOENT; + goto fail_dp_tx; + } + if (~dp_tx->features & SFC_DP_TX_FEAT_MULTI_PROCESS) { + sfc_err(sa, "%s Tx datapath does not support multi-process", + sa->dp_tx_name); + rc = EINVAL; + goto fail_dp_tx_multi_process; + } + + dev->rx_pkt_burst = dp_rx->pkt_burst; + dev->tx_pkt_burst = dp_tx->pkt_burst; + dev->dev_ops = &sfc_eth_dev_secondary_ops; + + return 0; + +fail_dp_tx_multi_process: +fail_dp_tx: +fail_dp_rx_multi_process: +fail_dp_rx: + return rc; +} + +static void +sfc_eth_dev_secondary_clear_ops(struct rte_eth_dev *dev) +{ + dev->dev_ops = NULL; + dev->tx_pkt_burst = NULL; + dev->rx_pkt_burst = NULL; +} + static void sfc_register_dp(void) { @@ -1512,6 +1630,9 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) sfc_register_dp(); + if (rte_eal_process_type() != RTE_PROC_PRIMARY) + return -sfc_eth_dev_secondary_set_ops(dev); + /* Required for logging */ sa->pci_addr = pci_dev->addr; sa->port_id = dev->data->port_id; @@ -1595,8 +1716,14 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) static int sfc_eth_dev_uninit(struct rte_eth_dev *dev) { - struct sfc_adapter *sa = dev->data->dev_private; + struct sfc_adapter *sa; + + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { + sfc_eth_dev_secondary_clear_ops(dev); + return 0; + } + sa = dev->data->dev_private; sfc_log_init(sa, "entry"); sfc_adapter_lock(sa); -- 2.9.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process Andrew Rybchenko @ 2017-05-22 11:29 ` Ferruh Yigit 2017-05-22 12:07 ` Andrew Rybchenko 0 siblings, 1 reply; 15+ messages in thread From: Ferruh Yigit @ 2017-05-22 11:29 UTC (permalink / raw) To: Andrew Rybchenko, dev On 5/18/2017 3:00 PM, Andrew Rybchenko wrote: > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > Reviewed-by: Andy Moreton <amoreton@solarflare.com> <...> > Linux VFIO = Y > diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h > index 772a713..007ed24 100644 > --- a/drivers/net/sfc/sfc.h > +++ b/drivers/net/sfc/sfc.h > @@ -225,7 +225,18 @@ struct sfc_adapter { > uint8_t rss_key[SFC_RSS_KEY_SIZE]; > #endif > > + /* > + * Shared memory copy of the Rx datapath name to be used by > + * the secondary process to find Rx datapath to be used. > + */ > + char *dp_rx_name; Why not use sa->dp_rx->dp.name to find the dp_rx? That variable should be shared between processes already? <...> > diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c > index 1484bab..60812cb 100644 > --- a/drivers/net/sfc/sfc_ef10_rx.c > +++ b/drivers/net/sfc/sfc_ef10_rx.c > @@ -699,7 +699,7 @@ struct sfc_dp_rx sfc_ef10_rx = { > .type = SFC_DP_RX, > .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, > }, > - .features = 0, > + .features = SFC_DP_RX_FEAT_MULTI_PROCESS, Why this flag is needed, I mean why multi process support is not always enabled by default? <...> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process 2017-05-22 11:29 ` Ferruh Yigit @ 2017-05-22 12:07 ` Andrew Rybchenko 2017-05-22 12:36 ` Ferruh Yigit 0 siblings, 1 reply; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-22 12:07 UTC (permalink / raw) To: Ferruh Yigit, dev On 05/22/2017 02:29 PM, Ferruh Yigit wrote: > On 5/18/2017 3:00 PM, Andrew Rybchenko wrote: >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >> Reviewed-by: Andy Moreton <amoreton@solarflare.com> > <...> > >> Linux VFIO = Y >> diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h >> index 772a713..007ed24 100644 >> --- a/drivers/net/sfc/sfc.h >> +++ b/drivers/net/sfc/sfc.h >> @@ -225,7 +225,18 @@ struct sfc_adapter { >> uint8_t rss_key[SFC_RSS_KEY_SIZE]; >> #endif >> >> + /* >> + * Shared memory copy of the Rx datapath name to be used by >> + * the secondary process to find Rx datapath to be used. >> + */ >> + char *dp_rx_name; > Why not use sa->dp_rx->dp.name to find the dp_rx? That variable should > be shared between processes already? sa->dp_rx is a pointer to .data section (sfc_efx_rx or sfc_ef10_rx) which is (may be) different in primary and secondary processes. > <...> > >> diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c >> index 1484bab..60812cb 100644 >> --- a/drivers/net/sfc/sfc_ef10_rx.c >> +++ b/drivers/net/sfc/sfc_ef10_rx.c >> @@ -699,7 +699,7 @@ struct sfc_dp_rx sfc_ef10_rx = { >> .type = SFC_DP_RX, >> .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, >> }, >> - .features = 0, >> + .features = SFC_DP_RX_FEAT_MULTI_PROCESS, > Why this flag is needed, I mean why multi process support is not always > enabled by default? libefx-based datapath intensively uses function pointers (primary process function pointers stored in data structures). So, it does not work in multi process. > <...> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process 2017-05-22 12:07 ` Andrew Rybchenko @ 2017-05-22 12:36 ` Ferruh Yigit 2017-05-22 12:44 ` Andrew Rybchenko 2017-05-22 15:18 ` Sergio Gonzalez Monroy 0 siblings, 2 replies; 15+ messages in thread From: Ferruh Yigit @ 2017-05-22 12:36 UTC (permalink / raw) To: Andrew Rybchenko, dev, Sergio Gonzalez Monroy On 5/22/2017 1:07 PM, Andrew Rybchenko wrote: > On 05/22/2017 02:29 PM, Ferruh Yigit wrote: >> On 5/18/2017 3:00 PM, Andrew Rybchenko wrote: >>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>> Reviewed-by: Andy Moreton <amoreton@solarflare.com> >> <...> >> >>> Linux VFIO = Y >>> diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h >>> index 772a713..007ed24 100644 >>> --- a/drivers/net/sfc/sfc.h >>> +++ b/drivers/net/sfc/sfc.h >>> @@ -225,7 +225,18 @@ struct sfc_adapter { >>> uint8_t rss_key[SFC_RSS_KEY_SIZE]; >>> #endif >>> >>> + /* >>> + * Shared memory copy of the Rx datapath name to be used by >>> + * the secondary process to find Rx datapath to be used. >>> + */ >>> + char *dp_rx_name; >> Why not use sa->dp_rx->dp.name to find the dp_rx? That variable should >> be shared between processes already? > > sa->dp_rx is a pointer to .data section (sfc_efx_rx or sfc_ef10_rx) > which is (may be) different in primary and secondary processes. OK, thanks. Does it make sense to implement strdup as rte_strdup, so others can re-use it? @sergio, what do you think? > >> <...> >> >>> diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c >>> index 1484bab..60812cb 100644 >>> --- a/drivers/net/sfc/sfc_ef10_rx.c >>> +++ b/drivers/net/sfc/sfc_ef10_rx.c >>> @@ -699,7 +699,7 @@ struct sfc_dp_rx sfc_ef10_rx = { >>> .type = SFC_DP_RX, >>> .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, >>> }, >>> - .features = 0, >>> + .features = SFC_DP_RX_FEAT_MULTI_PROCESS, >> Why this flag is needed, I mean why multi process support is not always >> enabled by default? > > libefx-based datapath intensively uses function pointers (primary > process function pointers stored in data structures). So, it does not > work in multi process. But this currently added always, if I don't miss anything. And only checked once in secondary path and error returned if not set. Is there any code path that behaves different based on this flag? Or any case that this flags shouldn't be set? What happens if this flag removed, and assumed it is always set? (this and tx version of this flag) > >> <...> > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process 2017-05-22 12:36 ` Ferruh Yigit @ 2017-05-22 12:44 ` Andrew Rybchenko 2017-05-22 12:54 ` Ferruh Yigit 2017-05-22 15:18 ` Sergio Gonzalez Monroy 1 sibling, 1 reply; 15+ messages in thread From: Andrew Rybchenko @ 2017-05-22 12:44 UTC (permalink / raw) To: Ferruh Yigit, dev, Sergio Gonzalez Monroy On 05/22/2017 03:36 PM, Ferruh Yigit wrote: > On 5/22/2017 1:07 PM, Andrew Rybchenko wrote: >> On 05/22/2017 02:29 PM, Ferruh Yigit wrote: >>> On 5/18/2017 3:00 PM, Andrew Rybchenko wrote: >>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>>> Reviewed-by: Andy Moreton <amoreton@solarflare.com> >>> <...> >>> >>>> diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c >>>> index 1484bab..60812cb 100644 >>>> --- a/drivers/net/sfc/sfc_ef10_rx.c >>>> +++ b/drivers/net/sfc/sfc_ef10_rx.c >>>> @@ -699,7 +699,7 @@ struct sfc_dp_rx sfc_ef10_rx = { >>>> .type = SFC_DP_RX, >>>> .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, >>>> }, >>>> - .features = 0, >>>> + .features = SFC_DP_RX_FEAT_MULTI_PROCESS, >>> Why this flag is needed, I mean why multi process support is not always >>> enabled by default? >> libefx-based datapath intensively uses function pointers (primary >> process function pointers stored in data structures). So, it does not >> work in multi process. > But this currently added always, if I don't miss anything. And only > checked once in secondary path and error returned if not set. > > Is there any code path that behaves different based on this flag? Or any > case that this flags shouldn't be set? sfc_efx_rx (and sfc_efx_tx) does not have the flag. > What happens if this flag removed, and assumed it is always set? (this > and tx version of this flag) Init in the secondary process fails if datapath chosen by the primary process does not have the flag. >>> <...> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process 2017-05-22 12:44 ` Andrew Rybchenko @ 2017-05-22 12:54 ` Ferruh Yigit 0 siblings, 0 replies; 15+ messages in thread From: Ferruh Yigit @ 2017-05-22 12:54 UTC (permalink / raw) To: Andrew Rybchenko, dev, Sergio Gonzalez Monroy On 5/22/2017 1:44 PM, Andrew Rybchenko wrote: > On 05/22/2017 03:36 PM, Ferruh Yigit wrote: >> On 5/22/2017 1:07 PM, Andrew Rybchenko wrote: >>> On 05/22/2017 02:29 PM, Ferruh Yigit wrote: >>>> On 5/18/2017 3:00 PM, Andrew Rybchenko wrote: >>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>>>> Reviewed-by: Andy Moreton <amoreton@solarflare.com> >>>> <...> >>>> >>>>> diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c >>>>> index 1484bab..60812cb 100644 >>>>> --- a/drivers/net/sfc/sfc_ef10_rx.c >>>>> +++ b/drivers/net/sfc/sfc_ef10_rx.c >>>>> @@ -699,7 +699,7 @@ struct sfc_dp_rx sfc_ef10_rx = { >>>>> .type = SFC_DP_RX, >>>>> .hw_fw_caps = SFC_DP_HW_FW_CAP_EF10, >>>>> }, >>>>> - .features = 0, >>>>> + .features = SFC_DP_RX_FEAT_MULTI_PROCESS, >>>> Why this flag is needed, I mean why multi process support is not always >>>> enabled by default? >>> libefx-based datapath intensively uses function pointers (primary >>> process function pointers stored in data structures). So, it does not >>> work in multi process. >> But this currently added always, if I don't miss anything. And only >> checked once in secondary path and error returned if not set. >> >> Is there any code path that behaves different based on this flag? Or any >> case that this flags shouldn't be set? > > sfc_efx_rx (and sfc_efx_tx) does not have the flag. Got it, thanks. > >> What happens if this flag removed, and assumed it is always set? (this >> and tx version of this flag) > > Init in the secondary process fails if datapath chosen by the primary > process does not have the flag. > >>>> <...> > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process 2017-05-22 12:36 ` Ferruh Yigit 2017-05-22 12:44 ` Andrew Rybchenko @ 2017-05-22 15:18 ` Sergio Gonzalez Monroy 1 sibling, 0 replies; 15+ messages in thread From: Sergio Gonzalez Monroy @ 2017-05-22 15:18 UTC (permalink / raw) To: Ferruh Yigit, Andrew Rybchenko, dev On 22/05/2017 13:36, Ferruh Yigit wrote: > On 5/22/2017 1:07 PM, Andrew Rybchenko wrote: >> On 05/22/2017 02:29 PM, Ferruh Yigit wrote: >>> On 5/18/2017 3:00 PM, Andrew Rybchenko wrote: >>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>>> Reviewed-by: Andy Moreton <amoreton@solarflare.com> >>> <...> >>> >>>> Linux VFIO = Y >>>> diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h >>>> index 772a713..007ed24 100644 >>>> --- a/drivers/net/sfc/sfc.h >>>> +++ b/drivers/net/sfc/sfc.h >>>> @@ -225,7 +225,18 @@ struct sfc_adapter { >>>> uint8_t rss_key[SFC_RSS_KEY_SIZE]; >>>> #endif >>>> >>>> + /* >>>> + * Shared memory copy of the Rx datapath name to be used by >>>> + * the secondary process to find Rx datapath to be used. >>>> + */ >>>> + char *dp_rx_name; >>> Why not use sa->dp_rx->dp.name to find the dp_rx? That variable should >>> be shared between processes already? >> sa->dp_rx is a pointer to .data section (sfc_efx_rx or sfc_ef10_rx) >> which is (may be) different in primary and secondary processes. > OK, thanks. > Does it make sense to implement strdup as rte_strdup, so others can > re-use it? @sergio, what do you think? IMHO I would hold until there are more cases using it. Sergio ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process Andrew Rybchenko @ 2017-05-22 15:42 ` Ferruh Yigit 2 siblings, 0 replies; 15+ messages in thread From: Ferruh Yigit @ 2017-05-22 15:42 UTC (permalink / raw) To: Andrew Rybchenko, dev On 5/18/2017 3:00 PM, Andrew Rybchenko wrote: > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > Reviewed-by: Andy Moreton <amoreton@solarflare.com> Series applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-05-22 15:42 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-17 12:25 [dpdk-dev] [PATCH 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko 2017-05-17 12:25 ` [dpdk-dev] [PATCH 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko 2017-05-18 10:59 ` Ferruh Yigit 2017-05-18 14:01 ` Andrew Rybchenko 2017-05-17 12:25 ` [dpdk-dev] [PATCH 3/3] net/sfc: support multi-process Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 2/3] net/sfc: use locally stored data for logging Andrew Rybchenko 2017-05-18 14:00 ` [dpdk-dev] [PATCH v2 3/3] net/sfc: support multi-process Andrew Rybchenko 2017-05-22 11:29 ` Ferruh Yigit 2017-05-22 12:07 ` Andrew Rybchenko 2017-05-22 12:36 ` Ferruh Yigit 2017-05-22 12:44 ` Andrew Rybchenko 2017-05-22 12:54 ` Ferruh Yigit 2017-05-22 15:18 ` Sergio Gonzalez Monroy 2017-05-22 15:42 ` [dpdk-dev] [PATCH v2 1/3] net/sfc: carefully cleanup on init failure and shutdown Ferruh Yigit
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).