DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] dpdk vlan strip offload bug for I350 nic?
@ 2018-12-09  4:30 1534898891
  2018-12-09  5:23 `  =?gb18030?B?MTUzNDg5ODg5MQ==?=
  2018-12-10  7:53 ` [dpdk-dev] rte_eal_hotplug_remove() generates error message Hideyuki Yamashita
  0 siblings, 2 replies; 21+ messages in thread
From: 1534898891 @ 2018-12-09  4:30 UTC (permalink / raw)
  To: dev

Hi Guys:
I used the vlan strip offload in I350 nic case. VLAN tag is incorrect when packets are ICMP. Other packets are correct.
NIC:I350
DPDK version:18.02 OS:centos 7.4
Any help is appreciated.


Thanks.
Hongbowang.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] dpdk vlan strip offload bug for I350 nic?
  2018-12-09  4:30 [dpdk-dev] dpdk vlan strip offload bug for I350 nic? 1534898891
@ 2018-12-09  5:23 `  =?gb18030?B?MTUzNDg5ODg5MQ==?=
  2018-12-18  2:26   ` Zhao1, Wei
  2018-12-10  7:53 ` [dpdk-dev] rte_eal_hotplug_remove() generates error message Hideyuki Yamashita
  1 sibling, 1 reply; 21+ messages in thread
From: =?gb18030?B?MTUzNDg5ODg5MQ==?= @ 2018-12-09  5:23 UTC (permalink / raw)
  To: =?gb18030?B?ZGV2?=

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb18030", Size: 871 bytes --]

My server's port connect to switch with trunk. The gw IP on switch's vlan-if. I used kni send icmp reques to gw, but the vlan_tci of icmp reply packets is incorrect. Other packets are correct. such as the ping same subnet servers or other packets(tcp or udp) and switch gw ping my kni is OK. This is too weird. The attachment is my test program. I guess it's dpdk drive bug or firmware bug.


Any help is appreciated.


Thanks.



------------------ Original ------------------
From:  "±ùÀá"<1534898891@qq.com>;
Date:  Sun, Dec 9, 2018 12:30 PM
To:  "dev"<dev@dpdk.org>;

Subject:  dpdk vlan strip offload bug for I350 nic?



Hi Guys:
I used the vlan strip offload in I350 nic case. VLAN tag is incorrect when packets are ICMP. Other packets are correct.
NIC:I350
DPDK version:18.02 OS:centos 7.4
Any help is appreciated.


Thanks.
Hongbowang.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-09  4:30 [dpdk-dev] dpdk vlan strip offload bug for I350 nic? 1534898891
  2018-12-09  5:23 `  =?gb18030?B?MTUzNDg5ODg5MQ==?=
@ 2018-12-10  7:53 ` Hideyuki Yamashita
  2018-12-11  0:54   ` Hideyuki Yamashita
  1 sibling, 1 reply; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-10  7:53 UTC (permalink / raw)
  To: dev

Subject: 

Dear Thomas and all,

I hope you all get safely back home after DPDK summit.
(When I get back Japan, it is chilling. (start of winter))

On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
However, following syslog message is printed.
“EAL: Error: Invalid memory”

At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
“rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”

We tested above changes, but the result is the same (i.e., same error message is printed).
The debug log message says:
---
[primary]
VHOST_CONFIG: vhost-user server: socket created, fd: 17
VHOST_CONFIG: bind to /tmp/sock0
EAL: Error: Invalid memory
VHOST_CONFIG: vhost-user server: socket created, fd: 17
VHOST_CONFIG: bind to /tmp/sock0

[secondary]
APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
EAL: request: eal_dev_mp_request
EAL: msg: eal_dev_mp_request
EAL: request: bus_vdev_mp
EAL: msg: bus_vdev_mp
EAL: msg: bus_vdev_mp
EAL: reply: eal_dev_mp_request
EAL: msg: eal_dev_mp_request
rte_eth_promiscuous_disable: Function not supported
rte_eth_allmulticast_disable: Function not supported
APP: To Server: add
EAL: request: eal_dev_mp_request
EAL: msg: eal_dev_mp_request
EAL: reply: eal_dev_mp_request
EAL: msg: eal_dev_mp_request
APP: To Server: del
APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
EAL: request: eal_dev_mp_request
EAL: msg: eal_dev_mp_request
EAL: request: bus_vdev_mp
EAL: msg: bus_vdev_mp
EAL: msg: bus_vdev_mp
EAL: reply: eal_dev_mp_request
EAL: msg: eal_dev_mp_request
rte_eth_promiscuous_disable: Function not supported
rte_eth_allmulticast_disable: Function not supported
APP: To Server: add
---

We would like to ask:
1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?

Thanks in advance and have a nice day.

BR,
Hideyuki Yamashita
NTT TechnoCross

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-10  7:53 ` [dpdk-dev] rte_eal_hotplug_remove() generates error message Hideyuki Yamashita
@ 2018-12-11  0:54   ` Hideyuki Yamashita
  2018-12-17 10:02     ` Hideyuki Yamashita
  0 siblings, 1 reply; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-11  0:54 UTC (permalink / raw)
  To: dev; +Cc: Yasufumi Ogawa, Nakamura Hioryuki, Hideyuki Yamashita

Adding my colleague Yasufumi and Hiroyuki as CC.

We are waiting valuable advice from you.

Thanks in advance,
Hideyuki Yamashita
NTT TechnoCross

> 
> Dear Thomas and all,
> 
> I hope you all get safely back home after DPDK summit.
> (When I get back Japan, it is chilling. (start of winter))
> 
> On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> However, following syslog message is printed.
> “EAL: Error: Invalid memory”
> 
> At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> 
> We tested above changes, but the result is the same (i.e., same error message is printed).
> The debug log message says:
> ---
> [primary]
> VHOST_CONFIG: vhost-user server: socket created, fd: 17
> VHOST_CONFIG: bind to /tmp/sock0
> EAL: Error: Invalid memory
> VHOST_CONFIG: vhost-user server: socket created, fd: 17
> VHOST_CONFIG: bind to /tmp/sock0
> 
> [secondary]
> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> EAL: request: eal_dev_mp_request
> EAL: msg: eal_dev_mp_request
> EAL: request: bus_vdev_mp
> EAL: msg: bus_vdev_mp
> EAL: msg: bus_vdev_mp
> EAL: reply: eal_dev_mp_request
> EAL: msg: eal_dev_mp_request
> rte_eth_promiscuous_disable: Function not supported
> rte_eth_allmulticast_disable: Function not supported
> APP: To Server: add
> EAL: request: eal_dev_mp_request
> EAL: msg: eal_dev_mp_request
> EAL: reply: eal_dev_mp_request
> EAL: msg: eal_dev_mp_request
> APP: To Server: del
> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> EAL: request: eal_dev_mp_request
> EAL: msg: eal_dev_mp_request
> EAL: request: bus_vdev_mp
> EAL: msg: bus_vdev_mp
> EAL: msg: bus_vdev_mp
> EAL: reply: eal_dev_mp_request
> EAL: msg: eal_dev_mp_request
> rte_eth_promiscuous_disable: Function not supported
> rte_eth_allmulticast_disable: Function not supported
> APP: To Server: add
> ---
> 
> We would like to ask:
> 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> 
> Thanks in advance and have a nice day.
> 
> BR,
> Hideyuki Yamashita
> NTT TechnoCross

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-11  0:54   ` Hideyuki Yamashita
@ 2018-12-17 10:02     ` Hideyuki Yamashita
  2018-12-17 10:23       ` Burakov, Anatoly
  0 siblings, 1 reply; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-17 10:02 UTC (permalink / raw)
  To: dev, Hideyuki Yamashita; +Cc: Yasufumi Ogawa, Nakamura Hioryuki

Dear Thomas and all,

I took a look on dpdk code.
I firstly write qustions and my analisys 
on the current dpdk code follows after that.

[1.Questions]
I have several questions to ask again.
Is my understanding correct about followings

Q1: "EAL:ERROR, Invalid memory" is ignorable

Q2: there is no big difference between calling 
rte_eal_hotplug_remove(bus->name, dev->name)
and
rte_dev_remove(dev) because anyway it 
reaches to rte_pmd_vhost_remove and encounter 
the same error.

[2.snip from my code]
.....
        rte_eth_dev_close(port_id);
        ret = rte_dev_remove(dev);
        if (ret < 0)
                return ret;
        rte_eth_dev_release_port(&rte_eth_devices[port_id]);

