[dpdk-dev] vhost-switch example: huge memory need and CRC off-loading issue

2015-08-08 Thread Jan Kiszka
On 2015-08-08 02:39, Ouyang, Changchun wrote:
> 
> 
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Kiszka
>> Sent: Saturday, August 8, 2015 1:31 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] vhost-switch example: huge memory need and CRC off-
>> loading issue
>>
>> Hi again,
>>
>> two findings in the vhost-switch example code that can cause grey hair for
>> starters:
>>
>> - MAX_QUEUES of 512 causes pretty high memory need for the application
>>   (something between 1 and 2G) - is that really needed? I'm now running
>>   with 32, and I'm able to get away with 256M. Can we tune this
>>   default?
> 
> Don't think we need change default just because your platform is 32,
> Well, my platform is 128, other platform may have other value, :-)

Then let's make it configurable or explore the actual device needs
before allocating the buffer. The impact on memory consumption is way
too big to hard-code this, specifically as this is per physical port IIUC.

> 
>>
>> - hw_strip_crc is set to 0, but either the igb driver or the ET2 quad
>>   port adapter I'm using is ignoring this. It does strip the CRC, so
> 
> Igb and ET2 should NOT ignore it.

Well, they do not listen to us. What can I do to debug this?

According to eth_igb_rx_init, hw_strip_crc affects E1000_RCTL_SECRC in
the receiver control registers, and debugging confirms this (the bit is
unset in all E1000_RCTL accesses). But there is still no CRC at the end
of received packets. Possibly, there is some other knob that controls
CRC stripping?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux


[dpdk-dev] [PATCH] vchost: Notify application of ownership change

2015-08-08 Thread Jan Kiszka
On 2015-08-08 02:25, Ouyang, Changchun wrote:
> 
> 
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Kiszka
>> Sent: Saturday, August 8, 2015 1:21 AM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH] vchost: Notify application of ownership change
> 
> Vchost should be vhost in the title

Oops. Unless I need to resend for some other reason, I guess the commit
can fix this up.

> 
>>
>> On VHOST_*_RESET_OWNER, we reinitialize the device but without telling
>> the application. That will cause crashes when it continues to invoke vhost
>> services on the device. Fix it by calling the destruction hook if the device 
>> is
>> still in use.
> What's your qemu version?

git head, see my other reply for details.

> Any validation work on this patch?

What do you mean with this? Test cases? Or steps to reproduce? For the
latter, just fire up a recent qemu, let the guest enable the virtio
device, then reboot or simply terminate qemu.

>>
>> Signed-off-by: Jan Kiszka 
>> ---
>>
>> This is the surprisingly simple answer to my questions in
>> http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
>>
>>  lib/librte_vhost/virtio-net.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c 
>> index
>> b520ec5..3c5b5b2 100644
>> --- a/lib/librte_vhost/virtio-net.c
>> +++ b/lib/librte_vhost/virtio-net.c
>> @@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
>>
>>  ll_dev = get_config_ll_entry(ctx);
>>
>> +if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
>> +notify_ops->destroy_device(_dev->dev);
>> +
> 
> I am not sure whether destroy_device here will affect the second time 
> init_device(below) and new_device(after the reset) or not.
> Need validation.

Cannot follow, what do you mean with "second time"? If the callback
could invoke something that causes cleanup_device to be called as well?
That's at least not the case with vhost-switch, but I'm far from being
familiar with the API to asses if that is possible in general.

Jan

> 
>>  cleanup_device(_dev->dev);
>>  init_device(_dev->dev);
>>
>> --
>> 2.1.4


-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux


[dpdk-dev] vhost-switch example: huge memory need and CRC off-loading issue

2015-08-08 Thread Ouyang, Changchun


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Kiszka
> Sent: Saturday, August 8, 2015 1:31 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] vhost-switch example: huge memory need and CRC off-
> loading issue
> 
> Hi again,
> 
> two findings in the vhost-switch example code that can cause grey hair for
> starters:
> 
> - MAX_QUEUES of 512 causes pretty high memory need for the application
>   (something between 1 and 2G) - is that really needed? I'm now running
>   with 32, and I'm able to get away with 256M. Can we tune this
>   default?

Don't think we need change default just because your platform is 32,
Well, my platform is 128, other platform may have other value, :-)

