[dpdk-stable] [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment

[dpdk-stable] [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment

[Jerin Jacob]

The struct rte_eth_dev_data is used in ethdev fastpath routines
and it not aligned to cache line size. This patch fixes the ethdev
data alignment.

The alignment was broken from the "first public release" changeset
where ethdev data address was aligned only to the first port.
Remaining ports alignment was defined by the size of the struct
(rte_eth_dev_data). This scheme is not guaranteed to be cache line
aligned all the time.

"ethdev: add port ownership" change set introduced a
rte_eth_dev_shared_data container for port ownership change,
This resulted in rte_eth_dev->data memory for the first port also
as cache unaligned.

Added a compiler alignment attribute to make sure
rte_eth_dev->data always cache aligned so that CPU/compiler
1) Avoid sharing the element with another cache line
2) Can load/store the elements in struct rte_eth_dev_data as
naturally aligned.

Some platform like thunderX could see performance regression of 1%
at "ethdev: add port ownership" change set with
1 port/1 queue l3fwd application and this patch fixes that regression.

example command:
sudo ./examples/l3fwd/build/l3fwd -c 0xff00 -- -p 0x1 --config="(0,0,9)"

Fixes: af75078fece3 ("first public release")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")

Cc: stable@dpdk.org

Cc: Matan Azrad <matan@mellanox.com>
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
---

v3:
- added more history/data in the git log
- added stable@dpdk.org in Cc

v2:
 - Change the git comments based on Matan's feedback
http://dpdk.org/dev/patchwork/patch/35104/ 

---
 lib/librte_ether/rte_ethdev_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_ether/rte_ethdev_core.h b/lib/librte_ether/rte_ethdev_core.h
