* [dpdk-dev] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure
@ 2020-04-07 4:23 wangyunjian
2020-04-07 15:15 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: wangyunjian @ 2020-04-07 4:23 UTC (permalink / raw)
To: dev; +Cc: keith.wiles, jerry.lilijun, xudingke, Yunjian Wang, stable
From: Yunjian Wang <wangyunjian@huawei.com>
Now the rxq->pool is mbuf concatenation, But its nb_segs is 1.
When do some sanity checks on the mbuf, it fails.
Fixes: 0781f5762cfe ("net/tap: support segmented mbufs")
CC: stable@dpdk.org
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
drivers/net/tap/rte_eth_tap.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index a9ba0ca68..703fcceb9 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -339,6 +339,23 @@ tap_rx_offload_get_queue_capa(void)
DEV_RX_OFFLOAD_TCP_CKSUM;
}
+static void
+tap_rxq_pool_free(struct rte_mbuf *pool)
+{
+ struct rte_mbuf *mbuf = pool;
+ uint16_t nb_segs = 1;
+
+ if (mbuf == NULL)
+ return;
+
+ while (mbuf->next) {
+ mbuf = mbuf->next;
+ nb_segs++;
+ }
+ pool->nb_segs = nb_segs;
+ rte_pktmbuf_free(pool);
+}
+
/* Callback to handle the rx burst of packets to the correct interface and
* file descriptor(s) in a multi-queue setup.
*/
@@ -389,7 +406,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
goto end;
seg->next = NULL;
- rte_pktmbuf_free(mbuf);
+ tap_rxq_pool_free(mbuf);
goto end;
}
@@ -1033,7 +1050,7 @@ tap_dev_close(struct rte_eth_dev *dev)
rxq = &internals->rxq[i];
close(process_private->rxq_fds[i]);
process_private->rxq_fds[i] = -1;
- rte_pktmbuf_free(rxq->pool);
+ tap_rxq_pool_free(rxq->pool);
rte_free(rxq->iovecs);
rxq->pool = NULL;
rxq->iovecs = NULL;
@@ -1072,7 +1089,7 @@ tap_rx_queue_release(void *queue)
if (process_private->rxq_fds[rxq->queue_id] > 0) {
close(process_private->rxq_fds[rxq->queue_id]);
process_private->rxq_fds[rxq->queue_id] = -1;
- rte_pktmbuf_free(rxq->pool);
+ tap_rxq_pool_free(rxq->pool);
rte_free(rxq->iovecs);
rxq->pool = NULL;
rxq->iovecs = NULL;
@@ -1480,7 +1497,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
return 0;
error:
- rte_pktmbuf_free(rxq->pool);
+ tap_rxq_pool_free(rxq->pool);
rxq->pool = NULL;
rte_free(rxq->iovecs);
rxq->iovecs = NULL;
@@ -2435,7 +2452,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
rxq = &internals->rxq[i];
close(process_private->rxq_fds[i]);
process_private->rxq_fds[i] = -1;
- rte_pktmbuf_free(rxq->pool);
+ tap_rxq_pool_free(rxq->pool);
rte_free(rxq->iovecs);
rxq->pool = NULL;
rxq->iovecs = NULL;
--
2.19.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure
2020-04-07 4:23 [dpdk-dev] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure wangyunjian
@ 2020-04-07 15:15 ` Ferruh Yigit
2020-04-07 15:38 ` Stephen Hemminger
2020-04-07 15:58 ` Ferruh Yigit
0 siblings, 2 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-04-07 15:15 UTC (permalink / raw)
To: wangyunjian, dev; +Cc: keith.wiles, jerry.lilijun, xudingke, stable
On 4/7/2020 5:23 AM, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian@huawei.com>
>
> Now the rxq->pool is mbuf concatenation, But its nb_segs is 1.
> When do some sanity checks on the mbuf, it fails.
+1, 'rxq->pool' seems Rx ring representation as linked mbufs and empty ones has
'nb_segs' values as 1.
>
> Fixes: 0781f5762cfe ("net/tap: support segmented mbufs")
> CC: stable@dpdk.org
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 27 ++++++++++++++++++++++-----
> 1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index a9ba0ca68..703fcceb9 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -339,6 +339,23 @@ tap_rx_offload_get_queue_capa(void)
> DEV_RX_OFFLOAD_TCP_CKSUM;
> }
>
> +static void
> +tap_rxq_pool_free(struct rte_mbuf *pool)
> +{
> + struct rte_mbuf *mbuf = pool;
> + uint16_t nb_segs = 1;
> +
> + if (mbuf == NULL)
> + return;
> +
> + while (mbuf->next) {
> + mbuf = mbuf->next;
> + nb_segs++;
> + }
> + pool->nb_segs = nb_segs;
> + rte_pktmbuf_free(pool);
> +}
Since you are already iterating the chain, why not free immediately instead of
calculating the nb_segs and making API go through the chain again, what about
following:
tap_rxq_pool_free(struct rte_mbuf *pool)
{
struct rte_mbuf *next;
while (pool) {
next = pool->next;
rte_pktmbuf_free(pool);
pool = next;
}
}
> +
> /* Callback to handle the rx burst of packets to the correct interface and
> * file descriptor(s) in a multi-queue setup.
> */
> @@ -389,7 +406,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> goto end;
>
> seg->next = NULL;
> - rte_pktmbuf_free(mbuf);
> + tap_rxq_pool_free(mbuf);
As far as I can see 'mbuf' should have correct 'nb_segs' value, and it can
continue to use 'rte_pktmbuf_free()'. If you can observe the problem can you
please try this?
>
> goto end;
> }
> @@ -1033,7 +1050,7 @@ tap_dev_close(struct rte_eth_dev *dev)
> rxq = &internals->rxq[i];
> close(process_private->rxq_fds[i]);
> process_private->rxq_fds[i] = -1;
> - rte_pktmbuf_free(rxq->pool);
> + tap_rxq_pool_free(rxq->pool);
> rte_free(rxq->iovecs);
> rxq->pool = NULL;
> rxq->iovecs = NULL;
> @@ -1072,7 +1089,7 @@ tap_rx_queue_release(void *queue)
> if (process_private->rxq_fds[rxq->queue_id] > 0) {
> close(process_private->rxq_fds[rxq->queue_id]);
> process_private->rxq_fds[rxq->queue_id] = -1;
> - rte_pktmbuf_free(rxq->pool);
> + tap_rxq_pool_free(rxq->pool);
> rte_free(rxq->iovecs);
> rxq->pool = NULL;
> rxq->iovecs = NULL;
> @@ -1480,7 +1497,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
> return 0;
>
> error:
> - rte_pktmbuf_free(rxq->pool);
> + tap_rxq_pool_free(rxq->pool);
> rxq->pool = NULL;
> rte_free(rxq->iovecs);
> rxq->iovecs = NULL;
> @@ -2435,7 +2452,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
> rxq = &internals->rxq[i];
> close(process_private->rxq_fds[i]);
> process_private->rxq_fds[i] = -1;
> - rte_pktmbuf_free(rxq->pool);
> + tap_rxq_pool_free(rxq->pool);
> rte_free(rxq->iovecs);
> rxq->pool = NULL;
> rxq->iovecs = NULL;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure
2020-04-07 15:15 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
@ 2020-04-07 15:38 ` Stephen Hemminger
2020-04-07 15:45 ` Ferruh Yigit
2020-04-07 15:58 ` Ferruh Yigit
1 sibling, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2020-04-07 15:38 UTC (permalink / raw)
To: Ferruh Yigit
Cc: wangyunjian, dev, keith.wiles, jerry.lilijun, xudingke, stable
On Tue, 7 Apr 2020 16:15:16 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > +static void
> > +tap_rxq_pool_free(struct rte_mbuf *pool)
> > +{
> > + struct rte_mbuf *mbuf = pool;
> > + uint16_t nb_segs = 1;
> > +
> > + if (mbuf == NULL)
> > + return;
> > +
> > + while (mbuf->next) {
> > + mbuf = mbuf->next;
> > + nb_segs++;
> > + }
> > + pool->nb_segs = nb_segs;
> > + rte_pktmbuf_free(pool);
> > +}
Since mbuf is going to be free, why bother with nb_segs.
Since rte_pktmbuf_free takes NULL as an argument, and frees the m->next chain
I don't see why not just
rte_pktmbuf_free(pool)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure
2020-04-07 15:38 ` Stephen Hemminger
@ 2020-04-07 15:45 ` Ferruh Yigit
2020-04-07 15:49 ` Ferruh Yigit
2020-04-07 16:08 ` Stephen Hemminger
0 siblings, 2 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-04-07 15:45 UTC (permalink / raw)
To: Stephen Hemminger
Cc: wangyunjian, dev, keith.wiles, jerry.lilijun, xudingke, stable
On 4/7/2020 4:38 PM, Stephen Hemminger wrote:
> On Tue, 7 Apr 2020 16:15:16 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
>>> +static void
>>> +tap_rxq_pool_free(struct rte_mbuf *pool)
>>> +{
>>> + struct rte_mbuf *mbuf = pool;
>>> + uint16_t nb_segs = 1;
>>> +
>>> + if (mbuf == NULL)
>>> + return;
>>> +
>>> + while (mbuf->next) {
>>> + mbuf = mbuf->next;
>>> + nb_segs++;
>>> + }
>>> + pool->nb_segs = nb_segs;
>>> + rte_pktmbuf_free(pool);
>>> +}
>
> Since mbuf is going to be free, why bother with nb_segs.
> Since rte_pktmbuf_free takes NULL as an argument, and frees the m->next chain
> I don't see why not just
> rte_pktmbuf_free(pool)
>
Chain is not constructed properly, 'nb_segs' is wrong, only 'rte_pktmbuf_free()'
call won't free all the chain but first mbuf.
This implementation is fixing 'nb_segs' sot that 'rte_pktmbuf_free()' can work
as you suggested.
Or I suggest iterate the list and fix all mbufs, instead of fixing 'nb_segs',
this may be one iteration less.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure
2020-04-07 15:45 ` Ferruh Yigit
@ 2020-04-07 15:49 ` Ferruh Yigit
2020-04-07 16:08 ` Stephen Hemminger
1 sibling, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-04-07 15:49 UTC (permalink / raw)
To: Stephen Hemminger
Cc: wangyunjian, dev, keith.wiles, jerry.lilijun, xudingke, stable
On 4/7/2020 4:45 PM, Ferruh Yigit wrote:
> On 4/7/2020 4:38 PM, Stephen Hemminger wrote:
>> On Tue, 7 Apr 2020 16:15:16 +0100
>> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>>>> +static void
>>>> +tap_rxq_pool_free(struct rte_mbuf *pool)
>>>> +{
>>>> + struct rte_mbuf *mbuf = pool;
>>>> + uint16_t nb_segs = 1;
>>>> +
>>>> + if (mbuf == NULL)
>>>> + return;
>>>> +
>>>> + while (mbuf->next) {
>>>> + mbuf = mbuf->next;
>>>> + nb_segs++;
>>>> + }
>>>> + pool->nb_segs = nb_segs;
>>>> + rte_pktmbuf_free(pool);
>>>> +}
>>
>> Since mbuf is going to be free, why bother with nb_segs.
>> Since rte_pktmbuf_free takes NULL as an argument, and frees the m->next chain
>> I don't see why not just
>> rte_pktmbuf_free(pool)
>>
>
> Chain is not constructed properly, 'nb_segs' is wrong, only 'rte_pktmbuf_free()'
> call won't free all the chain but first mbuf.
No, my bad. It will free all and will work as expected. But this is to fix the
mbuf sanity checks when they are enabled.
>
> This implementation is fixing 'nb_segs' sot that 'rte_pktmbuf_free()' can work
> as you suggested.
>
> Or I suggest iterate the list and fix all mbufs, instead of fixing 'nb_segs',
> this may be one iteration less.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure
2020-04-07 15:15 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2020-04-07 15:38 ` Stephen Hemminger
@ 2020-04-07 15:58 ` Ferruh Yigit
1 sibling, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-04-07 15:58 UTC (permalink / raw)
To: wangyunjian, dev; +Cc: keith.wiles, jerry.lilijun, xudingke, stable
On 4/7/2020 4:15 PM, Ferruh Yigit wrote:
> On 4/7/2020 5:23 AM, wangyunjian wrote:
>> From: Yunjian Wang <wangyunjian@huawei.com>
>>
>> Now the rxq->pool is mbuf concatenation, But its nb_segs is 1.
>> When do some sanity checks on the mbuf, it fails.
>
> +1, 'rxq->pool' seems Rx ring representation as linked mbufs and empty ones has
> 'nb_segs' values as 1.
>
>>
>> Fixes: 0781f5762cfe ("net/tap: support segmented mbufs")
>> CC: stable@dpdk.org
>>
>> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
>> ---
>> drivers/net/tap/rte_eth_tap.c | 27 ++++++++++++++++++++++-----
>> 1 file changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index a9ba0ca68..703fcceb9 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -339,6 +339,23 @@ tap_rx_offload_get_queue_capa(void)
>> DEV_RX_OFFLOAD_TCP_CKSUM;
>> }
>>
>> +static void
>> +tap_rxq_pool_free(struct rte_mbuf *pool)
>> +{
>> + struct rte_mbuf *mbuf = pool;
>> + uint16_t nb_segs = 1;
>> +
>> + if (mbuf == NULL)
>> + return;
>> +
>> + while (mbuf->next) {
>> + mbuf = mbuf->next;
>> + nb_segs++;
>> + }
>> + pool->nb_segs = nb_segs;
>> + rte_pktmbuf_free(pool);
>> +}
>
> Since you are already iterating the chain, why not free immediately instead of
> calculating the nb_segs and making API go through the chain again, what about
> following:
>
> tap_rxq_pool_free(struct rte_mbuf *pool)
> {
> struct rte_mbuf *next;
> while (pool) {
> next = pool->next;
> rte_pktmbuf_free(pool);
> pool = next;
> }
> }
Ignore this please, this may be still complaining in mbuf sanity check, so OK to
your usage.
>
>> +
>> /* Callback to handle the rx burst of packets to the correct interface and
>> * file descriptor(s) in a multi-queue setup.
>> */
>> @@ -389,7 +406,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>> goto end;
>>
>> seg->next = NULL;
>> - rte_pktmbuf_free(mbuf);
>> + tap_rxq_pool_free(mbuf);
>
> As far as I can see 'mbuf' should have correct 'nb_segs' value, and it can
> continue to use 'rte_pktmbuf_free()'. If you can observe the problem can you
> please try this?
>
>>
>> goto end;
>> }
>> @@ -1033,7 +1050,7 @@ tap_dev_close(struct rte_eth_dev *dev)
>> rxq = &internals->rxq[i];
>> close(process_private->rxq_fds[i]);
>> process_private->rxq_fds[i] = -1;
>> - rte_pktmbuf_free(rxq->pool);
>> + tap_rxq_pool_free(rxq->pool);
>> rte_free(rxq->iovecs);
>> rxq->pool = NULL;
>> rxq->iovecs = NULL;
>> @@ -1072,7 +1089,7 @@ tap_rx_queue_release(void *queue)
>> if (process_private->rxq_fds[rxq->queue_id] > 0) {
>> close(process_private->rxq_fds[rxq->queue_id]);
>> process_private->rxq_fds[rxq->queue_id] = -1;
>> - rte_pktmbuf_free(rxq->pool);
>> + tap_rxq_pool_free(rxq->pool);
>> rte_free(rxq->iovecs);
>> rxq->pool = NULL;
>> rxq->iovecs = NULL;
>> @@ -1480,7 +1497,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
>> return 0;
>>
>> error:
>> - rte_pktmbuf_free(rxq->pool);
>> + tap_rxq_pool_free(rxq->pool);
>> rxq->pool = NULL;
>> rte_free(rxq->iovecs);
>> rxq->iovecs = NULL;
>> @@ -2435,7 +2452,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
>> rxq = &internals->rxq[i];
>> close(process_private->rxq_fds[i]);
>> process_private->rxq_fds[i] = -1;
>> - rte_pktmbuf_free(rxq->pool);
>> + tap_rxq_pool_free(rxq->pool);
>> rte_free(rxq->iovecs);
>> rxq->pool = NULL;
>> rxq->iovecs = NULL;
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure
2020-04-07 15:45 ` Ferruh Yigit
2020-04-07 15:49 ` Ferruh Yigit
@ 2020-04-07 16:08 ` Stephen Hemminger
2020-04-08 1:10 ` wangyunjian
1 sibling, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2020-04-07 16:08 UTC (permalink / raw)
To: Ferruh Yigit
Cc: wangyunjian, dev, keith.wiles, jerry.lilijun, xudingke, stable
On Tue, 7 Apr 2020 16:45:59 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 4/7/2020 4:38 PM, Stephen Hemminger wrote:
> > On Tue, 7 Apr 2020 16:15:16 +0100
> > Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >
> >>> +static void
> >>> +tap_rxq_pool_free(struct rte_mbuf *pool)
> >>> +{
> >>> + struct rte_mbuf *mbuf = pool;
> >>> + uint16_t nb_segs = 1;
> >>> +
> >>> + if (mbuf == NULL)
> >>> + return;
> >>> +
> >>> + while (mbuf->next) {
> >>> + mbuf = mbuf->next;
> >>> + nb_segs++;
> >>> + }
> >>> + pool->nb_segs = nb_segs;
> >>> + rte_pktmbuf_free(pool);
> >>> +}
> >
> > Since mbuf is going to be free, why bother with nb_segs.
> > Since rte_pktmbuf_free takes NULL as an argument, and frees the m->next chain
> > I don't see why not just
> > rte_pktmbuf_free(pool)
> >
>
> Chain is not constructed properly, 'nb_segs' is wrong, only 'rte_pktmbuf_free()'
> call won't free all the chain but first mbuf.
>
> This implementation is fixing 'nb_segs' sot that 'rte_pktmbuf_free()' can work
> as you suggested.
>
> Or I suggest iterate the list and fix all mbufs, instead of fixing 'nb_segs',
> this may be one iteration less.
If you look at implementation of rte_pktmbuf_free() in current DPDK version
it does not care what nb_segs is set to.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure
2020-04-07 16:08 ` Stephen Hemminger
@ 2020-04-08 1:10 ` wangyunjian
0 siblings, 0 replies; 8+ messages in thread
From: wangyunjian @ 2020-04-08 1:10 UTC (permalink / raw)
To: Stephen Hemminger, Ferruh Yigit
Cc: dev, keith.wiles, Lilijun (Jerry), xudingke, stable
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, April 8, 2020 12:08 AM
> To: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org;
> keith.wiles@intel.com; Lilijun (Jerry) <jerry.lilijun@huawei.com>; xudingke
> <xudingke@huawei.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for
> mbuf's nb_segs failure
>
> On Tue, 7 Apr 2020 16:45:59 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> > On 4/7/2020 4:38 PM, Stephen Hemminger wrote:
> > > On Tue, 7 Apr 2020 16:15:16 +0100
> > > Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > >
> > >>> +static void
> > >>> +tap_rxq_pool_free(struct rte_mbuf *pool) {
> > >>> + struct rte_mbuf *mbuf = pool;
> > >>> + uint16_t nb_segs = 1;
> > >>> +
> > >>> + if (mbuf == NULL)
> > >>> + return;
> > >>> +
> > >>> + while (mbuf->next) {
> > >>> + mbuf = mbuf->next;
> > >>> + nb_segs++;
> > >>> + }
> > >>> + pool->nb_segs = nb_segs;
> > >>> + rte_pktmbuf_free(pool);
> > >>> +}
> > >
> > > Since mbuf is going to be free, why bother with nb_segs.
> > > Since rte_pktmbuf_free takes NULL as an argument, and frees the
> > > m->next chain I don't see why not just
> > > rte_pktmbuf_free(pool)
> > >
> >
> > Chain is not constructed properly, 'nb_segs' is wrong, only
> 'rte_pktmbuf_free()'
> > call won't free all the chain but first mbuf.
> >
> > This implementation is fixing 'nb_segs' sot that 'rte_pktmbuf_free()'
> > can work as you suggested.
> >
> > Or I suggest iterate the list and fix all mbufs, instead of fixing
> > 'nb_segs', this may be one iteration less.
>
> If you look at implementation of rte_pktmbuf_free() in current DPDK version it
> does not care what nb_segs is set to.
I found this problem with mbuf debug enabled.
Thanks
Yunjian
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-04-08 1:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-07 4:23 [dpdk-dev] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure wangyunjian
2020-04-07 15:15 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2020-04-07 15:38 ` Stephen Hemminger
2020-04-07 15:45 ` Ferruh Yigit
2020-04-07 15:49 ` Ferruh Yigit
2020-04-07 16:08 ` Stephen Hemminger
2020-04-08 1:10 ` wangyunjian
2020-04-07 15:58 ` 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).