[3. My analysis on dpdk code]
static int
  rte_pmd_vhost_remove(struct rte_vdev_device *dev)
  {
   ...........
         eth_dev_close(eth_dev);

          rte_free(vring_states[eth_dev->data->port_id]);
          vring_states[eth_dev->data->port_id] = NULL;

          rte_eth_dev_release_port(eth_dev);

As you can see in rte_eth_vhost.c
It calls both eth_dev_close and rte_eth_dev_release_port.
And inside both functions, it tries to free mac_addrs. 
        rte_free(dev->data->mac_addrs);       //in rth_dev_close
        rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port

I understand that is the reason why 
/* Free the memory space back to heap */
void rte_free(void *addr)
{
        if (addr == NULL) return;
        if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
                RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
}
encounter the error.
As an experiment, I commented out one of them, "ERR, Invalid memory"
disappered.

Thanks and BR,
Hideyuki Yamashita
NTT TechnoCross

> Adding my colleague Yasufumi and Hiroyuki as CC.
> 
> We are waiting valuable advice from you.
> 
> Thanks in advance,
> Hideyuki Yamashita
> NTT TechnoCross
> 
> > 
> > Dear Thomas and all,
> > 
> > I hope you all get safely back home after DPDK summit.
> > (When I get back Japan, it is chilling. (start of winter))
> > 
> > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > However, following syslog message is printed.
> > “EAL: Error: Invalid memory”
> > 
> > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > 
> > We tested above changes, but the result is the same (i.e., same error message is printed).
> > The debug log message says:
> > ---
> > [primary]
> > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > VHOST_CONFIG: bind to /tmp/sock0
> > EAL: Error: Invalid memory
> > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > VHOST_CONFIG: bind to /tmp/sock0
> > 
> > [secondary]
> > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > EAL: request: eal_dev_mp_request
> > EAL: msg: eal_dev_mp_request
> > EAL: request: bus_vdev_mp
> > EAL: msg: bus_vdev_mp
> > EAL: msg: bus_vdev_mp
> > EAL: reply: eal_dev_mp_request
> > EAL: msg: eal_dev_mp_request
> > rte_eth_promiscuous_disable: Function not supported
> > rte_eth_allmulticast_disable: Function not supported
> > APP: To Server: add
> > EAL: request: eal_dev_mp_request
> > EAL: msg: eal_dev_mp_request
> > EAL: reply: eal_dev_mp_request
> > EAL: msg: eal_dev_mp_request
> > APP: To Server: del
> > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > EAL: request: eal_dev_mp_request
> > EAL: msg: eal_dev_mp_request
> > EAL: request: bus_vdev_mp
> > EAL: msg: bus_vdev_mp
> > EAL: msg: bus_vdev_mp
> > EAL: reply: eal_dev_mp_request
> > EAL: msg: eal_dev_mp_request
> > rte_eth_promiscuous_disable: Function not supported
> > rte_eth_allmulticast_disable: Function not supported
> > APP: To Server: add
> > ---
> > 
> > We would like to ask:
> > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > 
> > Thanks in advance and have a nice day.
> > 
> > BR,
> > Hideyuki Yamashita
> > NTT TechnoCross
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-17 10:02     ` Hideyuki Yamashita
@ 2018-12-17 10:23       ` Burakov, Anatoly
  2018-12-17 10:38         ` Burakov, Anatoly
  2018-12-17 10:45         ` Hideyuki Yamashita
  0 siblings, 2 replies; 21+ messages in thread
From: Burakov, Anatoly @ 2018-12-17 10:23 UTC (permalink / raw)
  To: Hideyuki Yamashita, dev; +Cc: Yasufumi Ogawa, Nakamura Hioryuki

On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> Dear Thomas and all,
> 
> I took a look on dpdk code.
> I firstly write qustions and my analisys
> on the current dpdk code follows after that.
> 
> [1.Questions]
> I have several questions to ask again.
> Is my understanding correct about followings
> 
> Q1: "EAL:ERROR, Invalid memory" is ignorable
> 
> Q2: there is no big difference between calling
> rte_eal_hotplug_remove(bus->name, dev->name)
> and
> rte_dev_remove(dev) because anyway it
> reaches to rte_pmd_vhost_remove and encounter
> the same error.
> 
> [2.snip from my code]
> .....
>          rte_eth_dev_close(port_id);
>          ret = rte_dev_remove(dev);
>          if (ret < 0)
>                  return ret;
>          rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> 
> [3. My analysis on dpdk code]
> static int
>    rte_pmd_vhost_remove(struct rte_vdev_device *dev)
>    {
>     ...........
>           eth_dev_close(eth_dev);
> 
>            rte_free(vring_states[eth_dev->data->port_id]);
>            vring_states[eth_dev->data->port_id] = NULL;
> 
>            rte_eth_dev_release_port(eth_dev);
> 
> As you can see in rte_eth_vhost.c
> It calls both eth_dev_close and rte_eth_dev_release_port.
> And inside both functions, it tries to free mac_addrs.
>          rte_free(dev->data->mac_addrs);       //in rth_dev_close
>          rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> 
> I understand that is the reason why
> /* Free the memory space back to heap */
> void rte_free(void *addr)
> {
>          if (addr == NULL) return;
>          if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
>                  RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> }
> encounter the error.
> As an experiment, I commented out one of them, "ERR, Invalid memory"
> disappered.
> 
> Thanks and BR,
> Hideyuki Yamashita
> NTT TechnoCross
> 
>> Adding my colleague Yasufumi and Hiroyuki as CC.
>>
>> We are waiting valuable advice from you.
>>
>> Thanks in advance,
>> Hideyuki Yamashita
>> NTT TechnoCross
>>
>>>
>>> Dear Thomas and all,
>>>
>>> I hope you all get safely back home after DPDK summit.
>>> (When I get back Japan, it is chilling. (start of winter))
>>>
>>> On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
>>> However, following syslog message is printed.
>>> “EAL: Error: Invalid memory”
>>>
>>> At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
>>> Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
>>> “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
>>>
>>> We tested above changes, but the result is the same (i.e., same error message is printed).
>>> The debug log message says:
>>> ---
>>> [primary]
>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
>>> VHOST_CONFIG: bind to /tmp/sock0
>>> EAL: Error: Invalid memory
>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
>>> VHOST_CONFIG: bind to /tmp/sock0
>>>
>>> [secondary]
>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
>>> EAL: request: eal_dev_mp_request
>>> EAL: msg: eal_dev_mp_request
>>> EAL: request: bus_vdev_mp
>>> EAL: msg: bus_vdev_mp
>>> EAL: msg: bus_vdev_mp
>>> EAL: reply: eal_dev_mp_request
>>> EAL: msg: eal_dev_mp_request
>>> rte_eth_promiscuous_disable: Function not supported
>>> rte_eth_allmulticast_disable: Function not supported
>>> APP: To Server: add
>>> EAL: request: eal_dev_mp_request
>>> EAL: msg: eal_dev_mp_request
>>> EAL: reply: eal_dev_mp_request
>>> EAL: msg: eal_dev_mp_request
>>> APP: To Server: del
>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
>>> EAL: request: eal_dev_mp_request
>>> EAL: msg: eal_dev_mp_request
>>> EAL: request: bus_vdev_mp
>>> EAL: msg: bus_vdev_mp
>>> EAL: msg: bus_vdev_mp
>>> EAL: reply: eal_dev_mp_request
>>> EAL: msg: eal_dev_mp_request
>>> rte_eth_promiscuous_disable: Function not supported
>>> rte_eth_allmulticast_disable: Function not supported
>>> APP: To Server: add
>>> ---
>>>
>>> We would like to ask:
>>> 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
>>> 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
>>>
>>> Thanks in advance and have a nice day.
>>>
>>> BR,
>>> Hideyuki Yamashita
>>> NTT TechnoCross
>>
> 
> 
> 

Hi Hideyuki,

The error you're referring to (about invalid memory) means that you're  
trying to free a pointer that points to invalid memory. Meaning, either  
the pointer itself is not pointing to an allocated area, or it points to  
memory that has already been freed.

If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same  
area, this is a bug, because this would lead to double free, and  
rte_malloc will rightly complain about invalid memory. Now, malloc won't  
try to do anything with the invalid memory, so the error itself is  
harmless *as far as malloc is concerned*, but these attempts to free the  
memory twice should be fixed whereever they happen.

I'm not well-versed in dev infrastructure, so i wouldn't be able to say  
which one of the rte_free calls is an extra, unneeded one. This is  
something e.g. Thomas could help with, or the driver maintainer.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-17 10:23       ` Burakov, Anatoly
@ 2018-12-17 10:38         ` Burakov, Anatoly
  2018-12-17 11:03           ` Hideyuki Yamashita
  2018-12-17 10:45         ` Hideyuki Yamashita
  1 sibling, 1 reply; 21+ messages in thread
From: Burakov, Anatoly @ 2018-12-17 10:38 UTC (permalink / raw)
  To: Hideyuki Yamashita, dev; +Cc: Yasufumi Ogawa, Nakamura Hioryuki

On 17-Dec-18 10:23 AM, Burakov, Anatoly wrote:
> On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
>> Dear Thomas and all,
>>
>> I took a look on dpdk code.
>> I firstly write qustions and my analisys
>> on the current dpdk code follows after that.
>>
>> [1.Questions]
>> I have several questions to ask again.
>> Is my understanding correct about followings
>>
>> Q1: "EAL:ERROR, Invalid memory" is ignorable
>>
>> Q2: there is no big difference between calling
>> rte_eal_hotplug_remove(bus->name, dev->name)
>> and
>> rte_dev_remove(dev) because anyway it
>> reaches to rte_pmd_vhost_remove and encounter
>> the same error.
>>
>> [2.snip from my code]
>> .....
>>          rte_eth_dev_close(port_id);
>>          ret = rte_dev_remove(dev);
>>          if (ret < 0)
>>                  return ret;
>>          rte_eth_dev_release_port(&rte_eth_devices[port_id]);
>>
>> [3. My analysis on dpdk code]
>> static int
>>    rte_pmd_vhost_remove(struct rte_vdev_device *dev)
>>    {
>>     ...........
>>           eth_dev_close(eth_dev);
>>
>>            rte_free(vring_states[eth_dev->data->port_id]);
>>            vring_states[eth_dev->data->port_id] = NULL;
>>
>>            rte_eth_dev_release_port(eth_dev);
>>
>> As you can see in rte_eth_vhost.c
>> It calls both eth_dev_close and rte_eth_dev_release_port.
>> And inside both functions, it tries to free mac_addrs.
>>          rte_free(dev->data->mac_addrs);       //in rth_dev_close
>>          rte_free(eth_dev->data->mac_addrs);  //in 
>> rte_eth_dev_release_port
>>
>> I understand that is the reason why
>> /* Free the memory space back to heap */
>> void rte_free(void *addr)
>> {
>>          if (addr == NULL) return;
>>          if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
>>                  RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
>> }
>> encounter the error.
>> As an experiment, I commented out one of them, "ERR, Invalid memory"
>> disappered.
>>
>> Thanks and BR,
>> Hideyuki Yamashita
>> NTT TechnoCross
>>
>>> Adding my colleague Yasufumi and Hiroyuki as CC.
>>>
>>> We are waiting valuable advice from you.
>>>
>>> Thanks in advance,
>>> Hideyuki Yamashita
>>> NTT TechnoCross
>>>
>>>>
>>>> Dear Thomas and all,
>>>>
>>>> I hope you all get safely back home after DPDK summit.
>>>> (When I get back Japan, it is chilling. (start of winter))
>>>>
>>>> On DPDK 18.11.0, we tried to remove vhost device by using 
>>>> rte_eal_hotplug_remove().
>>>> However, following syslog message is printed.
>>>> “EAL: Error: Invalid memory”
>>>>
>>>> At DPDK summit San Jose, we had chance to ask Thomas how to handle 
>>>> the error message, and he gave us following advice:
>>>> Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
>>>> “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and 
>>>> rte_dev_remove(rte_dev)”
>>>>
>>>> We tested above changes, but the result is the same (i.e., same 
>>>> error message is printed).
>>>> The debug log message says:
>>>> ---
>>>> [primary]
>>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
>>>> VHOST_CONFIG: bind to /tmp/sock0
>>>> EAL: Error: Invalid memory
>>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
>>>> VHOST_CONFIG: bind to /tmp/sock0
>>>>
>>>> [secondary]
>>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
>>>> EAL: request: eal_dev_mp_request
>>>> EAL: msg: eal_dev_mp_request
>>>> EAL: request: bus_vdev_mp
>>>> EAL: msg: bus_vdev_mp
>>>> EAL: msg: bus_vdev_mp
>>>> EAL: reply: eal_dev_mp_request
>>>> EAL: msg: eal_dev_mp_request
>>>> rte_eth_promiscuous_disable: Function not supported
>>>> rte_eth_allmulticast_disable: Function not supported
>>>> APP: To Server: add
>>>> EAL: request: eal_dev_mp_request
>>>> EAL: msg: eal_dev_mp_request
>>>> EAL: reply: eal_dev_mp_request
>>>> EAL: msg: eal_dev_mp_request
>>>> APP: To Server: del
>>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
>>>> EAL: request: eal_dev_mp_request
>>>> EAL: msg: eal_dev_mp_request
>>>> EAL: request: bus_vdev_mp
>>>> EAL: msg: bus_vdev_mp
>>>> EAL: msg: bus_vdev_mp
>>>> EAL: reply: eal_dev_mp_request
>>>> EAL: msg: eal_dev_mp_request
>>>> rte_eth_promiscuous_disable: Function not supported
>>>> rte_eth_allmulticast_disable: Function not supported
>>>> APP: To Server: add
>>>> ---
>>>>
>>>> We would like to ask:
>>>> 1)    Is the message “EAL: Error: Invalid memory” ignorable or not? 
>>>> There is no obstacle except this message to re-add the vhost device.
>>>> 2)    Which is the better(best?) way to add/del vhost device 
>>>> “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
>>>>
>>>> Thanks in advance and have a nice day.
>>>>
>>>> BR,
>>>> Hideyuki Yamashita
>>>> NTT TechnoCross
>>>
>>
>>
>>
> 
> Hi Hideyuki,
> 
> The error you're referring to (about invalid memory) means that you're 
> trying to free a pointer that points to invalid memory. Meaning, either 
> the pointer itself is not pointing to an allocated area, or it points to 
> memory that has already been freed.
> 
> If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same 
> area, this is a bug, because this would lead to double free, and 
> rte_malloc will rightly complain about invalid memory. Now, malloc won't 
> try to do anything with the invalid memory, so the error itself is 
> harmless *as far as malloc is concerned*, but these attempts to free the 
> memory twice should be fixed whereever they happen.
> 
> I'm not well-versed in dev infrastructure, so i wouldn't be able to say 
> which one of the rte_free calls is an extra, unneeded one. This is 
> something e.g. Thomas could help with, or the driver maintainer.
> 

...with that said, to me, this looks suspicious:


 >          rte_eth_dev_close(port_id);
 >          ret = rte_dev_remove(dev);
 >          if (ret < 0)
 >                  return ret;
 >          rte_eth_dev_release_port(&rte_eth_devices[port_id]);

So, you're closing the port, then you remove the device, then you  
release port. From your own analysis, what dev_remove does is the following:

 >    rte_pmd_vhost_remove(struct rte_vdev_device *dev)
 >    {
 >     ...........
 >           eth_dev_close(eth_dev);
 >
 >            rte_free(vring_states[eth_dev->data->port_id]);
 >            vring_states[eth_dev->data->port_id] = NULL;
 >
 >            rte_eth_dev_release_port(eth_dev);

So, you have duplicate call to eth_dev_close() (which is likely not a  
problem), and a duplicate call to rte_eth_dev_release_port(). So, if  
rte_dev_remove() already calls rte_eth_dev_release_port(), why are you  
calling it in your code?

Now, as i said above, i'm not well-versed enough in the dev  
infrastructure to know which of the calls should go where, but i'm  
pretty sure you shouldn't call rte_eth_dev_release_port twice - this is  
likely what's causing your double-free.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-17 10:23       ` Burakov, Anatoly
  2018-12-17 10:38         ` Burakov, Anatoly
@ 2018-12-17 10:45         ` Hideyuki Yamashita
  2018-12-17 11:21           ` Burakov, Anatoly
  1 sibling, 1 reply; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-17 10:45 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Yasufumi Ogawa, Nakamura Hioryuki

> On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > Dear Thomas and all,
> >
> > I took a look on dpdk code.
> > I firstly write qustions and my analisys
> > on the current dpdk code follows after that.
> >
> > [1.Questions]
> > I have several questions to ask again.
> > Is my understanding correct about followings
> >
> > Q1: "EAL:ERROR, Invalid memory" is ignorable
> >
> > Q2: there is no big difference between calling
> > rte_eal_hotplug_remove(bus->name, dev->name)
> > and
> > rte_dev_remove(dev) because anyway it
> > reaches to rte_pmd_vhost_remove and encounter
> > the same error.
> >
> > [2.snip from my code]
> > .....
> >          rte_eth_dev_close(port_id);
> >          ret = rte_dev_remove(dev);
> >          if (ret < 0)
> >                  return ret;
> >          rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> >
> > [3. My analysis on dpdk code]
> > static int
> >    rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> >    {
> >     ...........
> >           eth_dev_close(eth_dev);
> >
> >            rte_free(vring_states[eth_dev->data->port_id]);
> >            vring_states[eth_dev->data->port_id] = NULL;
> >
> >            rte_eth_dev_release_port(eth_dev);
> >
> > As you can see in rte_eth_vhost.c
> > It calls both eth_dev_close and rte_eth_dev_release_port.
> > And inside both functions, it tries to free mac_addrs.
> >          rte_free(dev->data->mac_addrs);       //in rth_dev_close
> >          rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> >
> > I understand that is the reason why
> > /* Free the memory space back to heap */
> > void rte_free(void *addr)
> > {
> >          if (addr == NULL) return;
> >          if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> >                  RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > }
> > encounter the error.
> > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > disappered.
> >
> > Thanks and BR,
> > Hideyuki Yamashita
> > NTT TechnoCross
> >
> >> Adding my colleague Yasufumi and Hiroyuki as CC.
> >>
> >> We are waiting valuable advice from you.
> >>
> >> Thanks in advance,
> >> Hideyuki Yamashita
> >> NTT TechnoCross
> >>
> >>>
> >>> Dear Thomas and all,
> >>>
> >>> I hope you all get safely back home after DPDK summit.
> >>> (When I get back Japan, it is chilling. (start of winter))
> >>>
> >>> On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> >>> However, following syslog message is printed.
> >>> “EAL: Error: Invalid memory”
> >>>
> >>> At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> >>> Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> >>> “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> >>>
> >>> We tested above changes, but the result is the same (i.e., same error message is printed).
> >>> The debug log message says:
> >>> ---
> >>> [primary]
> >>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
> >>> VHOST_CONFIG: bind to /tmp/sock0
> >>> EAL: Error: Invalid memory
> >>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
> >>> VHOST_CONFIG: bind to /tmp/sock0
> >>>
> >>> [secondary]
> >>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> >>> EAL: request: eal_dev_mp_request
> >>> EAL: msg: eal_dev_mp_request
> >>> EAL: request: bus_vdev_mp
> >>> EAL: msg: bus_vdev_mp
> >>> EAL: msg: bus_vdev_mp
> >>> EAL: reply: eal_dev_mp_request
> >>> EAL: msg: eal_dev_mp_request
> >>> rte_eth_promiscuous_disable: Function not supported
> >>> rte_eth_allmulticast_disable: Function not supported
> >>> APP: To Server: add
> >>> EAL: request: eal_dev_mp_request
> >>> EAL: msg: eal_dev_mp_request
> >>> EAL: reply: eal_dev_mp_request
> >>> EAL: msg: eal_dev_mp_request
> >>> APP: To Server: del
> >>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> >>> EAL: request: eal_dev_mp_request
> >>> EAL: msg: eal_dev_mp_request
> >>> EAL: request: bus_vdev_mp
> >>> EAL: msg: bus_vdev_mp
> >>> EAL: msg: bus_vdev_mp
> >>> EAL: reply: eal_dev_mp_request
> >>> EAL: msg: eal_dev_mp_request
> >>> rte_eth_promiscuous_disable: Function not supported
> >>> rte_eth_allmulticast_disable: Function not supported
> >>> APP: To Server: add
> >>> ---
> >>>
> >>> We would like to ask:
> >>> 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> >>> 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> >>>
> >>> Thanks in advance and have a nice day.
> >>>
> >>> BR,
> >>> Hideyuki Yamashita
> >>> NTT TechnoCross
> >>
> >
> >
> >
> Hi Hideyuki,
> 
> The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> 
> If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> 
> I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> 
> --Thanks,
> Anatoly
Hello Anatoly,

Thanks for your reply for my newbie question.
Now I understand that this error is harmless from DPDK application(SPP)
point of view in practice. Thanks.
But anyway if there is a double free logic, it is a bug and should be
fixed.
The remaining issues are
1. If it is really a bug (or my mis-understanding)
2. If is is a bug which function should remove rte_free(mac_addrs)

Thanks,
Hideyuki Yamashita
NTT TechnoCross

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-17 10:38         ` Burakov, Anatoly
@ 2018-12-17 11:03           ` Hideyuki Yamashita
  0 siblings, 0 replies; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-17 11:03 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Yasufumi Ogawa, Nakamura Hioryuki

> On 17-Dec-18 10:23 AM, Burakov, Anatoly wrote:
> > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> >> Dear Thomas and all,
> >>
> >> I took a look on dpdk code.
> >> I firstly write qustions and my analisys
> >> on the current dpdk code follows after that.
> >>
> >> [1.Questions]
> >> I have several questions to ask again.
> >> Is my understanding correct about followings
> >>
> >> Q1: "EAL:ERROR, Invalid memory" is ignorable
> >>
> >> Q2: there is no big difference between calling
> >> rte_eal_hotplug_remove(bus->name, dev->name)
> >> and
> >> rte_dev_remove(dev) because anyway it
> >> reaches to rte_pmd_vhost_remove and encounter
> >> the same error.
> >>
> >> [2.snip from my code]
> >> .....
> >> ???????? rte_eth_dev_close(port_id);
> >> ???????? ret = rte_dev_remove(dev);
> >> ???????? if (ret < 0)
> >> ???????????????? return ret;
> >> ???????? rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> >>
> >> [3. My analysis on dpdk code]
> >> static int
> >> ?? rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> >> ?? {
> >> ??? ...........
> >> ????????  eth_dev_close(eth_dev);
> >>
> >> ?????????? rte_free(vring_states[eth_dev->data->port_id]);
> >> ?????????? vring_states[eth_dev->data->port_id] = NULL;
> >>
> >> ?????????? rte_eth_dev_release_port(eth_dev);
> >>
> >> As you can see in rte_eth_vhost.c
> >> It calls both eth_dev_close and rte_eth_dev_release_port.
> >> And inside both functions, it tries to free mac_addrs.
> >> ???????? rte_free(dev->data->mac_addrs);?????? //in rth_dev_close
> >> ???????? rte_free(eth_dev->data->mac_addrs);? //in
> >> rte_eth_dev_release_port
> >>
> >> I understand that is the reason why
> >> /* Free the memory space back to heap */
> >> void rte_free(void *addr)
> >> {
> >> ???????? if (addr == NULL) return;
> >> ???????? if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> >> ???????????????? RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> >> }
> >> encounter the error.
> >> As an experiment, I commented out one of them, "ERR, Invalid memory"
> >> disappered.
> >>
> >> Thanks and BR,
> >> Hideyuki Yamashita
> >> NTT TechnoCross
> >>
> >>> Adding my colleague Yasufumi and Hiroyuki as CC.
> >>>
> >>> We are waiting valuable advice from you.
> >>>
> >>> Thanks in advance,
> >>> Hideyuki Yamashita
> >>> NTT TechnoCross
> >>>
> >>>>
> >>>> Dear Thomas and all,
> >>>>
> >>>> I hope you all get safely back home after DPDK summit.
> >>>> (When I get back Japan, it is chilling. (start of winter))
> >>>>
> >>>> On DPDK 18.11.0, we tried to remove vhost device by using
> >>>> rte_eal_hotplug_remove().
> >>>> However, following syslog message is printed.
> >>>> “EAL: Error: Invalid memory”
> >>>>
> >>>> At DPDK summit San Jose, we had chance to ask Thomas how to handle
> >>>> the error message, and he gave us following advice:
> >>>> Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> >>>> “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and
> >>>> rte_dev_remove(rte_dev)”
> >>>>
> >>>> We tested above changes, but the result is the same (i.e., same
> >>>> error message is printed).
> >>>> The debug log message says:
> >>>> ---
> >>>> [primary]
> >>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
> >>>> VHOST_CONFIG: bind to /tmp/sock0
> >>>> EAL: Error: Invalid memory
> >>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
> >>>> VHOST_CONFIG: bind to /tmp/sock0
> >>>>
> >>>> [secondary]
> >>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> >>>> EAL: request: eal_dev_mp_request
> >>>> EAL: msg: eal_dev_mp_request
> >>>> EAL: request: bus_vdev_mp
> >>>> EAL: msg: bus_vdev_mp
> >>>> EAL: msg: bus_vdev_mp
> >>>> EAL: reply: eal_dev_mp_request
> >>>> EAL: msg: eal_dev_mp_request
> >>>> rte_eth_promiscuous_disable: Function not supported
> >>>> rte_eth_allmulticast_disable: Function not supported
> >>>> APP: To Server: add
> >>>> EAL: request: eal_dev_mp_request
> >>>> EAL: msg: eal_dev_mp_request
> >>>> EAL: reply: eal_dev_mp_request
> >>>> EAL: msg: eal_dev_mp_request
> >>>> APP: To Server: del
> >>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> >>>> EAL: request: eal_dev_mp_request
> >>>> EAL: msg: eal_dev_mp_request
> >>>> EAL: request: bus_vdev_mp
> >>>> EAL: msg: bus_vdev_mp
> >>>> EAL: msg: bus_vdev_mp
> >>>> EAL: reply: eal_dev_mp_request
> >>>> EAL: msg: eal_dev_mp_request
> >>>> rte_eth_promiscuous_disable: Function not supported
> >>>> rte_eth_allmulticast_disable: Function not supported
> >>>> APP: To Server: add
> >>>> ---
> >>>>
> >>>> We would like to ask:
> >>>> 1)??? Is the message “EAL: Error: Invalid memory” ignorable or not?
> >>>> There is no obstacle except this message to re-add the vhost device.
> >>>> 2)??? Which is the better(best?) way to add/del vhost device
> >>>> “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> >>>>
> >>>> Thanks in advance and have a nice day.
> >>>>
> >>>> BR,
> >>>> Hideyuki Yamashita
> >>>> NTT TechnoCross
> >>>
> >>
> >>
> >>
> >
> > Hi Hideyuki,
> >
> > The error you're referring to (about invalid memory) means that you're
> > trying to free a pointer that points to invalid memory. Meaning, either
> > the pointer itself is not pointing to an allocated area, or it points to
> > memory that has already been freed.
> >
> > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same
> > area, this is a bug, because this would lead to double free, and
> > rte_malloc will rightly complain about invalid memory. Now, malloc won't
> > try to do anything with the invalid memory, so the error itself is
> > harmless *as far as malloc is concerned*, but these attempts to free the
> > memory twice should be fixed whereever they happen.
> >
> > I'm not well-versed in dev infrastructure, so i wouldn't be able to say
> > which one of the rte_free calls is an extra, unneeded one. This is
> > something e.g. Thomas could help with, or the driver maintainer.
> > 
> ...with that said, to me, this looks suspicious:
> 
> 
>  >          rte_eth_dev_close(port_id);
>  >          ret = rte_dev_remove(dev);
>  >          if (ret < 0)
>  >                  return ret;
>  >          rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> 
> So, you're closing the port, then you remove the device, then you  release port. From your own analysis, what dev_remove does is the following:
> 
>  >    rte_pmd_vhost_remove(struct rte_vdev_device *dev)
>  >    {
>  >     ...........
>  >           eth_dev_close(eth_dev);
>  >
>  >            rte_free(vring_states[eth_dev->data->port_id]);
>  >            vring_states[eth_dev->data->port_id] = NULL;
>  >
>  >            rte_eth_dev_release_port(eth_dev);
> 
> So, you have duplicate call to eth_dev_close() (which is likely not a  problem), and a duplicate call to rte_eth_dev_release_port(). So, if  rte_dev_remove() already calls rte_eth_dev_release_port(), why are you  calling it in your code?
> 
> Now, as i said above, i'm not well-versed enough in the dev  infrastructure to know which of the calls should go where, but i'm  pretty sure you shouldn't call rte_eth_dev_release_port twice - this is  likely what's causing your double-free.
> 
> -- Thanks,
> Anatoly

Hello Anatoly,

You are right, I should not call rte_eth_dev_release_port.
As an experiment, I commented out rte_eth_dev_release_port.
But the same error remains.
(I compiled my app(SPP) after the modification)

VHOST_CONFIG: vhost-user server: socket created, fd: 15
VHOST_CONFIG: bind to /tmp/sock0
EAL: Error: Invalid memory

So I think something is still there in DPDK.

BR,
Hideyuki Yamashita
NTT TechnoCross

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-17 10:45         ` Hideyuki Yamashita
@ 2018-12-17 11:21           ` Burakov, Anatoly
  2018-12-17 12:10             ` Hideyuki Yamashita
  2018-12-18  2:39             ` Tiwei Bie
  0 siblings, 2 replies; 21+ messages in thread
From: Burakov, Anatoly @ 2018-12-17 11:21 UTC (permalink / raw)
  To: Hideyuki Yamashita; +Cc: dev, Yasufumi Ogawa, Nakamura Hioryuki

On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
>> On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
>>> Dear Thomas and all,
>>>
>>> I took a look on dpdk code.
>>> I firstly write qustions and my analisys
>>> on the current dpdk code follows after that.
>>>
>>> [1.Questions]
>>> I have several questions to ask again.
>>> Is my understanding correct about followings
>>>
>>> Q1: "EAL:ERROR, Invalid memory" is ignorable
>>>
>>> Q2: there is no big difference between calling
>>> rte_eal_hotplug_remove(bus->name, dev->name)
>>> and
>>> rte_dev_remove(dev) because anyway it
>>> reaches to rte_pmd_vhost_remove and encounter
>>> the same error.
>>>
>>> [2.snip from my code]
>>> .....
>>>           rte_eth_dev_close(port_id);
>>>           ret = rte_dev_remove(dev);
>>>           if (ret < 0)
>>>                   return ret;
>>>           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
>>>
>>> [3. My analysis on dpdk code]
>>> static int
>>>     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
>>>     {
>>>      ...........
>>>            eth_dev_close(eth_dev);
>>>
>>>             rte_free(vring_states[eth_dev->data->port_id]);
>>>             vring_states[eth_dev->data->port_id] = NULL;
>>>
>>>             rte_eth_dev_release_port(eth_dev);
>>>
>>> As you can see in rte_eth_vhost.c
>>> It calls both eth_dev_close and rte_eth_dev_release_port.
>>> And inside both functions, it tries to free mac_addrs.
>>>           rte_free(dev->data->mac_addrs);       //in rth_dev_close
>>>           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
>>>
>>> I understand that is the reason why
>>> /* Free the memory space back to heap */
>>> void rte_free(void *addr)
>>> {
>>>           if (addr == NULL) return;
>>>           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
>>>                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
>>> }
>>> encounter the error.
>>> As an experiment, I commented out one of them, "ERR, Invalid memory"
>>> disappered.
>>>
>>> Thanks and BR,
>>> Hideyuki Yamashita
>>> NTT TechnoCross
>>>
>>>> Adding my colleague Yasufumi and Hiroyuki as CC.
>>>>
>>>> We are waiting valuable advice from you.
>>>>
>>>> Thanks in advance,
>>>> Hideyuki Yamashita
>>>> NTT TechnoCross
>>>>
>>>>>
>>>>> Dear Thomas and all,
>>>>>
>>>>> I hope you all get safely back home after DPDK summit.
>>>>> (When I get back Japan, it is chilling. (start of winter))
>>>>>
>>>>> On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
>>>>> However, following syslog message is printed.
>>>>> “EAL: Error: Invalid memory”
>>>>>
>>>>> At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
>>>>> Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
>>>>> “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
>>>>>
>>>>> We tested above changes, but the result is the same (i.e., same error message is printed).
>>>>> The debug log message says:
>>>>> ---
>>>>> [primary]
>>>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
>>>>> VHOST_CONFIG: bind to /tmp/sock0
>>>>> EAL: Error: Invalid memory
>>>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
>>>>> VHOST_CONFIG: bind to /tmp/sock0
>>>>>
>>>>> [secondary]
>>>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
>>>>> EAL: request: eal_dev_mp_request
>>>>> EAL: msg: eal_dev_mp_request
>>>>> EAL: request: bus_vdev_mp
>>>>> EAL: msg: bus_vdev_mp
>>>>> EAL: msg: bus_vdev_mp
>>>>> EAL: reply: eal_dev_mp_request
>>>>> EAL: msg: eal_dev_mp_request
>>>>> rte_eth_promiscuous_disable: Function not supported
>>>>> rte_eth_allmulticast_disable: Function not supported
>>>>> APP: To Server: add
>>>>> EAL: request: eal_dev_mp_request
>>>>> EAL: msg: eal_dev_mp_request
>>>>> EAL: reply: eal_dev_mp_request
>>>>> EAL: msg: eal_dev_mp_request
>>>>> APP: To Server: del
>>>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
>>>>> EAL: request: eal_dev_mp_request
>>>>> EAL: msg: eal_dev_mp_request
>>>>> EAL: request: bus_vdev_mp
>>>>> EAL: msg: bus_vdev_mp
>>>>> EAL: msg: bus_vdev_mp
>>>>> EAL: reply: eal_dev_mp_request
>>>>> EAL: msg: eal_dev_mp_request
>>>>> rte_eth_promiscuous_disable: Function not supported
>>>>> rte_eth_allmulticast_disable: Function not supported
>>>>> APP: To Server: add
>>>>> ---
>>>>>
>>>>> We would like to ask:
>>>>> 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
>>>>> 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
>>>>>
>>>>> Thanks in advance and have a nice day.
>>>>>
>>>>> BR,
>>>>> Hideyuki Yamashita
>>>>> NTT TechnoCross
>>>>
>>>
>>>
>>>
>> Hi Hideyuki,
>>
>> The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
>>
>> If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
>>
>> I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
>>
>> --Thanks,
>> Anatoly
> Hello Anatoly,
> 
> Thanks for your reply for my newbie question.
> Now I understand that this error is harmless from DPDK application(SPP)
> point of view in practice. Thanks.
> But anyway if there is a double free logic, it is a bug and should be
> fixed.
> The remaining issues are
> 1. If it is really a bug (or my mis-understanding)
> 2. If is is a bug which function should remove rte_free(mac_addrs)

 From description, it looks like a bug. Correct usage of API  
(rte_dev_close() followed by rte_dev_remove()) should not trigger any  
errors. You might want to create a BugZilla entry describing the issue.

> 
> Thanks,
> Hideyuki Yamashita
> NTT TechnoCross
> 
> 


-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-17 11:21           ` Burakov, Anatoly
@ 2018-12-17 12:10             ` Hideyuki Yamashita
  2018-12-18  2:39             ` Tiwei Bie
  1 sibling, 0 replies; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-17 12:10 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Yasufumi Ogawa, Nakamura Hioryuki

> On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> >> On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> >>> Dear Thomas and all,
> >>>
> >>> I took a look on dpdk code.
> >>> I firstly write qustions and my analisys
> >>> on the current dpdk code follows after that.
> >>>
> >>> [1.Questions]
> >>> I have several questions to ask again.
> >>> Is my understanding correct about followings
> >>>
> >>> Q1: "EAL:ERROR, Invalid memory" is ignorable
> >>>
> >>> Q2: there is no big difference between calling
> >>> rte_eal_hotplug_remove(bus->name, dev->name)
> >>> and
> >>> rte_dev_remove(dev) because anyway it
> >>> reaches to rte_pmd_vhost_remove and encounter
> >>> the same error.
> >>>
> >>> [2.snip from my code]
> >>> .....
> >>>           rte_eth_dev_close(port_id);
> >>>           ret = rte_dev_remove(dev);
> >>>           if (ret < 0)
> >>>                   return ret;
> >>>           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> >>>
> >>> [3. My analysis on dpdk code]
> >>> static int
> >>>     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> >>>     {
> >>>      ...........
> >>>            eth_dev_close(eth_dev);
> >>>
> >>>             rte_free(vring_states[eth_dev->data->port_id]);
> >>>             vring_states[eth_dev->data->port_id] = NULL;
> >>>
> >>>             rte_eth_dev_release_port(eth_dev);
> >>>
> >>> As you can see in rte_eth_vhost.c
> >>> It calls both eth_dev_close and rte_eth_dev_release_port.
> >>> And inside both functions, it tries to free mac_addrs.
> >>>           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> >>>           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> >>>
> >>> I understand that is the reason why
> >>> /* Free the memory space back to heap */
> >>> void rte_free(void *addr)
> >>> {
> >>>           if (addr == NULL) return;
> >>>           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> >>>                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> >>> }
> >>> encounter the error.
> >>> As an experiment, I commented out one of them, "ERR, Invalid memory"
> >>> disappered.
> >>>
> >>> Thanks and BR,
> >>> Hideyuki Yamashita
> >>> NTT TechnoCross
> >>>
> >>>> Adding my colleague Yasufumi and Hiroyuki as CC.
> >>>>
> >>>> We are waiting valuable advice from you.
> >>>>
> >>>> Thanks in advance,
> >>>> Hideyuki Yamashita
> >>>> NTT TechnoCross
> >>>>
> >>>>>
> >>>>> Dear Thomas and all,
> >>>>>
> >>>>> I hope you all get safely back home after DPDK summit.
> >>>>> (When I get back Japan, it is chilling. (start of winter))
> >>>>>
> >>>>> On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> >>>>> However, following syslog message is printed.
> >>>>> “EAL: Error: Invalid memory”
> >>>>>
> >>>>> At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> >>>>> Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> >>>>> “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> >>>>>
> >>>>> We tested above changes, but the result is the same (i.e., same error message is printed).
> >>>>> The debug log message says:
> >>>>> ---
> >>>>> [primary]
> >>>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
> >>>>> VHOST_CONFIG: bind to /tmp/sock0
> >>>>> EAL: Error: Invalid memory
> >>>>> VHOST_CONFIG: vhost-user server: socket created, fd: 17
> >>>>> VHOST_CONFIG: bind to /tmp/sock0
> >>>>>
> >>>>> [secondary]
> >>>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> >>>>> EAL: request: eal_dev_mp_request
> >>>>> EAL: msg: eal_dev_mp_request
> >>>>> EAL: request: bus_vdev_mp
> >>>>> EAL: msg: bus_vdev_mp
> >>>>> EAL: msg: bus_vdev_mp
> >>>>> EAL: reply: eal_dev_mp_request
> >>>>> EAL: msg: eal_dev_mp_request
> >>>>> rte_eth_promiscuous_disable: Function not supported
> >>>>> rte_eth_allmulticast_disable: Function not supported
> >>>>> APP: To Server: add
> >>>>> EAL: request: eal_dev_mp_request
> >>>>> EAL: msg: eal_dev_mp_request
> >>>>> EAL: reply: eal_dev_mp_request
> >>>>> EAL: msg: eal_dev_mp_request
> >>>>> APP: To Server: del
> >>>>> APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> >>>>> EAL: request: eal_dev_mp_request
> >>>>> EAL: msg: eal_dev_mp_request
> >>>>> EAL: request: bus_vdev_mp
> >>>>> EAL: msg: bus_vdev_mp
> >>>>> EAL: msg: bus_vdev_mp
> >>>>> EAL: reply: eal_dev_mp_request
> >>>>> EAL: msg: eal_dev_mp_request
> >>>>> rte_eth_promiscuous_disable: Function not supported
> >>>>> rte_eth_allmulticast_disable: Function not supported
> >>>>> APP: To Server: add
> >>>>> ---
> >>>>>
> >>>>> We would like to ask:
> >>>>> 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> >>>>> 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> >>>>>
> >>>>> Thanks in advance and have a nice day.
> >>>>>
> >>>>> BR,
> >>>>> Hideyuki Yamashita
> >>>>> NTT TechnoCross
> >>>>
> >>>
> >>>
> >>>
> >> Hi Hideyuki,
> >>
> >> The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> >>
> >> If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> >>
> >> I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> >>
> >> --Thanks,
> >> Anatoly
> > Hello Anatoly,
> >
> > Thanks for your reply for my newbie question.
> > Now I understand that this error is harmless from DPDK application(SPP)
> > point of view in practice. Thanks.
> > But anyway if there is a double free logic, it is a bug and should be
> > fixed.
> > The remaining issues are
> > 1. If it is really a bug (or my mis-understanding)
> > 2. If is is a bug which function should remove rte_free(mac_addrs)
> 
>  From description, it looks like a bug. Correct usage of API (rte_dev_close() followed by rte_dev_remove()) should not trigger any errors. You might want to create a BugZilla entry describing the issue.
> 
> >
> > Thanks,
> > Hideyuki Yamashita
> > NTT TechnoCross
> >
> >
> 
> --Thanks,
> Anatoly
Hello 

Thanks for your reply again.
I would like to wait for a while(at least a day) to hear 
opnion from other people including Thomas or maintainer.
In addition I have to learn how to file bug in Bugzzilla
for DPDK.

Thanks,
Hideyuki Yamashita
NTT TechnoCross

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] dpdk vlan strip offload bug for I350 nic?
  2018-12-09  5:23 `  =?gb18030?B?MTUzNDg5ODg5MQ==?=
@ 2018-12-18  2:26   ` Zhao1, Wei
  0 siblings, 0 replies; 21+ messages in thread
From: Zhao1, Wei @ 2018-12-18  2:26 UTC (permalink / raw)
  To: 1534898891, dev

Hi,
    We can not  get your attachment at all.
VLAN STRIP is done by hardware NIC, not DPDK software.
BUT you can check the flag of packets output  from rte_eth_rx_burst(), 
Check bit of PKT_RX_VLAN in rxm->ol_flags.  Only if this bit is set rxm->vlan_tci is valid. 
For IMCP packet, maybe it is not set at all.
BTW, please give out the predicted and actual vlan_tci to us for debug.


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of 1534898891
> Sent: Sunday, December 9, 2018 1:23 PM
> To: dev <dev@dpdk.org>
> Subject: Re: [dpdk-dev] dpdk vlan strip offload bug for I350 nic?
> 
> My server's port connect to switch with trunk. The gw IP on switch's vlan-if. I
> used kni send icmp reques to gw, but the vlan_tci of icmp reply packets is
> incorrect. Other packets are correct. such as the ping same subnet servers or
> other packets(tcp or udp) and switch gw ping my kni is OK. This is too weird.
> The attachment is my test program. I guess it's dpdk drive bug or firmware
> bug.
> 
> 
> Any help is appreciated.
> 
> 
> Thanks.
> 
> 
> 
> ------------------ Original ------------------
> From:  "冰泪"<1534898891@qq.com>;
> Date:  Sun, Dec 9, 2018 12:30 PM
> To:  "dev"<dev@dpdk.org>;
> 
> Subject:  dpdk vlan strip offload bug for I350 nic?
> 
> 
> 
> Hi Guys:
> I used the vlan strip offload in I350 nic case. VLAN tag is incorrect when
> packets are ICMP. Other packets are correct.
> NIC:I350
> DPDK version:18.02 OS:centos 7.4
> Any help is appreciated.
> 
> 
> Thanks.
> Hongbowang.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-17 11:21           ` Burakov, Anatoly
  2018-12-17 12:10             ` Hideyuki Yamashita
@ 2018-12-18  2:39             ` Tiwei Bie
  2018-12-18  3:11               ` Hideyuki Yamashita
  1 sibling, 1 reply; 21+ messages in thread
From: Tiwei Bie @ 2018-12-18  2:39 UTC (permalink / raw)
  To: Burakov, Anatoly, Hideyuki Yamashita
  Cc: dev, Yasufumi Ogawa, Nakamura Hioryuki

On Mon, Dec 17, 2018 at 11:21:01AM +0000, Burakov, Anatoly wrote:
> On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> > > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > > > Dear Thomas and all,
> > > > 
> > > > I took a look on dpdk code.
> > > > I firstly write qustions and my analisys
> > > > on the current dpdk code follows after that.
> > > > 
> > > > [1.Questions]
> > > > I have several questions to ask again.
> > > > Is my understanding correct about followings
> > > > 
> > > > Q1: "EAL:ERROR, Invalid memory" is ignorable
> > > > 
> > > > Q2: there is no big difference between calling
> > > > rte_eal_hotplug_remove(bus->name, dev->name)
> > > > and
> > > > rte_dev_remove(dev) because anyway it
> > > > reaches to rte_pmd_vhost_remove and encounter
> > > > the same error.
> > > > 
> > > > [2.snip from my code]
> > > > .....
> > > >           rte_eth_dev_close(port_id);
> > > >           ret = rte_dev_remove(dev);
> > > >           if (ret < 0)
> > > >                   return ret;
> > > >           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> > > > 
> > > > [3. My analysis on dpdk code]
> > > > static int
> > > >     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> > > >     {
> > > >      ...........
> > > >            eth_dev_close(eth_dev);
> > > > 
> > > >             rte_free(vring_states[eth_dev->data->port_id]);
> > > >             vring_states[eth_dev->data->port_id] = NULL;
> > > > 
> > > >             rte_eth_dev_release_port(eth_dev);
> > > > 
> > > > As you can see in rte_eth_vhost.c
> > > > It calls both eth_dev_close and rte_eth_dev_release_port.
> > > > And inside both functions, it tries to free mac_addrs.
> > > >           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> > > >           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> > > > 
> > > > I understand that is the reason why
> > > > /* Free the memory space back to heap */
> > > > void rte_free(void *addr)
> > > > {
> > > >           if (addr == NULL) return;
> > > >           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > > >                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > > > }
> > > > encounter the error.
> > > > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > > > disappered.
> > > > 
> > > > Thanks and BR,
> > > > Hideyuki Yamashita
> > > > NTT TechnoCross
> > > > 
> > > > > Adding my colleague Yasufumi and Hiroyuki as CC.
> > > > > 
> > > > > We are waiting valuable advice from you.
> > > > > 
> > > > > Thanks in advance,
> > > > > Hideyuki Yamashita
> > > > > NTT TechnoCross
> > > > > 
> > > > > > 
> > > > > > Dear Thomas and all,
> > > > > > 
> > > > > > I hope you all get safely back home after DPDK summit.
> > > > > > (When I get back Japan, it is chilling. (start of winter))
> > > > > > 
> > > > > > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > > > > > However, following syslog message is printed.
> > > > > > “EAL: Error: Invalid memory”
> > > > > > 
> > > > > > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > > > > > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > > > > > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > > > > > 
> > > > > > We tested above changes, but the result is the same (i.e., same error message is printed).
> > > > > > The debug log message says:
> > > > > > ---
> > > > > > [primary]
> > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > EAL: Error: Invalid memory
> > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > 
> > > > > > [secondary]
> > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > EAL: request: eal_dev_mp_request
> > > > > > EAL: msg: eal_dev_mp_request
> > > > > > EAL: request: bus_vdev_mp
> > > > > > EAL: msg: bus_vdev_mp
> > > > > > EAL: msg: bus_vdev_mp
> > > > > > EAL: reply: eal_dev_mp_request
> > > > > > EAL: msg: eal_dev_mp_request
> > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > APP: To Server: add
> > > > > > EAL: request: eal_dev_mp_request
> > > > > > EAL: msg: eal_dev_mp_request
> > > > > > EAL: reply: eal_dev_mp_request
> > > > > > EAL: msg: eal_dev_mp_request
> > > > > > APP: To Server: del
> > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > EAL: request: eal_dev_mp_request
> > > > > > EAL: msg: eal_dev_mp_request
> > > > > > EAL: request: bus_vdev_mp
> > > > > > EAL: msg: bus_vdev_mp
> > > > > > EAL: msg: bus_vdev_mp
> > > > > > EAL: reply: eal_dev_mp_request
> > > > > > EAL: msg: eal_dev_mp_request
> > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > APP: To Server: add
> > > > > > ---
> > > > > > 
> > > > > > We would like to ask:
> > > > > > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > > > > > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > > > > > 
> > > > > > Thanks in advance and have a nice day.
> > > > > > 
> > > > > > BR,
> > > > > > Hideyuki Yamashita
> > > > > > NTT TechnoCross
> > > > > 
> > > > 
> > > > 
> > > > 
> > > Hi Hideyuki,
> > > 
> > > The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> > > 
> > > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> > > 
> > > I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> > > 
> > > --Thanks,
> > > Anatoly
> > Hello Anatoly,
> > 
> > Thanks for your reply for my newbie question.
> > Now I understand that this error is harmless from DPDK application(SPP)
> > point of view in practice. Thanks.
> > But anyway if there is a double free logic, it is a bug and should be
> > fixed.
> > The remaining issues are
> > 1. If it is really a bug (or my mis-understanding)
> > 2. If is is a bug which function should remove rte_free(mac_addrs)
> 
> From description, it looks like a bug. Correct usage of API (rte_dev_close()
> followed by rte_dev_remove()) should not trigger any errors. You might want
> to create a BugZilla entry describing the issue.

I also think it's a bug. The rte_free(dev->data->mac_addrs) in
vhost PMD's eth_dev_close() should be removed, as the common data
freeing has been moved to rte_eth_dev_release_port().

@Hideyuki, thanks for digging into this!

> 
> > 
> > Thanks,
> > Hideyuki Yamashita
> > NTT TechnoCross
> > 
> > 
> 
> 
> -- 
> Thanks,
> Anatoly

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-18  2:39             ` Tiwei Bie
@ 2018-12-18  3:11               ` Hideyuki Yamashita
  2018-12-18  5:12                 ` Tiwei Bie
  0 siblings, 1 reply; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-18  3:11 UTC (permalink / raw)
  To: Tiwei Bie; +Cc: Burakov, Anatoly, dev, Yasufumi Ogawa, Nakamura Hioryuki


------------------------------------------------> On Mon, Dec 17, 2018 at 11:21:01AM +0000, Burakov, Anatoly wrote:
> > On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> > > > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > > > > Dear Thomas and all,
> > > > > 
> > > > > I took a look on dpdk code.
> > > > > I firstly write qustions and my analisys
> > > > > on the current dpdk code follows after that.
> > > > > 
> > > > > [1.Questions]
> > > > > I have several questions to ask again.
> > > > > Is my understanding correct about followings
> > > > > 
> > > > > Q1: "EAL:ERROR, Invalid memory" is ignorable
> > > > > 
> > > > > Q2: there is no big difference between calling
> > > > > rte_eal_hotplug_remove(bus->name, dev->name)
> > > > > and
> > > > > rte_dev_remove(dev) because anyway it
> > > > > reaches to rte_pmd_vhost_remove and encounter
> > > > > the same error.
> > > > > 
> > > > > [2.snip from my code]
> > > > > .....
> > > > >           rte_eth_dev_close(port_id);
> > > > >           ret = rte_dev_remove(dev);
> > > > >           if (ret < 0)
> > > > >                   return ret;
> > > > >           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> > > > > 
> > > > > [3. My analysis on dpdk code]
> > > > > static int
> > > > >     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> > > > >     {
> > > > >      ...........
> > > > >            eth_dev_close(eth_dev);
> > > > > 
> > > > >             rte_free(vring_states[eth_dev->data->port_id]);
> > > > >             vring_states[eth_dev->data->port_id] = NULL;
> > > > > 
> > > > >             rte_eth_dev_release_port(eth_dev);
> > > > > 
> > > > > As you can see in rte_eth_vhost.c
> > > > > It calls both eth_dev_close and rte_eth_dev_release_port.
> > > > > And inside both functions, it tries to free mac_addrs.
> > > > >           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> > > > >           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> > > > > 
> > > > > I understand that is the reason why
> > > > > /* Free the memory space back to heap */
> > > > > void rte_free(void *addr)
> > > > > {
> > > > >           if (addr == NULL) return;
> > > > >           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > > > >                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > > > > }
> > > > > encounter the error.
> > > > > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > > > > disappered.
> > > > > 
> > > > > Thanks and BR,
> > > > > Hideyuki Yamashita
> > > > > NTT TechnoCross
> > > > > 
> > > > > > Adding my colleague Yasufumi and Hiroyuki as CC.
> > > > > > 
> > > > > > We are waiting valuable advice from you.
> > > > > > 
> > > > > > Thanks in advance,
> > > > > > Hideyuki Yamashita
> > > > > > NTT TechnoCross
> > > > > > 
> > > > > > > 
> > > > > > > Dear Thomas and all,
> > > > > > > 
> > > > > > > I hope you all get safely back home after DPDK summit.
> > > > > > > (When I get back Japan, it is chilling. (start of winter))
> > > > > > > 
> > > > > > > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > > > > > > However, following syslog message is printed.
> > > > > > > “EAL: Error: Invalid memory”
> > > > > > > 
> > > > > > > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > > > > > > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > > > > > > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > > > > > > 
> > > > > > > We tested above changes, but the result is the same (i.e., same error message is printed).
> > > > > > > The debug log message says:
> > > > > > > ---
> > > > > > > [primary]
> > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > EAL: Error: Invalid memory
> > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > 
> > > > > > > [secondary]
> > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > EAL: request: bus_vdev_mp
> > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > APP: To Server: add
> > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > APP: To Server: del
> > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > EAL: request: bus_vdev_mp
> > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > APP: To Server: add
> > > > > > > ---
> > > > > > > 
> > > > > > > We would like to ask:
> > > > > > > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > > > > > > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > > > > > > 
> > > > > > > Thanks in advance and have a nice day.
> > > > > > > 
> > > > > > > BR,
> > > > > > > Hideyuki Yamashita
> > > > > > > NTT TechnoCross
> > > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > Hi Hideyuki,
> > > > 
> > > > The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> > > > 
> > > > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> > > > 
> > > > I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> > > > 
> > > > --Thanks,
> > > > Anatoly
> > > Hello Anatoly,
> > > 
> > > Thanks for your reply for my newbie question.
> > > Now I understand that this error is harmless from DPDK application(SPP)
> > > point of view in practice. Thanks.
> > > But anyway if there is a double free logic, it is a bug and should be
> > > fixed.
> > > The remaining issues are
> > > 1. If it is really a bug (or my mis-understanding)
> > > 2. If is is a bug which function should remove rte_free(mac_addrs)
> > 
> > From description, it looks like a bug. Correct usage of API (rte_dev_close()
> > followed by rte_dev_remove()) should not trigger any errors. You might want
> > to create a BugZilla entry describing the issue.
> 
> I also think it's a bug. The rte_free(dev->data->mac_addrs) in
> vhost PMD's eth_dev_close() should be removed, as the common data
> freeing has been moved to rte_eth_dev_release_port().
> 
> @Hideyuki, thanks for digging into this!

Hello Twei,

I share the same view with you.
rte_eth_dev_release_port() is used in commonly in many pmds.
rte_dev_close() is vhost pmd local.

BTW, what is the difference between using
rte_eal_hotplug_remove and rte_eth_dev_close()+rte_dev_remove?
(I see no big difference right now from application developer point of
view)

Next question is what should I do next (you know I am very newbie).
File this bug into BUgzzila of DPDK?

Thanks for your confirming.

BR,
Hideyuki Yamashita
NTT TechnoCross
 
> > 
> > > 
> > > Thanks,
> > > Hideyuki Yamashita
> > > NTT TechnoCross
> > > 
> > > 
> > 
> > 
> > -- 
> > Thanks,
> > Anatoly

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-18  3:11               ` Hideyuki Yamashita
@ 2018-12-18  5:12                 ` Tiwei Bie
  2018-12-18  5:52                   ` Hideyuki Yamashita
  0 siblings, 1 reply; 21+ messages in thread
From: Tiwei Bie @ 2018-12-18  5:12 UTC (permalink / raw)
  To: Hideyuki Yamashita
  Cc: Burakov, Anatoly, dev, Yasufumi Ogawa, Nakamura Hioryuki

On Tue, Dec 18, 2018 at 12:11:33PM +0900, Hideyuki Yamashita wrote:
> 
> On Mon, Dec 17, 2018 at 11:21:01AM +0000, Burakov, Anatoly wrote:
> > > On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> > > > > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > > > > > Dear Thomas and all,
> > > > > > 
> > > > > > I took a look on dpdk code.
> > > > > > I firstly write qustions and my analisys
> > > > > > on the current dpdk code follows after that.
> > > > > > 
> > > > > > [1.Questions]
> > > > > > I have several questions to ask again.
> > > > > > Is my understanding correct about followings
> > > > > > 
> > > > > > Q1: "EAL:ERROR, Invalid memory" is ignorable
> > > > > > 
> > > > > > Q2: there is no big difference between calling
> > > > > > rte_eal_hotplug_remove(bus->name, dev->name)
> > > > > > and
> > > > > > rte_dev_remove(dev) because anyway it
> > > > > > reaches to rte_pmd_vhost_remove and encounter
> > > > > > the same error.
> > > > > > 
> > > > > > [2.snip from my code]
> > > > > > .....
> > > > > >           rte_eth_dev_close(port_id);
> > > > > >           ret = rte_dev_remove(dev);
> > > > > >           if (ret < 0)
> > > > > >                   return ret;
> > > > > >           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> > > > > > 
> > > > > > [3. My analysis on dpdk code]
> > > > > > static int
> > > > > >     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> > > > > >     {
> > > > > >      ...........
> > > > > >            eth_dev_close(eth_dev);
> > > > > > 
> > > > > >             rte_free(vring_states[eth_dev->data->port_id]);
> > > > > >             vring_states[eth_dev->data->port_id] = NULL;
> > > > > > 
> > > > > >             rte_eth_dev_release_port(eth_dev);
> > > > > > 
> > > > > > As you can see in rte_eth_vhost.c
> > > > > > It calls both eth_dev_close and rte_eth_dev_release_port.
> > > > > > And inside both functions, it tries to free mac_addrs.
> > > > > >           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> > > > > >           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> > > > > > 
> > > > > > I understand that is the reason why
> > > > > > /* Free the memory space back to heap */
> > > > > > void rte_free(void *addr)
> > > > > > {
> > > > > >           if (addr == NULL) return;
> > > > > >           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > > > > >                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > > > > > }
> > > > > > encounter the error.
> > > > > > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > > > > > disappered.
> > > > > > 
> > > > > > Thanks and BR,
> > > > > > Hideyuki Yamashita
> > > > > > NTT TechnoCross
> > > > > > 
> > > > > > > Adding my colleague Yasufumi and Hiroyuki as CC.
> > > > > > > 
> > > > > > > We are waiting valuable advice from you.
> > > > > > > 
> > > > > > > Thanks in advance,
> > > > > > > Hideyuki Yamashita
> > > > > > > NTT TechnoCross
> > > > > > > 
> > > > > > > > 
> > > > > > > > Dear Thomas and all,
> > > > > > > > 
> > > > > > > > I hope you all get safely back home after DPDK summit.
> > > > > > > > (When I get back Japan, it is chilling. (start of winter))
> > > > > > > > 
> > > > > > > > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > > > > > > > However, following syslog message is printed.
> > > > > > > > “EAL: Error: Invalid memory”
> > > > > > > > 
> > > > > > > > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > > > > > > > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > > > > > > > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > > > > > > > 
> > > > > > > > We tested above changes, but the result is the same (i.e., same error message is printed).
> > > > > > > > The debug log message says:
> > > > > > > > ---
> > > > > > > > [primary]
> > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > EAL: Error: Invalid memory
> > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > 
> > > > > > > > [secondary]
> > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > APP: To Server: add
> > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > APP: To Server: del
> > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > APP: To Server: add
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > We would like to ask:
> > > > > > > > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > > > > > > > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > > > > > > > 
> > > > > > > > Thanks in advance and have a nice day.
> > > > > > > > 
> > > > > > > > BR,
> > > > > > > > Hideyuki Yamashita
> > > > > > > > NTT TechnoCross
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > Hi Hideyuki,
> > > > > 
> > > > > The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> > > > > 
> > > > > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> > > > > 
> > > > > I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> > > > > 
> > > > > --Thanks,
> > > > > Anatoly
> > > > Hello Anatoly,
> > > > 
> > > > Thanks for your reply for my newbie question.
> > > > Now I understand that this error is harmless from DPDK application(SPP)
> > > > point of view in practice. Thanks.
> > > > But anyway if there is a double free logic, it is a bug and should be
> > > > fixed.
> > > > The remaining issues are
> > > > 1. If it is really a bug (or my mis-understanding)
> > > > 2. If is is a bug which function should remove rte_free(mac_addrs)
> > > 
> > > From description, it looks like a bug. Correct usage of API (rte_dev_close()
> > > followed by rte_dev_remove()) should not trigger any errors. You might want
> > > to create a BugZilla entry describing the issue.
> > 
> > I also think it's a bug. The rte_free(dev->data->mac_addrs) in
> > vhost PMD's eth_dev_close() should be removed, as the common data
> > freeing has been moved to rte_eth_dev_release_port().
> > 
> > @Hideyuki, thanks for digging into this!
> 
> Hello Twei,
> 
> I share the same view with you.
> rte_eth_dev_release_port() is used in commonly in many pmds.
> rte_dev_close() is vhost pmd local.
> 
> BTW, what is the difference between using
> rte_eal_hotplug_remove and rte_eth_dev_close()+rte_dev_remove?
> (I see no big difference right now from application developer point of
> view)

IMO, the difference is that the former one doesn't require
operating on rte_device directly.

> 
> Next question is what should I do next (you know I am very newbie).
> File this bug into BUgzzila of DPDK?

The issue is quite clear, I can fix it directly.

And as the fix is also quite clear, if you want to contribute
patch, you can also try to send a fix patch [1].

Either way is fine for me. Just let me know. :-)

[1] https://core.dpdk.org/contribute/#send


> 
> Thanks for your confirming.
> 
> BR,
> Hideyuki Yamashita
> NTT TechnoCross
>  
> > > 
> > > > 
> > > > Thanks,
> > > > Hideyuki Yamashita
> > > > NTT TechnoCross
> > > > 
> > > > 
> > > 
> > > 
> > > -- 
> > > Thanks,
> > > Anatoly
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-18  5:12                 ` Tiwei Bie
@ 2018-12-18  5:52                   ` Hideyuki Yamashita
  2018-12-18  6:25                     ` Tiwei Bie
  0 siblings, 1 reply; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-18  5:52 UTC (permalink / raw)
  To: Tiwei Bie; +Cc: Burakov, Anatoly, dev, Yasufumi Ogawa, Nakamura Hioryuki

> On Tue, Dec 18, 2018 at 12:11:33PM +0900, Hideyuki Yamashita wrote:
> > 
> > On Mon, Dec 17, 2018 at 11:21:01AM +0000, Burakov, Anatoly wrote:
> > > > On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> > > > > > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > > > > > > Dear Thomas and all,
> > > > > > > 
> > > > > > > I took a look on dpdk code.
> > > > > > > I firstly write qustions and my analisys
> > > > > > > on the current dpdk code follows after that.
> > > > > > > 
> > > > > > > [1.Questions]
> > > > > > > I have several questions to ask again.
> > > > > > > Is my understanding correct about followings
> > > > > > > 
> > > > > > > Q1: "EAL:ERROR, Invalid memory" is ignorable
> > > > > > > 
> > > > > > > Q2: there is no big difference between calling
> > > > > > > rte_eal_hotplug_remove(bus->name, dev->name)
> > > > > > > and
> > > > > > > rte_dev_remove(dev) because anyway it
> > > > > > > reaches to rte_pmd_vhost_remove and encounter
> > > > > > > the same error.
> > > > > > > 
> > > > > > > [2.snip from my code]
> > > > > > > .....
> > > > > > >           rte_eth_dev_close(port_id);
> > > > > > >           ret = rte_dev_remove(dev);
> > > > > > >           if (ret < 0)
> > > > > > >                   return ret;
> > > > > > >           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> > > > > > > 
> > > > > > > [3. My analysis on dpdk code]
> > > > > > > static int
> > > > > > >     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> > > > > > >     {
> > > > > > >      ...........
> > > > > > >            eth_dev_close(eth_dev);
> > > > > > > 
> > > > > > >             rte_free(vring_states[eth_dev->data->port_id]);
> > > > > > >             vring_states[eth_dev->data->port_id] = NULL;
> > > > > > > 
> > > > > > >             rte_eth_dev_release_port(eth_dev);
> > > > > > > 
> > > > > > > As you can see in rte_eth_vhost.c
> > > > > > > It calls both eth_dev_close and rte_eth_dev_release_port.
> > > > > > > And inside both functions, it tries to free mac_addrs.
> > > > > > >           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> > > > > > >           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> > > > > > > 
> > > > > > > I understand that is the reason why
> > > > > > > /* Free the memory space back to heap */
> > > > > > > void rte_free(void *addr)
> > > > > > > {
> > > > > > >           if (addr == NULL) return;
> > > > > > >           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > > > > > >                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > > > > > > }
> > > > > > > encounter the error.
> > > > > > > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > > > > > > disappered.
> > > > > > > 
> > > > > > > Thanks and BR,
> > > > > > > Hideyuki Yamashita
> > > > > > > NTT TechnoCross
> > > > > > > 
> > > > > > > > Adding my colleague Yasufumi and Hiroyuki as CC.
> > > > > > > > 
> > > > > > > > We are waiting valuable advice from you.
> > > > > > > > 
> > > > > > > > Thanks in advance,
> > > > > > > > Hideyuki Yamashita
> > > > > > > > NTT TechnoCross
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Dear Thomas and all,
> > > > > > > > > 
> > > > > > > > > I hope you all get safely back home after DPDK summit.
> > > > > > > > > (When I get back Japan, it is chilling. (start of winter))
> > > > > > > > > 
> > > > > > > > > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > > > > > > > > However, following syslog message is printed.
> > > > > > > > > “EAL: Error: Invalid memory”
> > > > > > > > > 
> > > > > > > > > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > > > > > > > > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > > > > > > > > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > > > > > > > > 
> > > > > > > > > We tested above changes, but the result is the same (i.e., same error message is printed).
> > > > > > > > > The debug log message says:
> > > > > > > > > ---
> > > > > > > > > [primary]
> > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > EAL: Error: Invalid memory
> > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > 
> > > > > > > > > [secondary]
> > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > APP: To Server: add
> > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > APP: To Server: del
> > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > APP: To Server: add
> > > > > > > > > ---
> > > > > > > > > 
> > > > > > > > > We would like to ask:
> > > > > > > > > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > > > > > > > > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > > > > > > > > 
> > > > > > > > > Thanks in advance and have a nice day.
> > > > > > > > > 
> > > > > > > > > BR,
> > > > > > > > > Hideyuki Yamashita
> > > > > > > > > NTT TechnoCross
> > > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > Hi Hideyuki,
> > > > > > 
> > > > > > The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> > > > > > 
> > > > > > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> > > > > > 
> > > > > > I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> > > > > > 
> > > > > > --Thanks,
> > > > > > Anatoly
> > > > > Hello Anatoly,
> > > > > 
> > > > > Thanks for your reply for my newbie question.
> > > > > Now I understand that this error is harmless from DPDK application(SPP)
> > > > > point of view in practice. Thanks.
> > > > > But anyway if there is a double free logic, it is a bug and should be
> > > > > fixed.
> > > > > The remaining issues are
> > > > > 1. If it is really a bug (or my mis-understanding)
> > > > > 2. If is is a bug which function should remove rte_free(mac_addrs)
> > > > 
> > > > From description, it looks like a bug. Correct usage of API (rte_dev_close()
> > > > followed by rte_dev_remove()) should not trigger any errors. You might want
> > > > to create a BugZilla entry describing the issue.
> > > 
> > > I also think it's a bug. The rte_free(dev->data->mac_addrs) in
> > > vhost PMD's eth_dev_close() should be removed, as the common data
> > > freeing has been moved to rte_eth_dev_release_port().
> > > 
> > > @Hideyuki, thanks for digging into this!
> > 
> > Hello Twei,
> > 
> > I share the same view with you.
> > rte_eth_dev_release_port() is used in commonly in many pmds.
> > rte_dev_close() is vhost pmd local.
> > 
> > BTW, what is the difference between using
> > rte_eal_hotplug_remove and rte_eth_dev_close()+rte_dev_remove?
> > (I see no big difference right now from application developer point of
> > view)
> 
> IMO, the difference is that the former one doesn't require
> operating on rte_device directly.
> 
> > 
> > Next question is what should I do next (you know I am very newbie).
> > File this bug into BUgzzila of DPDK?
> 
> The issue is quite clear, I can fix it directly.
> 
> And as the fix is also quite clear, if you want to contribute
> patch, you can also try to send a fix patch [1].
> 
> Either way is fine for me. Just let me know. :-)
> 
> [1] https://core.dpdk.org/contribute/#send
Hello,

Thanks for your guidance.

Well, I think it is a good chance to be "a contributer of DPDK".
(even for small one. I am getting excited already)
So let me give a try.

Thanks for your kindness.

BR,
Hideyuki Yamashita
NTT TechnoCross

> 
> > 
> > Thanks for your confirming.
> > 
> > BR,
> > Hideyuki Yamashita
> > NTT TechnoCross
> >  
> > > > 
> > > > > 
> > > > > Thanks,
> > > > > Hideyuki Yamashita
> > > > > NTT TechnoCross
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > > -- 
> > > > Thanks,
> > > > Anatoly
> > 
> > 

------------------------------------------------
NTTテクノクロス株式会社
フューチャーネットワーク事業部 第一事業ユニット
山下 英之(email: yamashita.hideyuki@po.ntt-tx.co.jp)
〒167-0043 東京都杉並区上荻1-2-1 荻窪インテグラルタワー14F
TEL 03-5347-8024    FAX  03-3392-2907
------------------------------------------------------

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-18  5:52                   ` Hideyuki Yamashita
@ 2018-12-18  6:25                     ` Tiwei Bie
  2018-12-18 11:30                       ` Hideyuki Yamashita
  0 siblings, 1 reply; 21+ messages in thread
From: Tiwei Bie @ 2018-12-18  6:25 UTC (permalink / raw)
  To: Hideyuki Yamashita
  Cc: Burakov, Anatoly, dev, Yasufumi Ogawa, Nakamura Hioryuki

On Tue, Dec 18, 2018 at 02:52:16PM +0900, Hideyuki Yamashita wrote:
> > On Tue, Dec 18, 2018 at 12:11:33PM +0900, Hideyuki Yamashita wrote:
> > > 
> > > On Mon, Dec 17, 2018 at 11:21:01AM +0000, Burakov, Anatoly wrote:
> > > > > On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> > > > > > > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > > > > > > > Dear Thomas and all,
> > > > > > > > 
> > > > > > > > I took a look on dpdk code.
> > > > > > > > I firstly write qustions and my analisys
> > > > > > > > on the current dpdk code follows after that.
> > > > > > > > 
> > > > > > > > [1.Questions]
> > > > > > > > I have several questions to ask again.
> > > > > > > > Is my understanding correct about followings
> > > > > > > > 
> > > > > > > > Q1: "EAL:ERROR, Invalid memory" is ignorable
> > > > > > > > 
> > > > > > > > Q2: there is no big difference between calling
> > > > > > > > rte_eal_hotplug_remove(bus->name, dev->name)
> > > > > > > > and
> > > > > > > > rte_dev_remove(dev) because anyway it
> > > > > > > > reaches to rte_pmd_vhost_remove and encounter
> > > > > > > > the same error.
> > > > > > > > 
> > > > > > > > [2.snip from my code]
> > > > > > > > .....
> > > > > > > >           rte_eth_dev_close(port_id);
> > > > > > > >           ret = rte_dev_remove(dev);
> > > > > > > >           if (ret < 0)
> > > > > > > >                   return ret;
> > > > > > > >           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> > > > > > > > 
> > > > > > > > [3. My analysis on dpdk code]
> > > > > > > > static int
> > > > > > > >     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> > > > > > > >     {
> > > > > > > >      ...........
> > > > > > > >            eth_dev_close(eth_dev);
> > > > > > > > 
> > > > > > > >             rte_free(vring_states[eth_dev->data->port_id]);
> > > > > > > >             vring_states[eth_dev->data->port_id] = NULL;
> > > > > > > > 
> > > > > > > >             rte_eth_dev_release_port(eth_dev);
> > > > > > > > 
> > > > > > > > As you can see in rte_eth_vhost.c
> > > > > > > > It calls both eth_dev_close and rte_eth_dev_release_port.
> > > > > > > > And inside both functions, it tries to free mac_addrs.
> > > > > > > >           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> > > > > > > >           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> > > > > > > > 
> > > > > > > > I understand that is the reason why
> > > > > > > > /* Free the memory space back to heap */
> > > > > > > > void rte_free(void *addr)
> > > > > > > > {
> > > > > > > >           if (addr == NULL) return;
> > > > > > > >           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > > > > > > >                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > > > > > > > }
> > > > > > > > encounter the error.
> > > > > > > > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > > > > > > > disappered.
> > > > > > > > 
> > > > > > > > Thanks and BR,
> > > > > > > > Hideyuki Yamashita
> > > > > > > > NTT TechnoCross
> > > > > > > > 
> > > > > > > > > Adding my colleague Yasufumi and Hiroyuki as CC.
> > > > > > > > > 
> > > > > > > > > We are waiting valuable advice from you.
> > > > > > > > > 
> > > > > > > > > Thanks in advance,
> > > > > > > > > Hideyuki Yamashita
> > > > > > > > > NTT TechnoCross
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Dear Thomas and all,
> > > > > > > > > > 
> > > > > > > > > > I hope you all get safely back home after DPDK summit.
> > > > > > > > > > (When I get back Japan, it is chilling. (start of winter))
> > > > > > > > > > 
> > > > > > > > > > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > > > > > > > > > However, following syslog message is printed.
> > > > > > > > > > “EAL: Error: Invalid memory”
> > > > > > > > > > 
> > > > > > > > > > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > > > > > > > > > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > > > > > > > > > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > > > > > > > > > 
> > > > > > > > > > We tested above changes, but the result is the same (i.e., same error message is printed).
> > > > > > > > > > The debug log message says:
> > > > > > > > > > ---
> > > > > > > > > > [primary]
> > > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > > EAL: Error: Invalid memory
> > > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > > 
> > > > > > > > > > [secondary]
> > > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > > APP: To Server: add
> > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > APP: To Server: del
> > > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > > APP: To Server: add
> > > > > > > > > > ---
> > > > > > > > > > 
> > > > > > > > > > We would like to ask:
> > > > > > > > > > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > > > > > > > > > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > > > > > > > > > 
> > > > > > > > > > Thanks in advance and have a nice day.
> > > > > > > > > > 
> > > > > > > > > > BR,
> > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > NTT TechnoCross
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > Hi Hideyuki,
> > > > > > > 
> > > > > > > The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> > > > > > > 
> > > > > > > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> > > > > > > 
> > > > > > > I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> > > > > > > 
> > > > > > > --Thanks,
> > > > > > > Anatoly
> > > > > > Hello Anatoly,
> > > > > > 
> > > > > > Thanks for your reply for my newbie question.
> > > > > > Now I understand that this error is harmless from DPDK application(SPP)
> > > > > > point of view in practice. Thanks.
> > > > > > But anyway if there is a double free logic, it is a bug and should be
> > > > > > fixed.
> > > > > > The remaining issues are
> > > > > > 1. If it is really a bug (or my mis-understanding)
> > > > > > 2. If is is a bug which function should remove rte_free(mac_addrs)
> > > > > 
> > > > > From description, it looks like a bug. Correct usage of API (rte_dev_close()
> > > > > followed by rte_dev_remove()) should not trigger any errors. You might want
> > > > > to create a BugZilla entry describing the issue.
> > > > 
> > > > I also think it's a bug. The rte_free(dev->data->mac_addrs) in
> > > > vhost PMD's eth_dev_close() should be removed, as the common data
> > > > freeing has been moved to rte_eth_dev_release_port().
> > > > 
> > > > @Hideyuki, thanks for digging into this!
> > > 
> > > Hello Twei,
> > > 
> > > I share the same view with you.
> > > rte_eth_dev_release_port() is used in commonly in many pmds.
> > > rte_dev_close() is vhost pmd local.
> > > 
> > > BTW, what is the difference between using
> > > rte_eal_hotplug_remove and rte_eth_dev_close()+rte_dev_remove?
> > > (I see no big difference right now from application developer point of
> > > view)
> > 
> > IMO, the difference is that the former one doesn't require
> > operating on rte_device directly.
> > 
> > > 
> > > Next question is what should I do next (you know I am very newbie).
> > > File this bug into BUgzzila of DPDK?
> > 
> > The issue is quite clear, I can fix it directly.
> > 
> > And as the fix is also quite clear, if you want to contribute
> > patch, you can also try to send a fix patch [1].
> > 
> > Either way is fine for me. Just let me know. :-)
> > 
> > [1] https://core.dpdk.org/contribute/#send
> Hello,
> 
> Thanks for your guidance.
> 
> Well, I think it is a good chance to be "a contributer of DPDK".
> (even for small one. I am getting excited already)
> So let me give a try.

Great! Looking forward to your patch. :-)

> 
> Thanks for your kindness.
> 
> BR,
> Hideyuki Yamashita
> NTT TechnoCross
> 
> > 
> > > 
> > > Thanks for your confirming.
> > > 
> > > BR,
> > > Hideyuki Yamashita
> > > NTT TechnoCross
> > >  
> > > > > 
> > > > > > 
> > > > > > Thanks,
> > > > > > Hideyuki Yamashita
> > > > > > NTT TechnoCross
> > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > > -- 
> > > > > Thanks,
> > > > > Anatoly
> > > 
> > > 
> 
> ------------------------------------------------
> NTTテクノクロス株式会社
> フューチャーネットワーク事業部 第一事業ユニット
> 山下 英之(email: yamashita.hideyuki@po.ntt-tx.co.jp)
> 〒167-0043 東京都杉並区上荻1-2-1 荻窪インテグラルタワー14F
> TEL 03-5347-8024    FAX  03-3392-2907
> ------------------------------------------------------
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-18  6:25                     ` Tiwei Bie
@ 2018-12-18 11:30                       ` Hideyuki Yamashita
  2018-12-18 13:09                         ` Tiwei Bie
  0 siblings, 1 reply; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-18 11:30 UTC (permalink / raw)
  To: Tiwei Bie; +Cc: Burakov, Anatoly, dev, Yasufumi Ogawa, Nakamura Hioryuki

> On Tue, Dec 18, 2018 at 02:52:16PM +0900, Hideyuki Yamashita wrote:
> > > On Tue, Dec 18, 2018 at 12:11:33PM +0900, Hideyuki Yamashita wrote:
> > > > 
> > > > On Mon, Dec 17, 2018 at 11:21:01AM +0000, Burakov, Anatoly wrote:
> > > > > > On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> > > > > > > > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > > > > > > > > Dear Thomas and all,
> > > > > > > > > 
> > > > > > > > > I took a look on dpdk code.
> > > > > > > > > I firstly write qustions and my analisys
> > > > > > > > > on the current dpdk code follows after that.
> > > > > > > > > 
> > > > > > > > > [1.Questions]
> > > > > > > > > I have several questions to ask again.
> > > > > > > > > Is my understanding correct about followings
> > > > > > > > > 
> > > > > > > > > Q1: "EAL:ERROR, Invalid memory" is ignorable
> > > > > > > > > 
> > > > > > > > > Q2: there is no big difference between calling
> > > > > > > > > rte_eal_hotplug_remove(bus->name, dev->name)
> > > > > > > > > and
> > > > > > > > > rte_dev_remove(dev) because anyway it
> > > > > > > > > reaches to rte_pmd_vhost_remove and encounter
> > > > > > > > > the same error.
> > > > > > > > > 
> > > > > > > > > [2.snip from my code]
> > > > > > > > > .....
> > > > > > > > >           rte_eth_dev_close(port_id);
> > > > > > > > >           ret = rte_dev_remove(dev);
> > > > > > > > >           if (ret < 0)
> > > > > > > > >                   return ret;
> > > > > > > > >           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> > > > > > > > > 
> > > > > > > > > [3. My analysis on dpdk code]
> > > > > > > > > static int
> > > > > > > > >     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> > > > > > > > >     {
> > > > > > > > >      ...........
> > > > > > > > >            eth_dev_close(eth_dev);
> > > > > > > > > 
> > > > > > > > >             rte_free(vring_states[eth_dev->data->port_id]);
> > > > > > > > >             vring_states[eth_dev->data->port_id] = NULL;
> > > > > > > > > 
> > > > > > > > >             rte_eth_dev_release_port(eth_dev);
> > > > > > > > > 
> > > > > > > > > As you can see in rte_eth_vhost.c
> > > > > > > > > It calls both eth_dev_close and rte_eth_dev_release_port.
> > > > > > > > > And inside both functions, it tries to free mac_addrs.
> > > > > > > > >           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> > > > > > > > >           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> > > > > > > > > 
> > > > > > > > > I understand that is the reason why
> > > > > > > > > /* Free the memory space back to heap */
> > > > > > > > > void rte_free(void *addr)
> > > > > > > > > {
> > > > > > > > >           if (addr == NULL) return;
> > > > > > > > >           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > > > > > > > >                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > > > > > > > > }
> > > > > > > > > encounter the error.
> > > > > > > > > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > > > > > > > > disappered.
> > > > > > > > > 
> > > > > > > > > Thanks and BR,
> > > > > > > > > Hideyuki Yamashita
> > > > > > > > > NTT TechnoCross
> > > > > > > > > 
> > > > > > > > > > Adding my colleague Yasufumi and Hiroyuki as CC.
> > > > > > > > > > 
> > > > > > > > > > We are waiting valuable advice from you.
> > > > > > > > > > 
> > > > > > > > > > Thanks in advance,
> > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > NTT TechnoCross
> > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Dear Thomas and all,
> > > > > > > > > > > 
> > > > > > > > > > > I hope you all get safely back home after DPDK summit.
> > > > > > > > > > > (When I get back Japan, it is chilling. (start of winter))
> > > > > > > > > > > 
> > > > > > > > > > > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > > > > > > > > > > However, following syslog message is printed.
> > > > > > > > > > > “EAL: Error: Invalid memory”
> > > > > > > > > > > 
> > > > > > > > > > > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > > > > > > > > > > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > > > > > > > > > > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > > > > > > > > > > 
> > > > > > > > > > > We tested above changes, but the result is the same (i.e., same error message is printed).
> > > > > > > > > > > The debug log message says:
> > > > > > > > > > > ---
> > > > > > > > > > > [primary]
> > > > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > > > EAL: Error: Invalid memory
> > > > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > > > 
> > > > > > > > > > > [secondary]
> > > > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > > > APP: To Server: add
> > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > APP: To Server: del
> > > > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > > > APP: To Server: add
> > > > > > > > > > > ---
> > > > > > > > > > > 
> > > > > > > > > > > We would like to ask:
> > > > > > > > > > > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > > > > > > > > > > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > > > > > > > > > > 
> > > > > > > > > > > Thanks in advance and have a nice day.
> > > > > > > > > > > 
> > > > > > > > > > > BR,
> > > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > > NTT TechnoCross
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > Hi Hideyuki,
> > > > > > > > 
> > > > > > > > The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> > > > > > > > 
> > > > > > > > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> > > > > > > > 
> > > > > > > > I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> > > > > > > > 
> > > > > > > > --Thanks,
> > > > > > > > Anatoly
> > > > > > > Hello Anatoly,
> > > > > > > 
> > > > > > > Thanks for your reply for my newbie question.
> > > > > > > Now I understand that this error is harmless from DPDK application(SPP)
> > > > > > > point of view in practice. Thanks.
> > > > > > > But anyway if there is a double free logic, it is a bug and should be
> > > > > > > fixed.
> > > > > > > The remaining issues are
> > > > > > > 1. If it is really a bug (or my mis-understanding)
> > > > > > > 2. If is is a bug which function should remove rte_free(mac_addrs)
> > > > > > 
> > > > > > From description, it looks like a bug. Correct usage of API (rte_dev_close()
> > > > > > followed by rte_dev_remove()) should not trigger any errors. You might want
> > > > > > to create a BugZilla entry describing the issue.
> > > > > 
> > > > > I also think it's a bug. The rte_free(dev->data->mac_addrs) in
> > > > > vhost PMD's eth_dev_close() should be removed, as the common data
> > > > > freeing has been moved to rte_eth_dev_release_port().
> > > > > 
> > > > > @Hideyuki, thanks for digging into this!
> > > > 
> > > > Hello Twei,
> > > > 
> > > > I share the same view with you.
> > > > rte_eth_dev_release_port() is used in commonly in many pmds.
> > > > rte_dev_close() is vhost pmd local.
> > > > 
> > > > BTW, what is the difference between using
> > > > rte_eal_hotplug_remove and rte_eth_dev_close()+rte_dev_remove?
> > > > (I see no big difference right now from application developer point of
> > > > view)
> > > 
> > > IMO, the difference is that the former one doesn't require
> > > operating on rte_device directly.
> > > 
> > > > 
> > > > Next question is what should I do next (you know I am very newbie).
> > > > File this bug into BUgzzila of DPDK?
> > > 
> > > The issue is quite clear, I can fix it directly.
> > > 
> > > And as the fix is also quite clear, if you want to contribute
> > > patch, you can also try to send a fix patch [1].
> > > 
> > > Either way is fine for me. Just let me know. :-)
> > > 
> > > [1] https://core.dpdk.org/contribute/#send
> > Hello,
> > 
> > Thanks for your guidance.
> > 
> > Well, I think it is a good chance to be "a contributer of DPDK".
> > (even for small one. I am getting excited already)
> > So let me give a try.
> 
> Great! Looking forward to your patch. :-)
Hello,

I posted my very first patch to DPDK.
After that very basic questions come up
to my mind.
When will those will be released(if that is merged safely)?
That is my natural question as application developer.
(Next release 19.02 of course?)

I need to craeate patch also to SPP(my app)
and I have to write down something about 
fix release timing of this bug in my commit message
to notify the SPP users.
(e.g. Until the next release of DPDK, you may encounter
"EAL:Error Invalid memory" when deleteing vhost pmd.)

Thanks in advance for your kind advice to newbie.

BR,
Hideyuki Yamashita
NTT TechnoCross
 
> > 
> > Thanks for your kindness.
> > 
> > BR,
> > Hideyuki Yamashita
> > NTT TechnoCross
> > 
> > > 
> > > > 
> > > > Thanks for your confirming.
> > > > 
> > > > BR,
> > > > Hideyuki Yamashita
> > > > NTT TechnoCross
> > > >  
> > > > > > 
> > > > > > > 
> > > > > > > Thanks,
> > > > > > > Hideyuki Yamashita
> > > > > > > NTT TechnoCross
> > > > > > > 
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > > -- 
> > > > > > Thanks,
> > > > > > Anatoly
> > > > 
> > > > 
> > 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-18 11:30                       ` Hideyuki Yamashita
@ 2018-12-18 13:09                         ` Tiwei Bie
  2018-12-27  5:52                           ` Hideyuki Yamashita
  0 siblings, 1 reply; 21+ messages in thread
From: Tiwei Bie @ 2018-12-18 13:09 UTC (permalink / raw)
  To: Hideyuki Yamashita
  Cc: Burakov, Anatoly, dev, Yasufumi Ogawa, Nakamura Hioryuki,
	maxime.coquelin, zhihong.wang

On Tue, Dec 18, 2018 at 08:30:16PM +0900, Hideyuki Yamashita wrote:
> > On Tue, Dec 18, 2018 at 02:52:16PM +0900, Hideyuki Yamashita wrote:
> > > > On Tue, Dec 18, 2018 at 12:11:33PM +0900, Hideyuki Yamashita wrote:
> > > > > 
> > > > > On Mon, Dec 17, 2018 at 11:21:01AM +0000, Burakov, Anatoly wrote:
> > > > > > > On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> > > > > > > > > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > > > > > > > > > Dear Thomas and all,
> > > > > > > > > > 
> > > > > > > > > > I took a look on dpdk code.
> > > > > > > > > > I firstly write qustions and my analisys
> > > > > > > > > > on the current dpdk code follows after that.
> > > > > > > > > > 
> > > > > > > > > > [1.Questions]
> > > > > > > > > > I have several questions to ask again.
> > > > > > > > > > Is my understanding correct about followings
> > > > > > > > > > 
> > > > > > > > > > Q1: "EAL:ERROR, Invalid memory" is ignorable
> > > > > > > > > > 
> > > > > > > > > > Q2: there is no big difference between calling
> > > > > > > > > > rte_eal_hotplug_remove(bus->name, dev->name)
> > > > > > > > > > and
> > > > > > > > > > rte_dev_remove(dev) because anyway it
> > > > > > > > > > reaches to rte_pmd_vhost_remove and encounter
> > > > > > > > > > the same error.
> > > > > > > > > > 
> > > > > > > > > > [2.snip from my code]
> > > > > > > > > > .....
> > > > > > > > > >           rte_eth_dev_close(port_id);
> > > > > > > > > >           ret = rte_dev_remove(dev);
> > > > > > > > > >           if (ret < 0)
> > > > > > > > > >                   return ret;
> > > > > > > > > >           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> > > > > > > > > > 
> > > > > > > > > > [3. My analysis on dpdk code]
> > > > > > > > > > static int
> > > > > > > > > >     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> > > > > > > > > >     {
> > > > > > > > > >      ...........
> > > > > > > > > >            eth_dev_close(eth_dev);
> > > > > > > > > > 
> > > > > > > > > >             rte_free(vring_states[eth_dev->data->port_id]);
> > > > > > > > > >             vring_states[eth_dev->data->port_id] = NULL;
> > > > > > > > > > 
> > > > > > > > > >             rte_eth_dev_release_port(eth_dev);
> > > > > > > > > > 
> > > > > > > > > > As you can see in rte_eth_vhost.c
> > > > > > > > > > It calls both eth_dev_close and rte_eth_dev_release_port.
> > > > > > > > > > And inside both functions, it tries to free mac_addrs.
> > > > > > > > > >           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> > > > > > > > > >           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> > > > > > > > > > 
> > > > > > > > > > I understand that is the reason why
> > > > > > > > > > /* Free the memory space back to heap */
> > > > > > > > > > void rte_free(void *addr)
> > > > > > > > > > {
> > > > > > > > > >           if (addr == NULL) return;
> > > > > > > > > >           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > > > > > > > > >                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > > > > > > > > > }
> > > > > > > > > > encounter the error.
> > > > > > > > > > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > > > > > > > > > disappered.
> > > > > > > > > > 
> > > > > > > > > > Thanks and BR,
> > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > NTT TechnoCross
> > > > > > > > > > 
> > > > > > > > > > > Adding my colleague Yasufumi and Hiroyuki as CC.
> > > > > > > > > > > 
> > > > > > > > > > > We are waiting valuable advice from you.
> > > > > > > > > > > 
> > > > > > > > > > > Thanks in advance,
> > > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > > NTT TechnoCross
> > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > Dear Thomas and all,
> > > > > > > > > > > > 
> > > > > > > > > > > > I hope you all get safely back home after DPDK summit.
> > > > > > > > > > > > (When I get back Japan, it is chilling. (start of winter))
> > > > > > > > > > > > 
> > > > > > > > > > > > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > > > > > > > > > > > However, following syslog message is printed.
> > > > > > > > > > > > “EAL: Error: Invalid memory”
> > > > > > > > > > > > 
> > > > > > > > > > > > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > > > > > > > > > > > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > > > > > > > > > > > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > > > > > > > > > > > 
> > > > > > > > > > > > We tested above changes, but the result is the same (i.e., same error message is printed).
> > > > > > > > > > > > The debug log message says:
> > > > > > > > > > > > ---
> > > > > > > > > > > > [primary]
> > > > > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > > > > EAL: Error: Invalid memory
> > > > > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > > > > 
> > > > > > > > > > > > [secondary]
> > > > > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > > > > APP: To Server: add
> > > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > APP: To Server: del
> > > > > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > > > > APP: To Server: add
> > > > > > > > > > > > ---
> > > > > > > > > > > > 
> > > > > > > > > > > > We would like to ask:
> > > > > > > > > > > > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > > > > > > > > > > > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > > > > > > > > > > > 
> > > > > > > > > > > > Thanks in advance and have a nice day.
> > > > > > > > > > > > 
> > > > > > > > > > > > BR,
> > > > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > > > NTT TechnoCross
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > Hi Hideyuki,
> > > > > > > > > 
> > > > > > > > > The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> > > > > > > > > 
> > > > > > > > > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> > > > > > > > > 
> > > > > > > > > I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> > > > > > > > > 
> > > > > > > > > --Thanks,
> > > > > > > > > Anatoly
> > > > > > > > Hello Anatoly,
> > > > > > > > 
> > > > > > > > Thanks for your reply for my newbie question.
> > > > > > > > Now I understand that this error is harmless from DPDK application(SPP)
> > > > > > > > point of view in practice. Thanks.
> > > > > > > > But anyway if there is a double free logic, it is a bug and should be
> > > > > > > > fixed.
> > > > > > > > The remaining issues are
> > > > > > > > 1. If it is really a bug (or my mis-understanding)
> > > > > > > > 2. If is is a bug which function should remove rte_free(mac_addrs)
> > > > > > > 
> > > > > > > From description, it looks like a bug. Correct usage of API (rte_dev_close()
> > > > > > > followed by rte_dev_remove()) should not trigger any errors. You might want
> > > > > > > to create a BugZilla entry describing the issue.
> > > > > > 
> > > > > > I also think it's a bug. The rte_free(dev->data->mac_addrs) in
> > > > > > vhost PMD's eth_dev_close() should be removed, as the common data
> > > > > > freeing has been moved to rte_eth_dev_release_port().
> > > > > > 
> > > > > > @Hideyuki, thanks for digging into this!
> > > > > 
> > > > > Hello Twei,
> > > > > 
> > > > > I share the same view with you.
> > > > > rte_eth_dev_release_port() is used in commonly in many pmds.
> > > > > rte_dev_close() is vhost pmd local.
> > > > > 
> > > > > BTW, what is the difference between using
> > > > > rte_eal_hotplug_remove and rte_eth_dev_close()+rte_dev_remove?
> > > > > (I see no big difference right now from application developer point of
> > > > > view)
> > > > 
> > > > IMO, the difference is that the former one doesn't require
> > > > operating on rte_device directly.
> > > > 
> > > > > 
> > > > > Next question is what should I do next (you know I am very newbie).
> > > > > File this bug into BUgzzila of DPDK?
> > > > 
> > > > The issue is quite clear, I can fix it directly.
> > > > 
> > > > And as the fix is also quite clear, if you want to contribute
> > > > patch, you can also try to send a fix patch [1].
> > > > 
> > > > Either way is fine for me. Just let me know. :-)
> > > > 
> > > > [1] https://core.dpdk.org/contribute/#send
> > > Hello,
> > > 
> > > Thanks for your guidance.
> > > 
> > > Well, I think it is a good chance to be "a contributer of DPDK".
> > > (even for small one. I am getting excited already)
> > > So let me give a try.
> > 
> > Great! Looking forward to your patch. :-)
> Hello,
> 
> I posted my very first patch to DPDK.
> After that very basic questions come up
> to my mind.
> When will those will be released(if that is merged safely)?

For the release time, you can check the schedule of the
next release from below link:

https://core.dpdk.org/roadmap/#dates

> That is my natural question as application developer.
> (Next release 19.02 of course?)

Yes.

> 
> I need to craeate patch also to SPP(my app)
> and I have to write down something about 
> fix release timing of this bug in my commit message
> to notify the SPP users.
> (e.g. Until the next release of DPDK, you may encounter
> "EAL:Error Invalid memory" when deleteing vhost pmd.)
> 
> Thanks in advance for your kind advice to newbie.
> 
> BR,
> Hideyuki Yamashita
> NTT TechnoCross
>  
> > > 
> > > Thanks for your kindness.
> > > 
> > > BR,
> > > Hideyuki Yamashita
> > > NTT TechnoCross
> > > 
> > > > 
> > > > > 
> > > > > Thanks for your confirming.
> > > > > 
> > > > > BR,
> > > > > Hideyuki Yamashita
> > > > > NTT TechnoCross
> > > > >  
> > > > > > > 
> > > > > > > > 
> > > > > > > > Thanks,
> > > > > > > > Hideyuki Yamashita
> > > > > > > > NTT TechnoCross
> > > > > > > > 
> > > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > -- 
> > > > > > > Thanks,
> > > > > > > Anatoly
> > > > > 
> > > > > 
> > > 
> 
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-18 13:09                         ` Tiwei Bie
@ 2018-12-27  5:52                           ` Hideyuki Yamashita
  2019-01-02  9:19                             ` Tiwei Bie
  0 siblings, 1 reply; 21+ messages in thread
From: Hideyuki Yamashita @ 2018-12-27  5:52 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: Burakov, Anatoly, dev, Yasufumi Ogawa, Nakamura Hioryuki,
	maxime.coquelin, zhihong.wang

> On Tue, Dec 18, 2018 at 08:30:16PM +0900, Hideyuki Yamashita wrote:
> > > On Tue, Dec 18, 2018 at 02:52:16PM +0900, Hideyuki Yamashita wrote:
> > > > > On Tue, Dec 18, 2018 at 12:11:33PM +0900, Hideyuki Yamashita wrote:
> > > > > > 
> > > > > > On Mon, Dec 17, 2018 at 11:21:01AM +0000, Burakov, Anatoly wrote:
> > > > > > > > On 17-Dec-18 10:45 AM, Hideyuki Yamashita wrote:
> > > > > > > > > > On 17-Dec-18 10:02 AM, Hideyuki Yamashita wrote:
> > > > > > > > > > > Dear Thomas and all,
> > > > > > > > > > > 
> > > > > > > > > > > I took a look on dpdk code.
> > > > > > > > > > > I firstly write qustions and my analisys
> > > > > > > > > > > on the current dpdk code follows after that.
> > > > > > > > > > > 
> > > > > > > > > > > [1.Questions]
> > > > > > > > > > > I have several questions to ask again.
> > > > > > > > > > > Is my understanding correct about followings
> > > > > > > > > > > 
> > > > > > > > > > > Q1: "EAL:ERROR, Invalid memory" is ignorable
> > > > > > > > > > > 
> > > > > > > > > > > Q2: there is no big difference between calling
> > > > > > > > > > > rte_eal_hotplug_remove(bus->name, dev->name)
> > > > > > > > > > > and
> > > > > > > > > > > rte_dev_remove(dev) because anyway it
> > > > > > > > > > > reaches to rte_pmd_vhost_remove and encounter
> > > > > > > > > > > the same error.
> > > > > > > > > > > 
> > > > > > > > > > > [2.snip from my code]
> > > > > > > > > > > .....
> > > > > > > > > > >           rte_eth_dev_close(port_id);
> > > > > > > > > > >           ret = rte_dev_remove(dev);
> > > > > > > > > > >           if (ret < 0)
> > > > > > > > > > >                   return ret;
> > > > > > > > > > >           rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> > > > > > > > > > > 
> > > > > > > > > > > [3. My analysis on dpdk code]
> > > > > > > > > > > static int
> > > > > > > > > > >     rte_pmd_vhost_remove(struct rte_vdev_device *dev)
> > > > > > > > > > >     {
> > > > > > > > > > >      ...........
> > > > > > > > > > >            eth_dev_close(eth_dev);
> > > > > > > > > > > 
> > > > > > > > > > >             rte_free(vring_states[eth_dev->data->port_id]);
> > > > > > > > > > >             vring_states[eth_dev->data->port_id] = NULL;
> > > > > > > > > > > 
> > > > > > > > > > >             rte_eth_dev_release_port(eth_dev);
> > > > > > > > > > > 
> > > > > > > > > > > As you can see in rte_eth_vhost.c
> > > > > > > > > > > It calls both eth_dev_close and rte_eth_dev_release_port.
> > > > > > > > > > > And inside both functions, it tries to free mac_addrs.
> > > > > > > > > > >           rte_free(dev->data->mac_addrs);       //in rth_dev_close
> > > > > > > > > > >           rte_free(eth_dev->data->mac_addrs);  //in rte_eth_dev_release_port
> > > > > > > > > > > 
> > > > > > > > > > > I understand that is the reason why
> > > > > > > > > > > /* Free the memory space back to heap */
> > > > > > > > > > > void rte_free(void *addr)
> > > > > > > > > > > {
> > > > > > > > > > >           if (addr == NULL) return;
> > > > > > > > > > >           if (malloc_heap_free(malloc_elem_from_data(addr)) < 0)
> > > > > > > > > > >                   RTE_LOG(ERR, EAL, "Error: Invalid memory\n");
> > > > > > > > > > > }
> > > > > > > > > > > encounter the error.
> > > > > > > > > > > As an experiment, I commented out one of them, "ERR, Invalid memory"
> > > > > > > > > > > disappered.
> > > > > > > > > > > 
> > > > > > > > > > > Thanks and BR,
> > > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > > NTT TechnoCross
> > > > > > > > > > > 
> > > > > > > > > > > > Adding my colleague Yasufumi and Hiroyuki as CC.
> > > > > > > > > > > > 
> > > > > > > > > > > > We are waiting valuable advice from you.
> > > > > > > > > > > > 
> > > > > > > > > > > > Thanks in advance,
> > > > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > > > NTT TechnoCross
> > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Dear Thomas and all,
> > > > > > > > > > > > > 
> > > > > > > > > > > > > I hope you all get safely back home after DPDK summit.
> > > > > > > > > > > > > (When I get back Japan, it is chilling. (start of winter))
> > > > > > > > > > > > > 
> > > > > > > > > > > > > On DPDK 18.11.0, we tried to remove vhost device by using rte_eal_hotplug_remove().
> > > > > > > > > > > > > However, following syslog message is printed.
> > > > > > > > > > > > > “EAL: Error: Invalid memory”
> > > > > > > > > > > > > 
> > > > > > > > > > > > > At DPDK summit San Jose, we had chance to ask Thomas how to handle the error message, and he gave us following advice:
> > > > > > > > > > > > > Replace “rte_eal_hotplug_add()” to “rte_dev_probe(devargs)” and
> > > > > > > > > > > > > “rte_eal_hotplug_remove()” to “rte_eth_dev_close() and rte_dev_remove(rte_dev)”
> > > > > > > > > > > > > 
> > > > > > > > > > > > > We tested above changes, but the result is the same (i.e., same error message is printed).
> > > > > > > > > > > > > The debug log message says:
> > > > > > > > > > > > > ---
> > > > > > > > > > > > > [primary]
> > > > > > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > > > > > EAL: Error: Invalid memory
> > > > > > > > > > > > > VHOST_CONFIG: vhost-user server: socket created, fd: 17
> > > > > > > > > > > > > VHOST_CONFIG: bind to /tmp/sock0
> > > > > > > > > > > > > 
> > > > > > > > > > > > > [secondary]
> > > > > > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > > > > > APP: To Server: add
> > > > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > > APP: To Server: del
> > > > > > > > > > > > > APP: devargs=eth_vhost0,iface=/tmp/sock0,queues=1
> > > > > > > > > > > > > EAL: request: eal_dev_mp_request
> > > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > > EAL: request: bus_vdev_mp
> > > > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > > > EAL: msg: bus_vdev_mp
> > > > > > > > > > > > > EAL: reply: eal_dev_mp_request
> > > > > > > > > > > > > EAL: msg: eal_dev_mp_request
> > > > > > > > > > > > > rte_eth_promiscuous_disable: Function not supported
> > > > > > > > > > > > > rte_eth_allmulticast_disable: Function not supported
> > > > > > > > > > > > > APP: To Server: add
> > > > > > > > > > > > > ---
> > > > > > > > > > > > > 
> > > > > > > > > > > > > We would like to ask:
> > > > > > > > > > > > > 1)	Is the message “EAL: Error: Invalid memory” ignorable or not? There is no obstacle except this message to re-add the vhost device.
> > > > > > > > > > > > > 2)	Which is the better(best?) way to add/del vhost device “rte_eal_hotplug_add/remove()” or the way Thomas suggested?
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Thanks in advance and have a nice day.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > BR,
> > > > > > > > > > > > > Hideyuki Yamashita
> > > > > > > > > > > > > NTT TechnoCross
> > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > Hi Hideyuki,
> > > > > > > > > > 
> > > > > > > > > > The error you're referring to (about invalid memory) means that you're trying to free a pointer that points to invalid memory. Meaning, either the pointer itself is not pointing to an allocated area, or it points to memory that has already been freed.
> > > > > > > > > > 
> > > > > > > > > > If dev->data->mac_addrs and eth_dev->data->mac_addrs point to the same area, this is a bug, because this would lead to double free, and rte_malloc will rightly complain about invalid memory. Now, malloc won't try to do anything with the invalid memory, so the error itself is harmless *as far as malloc is concerned*, but these attempts to free the memory twice should be fixed whereever they happen.
> > > > > > > > > > 
> > > > > > > > > > I'm not well-versed in dev infrastructure, so i wouldn't be able to say which one of the rte_free calls is an extra, unneeded one. This is something e.g. Thomas could help with, or the driver maintainer.
> > > > > > > > > > 
> > > > > > > > > > --Thanks,
> > > > > > > > > > Anatoly
> > > > > > > > > Hello Anatoly,
> > > > > > > > > 
> > > > > > > > > Thanks for your reply for my newbie question.
> > > > > > > > > Now I understand that this error is harmless from DPDK application(SPP)
> > > > > > > > > point of view in practice. Thanks.
> > > > > > > > > But anyway if there is a double free logic, it is a bug and should be
> > > > > > > > > fixed.
> > > > > > > > > The remaining issues are
> > > > > > > > > 1. If it is really a bug (or my mis-understanding)
> > > > > > > > > 2. If is is a bug which function should remove rte_free(mac_addrs)
> > > > > > > > 
> > > > > > > > From description, it looks like a bug. Correct usage of API (rte_dev_close()
> > > > > > > > followed by rte_dev_remove()) should not trigger any errors. You might want
> > > > > > > > to create a BugZilla entry describing the issue.
> > > > > > > 
> > > > > > > I also think it's a bug. The rte_free(dev->data->mac_addrs) in
> > > > > > > vhost PMD's eth_dev_close() should be removed, as the common data
> > > > > > > freeing has been moved to rte_eth_dev_release_port().
> > > > > > > 
> > > > > > > @Hideyuki, thanks for digging into this!
> > > > > > 
> > > > > > Hello Twei,
> > > > > > 
> > > > > > I share the same view with you.
> > > > > > rte_eth_dev_release_port() is used in commonly in many pmds.
> > > > > > rte_dev_close() is vhost pmd local.
> > > > > > 
> > > > > > BTW, what is the difference between using
> > > > > > rte_eal_hotplug_remove and rte_eth_dev_close()+rte_dev_remove?
> > > > > > (I see no big difference right now from application developer point of
> > > > > > view)
> > > > > 
> > > > > IMO, the difference is that the former one doesn't require
> > > > > operating on rte_device directly.
> > > > > 
> > > > > > 
> > > > > > Next question is what should I do next (you know I am very newbie).
> > > > > > File this bug into BUgzzila of DPDK?
> > > > > 
> > > > > The issue is quite clear, I can fix it directly.
> > > > > 
> > > > > And as the fix is also quite clear, if you want to contribute
> > > > > patch, you can also try to send a fix patch [1].
> > > > > 
> > > > > Either way is fine for me. Just let me know. :-)
> > > > > 
> > > > > [1] https://core.dpdk.org/contribute/#send
> > > > Hello,
> > > > 
> > > > Thanks for your guidance.
> > > > 
> > > > Well, I think it is a good chance to be "a contributer of DPDK".
> > > > (even for small one. I am getting excited already)
> > > > So let me give a try.
> > > 
> > > Great! Looking forward to your patch. :-)
> > Hello,
> > 
> > I posted my very first patch to DPDK.
> > After that very basic questions come up
> > to my mind.
> > When will those will be released(if that is merged safely)?
> 
> For the release time, you can check the schedule of the
> next release from below link:
> 
> https://core.dpdk.org/roadmap/#dates
> 
> > That is my natural question as application developer.
> > (Next release 19.02 of course?)
> 
> Yes.
Dear Tiwei and all,

I have question.

Q1.
I CCed my patch to stable@dpdk.org
Is that mean it will be reflected into 18.11 stable release?
The reason why I ask this is I think it is better that bug should 
be reflected into LTS because I and SPP users mainly use DPDK LTS(18.11).

Q2.
If yes, when will those stable release will be released?
(e.g. once a month )

Q3.
I understand my patch is acked and merged into 
dpdk-next-virtio.
What is dpdk-next-virtio?
It is the branch which will be reflected into next release?

Thanks in advance!

BR,
Hideyuki Yamashita
NTT TechnoCross
 
> > 
> > I need to craeate patch also to SPP(my app)
> > and I have to write down something about 
> > fix release timing of this bug in my commit message
> > to notify the SPP users.
> > (e.g. Until the next release of DPDK, you may encounter
> > "EAL:Error Invalid memory" when deleteing vhost pmd.)
> > 
> > Thanks in advance for your kind advice to newbie.
> > 
> > BR,
> > Hideyuki Yamashita
> > NTT TechnoCross
> >  
> > > > 
> > > > Thanks for your kindness.
> > > > 
> > > > BR,
> > > > Hideyuki Yamashita
> > > > NTT TechnoCross
> > > > 
> > > > > 
> > > > > > 
> > > > > > Thanks for your confirming.
> > > > > > 
> > > > > > BR,
> > > > > > Hideyuki Yamashita
> > > > > > NTT TechnoCross
> > > > > >  
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Thanks,
> > > > > > > > > Hideyuki Yamashita
> > > > > > > > > NTT TechnoCross
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > -- 
> > > > > > > > Thanks,
> > > > > > > > Anatoly
> > > > > > 
> > > > > > 
> > > > 
> > 
> > 
> > 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [dpdk-dev] rte_eal_hotplug_remove() generates error message
  2018-12-27  5:52                           ` Hideyuki Yamashita
@ 2019-01-02  9:19                             ` Tiwei Bie
  0 siblings, 0 replies; 21+ messages in thread
From: Tiwei Bie @ 2019-01-02  9:19 UTC (permalink / raw)
  To: Hideyuki Yamashita
  Cc: Burakov, Anatoly, dev, Yasufumi Ogawa, Nakamura Hioryuki,
	maxime.coquelin, zhihong.wang

On Thu, Dec 27, 2018 at 02:52:17PM +0900, Hideyuki Yamashita wrote:
> Q1.
> I CCed my patch to stable@dpdk.org
> Is that mean it will be reflected into 18.11 stable release?

Yes.

> The reason why I ask this is I think it is better that bug should 
> be reflected into LTS because I and SPP users mainly use DPDK LTS(18.11).
> 
> Q2.
> If yes, when will those stable release will be released?
> (e.g. once a month )

Plans can be found here:
https://core.dpdk.org/roadmap/#stable

> 
> Q3.
> I understand my patch is acked and merged into 
> dpdk-next-virtio.
> What is dpdk-next-virtio?
> It is the branch which will be reflected into next release?

It's already in main repo now:
http://git.dpdk.org/dpdk/commit/drivers/net/vhost?id=6e3339ca07734e59cd0c24594e3014ab49a0ffc0

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2019-01-02  9:21 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-09  4:30 [dpdk-dev] dpdk vlan strip offload bug for I350 nic? 1534898891
2018-12-09  5:23 `  =?gb18030?B?MTUzNDg5ODg5MQ==?=
2018-12-18  2:26   ` Zhao1, Wei
2018-12-10  7:53 ` [dpdk-dev] rte_eal_hotplug_remove() generates error message Hideyuki Yamashita
2018-12-11  0:54   ` Hideyuki Yamashita
2018-12-17 10:02     ` Hideyuki Yamashita
2018-12-17 10:23       ` Burakov, Anatoly
2018-12-17 10:38         ` Burakov, Anatoly
2018-12-17 11:03           ` Hideyuki Yamashita
2018-12-17 10:45         ` Hideyuki Yamashita
2018-12-17 11:21           ` Burakov, Anatoly
2018-12-17 12:10             ` Hideyuki Yamashita
2018-12-18  2:39             ` Tiwei Bie
2018-12-18  3:11               ` Hideyuki Yamashita
2018-12-18  5:12                 ` Tiwei Bie
2018-12-18  5:52                   ` Hideyuki Yamashita
2018-12-18  6:25                     ` Tiwei Bie
2018-12-18 11:30                       ` Hideyuki Yamashita
2018-12-18 13:09                         ` Tiwei Bie
2018-12-27  5:52                           ` Hideyuki Yamashita
2019-01-02  9:19                             ` Tiwei Bie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).