index 315b31723..e5681e466 100644
--- a/lib/librte_ether/rte_ethdev_core.h
+++ b/lib/librte_ether/rte_ethdev_core.h
@@ -601,7 +601,7 @@ struct rte_eth_dev_data {
 	struct rte_vlan_filter_conf vlan_filter_conf;
 	/**< VLAN filter configuration. */
 	struct rte_eth_dev_owner owner; /**< The port owner. */
-};
+} __rte_cache_aligned;
 
 /**
  * @internal
-- 
2.16.1

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment

[Matan Azrad]

Hi Jerin

From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> The struct rte_eth_dev_data is used in ethdev fastpath routines and it not
> aligned to cache line size. This patch fixes the ethdev data alignment.
> 
> The alignment was broken from the "first public release" changeset where
> ethdev data address was aligned only to the first port.
> Remaining ports alignment was defined by the size of the struct
> (rte_eth_dev_data). This scheme is not guaranteed to be cache line aligned
> all the time.
> 
> "ethdev: add port ownership" change set introduced a
> rte_eth_dev_shared_data container for port ownership change, This
> resulted in rte_eth_dev->data memory for the first port also as cache
> unaligned.
> 
> Added a compiler alignment attribute to make sure rte_eth_dev->data
> always cache aligned so that CPU/compiler
> 1) Avoid sharing the element with another cache line
> 2) Can load/store the elements in struct rte_eth_dev_data as naturally
> aligned.
> 
> Some platform like thunderX could see performance regression of 1% at
> "ethdev: add port ownership" change set with
> 1 port/1 queue l3fwd application and this patch fixes that regression.
> 
> example command:
> sudo ./examples/l3fwd/build/l3fwd -c 0xff00 -- -p 0x1 --config="(0,0,9)"
> 
> Fixes: af75078fece3 ("first public release")
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")

I don't think you need the add the 5b7ba31148a8 fix line.
Maybe think about it in the next way:
Is your fix can stay as a same fix before the port ownership feature addition?
If yes, You are not fixing it.

I think that the port ownership just exposed the issue, did not initiated it.

> Cc: stable@dpdk.org
> 
> Cc: Matan Azrad <matan@mellanox.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> ---
> 
> v3:
> - added more history/data in the git log
> - added stable@dpdk.org in Cc
> 
> v2:
>  - Change the git comments based on Matan's feedback
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpd
> k.org%2Fdev%2Fpatchwork%2Fpatch%2F35104%2F&data=02%7C01%7Cmata
> n%40mellanox.com%7Cd491698ead5f48626c1408d5721a8331%7C8c5c71989d
> be4cf9a2099e499881116c%7C0%7C0%7C636540380570748030&sdata=if8cCrk3
> U0XqWp8NE%2BEP1BjuPHX95HHN0f1yjk6krV0%3D&reserved=0
> 
> ---
>  lib/librte_ether/rte_ethdev_core.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev_core.h
> b/lib/librte_ether/rte_ethdev_core.h
> index 315b31723..e5681e466 100644
> --- a/lib/librte_ether/rte_ethdev_core.h
> +++ b/lib/librte_ether/rte_ethdev_core.h
> @@ -601,7 +601,7 @@ struct rte_eth_dev_data {
>  	struct rte_vlan_filter_conf vlan_filter_conf;
>  	/**< VLAN filter configuration. */
>  	struct rte_eth_dev_owner owner; /**< The port owner. */ -};
> +} __rte_cache_aligned;
> 
>  /**
>   * @internal
> --
> 2.16.1

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment

[Jerin Jacob]

-----Original Message-----
> Date: Mon, 12 Feb 2018 13:44:54 +0000
> From: Matan Azrad <matan@mellanox.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> CC: "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>, "stable@dpdk.org"
>  <stable@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>, Konstantin
>  Ananyev <konstantin.ananyev@intel.com>, Pavan Nikhilesh
>  <pbhagavatula@caviumnetworks.com>
> Subject: RE: [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment
> 
> Hi Jerin
> 
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > The struct rte_eth_dev_data is used in ethdev fastpath routines and it not
> > aligned to cache line size. This patch fixes the ethdev data alignment.
> > 
> > The alignment was broken from the "first public release" changeset where
> > ethdev data address was aligned only to the first port.
> > Remaining ports alignment was defined by the size of the struct
> > (rte_eth_dev_data). This scheme is not guaranteed to be cache line aligned
> > all the time.
> > 
> > "ethdev: add port ownership" change set introduced a
> > rte_eth_dev_shared_data container for port ownership change, This
> > resulted in rte_eth_dev->data memory for the first port also as cache
> > unaligned.
> > 
> > Added a compiler alignment attribute to make sure rte_eth_dev->data
> > always cache aligned so that CPU/compiler
> > 1) Avoid sharing the element with another cache line
> > 2) Can load/store the elements in struct rte_eth_dev_data as naturally
> > aligned.
> > 
> > Some platform like thunderX could see performance regression of 1% at
> > "ethdev: add port ownership" change set with
> > 1 port/1 queue l3fwd application and this patch fixes that regression.
> > 
> > example command:
> > sudo ./examples/l3fwd/build/l3fwd -c 0xff00 -- -p 0x1 --config="(0,0,9)"
> > 
> > Fixes: af75078fece3 ("first public release")
> > Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> 
> I don't think you need the add the 5b7ba31148a8 fix line.
> Maybe think about it in the next way:
> Is your fix can stay as a same fix before the port ownership feature addition?
> If yes, You are not fixing it.

I don't think so. 5b7ba31148a8 breaking the first port alignment case(as
mentioned in the commit log clearly).
First port alignment was correct prior to 5b7ba31148a8 change set

Do you agree?

> 
> I think that the port ownership just exposed the issue, did not initiated it.
> 
> > Cc: stable@dpdk.org
> > 
> > Cc: Matan Azrad <matan@mellanox.com>
> > Cc: Thomas Monjalon <thomas@monjalon.net>
> > Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > ---
> > 
> > v3:
> > - added more history/data in the git log
> > - added stable@dpdk.org in Cc
> > 
> > v2:
> >  - Change the git comments based on Matan's feedback
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpd
> > k.org%2Fdev%2Fpatchwork%2Fpatch%2F35104%2F&data=02%7C01%7Cmata
> > n%40mellanox.com%7Cd491698ead5f48626c1408d5721a8331%7C8c5c71989d
> > be4cf9a2099e499881116c%7C0%7C0%7C636540380570748030&sdata=if8cCrk3
> > U0XqWp8NE%2BEP1BjuPHX95HHN0f1yjk6krV0%3D&reserved=0
> > 
> > ---
> >  lib/librte_ether/rte_ethdev_core.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/librte_ether/rte_ethdev_core.h
> > b/lib/librte_ether/rte_ethdev_core.h
> > index 315b31723..e5681e466 100644
> > --- a/lib/librte_ether/rte_ethdev_core.h
> > +++ b/lib/librte_ether/rte_ethdev_core.h
> > @@ -601,7 +601,7 @@ struct rte_eth_dev_data {
> >  	struct rte_vlan_filter_conf vlan_filter_conf;
> >  	/**< VLAN filter configuration. */
> >  	struct rte_eth_dev_owner owner; /**< The port owner. */ -};
> > +} __rte_cache_aligned;
> > 
> >  /**
> >   * @internal
> > --
> > 2.16.1
> 

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment

[Matan Azrad]

From: Jerin Jacob, Sent: Monday, February 12, 2018 3:51 PM
> -----Original Message-----
> > Date: Mon, 12 Feb 2018 13:44:54 +0000
> > From: Matan Azrad <matan@mellanox.com>
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
> >  <dev@dpdk.org>
> > CC: "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>, "stable@dpdk.org"
> >  <stable@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>,
> Konstantin
> > Ananyev <konstantin.ananyev@intel.com>, Pavan Nikhilesh
> > <pbhagavatula@caviumnetworks.com>
> > Subject: RE: [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment
> >
> > Hi Jerin
> >
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > > The struct rte_eth_dev_data is used in ethdev fastpath routines and
> > > it not aligned to cache line size. This patch fixes the ethdev data
> alignment.
> > >
> > > The alignment was broken from the "first public release" changeset
> > > where ethdev data address was aligned only to the first port.
> > > Remaining ports alignment was defined by the size of the struct
> > > (rte_eth_dev_data). This scheme is not guaranteed to be cache line
> > > aligned all the time.
> > >
> > > "ethdev: add port ownership" change set introduced a
> > > rte_eth_dev_shared_data container for port ownership change, This
> > > resulted in rte_eth_dev->data memory for the first port also as
> > > cache unaligned.
> > >
> > > Added a compiler alignment attribute to make sure rte_eth_dev->data
> > > always cache aligned so that CPU/compiler
> > > 1) Avoid sharing the element with another cache line
> > > 2) Can load/store the elements in struct rte_eth_dev_data as
> > > naturally aligned.
> > >
> > > Some platform like thunderX could see performance regression of 1%
> > > at
> > > "ethdev: add port ownership" change set with
> > > 1 port/1 queue l3fwd application and this patch fixes that regression.
> > >
> > > example command:
> > > sudo ./examples/l3fwd/build/l3fwd -c 0xff00 -- -p 0x1 --config="(0,0,9)"
> > >
> > > Fixes: af75078fece3 ("first public release")
> > > Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> >
> > I don't think you need the add the 5b7ba31148a8 fix line.
> > Maybe think about it in the next way:
> > Is your fix can stay as a same fix before the port ownership feature
> addition?
> > If yes, You are not fixing it.
> 
> I don't think so. 5b7ba31148a8 breaking the first port alignment case(as
> mentioned in the commit log clearly).
> First port alignment was correct prior to 5b7ba31148a8 change set
> 
> Do you agree?

No.
The root cause of the issue this fix is fixing, is that the structure is not aligned, it is not relevant to port ownership.
The port ownership just exposes it.

> >
> > I think that the port ownership just exposed the issue, did not initiated it.
> >
> > > Cc: stable@dpdk.org
> > >
> > > Cc: Matan Azrad <matan@mellanox.com>
> > > Cc: Thomas Monjalon <thomas@monjalon.net>
> > > Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > >
> > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > ---
> > >
> > > v3:
> > > - added more history/data in the git log
> > > - added stable@dpdk.org in Cc
> > >
> > > v2:
> > >  - Change the git comments based on Matan's feedback
> > >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdp
> > > d
> k.org%2Fdev%2Fpatchwork%2Fpatch%2F35104%2F&data=02%7C01%7Cmata
> > >
> n%40mellanox.com%7Cd491698ead5f48626c1408d5721a8331%7C8c5c71989d
> > >
> be4cf9a2099e499881116c%7C0%7C0%7C636540380570748030&sdata=if8cCrk3
> > > U0XqWp8NE%2BEP1BjuPHX95HHN0f1yjk6krV0%3D&reserved=0
> > >
> > > ---
> > >  lib/librte_ether/rte_ethdev_core.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/lib/librte_ether/rte_ethdev_core.h
> > > b/lib/librte_ether/rte_ethdev_core.h
> > > index 315b31723..e5681e466 100644
> > > --- a/lib/librte_ether/rte_ethdev_core.h
> > > +++ b/lib/librte_ether/rte_ethdev_core.h
> > > @@ -601,7 +601,7 @@ struct rte_eth_dev_data {
> > >  	struct rte_vlan_filter_conf vlan_filter_conf;
> > >  	/**< VLAN filter configuration. */
> > >  	struct rte_eth_dev_owner owner; /**< The port owner. */ -};
> > > +} __rte_cache_aligned;
> > >
> > >  /**
> > >   * @internal
> > > --
> > > 2.16.1
> >

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment

[Jerin Jacob]

-----Original Message-----
> Date: Mon, 12 Feb 2018 14:02:17 +0000
> From: Matan Azrad <matan@mellanox.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "ferruh.yigit@intel.com"
>  <ferruh.yigit@intel.com>, "stable@dpdk.org" <stable@dpdk.org>, Thomas
>  Monjalon <thomas@monjalon.net>, Konstantin Ananyev
>  <konstantin.ananyev@intel.com>, Pavan Nikhilesh
>  <pbhagavatula@caviumnetworks.com>
> Subject: RE: [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment
> 
> 
> 
> From: Jerin Jacob, Sent: Monday, February 12, 2018 3:51 PM
> > -----Original Message-----
> > > Date: Mon, 12 Feb 2018 13:44:54 +0000
> > > From: Matan Azrad <matan@mellanox.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
> > >  <dev@dpdk.org>
> > > CC: "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>, "stable@dpdk.org"
> > >  <stable@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>,
> > Konstantin
> > > Ananyev <konstantin.ananyev@intel.com>, Pavan Nikhilesh
> > > <pbhagavatula@caviumnetworks.com>
> > > Subject: RE: [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment
> > >
> > > Hi Jerin
> > >
> > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > > > The struct rte_eth_dev_data is used in ethdev fastpath routines and
> > > > it not aligned to cache line size. This patch fixes the ethdev data
> > alignment.
> > > >
> > > > The alignment was broken from the "first public release" changeset
> > > > where ethdev data address was aligned only to the first port.
> > > > Remaining ports alignment was defined by the size of the struct
> > > > (rte_eth_dev_data). This scheme is not guaranteed to be cache line
> > > > aligned all the time.
> > > >
> > > > "ethdev: add port ownership" change set introduced a
> > > > rte_eth_dev_shared_data container for port ownership change, This
> > > > resulted in rte_eth_dev->data memory for the first port also as
> > > > cache unaligned.
> > > >
> > > > Added a compiler alignment attribute to make sure rte_eth_dev->data
> > > > always cache aligned so that CPU/compiler
> > > > 1) Avoid sharing the element with another cache line
> > > > 2) Can load/store the elements in struct rte_eth_dev_data as
> > > > naturally aligned.
> > > >
> > > > Some platform like thunderX could see performance regression of 1%
> > > > at
> > > > "ethdev: add port ownership" change set with
> > > > 1 port/1 queue l3fwd application and this patch fixes that regression.
> > > >
> > > > example command:
> > > > sudo ./examples/l3fwd/build/l3fwd -c 0xff00 -- -p 0x1 --config="(0,0,9)"
> > > >
> > > > Fixes: af75078fece3 ("first public release")
> > > > Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> > >
> > > I don't think you need the add the 5b7ba31148a8 fix line.
> > > Maybe think about it in the next way:
> > > Is your fix can stay as a same fix before the port ownership feature
> > addition?
> > > If yes, You are not fixing it.
> > 
> > I don't think so. 5b7ba31148a8 breaking the first port alignment case(as
> > mentioned in the commit log clearly).
> > First port alignment was correct prior to 5b7ba31148a8 change set
> > 
> > Do you agree?
> 
> No.
> The root cause of the issue this fix is fixing, is that the structure is not aligned, it is not relevant to port ownership.
> The port ownership just exposes it.

My point is simple. First port alignment was correct prior to 5b7ba31148a8 change set.
it is broken from there.

It is minor thing. I let, ethdev maintainer to decide what to added in Fixes on apply.

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment

[Ferruh Yigit]

On 2/12/2018 2:11 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Mon, 12 Feb 2018 14:02:17 +0000
>> From: Matan Azrad <matan@mellanox.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> CC: "dev@dpdk.org" <dev@dpdk.org>, "ferruh.yigit@intel.com"
>>  <ferruh.yigit@intel.com>, "stable@dpdk.org" <stable@dpdk.org>, Thomas
>>  Monjalon <thomas@monjalon.net>, Konstantin Ananyev
>>  <konstantin.ananyev@intel.com>, Pavan Nikhilesh
>>  <pbhagavatula@caviumnetworks.com>
>> Subject: RE: [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment
>>
>>
>>
>> From: Jerin Jacob, Sent: Monday, February 12, 2018 3:51 PM
>>> -----Original Message-----
>>>> Date: Mon, 12 Feb 2018 13:44:54 +0000
>>>> From: Matan Azrad <matan@mellanox.com>
>>>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
>>>>  <dev@dpdk.org>
>>>> CC: "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>, "stable@dpdk.org"
>>>>  <stable@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>,
>>> Konstantin
>>>> Ananyev <konstantin.ananyev@intel.com>, Pavan Nikhilesh
>>>> <pbhagavatula@caviumnetworks.com>
>>>> Subject: RE: [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment
>>>>
>>>> Hi Jerin
>>>>
>>>> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
>>>>> The struct rte_eth_dev_data is used in ethdev fastpath routines and
>>>>> it not aligned to cache line size. This patch fixes the ethdev data
>>> alignment.
>>>>>
>>>>> The alignment was broken from the "first public release" changeset
>>>>> where ethdev data address was aligned only to the first port.
>>>>> Remaining ports alignment was defined by the size of the struct
>>>>> (rte_eth_dev_data). This scheme is not guaranteed to be cache line
>>>>> aligned all the time.
>>>>>
>>>>> "ethdev: add port ownership" change set introduced a
>>>>> rte_eth_dev_shared_data container for port ownership change, This
>>>>> resulted in rte_eth_dev->data memory for the first port also as
>>>>> cache unaligned.
>>>>>
>>>>> Added a compiler alignment attribute to make sure rte_eth_dev->data
>>>>> always cache aligned so that CPU/compiler
>>>>> 1) Avoid sharing the element with another cache line
>>>>> 2) Can load/store the elements in struct rte_eth_dev_data as
>>>>> naturally aligned.
>>>>>
>>>>> Some platform like thunderX could see performance regression of 1%
>>>>> at
>>>>> "ethdev: add port ownership" change set with
>>>>> 1 port/1 queue l3fwd application and this patch fixes that regression.
>>>>>
>>>>> example command:
>>>>> sudo ./examples/l3fwd/build/l3fwd -c 0xff00 -- -p 0x1 --config="(0,0,9)"
>>>>>
>>>>> Fixes: af75078fece3 ("first public release")
>>>>> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
>>>>
>>>> I don't think you need the add the 5b7ba31148a8 fix line.
>>>> Maybe think about it in the next way:
>>>> Is your fix can stay as a same fix before the port ownership feature
>>> addition?
>>>> If yes, You are not fixing it.
>>>
>>> I don't think so. 5b7ba31148a8 breaking the first port alignment case(as
>>> mentioned in the commit log clearly).
>>> First port alignment was correct prior to 5b7ba31148a8 change set
>>>
>>> Do you agree?
>>
>> No.
>> The root cause of the issue this fix is fixing, is that the structure is not aligned, it is not relevant to port ownership.
>> The port ownership just exposes it.
> 
> My point is simple. First port alignment was correct prior to 5b7ba31148a8 change set.
> it is broken from there.

+1 for this logic, adding fixes line makes sense for me.

> 
> It is minor thing. I let, ethdev maintainer to decide what to added in Fixes on apply.
> 

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] ethdev: fix ethdev data alignment

[Thomas Monjalon]

12/02/2018 14:13, Jerin Jacob:
> The struct rte_eth_dev_data is used in ethdev fastpath routines
> and it not aligned to cache line size. This patch fixes the ethdev
> data alignment.
> 
> The alignment was broken from the "first public release" changeset
> where ethdev data address was aligned only to the first port.
> Remaining ports alignment was defined by the size of the struct
> (rte_eth_dev_data). This scheme is not guaranteed to be cache line
> aligned all the time.
> 
> "ethdev: add port ownership" change set introduced a
> rte_eth_dev_shared_data container for port ownership change,
> This resulted in rte_eth_dev->data memory for the first port also
> as cache unaligned.
> 
> Added a compiler alignment attribute to make sure
> rte_eth_dev->data always cache aligned so that CPU/compiler
> 1) Avoid sharing the element with another cache line
> 2) Can load/store the elements in struct rte_eth_dev_data as
> naturally aligned.
> 
> Some platform like thunderX could see performance regression of 1%
> at "ethdev: add port ownership" change set with
> 1 port/1 queue l3fwd application and this patch fixes that regression.
> 
> example command:
> sudo ./examples/l3fwd/build/l3fwd -c 0xff00 -- -p 0x1 --config="(0,0,9)"
> 
> Fixes: af75078fece3 ("first public release")
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> 
> Cc: stable@dpdk.org
> 
> Cc: Matan Azrad <matan@mellanox.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> ---
Acked-by: Thomas Monjalon <thomas@monjalon.net>

Applied, thanks