diff mbox

[dpdk-dev,2/2] Remove RTE_MBUF_REFCNT references

Message ID 1424102913-18944-3-git-send-email-sergio.gonzalez.monroy@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Sergio Gonzalez Monroy Feb. 16, 2015, 4:08 p.m. UTC
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

Olivier Matz Feb. 18, 2015, 9:16 a.m. UTC | #1
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
Bruce Richardson Feb. 18, 2015, 9:35 a.m. UTC | #2
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
>
Ananyev, Konstantin Feb. 18, 2015, 9:48 a.m. UTC | #3
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
> >
Olivier Matz Feb. 18, 2015, 9:52 a.m. UTC | #4
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
>>
Bruce Richardson Feb. 18, 2015, 10 a.m. UTC | #5
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
> > >
Olivier Matz Feb. 18, 2015, 10:14 a.m. UTC | #6
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
>>>>
Ananyev, Konstantin Feb. 18, 2015, 10:22 a.m. UTC | #7
> -----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
> >>>>
Bruce Richardson Feb. 18, 2015, 10:22 a.m. UTC | #8
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
Olivier Matz Feb. 18, 2015, 10:33 a.m. UTC | #9
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
Bruce Richardson Feb. 18, 2015, 10:37 a.m. UTC | #10
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?
Ananyev, Konstantin Feb. 18, 2015, 10:47 a.m. UTC | #11
> -----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
>
Olivier Matz Feb. 18, 2015, 10:47 a.m. UTC | #12
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>
Olivier Matz Feb. 18, 2015, 11:01 a.m. UTC | #13
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
diff mbox

Patch

diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 579ebbf..54895ab 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -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),
 
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index e86ba22..9de6dea 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -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;
 }
diff --git a/config/common_bsdapp b/config/common_bsdapp
index 57bacb8..ad2d3cb 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -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
 
diff --git a/config/common_linuxapp b/config/common_linuxapp
index d428f84..5af645f 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -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
 
diff --git a/examples/Makefile b/examples/Makefile
index 81f1d2f..5a784d4 100644
--- a/examples/Makefile
+++ b/examples/Makefile
@@ -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
diff --git a/examples/ip_fragmentation/Makefile b/examples/ip_fragmentation/Makefile
index ea0e11f..c321e6a 100644
--- a/examples/ip_fragmentation/Makefile
+++ b/examples/ip_fragmentation/Makefile
@@ -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
 
diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
index b0a4106..e70fdc7 100644
--- a/examples/ip_pipeline/Makefile
+++ b/examples/ip_pipeline/Makefile
@@ -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
diff --git a/examples/ip_pipeline/main.c b/examples/ip_pipeline/main.c
index 2c53877..7eee187 100644
--- a/examples/ip_pipeline/main.c
+++ b/examples/ip_pipeline/main.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",
diff --git a/examples/ipv4_multicast/Makefile b/examples/ipv4_multicast/Makefile
index 604cebe..44f0a3b 100644
--- a/examples/ipv4_multicast/Makefile
+++ b/examples/ipv4_multicast/Makefile
@@ -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
 
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 5e341d6..592bc28 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -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. */
diff --git a/lib/librte_ip_frag/Makefile b/lib/librte_ip_frag/Makefile
index fe926f7..9d06780 100644
--- a/lib/librte_ip_frag/Makefile
+++ b/lib/librte_ip_frag/Makefile
@@ -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
diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
index 1083d44..f673728 100644
--- a/lib/librte_ip_frag/rte_ip_frag.h
+++ b/lib/librte_ip_frag/rte_ip_frag.h
@@ -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.
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 1b14e02..470e3c2 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -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)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 12e7545..d168488 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -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.
  *
diff --git a/lib/librte_pmd_bond/Makefile b/lib/librte_pmd_bond/Makefile
index d6c81a8..e9d94e1 100644
--- a/lib/librte_pmd_bond/Makefile
+++ b/lib/librte_pmd_bond/Makefile
@@ -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
 #
diff --git a/lib/librte_pmd_bond/rte_eth_bond.h b/lib/librte_pmd_bond/rte_eth_bond.h
index 7177983..99568d4 100644
--- a/lib/librte_pmd_bond/rte_eth_bond.h
+++ b/lib/librte_pmd_bond/rte_eth_bond.h
@@ -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).
  *
diff --git a/lib/librte_pmd_bond/rte_eth_bond_args.c b/lib/librte_pmd_bond/rte_eth_bond_args.c
index ca4de38..ae76bc6 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_args.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_args.c
@@ -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;
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index 09b0f30..b90bf48 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -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;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index b54cb19..8c95bd3 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -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;
diff --git a/lib/librte_port/Makefile b/lib/librte_port/Makefile
index 0e38452..de960fc 100644
--- a/lib/librte_port/Makefile
+++ b/lib/librte_port/Makefile
@@ -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