Re: [dpdk-dev] [PATCH] net/nfb: remove resources when dev is closed
On 08/09, Rastislav Cernay wrote:
>From: Rastislav Cernay
>
>The rte_eth_dev_close() function now handles freeing resources for
>devices (e.g., mac_addrs). To conform with the new close() behaviour we
>are asserting the RTE_ETH_DEV_CLOSE_REMOVE flag so that
>rte_eth_dev_close() releases all device level dynamic memory.
>
>Signed-off-by: Rastislav Cernay
>---
> drivers/net/nfb/nfb_ethdev.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/nfb/nfb_ethdev.c b/drivers/net/nfb/nfb_ethdev.c
>index c3119a0..4a19979 100644
>--- a/drivers/net/nfb/nfb_ethdev.c
>+++ b/drivers/net/nfb/nfb_ethdev.c
>@@ -210,12 +210,17 @@
> static void
> nfb_eth_dev_close(struct rte_eth_dev *dev)
> {
>+ struct pmd_internals *internals = (struct pmd_internals *)
>+ dev->data->dev_private;
The device private pointer (dev_private) is of type void *, therefore this
cast is unnecessary.
> uint16_t i;
> uint16_t nb_rx = dev->data->nb_rx_queues;
> uint16_t nb_tx = dev->data->nb_tx_queues;
>
> nfb_eth_dev_stop(dev);
>
>+ nfb_nc_rxmac_deinit(internals->rxmac, internals->max_rxmac);
>+ nfb_nc_txmac_deinit(internals->txmac, internals->max_txmac);
>+
> for (i = 0; i < nb_rx; i++) {
> nfb_eth_rx_queue_release(dev->data->rx_queues[i]);
> dev->data->rx_queues[i] = NULL;
>@@ -226,6 +231,9 @@
> dev->data->tx_queues[i] = NULL;
> }
> dev->data->nb_tx_queues = 0;
>+
>+ rte_free(dev->data->mac_addrs);
>+ dev->data->mac_addrs = NULL;
> }
>
> /**
>@@ -446,6 +454,9 @@
> rte_kvargs_free(kvlist);
> }
>
>+ /* Let rte_eth_dev_close() release the port resources */
>+ dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
>+
> /*
>* Get number of available DMA RX and TX queues, which is maximum
>* number of queues that can be created and store it in private device
>@@ -520,15 +531,10 @@
> static int
> nfb_eth_dev_uninit(struct rte_eth_dev *dev)
> {
>- struct rte_eth_dev_data *data = dev->data;
>- struct pmd_internals *internals = (struct pmd_internals *)
>- data->dev_private;
>-
> struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> struct rte_pci_addr *pci_addr = &pci_dev->addr;
>
>- nfb_nc_rxmac_deinit(internals->rxmac, internals->max_rxmac);
>- nfb_nc_txmac_deinit(internals->txmac, internals->max_txmac);
>+ nfb_eth_dev_close(dev);
>
> RTE_LOG(INFO, PMD, "NFB device ("
> PCI_PRI_FMT ") successfully uninitialized\n",
>--
>1.8.3.1
>
For the rest,
Reviewed-by: Xiaolong Ye
Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice about changes to ethernet structures
06/08/2019 13:26, Thomas Monjalon: > > >> Tell users about upcoming changes to rte_ether_addr and > > >> rte_ether_header. > > >> > > >> Signed-off-by: Stephen Hemminger > > >> Acked-by: Bruce Richardson > > > > > > Acked-by: Andrew Rybchenko > > > > Acked-by: Ferruh Yigit > > Acked-by: Thomas Monjalon Applied, thanks
Re: [dpdk-dev] [PATCH] doc: add deprecation notice to fix ethdev API returning void
06/08/2019 13:24, Thomas Monjalon: > 23/07/2019 16:22, Ferruh Yigit: > > On 7/23/2019 3:07 PM, Andrew Rybchenko wrote: > > > void return value is bad for get API (like rte_eth_dev_info-get()) > > > since caller does not know if the function does its job or not and > > > output value is filled in. > > > > > > void return value is bad for state changing API (like > > > rte_eth_promiscuous_enable()) since caller should use get API > > > to understand if state is really changed. > > > > > > Signed-off-by: Andrew Rybchenko > > Acked-by: Ferruh Yigit > Acked-by: Thomas Monjalon > Acked-by: Jerin Jacob Applied, thanks
Re: [dpdk-dev] [PATCH v2] doc: announce removal of old port count function
> >> The function rte_eth_dev_count() was marked as deprecated in DPDK 18.05
> >> in commit d9a42a69febf ("ethdev: deprecate port count function").
> >> It is planned to be removed after the next LTS release.
> >>
> >> Signed-off-by: Thomas Monjalon
> >> Acked-by: David Marchand
> > Acked-by: Konstantin Ananyev
> Acked-by: Andrew Rybchenko
> Acked-by: Jerin Jacob
Applied
Re: [dpdk-dev] [PATCH] doc: announce ABI change for RSS hash funtion
06/08/2019 16:45, Ananyev, Konstantin: > From: Thomas Monjalon > > 04/07/2019 06:43, simei: > > > From: Simei Su > > > > > > Add new field SYMMETRIC_TOEPLITZ in rte_eth_hash_function. This > > > can support symmetric hash function by rte_flow RSS action. > > > > > > Signed-off-by: Simei Su > > > --- > > > +* ethdev: New member in ``rte_eth_hash_funtion`` to support symmetric > > > hash funtion. > > > > That's unfortunate there is a typo in the name of the enum you want to > > change. > > > > Do you have any reference to the algo you want to support? A paper maybe? > > If I am not mistaken that's just an intent to enable symmetric RSS > hash-function via standard RSS rte_flow API - > feature already available with i40e and newest Intel HW. > (AFAIK on i40e right now it could be configured via > RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT). > If so, then I think the author may need to mention what PMDs will support > that feature in 19.11. Without any more comment, this patch cannot be accepted in 19.08.
Re: [dpdk-dev] [patch v5] doc: announce API change in ethdev offload flags
> > Add new offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and > > ``DEV_RX_OFFLOAD_FLOW_MARK``. > > Add new function ``rte_eth_dev_set_supported_ptypes`` to allow application > > to set specific ptypes to be updated in ``rte_mbuf::packet_type`` > > > > Signed-off-by: Pavan Nikhilesh > > Acked-by: Andrew Rybchenko > > Acked-by: Jerin Jacob > > Acked-by: Hemant Agrawal > Acked-by: Konstantin Ananyev We'll probably have to discuss more the details of these new APIs, but the overall idea looks good. I am not sure there is any API or ABI breakage here, so the announce may not be required. Anyway applied, thanks.
Re: [dpdk-dev] [PATCH 2/2] doc: announce new mbuf field for LRO
06/08/2019 20:17, Andrew Rybchenko: > On 8/6/19 5:56 PM, Matan Azrad wrote: > > The API breakage is because the ``tso_segsz`` field was documented for > > LRO. > > > > The ``tso_segsz`` field in mbuf indicates the size of each segment in > > the LRO packet in Rx path and should be provided by the LRO packet > > port. > > > > While the generic LRO packet may aggregate different segments sizes in > > one packet, it is impossible to expose this information for each segment > > by one field and it doesn't make sense to expose all the segments sizes > > in the mbuf. > > > > A new field may be added as union with the above field to expose the > > number of segments aggregated in the LRO packet. > > > > Signed-off-by: Matan Azrad > > --- > > --- a/doc/guides/rel_notes/deprecation.rst > > +++ b/doc/guides/rel_notes/deprecation.rst > > +* mbuf: Remove ``tso_segsz`` mbuf field providing for LRO support. Use > > union > > + block for the field memory to be shared with a new field ``lro_segs_n`` > > + indicates the number of segments aggregated in the LRO packet. > > I think that the number of segments is more logical in the case of LRO. > The question (already asked by Konstantin) is why it is needed at all > (statistics?). If so, it still makes sense. > > Segment size is misleading here, since not all segments > could be the same size. So, > > Acked-by: Andrew Rybchenko > > As far as I can see bnxt and qede do not fill it in. > mlx5 and vmxnet3 have the number of segments (vmxnet3 has segment > size sometimes and sometimes use a function to guess the value). > So both will win from the change. > It looks like virtio does not have number of segments. CC Maxime to > comment. I support improving the API for LRO. Unfortunately, the consensus is not strong enough at the moment. Anyway we should avoid any API breakage in 19.11, so I suggest to do only non-breaking additions in 19.11 if possible: - fill a new unioned field for LRO segments number - not set PKT_RX_LRO (which is still related to tso_segsz)
Re: [dpdk-dev] [PATCH 1/2] doc: announce ethdev ABI change for LRO fields
06/08/2019 17:27, Andrew Rybchenko: > On 8/6/19 5:56 PM, Matan Azrad wrote: > > It may be needed by the user to limit the LRO session packet size. > > In order to allow the above limitation, a new Rx configuration may be > > added for the maximum LRO session size. > > > > A new capability may be added too to expose the maximum LRO session size > > supported by the port. > > > > Signed-off-by: Matan Azrad > > --- > > +* ethdev: new 32-bit fields may be added for maximum LRO session size, in > > + struct ``rte_eth_dev_info`` for the port capability and in struct > > + ``rte_eth_rxmode`` for the port configuration. > > Acked-by: Andrew Rybchenko > > I don't know examples when the information is required, but can imagine. Acked-by: Thomas Monjalon We have only 2 acks but as they are simple new fields, better to announce their addition in advance and allow for more discussion during 19.11 release cycle. Applied (only patch 1 of 2)
Re: [dpdk-dev] [PATCH] net/bnxt: revert fix traffic stall on stop/start
09/08/2019 19:46, Ajit Khaparde:
> On Fri, Aug 9, 2019 at 10:22 AM Lance Richardson <
> lance.richard...@broadcom.com> wrote:
>
> > This reverts commit aa2c00702bad7b2c742e11a86cb9dbbb8364fd88, which
> > introduced the possibility of an invalid address exception when running
> > an application with a stopped receive queue. The issues with rxq stop/start
> > will be revisited in the 19.11 release timeframe.
> >
> > Fixes: aa2c00702bad ("net/bnxt: fix traffic stall on Rx queue stop/start")
> > Signed-off-by: Lance Richardson
> >
> Acked-by: Ajit Khaparde
Cc: sta...@dpdk.org
Applied at last minute for 19.08
Re: [dpdk-dev] [PATCH] examples/qos_meter: fix meter color type conversion
09/08/2019 13:56, Dumitrescu, Cristian:
> From: Singh, Jasvinder
> > In APP_MODE_SRTCM_COLOR_AWARE mode, sample app compilation fails
> > due to wrong meter color type conversion.
> >
> > Fixes: fc8a10d8527a ("examples/qos_meter: add color policy")
This is not the root cause. Replaced with:
Fixes: c1656328dbc2 ("meter: replace color definitions")
Cc: sta...@dpdk.org
> > error log-
> > /qos_meter/main.c:error: conversion to incomplete type
> > (enum rte_meter_color) input_color);
This log should be before "Fixes" line.
> > Signed-off-by: Jasvinder Singh
> > Reported-by: Peng Yuan
Reported-by should be before SoB line (chronological order).
> Acked-by: Cristian Dumitrescu
Applied
