[dpdk-dev,2/2] Remove RTE_MBUF_REFCNT references
Commit Message
This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
field in the mbuf struct permanently.
Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
app/test/test_link_bonding.c | 15 ---------------
app/test/test_mbuf.c | 17 ++++-------------
config/common_bsdapp | 1 -
config/common_linuxapp | 1 -
examples/Makefile | 4 ++--
examples/ip_fragmentation/Makefile | 4 ----
examples/ip_pipeline/Makefile | 3 ---
examples/ip_pipeline/main.c | 5 -----
examples/ipv4_multicast/Makefile | 4 ----
examples/vhost/main.c | 13 -------------
lib/librte_ip_frag/Makefile | 4 ----
lib/librte_ip_frag/rte_ip_frag.h | 4 ----
lib/librte_mbuf/rte_mbuf.c | 2 --
lib/librte_mbuf/rte_mbuf.h | 30 ------------------------------
lib/librte_pmd_bond/Makefile | 4 ----
lib/librte_pmd_bond/rte_eth_bond.h | 2 --
lib/librte_pmd_bond/rte_eth_bond_args.c | 2 --
lib/librte_pmd_bond/rte_eth_bond_pmd.c | 10 ----------
lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 8 --------
lib/librte_port/Makefile | 4 ----
20 files changed, 6 insertions(+), 131 deletions(-)
Comments
Hi Sergio,
On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> field in the mbuf struct permanently.
>
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
I think removing the refcount compile option goes in the right
direction. However, activating the refcount will break the applications
that reserve a private zone in mbufs. This is due to the macros
RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
data buffer.
For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
mbuf pool could store the size of the private size like it's done
for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
or m->pool, we can retrieve the mbuf pool and this value, then
compute the buffer address.
For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
a backpointer to the mbuf is always located before the data buffer,
but it looks difficult to do.
Another idea would be to add a field in indirect mbufs that stores
the pointer to the "parent" mbuf.
Regards,
Olivier
On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> Hi Sergio,
>
> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >field in the mbuf struct permanently.
> >
> >Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>
> I think removing the refcount compile option goes in the right
> direction. However, activating the refcount will break the applications
> that reserve a private zone in mbufs. This is due to the macros
> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> data buffer.
>
While I understand how the macros make certain assumptions, how does activating
the refcnt specifically lead to the problems you describe? Could you explain
that part in a bit more detail?
Thanks,
/Bruce
> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> mbuf pool could store the size of the private size like it's done
> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> or m->pool, we can retrieve the mbuf pool and this value, then
> compute the buffer address.
>
> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> a backpointer to the mbuf is always located before the data buffer,
> but it looks difficult to do.
>
> Another idea would be to add a field in indirect mbufs that stores
> the pointer to the "parent" mbuf.
>
> Regards,
> Olivier
>
Hi lads,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Wednesday, February 18, 2015 9:36 AM
> To: Olivier MATZ
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
>
> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> > Hi Sergio,
> >
> > On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> > >This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> > >field in the mbuf struct permanently.
> > >
> > >Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >
> > I think removing the refcount compile option goes in the right
> > direction. However, activating the refcount will break the applications
> > that reserve a private zone in mbufs. This is due to the macros
> > RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> > the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> > data buffer.
> >
>
> While I understand how the macros make certain assumptions, how does activating
> the refcnt specifically lead to the problems you describe? Could you explain
> that part in a bit more detail?
>
> Thanks,
> /Bruce
>
Olivier, I also don't understand your concern here.
As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
The only principal change here, is that we don't rely more on RTE_MBUF_FROM_BADDR to determine,
Is that indirect mbuf or not.
Instead we use a special falg for that purpose:
-#define RTE_MBUF_INDIRECT(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
+#define RTE_MBUF_INDIRECT(mb) (mb->ol_flags & IND_ATTACHED_MBUF)
BTW, Sergio as I said before, I think it should be:
#define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
Konstantin
> > For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> > mbuf pool could store the size of the private size like it's done
> > for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> > or m->pool, we can retrieve the mbuf pool and this value, then
> > compute the buffer address.
> >
> > For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> > a backpointer to the mbuf is always located before the data buffer,
> > but it looks difficult to do.
> >
> > Another idea would be to add a field in indirect mbufs that stores
> > the pointer to the "parent" mbuf.
> >
> > Regards,
> > Olivier
> >
Hi Bruce,
On 02/18/2015 10:35 AM, Bruce Richardson wrote:
> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
>> Hi Sergio,
>>
>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
>>> field in the mbuf struct permanently.
>>>
>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>>
>> I think removing the refcount compile option goes in the right
>> direction. However, activating the refcount will break the applications
>> that reserve a private zone in mbufs. This is due to the macros
>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
>> data buffer.
>>
>
> While I understand how the macros make certain assumptions, how does activating
> the refcnt specifically lead to the problems you describe? Could you explain
> that part in a bit more detail?
Indeed, you are right, removing the refcount option does not break
the applications if they do not use it. So there is probably no need
to fix that problem in this patch series.
However we should consider this problem as the refcount (which can
not be deactivated now) is not compatible with the private mbuf data.
By the way, once we are on this, shouldn't we consider removing the
RTE_MBUF_REFCNT_ATOMIC compile option?
Regards,
Olivier
>
> Thanks,
> /Bruce
>
>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
>> mbuf pool could store the size of the private size like it's done
>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
>> or m->pool, we can retrieve the mbuf pool and this value, then
>> compute the buffer address.
>>
>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
>> a backpointer to the mbuf is always located before the data buffer,
>> but it looks difficult to do.
>>
>> Another idea would be to add a field in indirect mbufs that stores
>> the pointer to the "parent" mbuf.
>>
>> Regards,
>> Olivier
>>
On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> Hi lads,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Wednesday, February 18, 2015 9:36 AM
> > To: Olivier MATZ
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >
> > On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> > > Hi Sergio,
> > >
> > > On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> > > >This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> > > >field in the mbuf struct permanently.
> > > >
> > > >Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > >
> > > I think removing the refcount compile option goes in the right
> > > direction. However, activating the refcount will break the applications
> > > that reserve a private zone in mbufs. This is due to the macros
> > > RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> > > the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> > > data buffer.
> > >
> >
> > While I understand how the macros make certain assumptions, how does activating
> > the refcnt specifically lead to the problems you describe? Could you explain
> > that part in a bit more detail?
> >
> > Thanks,
> > /Bruce
> >
>
> Olivier, I also don't understand your concern here.
> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> The only principal change here, is that we don't rely more on RTE_MBUF_FROM_BADDR to determine,
> Is that indirect mbuf or not.
> Instead we use a special falg for that purpose:
>
> -#define RTE_MBUF_INDIRECT(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> +#define RTE_MBUF_INDIRECT(mb) (mb->ol_flags & IND_ATTACHED_MBUF)
>
> BTW, Sergio as I said before, I think it should be:
> #define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
>
> Konstantin
>
>
> > > For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> > > mbuf pool could store the size of the private size like it's done
> > > for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> > > or m->pool, we can retrieve the mbuf pool and this value, then
> > > compute the buffer address.
Agreed, that makes sense.
> > >
> > > For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> > > a backpointer to the mbuf is always located before the data buffer,
> > > but it looks difficult to do.
On the other hand, with the proposed refcnt change Sergio proposes, we no
longer use this macro in any of the built-in mbuf handling for freeing mbufs.
Does this need to be solved at anything other than the application level?
/Bruce
> > >
> > > Another idea would be to add a field in indirect mbufs that stores
> > > the pointer to the "parent" mbuf.
> > >
> > > Regards,
> > > Olivier
> > >
On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
>> Hi lads,
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
>>> Sent: Wednesday, February 18, 2015 9:36 AM
>>> To: Olivier MATZ
>>> Cc: dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
>>>
>>> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
>>>> Hi Sergio,
>>>>
>>>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
>>>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
>>>>> field in the mbuf struct permanently.
>>>>>
>>>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>>>>
>>>> I think removing the refcount compile option goes in the right
>>>> direction. However, activating the refcount will break the applications
>>>> that reserve a private zone in mbufs. This is due to the macros
>>>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
>>>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
>>>> data buffer.
>>>>
>>>
>>> While I understand how the macros make certain assumptions, how does activating
>>> the refcnt specifically lead to the problems you describe? Could you explain
>>> that part in a bit more detail?
>>>
>>> Thanks,
>>> /Bruce
>>>
>>
>> Olivier, I also don't understand your concern here.
>> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
>> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
>> The only principal change here, is that we don't rely more on RTE_MBUF_FROM_BADDR to determine,
>> Is that indirect mbuf or not.
>> Instead we use a special falg for that purpose:
>>
>> -#define RTE_MBUF_INDIRECT(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
>> +#define RTE_MBUF_INDIRECT(mb) (mb->ol_flags & IND_ATTACHED_MBUF)
>>
>> BTW, Sergio as I said before, I think it should be:
>> #define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
>>
>> Konstantin
>>
>>
>>>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
>>>> mbuf pool could store the size of the private size like it's done
>>>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
>>>> or m->pool, we can retrieve the mbuf pool and this value, then
>>>> compute the buffer address.
>
> Agreed, that makes sense.
>
>>>>
>>>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
>>>> a backpointer to the mbuf is always located before the data buffer,
>>>> but it looks difficult to do.
>
> On the other hand, with the proposed refcnt change Sergio proposes, we no
> longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> Does this need to be solved at anything other than the application level?
It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
parent mbuf (direct) from the indirect mbuf beeing freed.
>>>>
>>>> Another idea would be to add a field in indirect mbufs that stores
>>>> the pointer to the "parent" mbuf.
>>>>
>>>> Regards,
>>>> Olivier
>>>>
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, February 18, 2015 10:15 AM
> To: Richardson, Bruce; Ananyev, Konstantin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
>
> On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> > On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> >> Hi lads,
> >>
> >>> -----Original Message-----
> >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> >>> Sent: Wednesday, February 18, 2015 9:36 AM
> >>> To: Olivier MATZ
> >>> Cc: dev@dpdk.org
> >>> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >>>
> >>> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> >>>> Hi Sergio,
> >>>>
> >>>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >>>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >>>>> field in the mbuf struct permanently.
> >>>>>
> >>>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >>>>
> >>>> I think removing the refcount compile option goes in the right
> >>>> direction. However, activating the refcount will break the applications
> >>>> that reserve a private zone in mbufs. This is due to the macros
> >>>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> >>>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> >>>> data buffer.
> >>>>
> >>>
> >>> While I understand how the macros make certain assumptions, how does activating
> >>> the refcnt specifically lead to the problems you describe? Could you explain
> >>> that part in a bit more detail?
> >>>
> >>> Thanks,
> >>> /Bruce
> >>>
> >>
> >> Olivier, I also don't understand your concern here.
> >> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> >> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> >> The only principal change here, is that we don't rely more on RTE_MBUF_FROM_BADDR to determine,
> >> Is that indirect mbuf or not.
> >> Instead we use a special falg for that purpose:
> >>
> >> -#define RTE_MBUF_INDIRECT(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> >> +#define RTE_MBUF_INDIRECT(mb) (mb->ol_flags & IND_ATTACHED_MBUF)
> >>
> >> BTW, Sergio as I said before, I think it should be:
> >> #define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >>
> >> Konstantin
> >>
> >>
> >>>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> >>>> mbuf pool could store the size of the private size like it's done
> >>>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> >>>> or m->pool, we can retrieve the mbuf pool and this value, then
> >>>> compute the buffer address.
> >
> > Agreed, that makes sense.
> >
> >>>>
> >>>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> >>>> a backpointer to the mbuf is always located before the data buffer,
> >>>> but it looks difficult to do.
> >
> > On the other hand, with the proposed refcnt change Sergio proposes, we no
> > longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> > Does this need to be solved at anything other than the application level?
>
> It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
> parent mbuf (direct) from the indirect mbuf beeing freed.
>
Yes, if the INDIRECT flag is set.
Though I still don't understand, what is the problem with these 2 macros with that patch?
Why we need to replace it with something?
What exactly you think will be broken?
Konstantin
>
>
> >>>>
> >>>> Another idea would be to add a field in indirect mbufs that stores
> >>>> the pointer to the "parent" mbuf.
> >>>>
> >>>> Regards,
> >>>> Olivier
> >>>>
On Wed, Feb 18, 2015 at 11:14:42AM +0100, Olivier MATZ wrote:
> On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> >On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> >>Hi lads,
> >>
> >>>-----Original Message-----
> >>>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> >>>Sent: Wednesday, February 18, 2015 9:36 AM
> >>>To: Olivier MATZ
> >>>Cc: dev@dpdk.org
> >>>Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >>>
> >>>On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> >>>>Hi Sergio,
> >>>>
> >>>>On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >>>>>This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >>>>>field in the mbuf struct permanently.
> >>>>>
> >>>>>Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >>>>
> >>>>I think removing the refcount compile option goes in the right
> >>>>direction. However, activating the refcount will break the applications
> >>>>that reserve a private zone in mbufs. This is due to the macros
> >>>>RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> >>>>the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> >>>>data buffer.
> >>>>
> >>>
> >>>While I understand how the macros make certain assumptions, how does activating
> >>>the refcnt specifically lead to the problems you describe? Could you explain
> >>>that part in a bit more detail?
> >>>
> >>>Thanks,
> >>>/Bruce
> >>>
> >>
> >>Olivier, I also don't understand your concern here.
> >>As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> >>They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> >>The only principal change here, is that we don't rely more on RTE_MBUF_FROM_BADDR to determine,
> >>Is that indirect mbuf or not.
> >>Instead we use a special falg for that purpose:
> >>
> >>-#define RTE_MBUF_INDIRECT(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> >>+#define RTE_MBUF_INDIRECT(mb) (mb->ol_flags & IND_ATTACHED_MBUF)
> >>
> >>BTW, Sergio as I said before, I think it should be:
> >>#define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >>
> >>Konstantin
> >>
> >>
> >>>>For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> >>>>mbuf pool could store the size of the private size like it's done
> >>>>for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> >>>>or m->pool, we can retrieve the mbuf pool and this value, then
> >>>>compute the buffer address.
> >
> >Agreed, that makes sense.
> >
> >>>>
> >>>>For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> >>>>a backpointer to the mbuf is always located before the data buffer,
> >>>>but it looks difficult to do.
> >
> >On the other hand, with the proposed refcnt change Sergio proposes, we no
> >longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> >Does this need to be solved at anything other than the application level?
>
> It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
> parent mbuf (direct) from the indirect mbuf beeing freed.
>
Yes, my bad.
How was this managed before, since refcnt field seems to be necessary in order
to effectively manage indirect mbufs? Is this just the case that this is something
that never worked and that needs to be solved, or is it something that was
working that this patch will now break?
Thanks,
/Bruce
Hi,
On 02/18/2015 11:22 AM, Bruce Richardson wrote:
> On Wed, Feb 18, 2015 at 11:14:42AM +0100, Olivier MATZ wrote:
>> On 02/18/2015 11:00 AM, Bruce Richardson wrote:
>>> On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
>>>> Hi lads,
>>>>
>>>>> -----Original Message-----
>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
>>>>> Sent: Wednesday, February 18, 2015 9:36 AM
>>>>> To: Olivier MATZ
>>>>> Cc: dev@dpdk.org
>>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
>>>>>
>>>>> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
>>>>>> Hi Sergio,
>>>>>>
>>>>>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
>>>>>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
>>>>>>> field in the mbuf struct permanently.
>>>>>>>
>>>>>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>>>>>>
>>>>>> I think removing the refcount compile option goes in the right
>>>>>> direction. However, activating the refcount will break the applications
>>>>>> that reserve a private zone in mbufs. This is due to the macros
>>>>>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
>>>>>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
>>>>>> data buffer.
>>>>>>
>>>>>
>>>>> While I understand how the macros make certain assumptions, how does activating
>>>>> the refcnt specifically lead to the problems you describe? Could you explain
>>>>> that part in a bit more detail?
>>>>>
>>>>> Thanks,
>>>>> /Bruce
>>>>>
>>>>
>>>> Olivier, I also don't understand your concern here.
>>>> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
>>>> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
>>>> The only principal change here, is that we don't rely more on RTE_MBUF_FROM_BADDR to determine,
>>>> Is that indirect mbuf or not.
>>>> Instead we use a special falg for that purpose:
>>>>
>>>> -#define RTE_MBUF_INDIRECT(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
>>>> +#define RTE_MBUF_INDIRECT(mb) (mb->ol_flags & IND_ATTACHED_MBUF)
>>>>
>>>> BTW, Sergio as I said before, I think it should be:
>>>> #define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
>>>>
>>>> Konstantin
>>>>
>>>>
>>>>>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
>>>>>> mbuf pool could store the size of the private size like it's done
>>>>>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
>>>>>> or m->pool, we can retrieve the mbuf pool and this value, then
>>>>>> compute the buffer address.
>>>
>>> Agreed, that makes sense.
>>>
>>>>>>
>>>>>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
>>>>>> a backpointer to the mbuf is always located before the data buffer,
>>>>>> but it looks difficult to do.
>>>
>>> On the other hand, with the proposed refcnt change Sergio proposes, we no
>>> longer use this macro in any of the built-in mbuf handling for freeing mbufs.
>>> Does this need to be solved at anything other than the application level?
>>
>> It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
>> parent mbuf (direct) from the indirect mbuf beeing freed.
>>
> Yes, my bad.
> How was this managed before, since refcnt field seems to be necessary in order
> to effectively manage indirect mbufs? Is this just the case that this is something
> that never worked and that needs to be solved, or is it something that was
> working that this patch will now break?
This is something that never worked before: refcounts are not compatible
with reserving private data in mbufs. This patch does not change the
issue, it is still there.
Before the patch, an application that wanted to reserve a private
data could disable refcounts at compile-time.
After the patch, the solution is just to avoid using refcounts.
Regards,
Olivier
On Wed, Feb 18, 2015 at 11:33:48AM +0100, Olivier MATZ wrote:
> Hi,
>
> On 02/18/2015 11:22 AM, Bruce Richardson wrote:
> >On Wed, Feb 18, 2015 at 11:14:42AM +0100, Olivier MATZ wrote:
> >>On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> >>>On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> >>>>Hi lads,
> >>>>
> >>>>>-----Original Message-----
> >>>>>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> >>>>>Sent: Wednesday, February 18, 2015 9:36 AM
> >>>>>To: Olivier MATZ
> >>>>>Cc: dev@dpdk.org
> >>>>>Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >>>>>
> >>>>>On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> >>>>>>Hi Sergio,
> >>>>>>
> >>>>>>On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >>>>>>>This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >>>>>>>field in the mbuf struct permanently.
> >>>>>>>
> >>>>>>>Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >>>>>>
> >>>>>>I think removing the refcount compile option goes in the right
> >>>>>>direction. However, activating the refcount will break the applications
> >>>>>>that reserve a private zone in mbufs. This is due to the macros
> >>>>>>RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> >>>>>>the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> >>>>>>data buffer.
> >>>>>>
> >>>>>
> >>>>>While I understand how the macros make certain assumptions, how does activating
> >>>>>the refcnt specifically lead to the problems you describe? Could you explain
> >>>>>that part in a bit more detail?
> >>>>>
> >>>>>Thanks,
> >>>>>/Bruce
> >>>>>
> >>>>
> >>>>Olivier, I also don't understand your concern here.
> >>>>As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> >>>>They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> >>>>The only principal change here, is that we don't rely more on RTE_MBUF_FROM_BADDR to determine,
> >>>>Is that indirect mbuf or not.
> >>>>Instead we use a special falg for that purpose:
> >>>>
> >>>>-#define RTE_MBUF_INDIRECT(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> >>>>+#define RTE_MBUF_INDIRECT(mb) (mb->ol_flags & IND_ATTACHED_MBUF)
> >>>>
> >>>>BTW, Sergio as I said before, I think it should be:
> >>>>#define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >>>>
> >>>>Konstantin
> >>>>
> >>>>
> >>>>>>For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> >>>>>>mbuf pool could store the size of the private size like it's done
> >>>>>>for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> >>>>>>or m->pool, we can retrieve the mbuf pool and this value, then
> >>>>>>compute the buffer address.
> >>>
> >>>Agreed, that makes sense.
> >>>
> >>>>>>
> >>>>>>For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> >>>>>>a backpointer to the mbuf is always located before the data buffer,
> >>>>>>but it looks difficult to do.
> >>>
> >>>On the other hand, with the proposed refcnt change Sergio proposes, we no
> >>>longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> >>>Does this need to be solved at anything other than the application level?
> >>
> >>It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
> >>parent mbuf (direct) from the indirect mbuf beeing freed.
> >>
> >Yes, my bad.
> >How was this managed before, since refcnt field seems to be necessary in order
> >to effectively manage indirect mbufs? Is this just the case that this is something
> >that never worked and that needs to be solved, or is it something that was
> >working that this patch will now break?
>
> This is something that never worked before: refcounts are not compatible
> with reserving private data in mbufs. This patch does not change the
> issue, it is still there.
>
> Before the patch, an application that wanted to reserve a private
> data could disable refcounts at compile-time.
> After the patch, the solution is just to avoid using refcounts.
>
> Regards,
> Olivier
>
Thanks for clarifying.
So, you ok with this patch as a step in the right direction?
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, February 18, 2015 10:34 AM
> To: Richardson, Bruce
> Cc: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
>
> Hi,
>
> On 02/18/2015 11:22 AM, Bruce Richardson wrote:
> > On Wed, Feb 18, 2015 at 11:14:42AM +0100, Olivier MATZ wrote:
> >> On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> >>> On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> >>>> Hi lads,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> >>>>> Sent: Wednesday, February 18, 2015 9:36 AM
> >>>>> To: Olivier MATZ
> >>>>> Cc: dev@dpdk.org
> >>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >>>>>
> >>>>> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> >>>>>> Hi Sergio,
> >>>>>>
> >>>>>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >>>>>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >>>>>>> field in the mbuf struct permanently.
> >>>>>>>
> >>>>>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >>>>>>
> >>>>>> I think removing the refcount compile option goes in the right
> >>>>>> direction. However, activating the refcount will break the applications
> >>>>>> that reserve a private zone in mbufs. This is due to the macros
> >>>>>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> >>>>>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> >>>>>> data buffer.
> >>>>>>
> >>>>>
> >>>>> While I understand how the macros make certain assumptions, how does activating
> >>>>> the refcnt specifically lead to the problems you describe? Could you explain
> >>>>> that part in a bit more detail?
> >>>>>
> >>>>> Thanks,
> >>>>> /Bruce
> >>>>>
> >>>>
> >>>> Olivier, I also don't understand your concern here.
> >>>> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> >>>> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> >>>> The only principal change here, is that we don't rely more on RTE_MBUF_FROM_BADDR to determine,
> >>>> Is that indirect mbuf or not.
> >>>> Instead we use a special falg for that purpose:
> >>>>
> >>>> -#define RTE_MBUF_INDIRECT(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> >>>> +#define RTE_MBUF_INDIRECT(mb) (mb->ol_flags & IND_ATTACHED_MBUF)
> >>>>
> >>>> BTW, Sergio as I said before, I think it should be:
> >>>> #define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >>>>
> >>>> Konstantin
> >>>>
> >>>>
> >>>>>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> >>>>>> mbuf pool could store the size of the private size like it's done
> >>>>>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> >>>>>> or m->pool, we can retrieve the mbuf pool and this value, then
> >>>>>> compute the buffer address.
> >>>
> >>> Agreed, that makes sense.
> >>>
> >>>>>>
> >>>>>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> >>>>>> a backpointer to the mbuf is always located before the data buffer,
> >>>>>> but it looks difficult to do.
> >>>
> >>> On the other hand, with the proposed refcnt change Sergio proposes, we no
> >>> longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> >>> Does this need to be solved at anything other than the application level?
> >>
> >> It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
> >> parent mbuf (direct) from the indirect mbuf beeing freed.
> >>
> > Yes, my bad.
> > How was this managed before, since refcnt field seems to be necessary in order
> > to effectively manage indirect mbufs? Is this just the case that this is something
> > that never worked and that needs to be solved, or is it something that was
> > working that this patch will now break?
>
> This is something that never worked before: refcounts are not compatible
> with reserving private data in mbufs. This patch does not change the
> issue, it is still there.
>
> Before the patch, an application that wanted to reserve a private
> data could disable refcounts at compile-time.
> After the patch, the solution is just to avoid using refcounts.
I'd say avoid using mbuf_attach/detach.
refcnt itself has nothing to do with that.
I finally understood what you are talking about ...
About private data - I suppose it is a matter of another patch.
I still think it would be better to reserve private data space before mbuf, not after
(at mbuf pool initialisation time).
Then *BADDR* macros could be unaffected.
Konstantin
>
> Regards,
> Olivier
>
Hi,
On 02/18/2015 11:37 AM, Bruce Richardson wrote:
>>> How was this managed before, since refcnt field seems to be necessary in order
>>> to effectively manage indirect mbufs? Is this just the case that this is something
>>> that never worked and that needs to be solved, or is it something that was
>>> working that this patch will now break?
>>
>> This is something that never worked before: refcounts are not compatible
>> with reserving private data in mbufs. This patch does not change the
>> issue, it is still there.
>>
>> Before the patch, an application that wanted to reserve a private
>> data could disable refcounts at compile-time.
>> After the patch, the solution is just to avoid using refcounts.
>>
>> Regards,
>> Olivier
>>
> Thanks for clarifying.
> So, you ok with this patch as a step in the right direction?
Yep,
Acked-by: Olivier Matz <olivier.matz@6wind.com>
Hi Konstantin,
On 02/18/2015 11:47 AM, Ananyev, Konstantin wrote:
>>> How was this managed before, since refcnt field seems to be necessary in order
>>> to effectively manage indirect mbufs? Is this just the case that this is something
>>> that never worked and that needs to be solved, or is it something that was
>>> working that this patch will now break?
>>
>> This is something that never worked before: refcounts are not compatible
>> with reserving private data in mbufs. This patch does not change the
>> issue, it is still there.
>>
>> Before the patch, an application that wanted to reserve a private
>> data could disable refcounts at compile-time.
>> After the patch, the solution is just to avoid using refcounts.
>
> I'd say avoid using mbuf_attach/detach.
> refcnt itself has nothing to do with that.
> I finally understood what you are talking about ...
> About private data - I suppose it is a matter of another patch.
> I still think it would be better to reserve private data space before mbuf, not after
> (at mbuf pool initialisation time).
> Then *BADDR* macros could be unaffected.
Indeed that could be a good idea.
Regards,
Olivier
@@ -708,9 +708,7 @@ test_set_bonding_mode(void)
int bonding_modes[] = { BONDING_MODE_ROUND_ROBIN,
BONDING_MODE_ACTIVE_BACKUP,
BONDING_MODE_BALANCE,
-#ifdef RTE_MBUF_REFCNT
BONDING_MODE_BROADCAST
-#endif
};
/* Test supported link bonding modes */
@@ -1425,7 +1423,6 @@ test_roundrobin_tx_burst(void)
return remove_slaves_and_stop_bonded_device();
}
-#ifdef RTE_MBUF_REFCNT
static int
verify_mbufs_ref_count(struct rte_mbuf **mbufs, int nb_mbufs, int val)
{
@@ -1439,8 +1436,6 @@ verify_mbufs_ref_count(struct rte_mbuf **mbufs, int nb_mbufs, int val)
}
return 0;
}
-#endif
-
static void
free_mbufs(struct rte_mbuf **mbufs, int nb_mbufs)
@@ -1545,12 +1540,10 @@ test_roundrobin_tx_burst_slave_tx_fail(void)
(unsigned int)port_stats.opackets, slave_expected_tx_count);
}
-#ifdef RTE_MBUF_REFCNT
/* Verify that all mbufs have a ref value of zero */
TEST_ASSERT_SUCCESS(verify_mbufs_ref_count(&pkt_burst[tx_count],
TEST_RR_SLAVE_TX_FAIL_PACKETS_COUNT, 1),
"mbufs refcnts not as expected");
-#endif
free_mbufs(&pkt_burst[tx_count], TEST_RR_SLAVE_TX_FAIL_PACKETS_COUNT);
/* Clean up and remove slaves from bonded device */
@@ -3056,12 +3049,10 @@ test_balance_tx_burst_slave_tx_fail(void)
(unsigned int)port_stats.opackets,
TEST_BAL_SLAVE_TX_FAIL_BURST_SIZE_2);
-#ifdef RTE_MBUF_REFCNT
/* Verify that all mbufs have a ref value of zero */
TEST_ASSERT_SUCCESS(verify_mbufs_ref_count(&pkts_burst_1[tx_count_1],
TEST_BAL_SLAVE_TX_FAIL_PACKETS_COUNT, 1),
"mbufs refcnts not as expected");
-#endif
free_mbufs(&pkts_burst_1[tx_count_1],
TEST_BAL_SLAVE_TX_FAIL_PACKETS_COUNT);
@@ -3472,9 +3463,6 @@ test_balance_verify_slave_link_status_change_behaviour(void)
return remove_slaves_and_stop_bonded_device();
}
-#ifdef RTE_MBUF_REFCNT
-/** Broadcast Mode Tests */
-
static int
test_broadcast_tx_burst(void)
{
@@ -4001,7 +3989,6 @@ test_broadcast_verify_slave_link_status_change_behaviour(void)
/* Clean up and remove slaves from bonded device */
return remove_slaves_and_stop_bonded_device();
}
-#endif
static int
test_reconfigure_bonded_device(void)
@@ -4592,14 +4579,12 @@ static struct unit_test_suite link_bonding_test_suite = {
TEST_CASE(test_tlb_verify_mac_assignment),
TEST_CASE(test_tlb_verify_promiscuous_enable_disable),
TEST_CASE(test_tlb_verify_slave_link_status_change_failover),
-#ifdef RTE_MBUF_REFCNT
TEST_CASE(test_broadcast_tx_burst),
TEST_CASE(test_broadcast_tx_burst_slave_tx_fail),
TEST_CASE(test_broadcast_rx_burst),
TEST_CASE(test_broadcast_verify_promiscuous_enable_disable),
TEST_CASE(test_broadcast_verify_mac_assignment),
TEST_CASE(test_broadcast_verify_slave_link_status_change_behaviour),
-#endif
TEST_CASE(test_reconfigure_bonded_device),
TEST_CASE(test_close_bonded_device),
@@ -81,7 +81,7 @@
static struct rte_mempool *pktmbuf_pool = NULL;
-#if defined RTE_MBUF_REFCNT && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
static struct rte_mempool *refcnt_pool = NULL;
static struct rte_ring *refcnt_mbuf_ring = NULL;
@@ -322,9 +322,6 @@ fail:
static int
testclone_testupdate_testdetach(void)
{
-#ifndef RTE_MBUF_REFCNT
- return 0;
-#else
struct rte_mbuf *mc = NULL;
struct rte_mbuf *clone = NULL;
@@ -363,7 +360,6 @@ fail:
if (mc)
rte_pktmbuf_free(mc);
return -1;
-#endif /* RTE_MBUF_REFCNT */
}
#undef GOTO_FAIL
@@ -396,13 +392,11 @@ test_pktmbuf_pool(void)
printf("Error pool not empty");
ret = -1;
}
-#ifdef RTE_MBUF_REFCNT
extra = rte_pktmbuf_clone(m[0], pktmbuf_pool);
if(extra != NULL) {
printf("Error pool not empty");
ret = -1;
}
-#endif
/* free them */
for (i=0; i<NB_MBUF; i++) {
if (m[i] != NULL)
@@ -504,12 +498,11 @@ test_pktmbuf_free_segment(void)
/*
* Stress test for rte_mbuf atomic refcnt.
- * Implies that:
- * RTE_MBUF_REFCNT and RTE_MBUF_REFCNT_ATOMIC are both defined.
+ * Implies that RTE_MBUF_REFCNT_ATOMIC is defined.
* For more efficency, recomended to run with RTE_LIBRTE_MBUF_DEBUG defined.
*/
-#if defined RTE_MBUF_REFCNT && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
static int
test_refcnt_slave(__attribute__((unused)) void *arg)
@@ -614,7 +607,7 @@ test_refcnt_master(void)
static int
test_refcnt_mbuf(void)
{
-#if defined RTE_MBUF_REFCNT && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
unsigned lnum, master, slave, tref;
@@ -747,7 +740,6 @@ test_failing_mbuf_sanity_check(void)
return -1;
}
-#ifdef RTE_MBUF_REFCNT
badbuf = *buf;
badbuf.refcnt = 0;
if (verify_mbuf_check_panics(&badbuf)) {
@@ -761,7 +753,6 @@ test_failing_mbuf_sanity_check(void)
printf("Error with bad-refcnt(MAX) mbuf test\n");
return -1;
}
-#endif
return 0;
}
@@ -247,7 +247,6 @@ CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
#
CONFIG_RTE_LIBRTE_MBUF=y
CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
-CONFIG_RTE_MBUF_REFCNT=y
CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
CONFIG_RTE_PKTMBUF_HEADROOM=128
@@ -255,7 +255,6 @@ CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
#
CONFIG_RTE_LIBRTE_MBUF=y
CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
-CONFIG_RTE_MBUF_REFCNT=y
CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
CONFIG_RTE_PKTMBUF_HEADROOM=128
@@ -46,8 +46,8 @@ DIRS-y += exception_path
DIRS-y += helloworld
DIRS-y += ip_pipeline
DIRS-y += ip_reassembly
-DIRS-$(CONFIG_RTE_MBUF_REFCNT) += ip_fragmentation
-DIRS-$(CONFIG_RTE_MBUF_REFCNT) += ipv4_multicast
+DIRS-$(CONFIG_RTE_IP_FRAG) += ip_fragmentation
+DIRS-y += ipv4_multicast
DIRS-$(CONFIG_RTE_LIBRTE_KNI) += kni
DIRS-y += l2fwd
DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += l2fwd-ivshmem
@@ -39,10 +39,6 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
include $(RTE_SDK)/mk/rte.vars.mk
-ifneq ($(CONFIG_RTE_MBUF_REFCNT),y)
-$(error This application requires RTE_MBUF_REFCNT to be enabled)
-endif
-
# binary name
APP = ip_fragmentation
@@ -51,11 +51,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_tx.c
SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_flow_classification.c
SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_routing.c
SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_passthrough.c
-
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_ipv4_frag.c
SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_ipv4_ras.c
-endif
ifeq ($(CONFIG_RTE_LIBRTE_ACL),y)
SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_firewall.c
@@ -148,17 +148,12 @@ app_lcore_main_loop(__attribute__((unused)) void *arg)
rte_exit(EXIT_FAILURE, "ACL not present in build\n");
#endif
-#ifdef RTE_MBUF_REFCNT
case APP_CORE_IPV4_FRAG:
app_main_loop_pipeline_ipv4_frag();
return 0;
case APP_CORE_IPV4_RAS:
app_main_loop_pipeline_ipv4_ras();
return 0;
-#else
- rte_exit(EXIT_FAILURE,
- "mbuf chaining not present in build\n");
-#endif
default:
rte_panic("%s: Invalid core type for core %u\n",
@@ -39,10 +39,6 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
include $(RTE_SDK)/mk/rte.vars.mk
-ifneq ($(CONFIG_RTE_MBUF_REFCNT),y)
-$(error This application requires RTE_MBUF_REFCNT to be enabled)
-endif
-
# binary name
APP = ipv4_multicast
@@ -726,19 +726,6 @@ us_vhost_parse_args(int argc, char **argv)
return -1;
} else
zero_copy = ret;
-
- if (zero_copy) {
-#ifdef RTE_MBUF_REFCNT
- RTE_LOG(ERR, VHOST_CONFIG, "Before running "
- "zero copy vhost APP, please "
- "disable RTE_MBUF_REFCNT\n"
- "in config file and then rebuild DPDK "
- "core lib!\n"
- "Otherwise please disable zero copy "
- "flag in command line!\n");
- return -1;
-#endif
- }
}
/* Specify the descriptor number on RX. */
@@ -42,12 +42,8 @@ EXPORT_MAP := rte_ipfrag_version.map
LIBABIVER := 1
#source files
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv4_fragmentation.c
SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv6_fragmentation.c
-else
-$(info WARNING: Fragmentation feature is disabled because it needs MBUF_REFCNT.)
-endif
SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv4_reassembly.c
SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv6_reassembly.c
SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ip_frag_common.c
@@ -180,7 +180,6 @@ rte_ip_frag_table_destroy( struct rte_ip_frag_tbl *tbl)
rte_free(tbl);
}
-#ifdef RTE_MBUF_REFCNT
/**
* This function implements the fragmentation of IPv6 packets.
*
@@ -209,7 +208,6 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
uint16_t mtu_size,
struct rte_mempool *pool_direct,
struct rte_mempool *pool_indirect);
-#endif
/*
* This function implements reassembly of fragmented IPv6 packets.
@@ -258,7 +256,6 @@ rte_ipv6_frag_get_ipv6_fragment_header(struct ipv6_hdr *hdr)
return NULL;
}
-#ifdef RTE_MBUF_REFCNT
/**
* IPv4 fragmentation.
*
@@ -287,7 +284,6 @@ int32_t rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in,
uint16_t nb_pkts_out, uint16_t mtu_size,
struct rte_mempool *pool_direct,
struct rte_mempool *pool_indirect);
-#endif
/*
* This function implements reassembly of fragmented IPv4 packets.
@@ -146,11 +146,9 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
if (m->buf_addr == NULL)
rte_panic("bad virt addr\n");
-#ifdef RTE_MBUF_REFCNT
uint16_t cnt = rte_mbuf_refcnt_read(m);
if ((cnt == 0) || (cnt == UINT16_MAX))
rte_panic("bad ref cnt\n");
-#endif
/* nothing to check for sub-segments */
if (is_header == 0)
@@ -220,11 +220,8 @@ struct rte_mbuf {
* config option.
*/
union {
-#ifdef RTE_MBUF_REFCNT
rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
uint16_t refcnt; /**< Non-atomically accessed refcnt */
-#endif
- uint16_t refcnt_reserved; /**< Do not use this field */
};
uint8_t nb_segs; /**< Number of segments. */
uint8_t port; /**< Input port. */
@@ -354,7 +351,6 @@ if (!(exp)) { \
#endif /* RTE_LIBRTE_MBUF_DEBUG */
-#ifdef RTE_MBUF_REFCNT
#ifdef RTE_MBUF_REFCNT_ATOMIC
/**
@@ -436,15 +432,6 @@ rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
rte_prefetch0(m); \
} while (0)
-#else /* ! RTE_MBUF_REFCNT */
-
-/** Mbuf prefetch */
-#define RTE_MBUF_PREFETCH_TO_FREE(m) do { } while(0)
-
-#define rte_mbuf_refcnt_set(m,v) do { } while(0)
-
-#endif /* RTE_MBUF_REFCNT */
-
/**
* Sanity checks on an mbuf.
@@ -479,10 +466,8 @@ static inline struct rte_mbuf *__rte_mbuf_raw_alloc(struct rte_mempool *mp)
if (rte_mempool_get(mp, &mb) < 0)
return NULL;
m = (struct rte_mbuf *)mb;
-#ifdef RTE_MBUF_REFCNT
RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(m) == 0);
rte_mbuf_refcnt_set(m, 1);
-#endif /* RTE_MBUF_REFCNT */
return (m);
}
@@ -497,9 +482,7 @@ static inline struct rte_mbuf *__rte_mbuf_raw_alloc(struct rte_mempool *mp)
static inline void __attribute__((always_inline))
__rte_mbuf_raw_free(struct rte_mbuf *m)
{
-#ifdef RTE_MBUF_REFCNT
RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(m) == 0);
-#endif /* RTE_MBUF_REFCNT */
rte_mempool_put(m->pool, m);
}
@@ -674,8 +657,6 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
return (m);
}
-#ifdef RTE_MBUF_REFCNT
-
/**
* Attach packet mbuf to another packet mbuf.
* After attachment we refer the mbuf we attached as 'indirect',
@@ -749,15 +730,11 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
m->ol_flags = 0;
}
-#endif /* RTE_MBUF_REFCNT */
-
-
static inline struct rte_mbuf* __attribute__((always_inline))
__rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
{
__rte_mbuf_sanity_check(m, 0);
-#ifdef RTE_MBUF_REFCNT
if (likely (rte_mbuf_refcnt_read(m) == 1) ||
likely (rte_mbuf_refcnt_update(m, -1) == 0)) {
@@ -773,12 +750,9 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
if (rte_mbuf_refcnt_update(md, -1) == 0)
__rte_mbuf_raw_free(md);
}
-#endif
return(m);
-#ifdef RTE_MBUF_REFCNT
}
return (NULL);
-#endif
}
/**
@@ -821,8 +795,6 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m)
}
}
-#ifdef RTE_MBUF_REFCNT
-
/**
* Creates a "clone" of the given packet mbuf.
*
@@ -897,8 +869,6 @@ static inline void rte_pktmbuf_refcnt_update(struct rte_mbuf *m, int16_t v)
} while ((m = m->next) != NULL);
}
-#endif /* RTE_MBUF_REFCNT */
-
/**
* Get the headroom in a packet mbuf.
*
@@ -51,10 +51,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_pmd.c
SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_args.c
SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_8023ad.c
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),n)
-$(info WARNING: Link Bonding Broadcast mode is disabled because it needs MBUF_REFCNT.)
-endif
-
#
# Export include files
#
@@ -71,12 +71,10 @@ extern "C" {
* slaves using one of three available transmit policies - l2, l2+3 or l3+4.
* See BALANCE_XMIT_POLICY macros definitions for further details on transmit
* policies. */
-#ifdef RTE_MBUF_REFCNT
#define BONDING_MODE_BROADCAST (3)
/**< Broadcast (Mode 3).
* In this mode all transmitted packets will be transmitted on all available
* active slaves of the bonded. */
-#endif
#define BONDING_MODE_8023AD (4)
/**< 802.3AD (Mode 4).
*
@@ -170,9 +170,7 @@ bond_ethdev_parse_slave_mode_kvarg(const char *key __rte_unused,
case BONDING_MODE_ROUND_ROBIN:
case BONDING_MODE_ACTIVE_BACKUP:
case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
case BONDING_MODE_BROADCAST:
-#endif
case BONDING_MODE_8023AD:
case BONDING_MODE_ADAPTIVE_TRANSMIT_LOAD_BALANCING:
return 0;
@@ -681,7 +681,6 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
return num_tx_total;
}
-#ifdef RTE_MBUF_REFCNT
static uint16_t
bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs,
uint16_t nb_pkts)
@@ -741,7 +740,6 @@ bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs,
return max_nb_of_tx_pkts;
}
-#endif
void
link_properties_set(struct rte_eth_dev *bonded_eth_dev,
@@ -839,9 +837,7 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
switch (internals->mode) {
case BONDING_MODE_ROUND_ROBIN:
case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
case BONDING_MODE_BROADCAST:
-#endif
for (i = 0; i < internals->slave_count; i++) {
if (mac_address_set(&rte_eth_devices[internals->slaves[i].port_id],
bonded_eth_dev->data->mac_addrs)) {
@@ -901,12 +897,10 @@ bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, int mode)
eth_dev->tx_pkt_burst = bond_ethdev_tx_burst_balance;
eth_dev->rx_pkt_burst = bond_ethdev_rx_burst;
break;
-#ifdef RTE_MBUF_REFCNT
case BONDING_MODE_BROADCAST:
eth_dev->tx_pkt_burst = bond_ethdev_tx_burst_broadcast;
eth_dev->rx_pkt_burst = bond_ethdev_rx_burst;
break;
-#endif
case BONDING_MODE_8023AD:
if (bond_mode_8023ad_enable(eth_dev) != 0)
return -1;
@@ -1410,9 +1404,7 @@ bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev)
/* Promiscuous mode is propagated to all slaves */
case BONDING_MODE_ROUND_ROBIN:
case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
case BONDING_MODE_BROADCAST:
-#endif
for (i = 0; i < internals->slave_count; i++)
rte_eth_promiscuous_enable(internals->slaves[i].port_id);
break;
@@ -1439,9 +1431,7 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev)
/* Promiscuous mode is propagated to all slaves */
case BONDING_MODE_ROUND_ROBIN:
case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
case BONDING_MODE_BROADCAST:
-#endif
for (i = 0; i < internals->slave_count; i++)
rte_eth_promiscuous_disable(internals->slaves[i].port_id);
break;
@@ -540,20 +540,12 @@ ixgbe_tx_free_bufs(struct igb_tx_queue *txq)
*/
txep = &((struct igb_tx_entry_v *)txq->sw_ring)[txq->tx_next_dd -
(n - 1)];
-#ifdef RTE_MBUF_REFCNT
m = __rte_pktmbuf_prefree_seg(txep[0].mbuf);
-#else
- m = txep[0].mbuf;
-#endif
if (likely(m != NULL)) {
free[0] = m;
nb_free = 1;
for (i = 1; i < n; i++) {
-#ifdef RTE_MBUF_REFCNT
m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
-#else
- m = txep[i].mbuf;
-#endif
if (likely(m != NULL)) {
if (likely(m->pool == free[0]->pool))
free[nb_free++] = m;
@@ -49,9 +49,7 @@ LIBABIVER := 1
SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ethdev.c
SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ring.c
ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y)
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_frag.c
-endif
SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ras.c
endif
SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_sched.c
@@ -62,9 +60,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port.h
SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ethdev.h
SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ring.h
ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y)
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_frag.h
-endif
SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ras.h
endif
SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_sched.h