> 
> - hw_strip_crc is set to 0, but either the igb driver or the ET2 quad
>   port adapter I'm using is ignoring this. It does strip the CRC, so

Igb and ET2 should NOT ignore it.

>   does software, and I'm losing 4 bytes on each unpadded packet. Known
>   issue?
> 
> Thanks,
> Jan
> 
> --
> Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate
> Competence Center Embedded Linux


[dpdk-dev] [PATCH] vchost: Notify application of ownership change

2015-08-08 Thread Ouyang, Changchun


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Kiszka
> Sent: Saturday, August 8, 2015 1:21 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] vchost: Notify application of ownership change

Vchost should be vhost in the title

> 
> On VHOST_*_RESET_OWNER, we reinitialize the device but without telling
> the application. That will cause crashes when it continues to invoke vhost
> services on the device. Fix it by calling the destruction hook if the device 
> is
> still in use.
What's your qemu version?
Any validation work on this patch?
> 
> Signed-off-by: Jan Kiszka 
> ---
> 
> This is the surprisingly simple answer to my questions in
> http://thread.gmane.org/gmane.comp.networking.dpdk.devel/22661.
> 
>  lib/librte_vhost/virtio-net.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c 
> index
> b520ec5..3c5b5b2 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
> 
>   ll_dev = get_config_ll_entry(ctx);
> 
> + if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
> + notify_ops->destroy_device(_dev->dev);
> +

I am not sure whether destroy_device here will affect the second time 
init_device(below) and new_device(after the reset) or not.
Need validation.

>   cleanup_device(_dev->dev);
>   init_device(_dev->dev);
> 
> --
> 2.1.4


[dpdk-dev] [PATCH] testpmd: modify the mac of csum forwarding

2015-08-08 Thread Ouyang, Changchun


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Zhang, Helin
> Sent: Saturday, August 8, 2015 12:07 AM
> To: Qiu, Michael; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] testpmd: modify the mac of csum
> forwarding
> 
> 
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Michael Qiu
> > Sent: Thursday, August 6, 2015 8:29 PM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] [PATCH] testpmd: modify the mac of csum forwarding
> >
> > For some ethnet-switch like intel RRC, all the packet forwarded out by
> > DPDK will be dropped in switch side, so the packet generator will never
> receive the packet.
> Is it because of anti-sproof? E.g. When the hardware found that the dest mac
> is the port itself, then it will be dropped during TX.
> You need to tell the root cause, and why we need to modify like this.
> 
> >
> > Signed-off-by: Michael Qiu 
> > ---
> >  app/test-pmd/csumonly.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > 1bf3485..bf8af1d 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -550,6 +550,10 @@ pkt_burst_checksum_forward(struct fwd_stream
> *fs)
> >  * and inner headers */
> >
> > eth_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
> > +   ether_addr_copy(_eth_addrs[fs->peer_addr],
> > +   _hdr->d_addr);
> > +   ether_addr_copy([fs->tx_port].eth_addr,
> > +   _hdr->s_addr);
> Is it really necessary? Why other NICs do not need this?
> 

Seems the behavior changes from io fwd into mac fwd?

> > parse_ethernet(eth_hdr, );
> > l3_hdr = (char *)eth_hdr + info.l2_len;
> >
> > --
> > 1.9.3



[dpdk-dev] vhost: Problem RESET_OWNER processing

2015-08-08 Thread Ouyang, Changchun


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Kiszka
> Sent: Friday, August 7, 2015 10:04 PM
> To: dev at dpdk.org; Xie, Huawei
> Subject: [dpdk-dev] vhost: Problem RESET_OWNER processing
> 
> Hi,
> 
> I was wondering if I'm alone with this: the vhost-switch example crashes on
> client disconnects if the client send a RESET_OWNER message. That's at least
> the case for QEMU and vhost-user mode (I suppose vhost-cuse is legacy

What's your qemu version?

> now). And it really ruins the party when playing with this because every VM
> shutdown or guest reboot triggers.
> 
> I was looking deeper in the librte_vhost, and I found that reset_owner() is
> doing cleanup_device and then init_device - but without letting the user
> know. So vhost-switch crashed in its main loop over continuing to use the
> device, namely calling rte_vhost_dequeue_burst (with
> dev->virtqueue[]->avail == NULL).
> 
> Do we simply need another hook in the vhost API, similar to the destruction
> notification?
> 
> Jan
> 
> --
> Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate
> Competence Center Embedded Linux