On Thu, Jan 22, 2015 at 09:56:48AM +0000, Ferriter, Cian wrote: > Hey guys, > > I just wanted to ask is there anything more that can be done with this patch > or is it in an acceptable state for pushing? > > Cian
At this stage I think I'm ok with the patch contents, unless anyone else objects. However, your patch submission is missing the sign-off line needed before it can be committed. Can you please resubmit with the proper sign-off. [See http://www.dpdk.org/dev] Regards, /Bruce > > -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ferriter, Cian > Sent: Monday, January 19, 2015 6:39 PM > To: Richardson, Bruce > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] lib/librte_ether: change socket_id passed to > rte_memzone_reserve > > I would be happy with the original suggestion. If the ethdev data for a port > in use is in cache it removes the performance concern associated the current > setup and my fix. The original suggestion also fixes the crash that I was > seeing because of memory being reserved from a numa node with no > "--socket-mem" allocated. > > Cian > > -----Original Message----- > From: Richardson, Bruce > Sent: Wednesday, January 14, 2015 10:10 AM > To: Ferriter, Cian > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] lib/librte_ether: change socket_id passed to > rte_memzone_reserve > > On Tue, Jan 13, 2015 at 06:05:25PM +0000, Ferriter, Cian wrote: > > Comments on alternative solutions: > > 1) how would this solution work when there is no NIC present, and > > "rte_eth_from_rings" is called? Here, could you have an else where the > > socket id of the master core is passed to the "memzone_reserve"? > > 2) how would you advise making this change? I have looked at where > > "rte_eth_dev_allocate" is being called and in all but one case, there is a > > "numa_id" that could be passed in. This isn't the case for " > > rte_eth_dev_init" however, is there an easy solution for this? Would there > > now need to be an "rte_eth_dev_data" struct for each socket that there is a > > NIC attached to, reserving memory from that socket? > > > > Cian > > While I think the issues you highlight can probably be overcome, I'm not so > sure any more how much it matters what numa node this is allocated on. The > ethdev data for any port in use by a port should be in the cache. In that > case, if it doesn't matter, your original suggestion would work fine. > > /Bruce > > > > > -----Original Message----- > > From: Richardson, Bruce > > Sent: Tuesday, January 13, 2015 1:56 PM > > To: Ferriter, Cian > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] lib/librte_ether: change socket_id > > passed to rte_memzone_reserve > > > > On Tue, Jan 13, 2015 at 09:23:16AM +0000, Ferriter, Cian wrote: > > > Passing a socket id of "rte_socket_id()" can cause problems in non DPDK > > > applications as there is a dependency on the current logical core we are > > > running on. > > > Passing " rte_lcore_to_socket_id(rte_get_master_lcore())" as the socket > > > id to rte_memzone_reserve resolves these issues as the master lcore > > > doesn't change. > > > > > > > The only trouble is that when affinitizing the memory for the NICs to the > > socket of the master lcore, it gives us no way to correctly configure an > > app to use NICs connected to two different sockets on the one system. All > > memory for all NICs will end up on the same socket. Two possible > > alternative solutions: > > 1) affinitize memory to the socket the NIC is connected to > > 2) add a socket parameter to the API calls to allow the user complete > > control over their memory allocations > > > > Obviously the second one breaks backward compatibility (assume we modify > > existing API call), but is more powerful. > > > > Thoughts? > > > > /Bruce > > > > > -----Original Message----- > > > From: Ferriter, Cian > > > Sent: Tuesday, January 13, 2015 9:22 AM > > > To: dev at dpdk.org > > > Cc: Ferriter, Cian > > > Subject: [PATCH] lib/librte_ether: change socket_id passed to > > > rte_memzone_reserve > > > > > > Change the socket id that is passed to rte_memzone_reserve from the > > > socket id of current logical core to the socket id of the master_lcore. > > > --- > > > lib/librte_ether/rte_ethdev.c | 2 +- > > > 1 files changed, 1 insertions(+), 1 deletions(-) mode change > > > 100644 => 100755 lib/librte_ether/rte_ethdev.c > > > > > > diff --git a/lib/librte_ether/rte_ethdev.c > > > b/lib/librte_ether/rte_ethdev.c old mode 100644 new mode 100755 > > > index 95f2ceb..835540d > > > --- a/lib/librte_ether/rte_ethdev.c > > > +++ b/lib/librte_ether/rte_ethdev.c > > > @@ -184,7 +184,7 @@ rte_eth_dev_data_alloc(void) > > > if (rte_eal_process_type() == RTE_PROC_PRIMARY){ > > > mz = rte_memzone_reserve(MZ_RTE_ETH_DEV_DATA, > > > RTE_MAX_ETHPORTS * sizeof(*rte_eth_dev_data), > > > - rte_socket_id(), flags); > > > + rte_lcore_to_socket_id(rte_get_master_lcore()), > > > flags); > > > } else > > > mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA); > > > if (mz == NULL) > > > -- > > > 1.7.4.1 > > >