diff mbox

[dpdk-dev,v2] add mtu set in virtio

Message ID 20160829230240.20164-1-sodey@sonusnet.com (mailing list archive)
State Superseded, archived
Delegated to: Yuanhan Liu
Headers

Commit Message

souvikdey33 Aug. 29, 2016, 11:02 p.m. UTC 
  Signed-off-by: Souvik Dey <sodey@sonusnet.com>

Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>

Virtio interfaces should also support setting of mtu, as in case of cloud
it is expected to have the consistent mtu across the infrastructure that
the dhcp server sends and not hardcoded to 1500(default).
---
 drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Maxime Coquelin Aug. 30, 2016, 7:57 a.m. UTC | #1 
Hi Souvik,

On 08/30/2016 01:02 AM, souvikdey33 wrote:
> Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>
> Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>
> Virtio interfaces should also support setting of mtu, as in case of cloud
> it is expected to have the consistent mtu across the infrastructure that
> the dhcp server sends and not hardcoded to 1500(default).
> ---
>  drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

FYI, there are some on-going changes in the VIRTIO specification
so that the VHOST interface exposes its MTU to its VIRTIO peer.
It may also be used as an alternative of what you patch achieves.

I am working on its implementation in Qemu/DPDK, our goal being to
reduce performance drops for small packets with Rx mergeable buffers
feature enabled.

Regards,
Maxime
souvikdey33 Sept. 1, 2016, 10:20 p.m. UTC | #2 
Hi Maxime,
	When is patches or new implementation going to come in the release ? if it is not 16.11 then, can we keep this change till the new virtio changes come in the release. And if it is already planned for 16.11, then can I get a little more information on that.

--
Regards,
Souvik

-----Original Message-----
From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] 
Sent: Tuesday, August 30, 2016 3:58 AM
To: Dey, Souvik <sodey@sonusnet.com>; stephen@networkplumber.org; huawei.xie@intel.com; yuanhan.liu@linux.intel.com
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] add mtu set in virtio

Hi Souvik,

On 08/30/2016 01:02 AM, souvikdey33 wrote:
> Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>
> Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>
> Virtio interfaces should also support setting of mtu, as in case of 
> cloud it is expected to have the consistent mtu across the 
> infrastructure that the dhcp server sends and not hardcoded to 1500(default).
> ---
>  drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

FYI, there are some on-going changes in the VIRTIO specification so that the VHOST interface exposes its MTU to its VIRTIO peer.
It may also be used as an alternative of what you patch achieves.

I am working on its implementation in Qemu/DPDK, our goal being to reduce performance drops for small packets with Rx mergeable buffers feature enabled.

Regards,
Maxime
Maxime Coquelin Sept. 2, 2016, 7:05 a.m. UTC | #3 
Hi Souvik,

On 09/02/2016 12:20 AM, Dey, Souvik wrote:
> Hi Maxime,
> 	When is patches or new implementation going to come in the release ? if it is not 16.11 then, can we keep this change till the new virtio changes come in the release. And if it is already planned for 16.11, then can I get a little more information on that.
>
I'm currently working on qemu part implementation, first RFC should be
sent next week.

Goal is to have it in 16.11, but I cannot commit, as the spec update
has not been acked yet.

For more information, you can start by having a look at the spec review:
https://lists.oasis-open.org/archives/virtio-dev/201608/msg00056.html

Regards,
Maxime

> --
> Regards,
> Souvik
>
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Tuesday, August 30, 2016 3:58 AM
> To: Dey, Souvik <sodey@sonusnet.com>; stephen@networkplumber.org; huawei.xie@intel.com; yuanhan.liu@linux.intel.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] add mtu set in virtio
>
> Hi Souvik,
>
> On 08/30/2016 01:02 AM, souvikdey33 wrote:
>> Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>>
>> Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
>> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>>
>> Virtio interfaces should also support setting of mtu, as in case of
>> cloud it is expected to have the consistent mtu across the
>> infrastructure that the dhcp server sends and not hardcoded to 1500(default).
>> ---
>>  drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>
> FYI, there are some on-going changes in the VIRTIO specification so that the VHOST interface exposes its MTU to its VIRTIO peer.
> It may also be used as an alternative of what you patch achieves.
>
> I am working on its implementation in Qemu/DPDK, our goal being to reduce performance drops for small packets with Rx mergeable buffers feature enabled.
>
> Regards,
> Maxime
>
souvikdey33 Sept. 7, 2016, 2:11 a.m. UTC | #4 
Hi Maxime,
	In that case let this fix be there till the time the new implementation comes in. We can re-visit the changes again in the new implementation and then decide to keep this or remove it. Hope this serves all the purposes.

--
Regards,
Souvik

-----Original Message-----
From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] 
Sent: Friday, September 2, 2016 3:06 AM
To: Dey, Souvik <sodey@sonusnet.com>; stephen@networkplumber.org
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] add mtu set in virtio

Hi Souvik,

On 09/02/2016 12:20 AM, Dey, Souvik wrote:
> Hi Maxime,
> 	When is patches or new implementation going to come in the release ? if it is not 16.11 then, can we keep this change till the new virtio changes come in the release. And if it is already planned for 16.11, then can I get a little more information on that.
>
I'm currently working on qemu part implementation, first RFC should be sent next week.

Goal is to have it in 16.11, but I cannot commit, as the spec update has not been acked yet.

For more information, you can start by having a look at the spec review:
https://lists.oasis-open.org/archives/virtio-dev/201608/msg00056.html

Regards,
Maxime

> --
> Regards,
> Souvik
>
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Tuesday, August 30, 2016 3:58 AM
> To: Dey, Souvik <sodey@sonusnet.com>; stephen@networkplumber.org; 
> huawei.xie@intel.com; yuanhan.liu@linux.intel.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] add mtu set in virtio
>
> Hi Souvik,
>
> On 08/30/2016 01:02 AM, souvikdey33 wrote:
>> Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>>
>> Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey 
>> <sodey@sonusnet.com>")
>> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>>
>> Virtio interfaces should also support setting of mtu, as in case of 
>> cloud it is expected to have the consistent mtu across the 
>> infrastructure that the dhcp server sends and not hardcoded to 1500(default).
>> ---
>>  drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>
> FYI, there are some on-going changes in the VIRTIO specification so that the VHOST interface exposes its MTU to its VIRTIO peer.
> It may also be used as an alternative of what you patch achieves.
>
> I am working on its implementation in Qemu/DPDK, our goal being to reduce performance drops for small packets with Rx mergeable buffers feature enabled.
>
> Regards,
> Maxime
>
Yuanhan Liu Sept. 7, 2016, 3:25 a.m. UTC | #5 
On Tue, Aug 30, 2016 at 09:57:39AM +0200, Maxime Coquelin wrote:
> Hi Souvik,
> 
> On 08/30/2016 01:02 AM, souvikdey33 wrote:
> >Signed-off-by: Souvik Dey <sodey@sonusnet.com>
> >
> >Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
> >Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> >
> >Virtio interfaces should also support setting of mtu, as in case of cloud
> >it is expected to have the consistent mtu across the infrastructure that
> >the dhcp server sends and not hardcoded to 1500(default).
> >---
> > drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> 
> FYI, there are some on-going changes in the VIRTIO specification
> so that the VHOST interface exposes its MTU to its VIRTIO peer.
> It may also be used as an alternative of what you patch achieves.
> 
> I am working on its implementation in Qemu/DPDK, our goal being to
> reduce performance drops for small packets with Rx mergeable buffers
> feature enabled.

Mind to educate me a bit on how that works?

	--yliu
Maxime Coquelin Sept. 7, 2016, 8:28 a.m. UTC | #6 
On 09/07/2016 04:11 AM, Dey, Souvik wrote:
> Hi Maxime,
> 	In that case let this fix be there till the time the new implementation comes in. We can re-visit the changes again in the new implementation and then decide to keep this or remove it. Hope this serves all the purposes.

Sure, I'm fine with your proposal.

Thanks,
Maxime
>
> --
> Regards,
> Souvik
>
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Friday, September 2, 2016 3:06 AM
> To: Dey, Souvik <sodey@sonusnet.com>; stephen@networkplumber.org
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] add mtu set in virtio
>
> Hi Souvik,
>
> On 09/02/2016 12:20 AM, Dey, Souvik wrote:
>> Hi Maxime,
>> 	When is patches or new implementation going to come in the release ? if it is not 16.11 then, can we keep this change till the new virtio changes come in the release. And if it is already planned for 16.11, then can I get a little more information on that.
>>
> I'm currently working on qemu part implementation, first RFC should be sent next week.
>
> Goal is to have it in 16.11, but I cannot commit, as the spec update has not been acked yet.
>
> For more information, you can start by having a look at the spec review:
> https://lists.oasis-open.org/archives/virtio-dev/201608/msg00056.html
>
> Regards,
> Maxime
>
>> --
>> Regards,
>> Souvik
>>
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Tuesday, August 30, 2016 3:58 AM
>> To: Dey, Souvik <sodey@sonusnet.com>; stephen@networkplumber.org;
>> huawei.xie@intel.com; yuanhan.liu@linux.intel.com
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v2] add mtu set in virtio
>>
>> Hi Souvik,
>>
>> On 08/30/2016 01:02 AM, souvikdey33 wrote:
>>> Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>>>
>>> Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey
>>> <sodey@sonusnet.com>")
>>> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>>>
>>> Virtio interfaces should also support setting of mtu, as in case of
>>> cloud it is expected to have the consistent mtu across the
>>> infrastructure that the dhcp server sends and not hardcoded to 1500(default).
>>> ---
>>>  drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>
>> FYI, there are some on-going changes in the VIRTIO specification so that the VHOST interface exposes its MTU to its VIRTIO peer.
>> It may also be used as an alternative of what you patch achieves.
>>
>> I am working on its implementation in Qemu/DPDK, our goal being to reduce performance drops for small packets with Rx mergeable buffers feature enabled.
>>
>> Regards,
>> Maxime
>>
Maxime Coquelin Sept. 7, 2016, 9:16 a.m. UTC | #7 
On 09/07/2016 05:25 AM, Yuanhan Liu wrote:
> On Tue, Aug 30, 2016 at 09:57:39AM +0200, Maxime Coquelin wrote:
>> Hi Souvik,
>>
>> On 08/30/2016 01:02 AM, souvikdey33 wrote:
>>> Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>>>
>>> Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
>>> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>>>
>>> Virtio interfaces should also support setting of mtu, as in case of cloud
>>> it is expected to have the consistent mtu across the infrastructure that
>>> the dhcp server sends and not hardcoded to 1500(default).
>>> ---
>>> drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>
>> FYI, there are some on-going changes in the VIRTIO specification
>> so that the VHOST interface exposes its MTU to its VIRTIO peer.
>> It may also be used as an alternative of what you patch achieves.
>>
>> I am working on its implementation in Qemu/DPDK, our goal being to
>> reduce performance drops for small packets with Rx mergeable buffers
>> feature enabled.
>
> Mind to educate me a bit on how that works?

Of course.

Basically, this is a way to advise the MTU we want in the guest.
In the guest, if GRO is not enabled:
  - In case of Kernel virtio-net, it could be used to
size the SKBs at the expected MTU. If possible, we could disable Rx
mergeable buffers.
  - In case of virtio PMD, if the MTU advised by host is lower than the
pre-allocated mbuf size for the receive queue, then we should not need
mergeable buffers.

Does that sound reasonnable?
Do I miss something?

Thanks,
Maxime
Yuanhan Liu Sept. 8, 2016, 7:30 a.m. UTC | #8 
On Wed, Sep 07, 2016 at 11:16:47AM +0200, Maxime Coquelin wrote:
> 
> 
> On 09/07/2016 05:25 AM, Yuanhan Liu wrote:
> >On Tue, Aug 30, 2016 at 09:57:39AM +0200, Maxime Coquelin wrote:
> >>Hi Souvik,
> >>
> >>On 08/30/2016 01:02 AM, souvikdey33 wrote:
> >>>Signed-off-by: Souvik Dey <sodey@sonusnet.com>
> >>>
> >>>Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
> >>>Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> >>>
> >>>Virtio interfaces should also support setting of mtu, as in case of cloud
> >>>it is expected to have the consistent mtu across the infrastructure that
> >>>the dhcp server sends and not hardcoded to 1500(default).
> >>>---
> >>>drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
> >>>1 file changed, 12 insertions(+)
> >>
> >>FYI, there are some on-going changes in the VIRTIO specification
> >>so that the VHOST interface exposes its MTU to its VIRTIO peer.
> >>It may also be used as an alternative of what you patch achieves.
> >>
> >>I am working on its implementation in Qemu/DPDK, our goal being to
> >>reduce performance drops for small packets with Rx mergeable buffers
> >>feature enabled.
> >
> >Mind to educate me a bit on how that works?
> 
> Of course.
> 
> Basically, this is a way to advise the MTU we want in the guest.
> In the guest, if GRO is not enabled:
>  - In case of Kernel virtio-net, it could be used to
> size the SKBs at the expected MTU. If possible, we could disable Rx
> mergeable buffers.
>  - In case of virtio PMD, if the MTU advised by host is lower than the
> pre-allocated mbuf size for the receive queue, then we should not need
> mergeable buffers.

Thanks for the explanation!

I see. So, the point is to avoid using mergeable buffers while it is
enabled.

> Does that sound reasonnable?

Yeah, maybe. Just don't know how well it may work in real life. Have
you got any rought data so far?

	--yliu

> Do I miss something?
> 
> Thanks,
> Maxime
Maxime Coquelin Sept. 8, 2016, 7:50 a.m. UTC | #9 
On 09/08/2016 09:30 AM, Yuanhan Liu wrote:
> On Wed, Sep 07, 2016 at 11:16:47AM +0200, Maxime Coquelin wrote:
>>
>>
>> On 09/07/2016 05:25 AM, Yuanhan Liu wrote:
>>> On Tue, Aug 30, 2016 at 09:57:39AM +0200, Maxime Coquelin wrote:
>>>> Hi Souvik,
>>>>
>>>> On 08/30/2016 01:02 AM, souvikdey33 wrote:
>>>>> Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>>>>>
>>>>> Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
>>>>> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>>>>>
>>>>> Virtio interfaces should also support setting of mtu, as in case of cloud
>>>>> it is expected to have the consistent mtu across the infrastructure that
>>>>> the dhcp server sends and not hardcoded to 1500(default).
>>>>> ---
>>>>> drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
>>>>> 1 file changed, 12 insertions(+)
>>>>
>>>> FYI, there are some on-going changes in the VIRTIO specification
>>>> so that the VHOST interface exposes its MTU to its VIRTIO peer.
>>>> It may also be used as an alternative of what you patch achieves.
>>>>
>>>> I am working on its implementation in Qemu/DPDK, our goal being to
>>>> reduce performance drops for small packets with Rx mergeable buffers
>>>> feature enabled.
>>>
>>> Mind to educate me a bit on how that works?
>>
>> Of course.
>>
>> Basically, this is a way to advise the MTU we want in the guest.
>> In the guest, if GRO is not enabled:
>>  - In case of Kernel virtio-net, it could be used to
>> size the SKBs at the expected MTU. If possible, we could disable Rx
>> mergeable buffers.
>>  - In case of virtio PMD, if the MTU advised by host is lower than the
>> pre-allocated mbuf size for the receive queue, then we should not need
>> mergeable buffers.
>
> Thanks for the explanation!
>
> I see. So, the point is to avoid using mergeable buffers while it is
> enabled.
>
>> Does that sound reasonnable?
>
> Yeah, maybe. Just don't know how well it may work in real life. Have
> you got any rought data so far?

The PoC is not done yet, only Qemu part is implemented.
But what we noticed is that for small packets, we have a 50%
degradation when rx mergeable buffers are on when running PVP
use-case.

Main part of the degradation is due an additional cache-miss in
virtio-pmd receive path, because we fetch the header to get the number
of buffer.

When sending only small packets and removing this access, we recover
25% of the degradation.

The 25% remaining part may be reduced significantly with Zhihong series.

Hope it answer your questions.

Thanks,
Maxime
Yuanhan Liu Sept. 8, 2016, 7:57 a.m. UTC | #10 
On Thu, Sep 08, 2016 at 09:50:34AM +0200, Maxime Coquelin wrote:
> 
> 
> On 09/08/2016 09:30 AM, Yuanhan Liu wrote:
> >On Wed, Sep 07, 2016 at 11:16:47AM +0200, Maxime Coquelin wrote:
> >>
> >>
> >>On 09/07/2016 05:25 AM, Yuanhan Liu wrote:
> >>>On Tue, Aug 30, 2016 at 09:57:39AM +0200, Maxime Coquelin wrote:
> >>>>Hi Souvik,
> >>>>
> >>>>On 08/30/2016 01:02 AM, souvikdey33 wrote:
> >>>>>Signed-off-by: Souvik Dey <sodey@sonusnet.com>
> >>>>>
> >>>>>Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey <sodey@sonusnet.com>")
> >>>>>Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> >>>>>
> >>>>>Virtio interfaces should also support setting of mtu, as in case of cloud
> >>>>>it is expected to have the consistent mtu across the infrastructure that
> >>>>>the dhcp server sends and not hardcoded to 1500(default).
> >>>>>---
> >>>>>drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
> >>>>>1 file changed, 12 insertions(+)
> >>>>
> >>>>FYI, there are some on-going changes in the VIRTIO specification
> >>>>so that the VHOST interface exposes its MTU to its VIRTIO peer.
> >>>>It may also be used as an alternative of what you patch achieves.
> >>>>
> >>>>I am working on its implementation in Qemu/DPDK, our goal being to
> >>>>reduce performance drops for small packets with Rx mergeable buffers
> >>>>feature enabled.
> >>>
> >>>Mind to educate me a bit on how that works?
> >>
> >>Of course.
> >>
> >>Basically, this is a way to advise the MTU we want in the guest.
> >>In the guest, if GRO is not enabled:
> >> - In case of Kernel virtio-net, it could be used to
> >>size the SKBs at the expected MTU. If possible, we could disable Rx
> >>mergeable buffers.
> >> - In case of virtio PMD, if the MTU advised by host is lower than the
> >>pre-allocated mbuf size for the receive queue, then we should not need
> >>mergeable buffers.
> >
> >Thanks for the explanation!
> >
> >I see. So, the point is to avoid using mergeable buffers while it is
> >enabled.
> >
> >>Does that sound reasonnable?
> >
> >Yeah, maybe. Just don't know how well it may work in real life. Have
> >you got any rought data so far?
> 
> The PoC is not done yet, only Qemu part is implemented.
> But what we noticed is that for small packets, we have a 50%
> degradation when rx mergeable buffers are on when running PVP
> use-case.
> 
> Main part of the degradation is due an additional cache-miss in
> virtio-pmd receive path, because we fetch the header to get the number
> of buffer.
> 
> When sending only small packets and removing this access, we recover
> 25% of the degradation.
> 
> The 25% remaining part may be reduced significantly with Zhihong series.
> 
> Hope it answer your questions.

Yes, it does and thanks for the info.

	--yliu
souvikdey33 Sept. 9, 2016, 3:44 a.m. UTC | #11 
Are we good to get this in for 16.11 and then revisit this when the VHOST improvements comes in. This will atleast take care of the gap between 16.11 and VHOST improvements coming in.

--
Regards,
Souvik

-----Original Message-----
From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] 
Sent: Thursday, September 8, 2016 3:57 AM
To: Maxime Coquelin <maxime.coquelin@redhat.com>
Cc: Dey, Souvik <sodey@sonusnet.com>; stephen@networkplumber.org; huawei.xie@intel.com; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] add mtu set in virtio

On Thu, Sep 08, 2016 at 09:50:34AM +0200, Maxime Coquelin wrote:
> 
> 
> On 09/08/2016 09:30 AM, Yuanhan Liu wrote:
> >On Wed, Sep 07, 2016 at 11:16:47AM +0200, Maxime Coquelin wrote:
> >>
> >>
> >>On 09/07/2016 05:25 AM, Yuanhan Liu wrote:
> >>>On Tue, Aug 30, 2016 at 09:57:39AM +0200, Maxime Coquelin wrote:
> >>>>Hi Souvik,
> >>>>
> >>>>On 08/30/2016 01:02 AM, souvikdey33 wrote:
> >>>>>Signed-off-by: Souvik Dey <sodey@sonusnet.com>
> >>>>>
> >>>>>Fixes: 1fb8e8896ca8 ("Signed-off-by: Souvik Dey 
> >>>>><sodey@sonusnet.com>")
> >>>>>Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> >>>>>
> >>>>>Virtio interfaces should also support setting of mtu, as in case 
> >>>>>of cloud it is expected to have the consistent mtu across the 
> >>>>>infrastructure that the dhcp server sends and not hardcoded to 1500(default).
> >>>>>---
> >>>>>drivers/net/virtio/virtio_ethdev.c | 12 ++++++++++++
> >>>>>1 file changed, 12 insertions(+)
> >>>>
> >>>>FYI, there are some on-going changes in the VIRTIO specification 
> >>>>so that the VHOST interface exposes its MTU to its VIRTIO peer.
> >>>>It may also be used as an alternative of what you patch achieves.
> >>>>
> >>>>I am working on its implementation in Qemu/DPDK, our goal being to 
> >>>>reduce performance drops for small packets with Rx mergeable 
> >>>>buffers feature enabled.
> >>>
> >>>Mind to educate me a bit on how that works?
> >>
> >>Of course.
> >>
> >>Basically, this is a way to advise the MTU we want in the guest.
> >>In the guest, if GRO is not enabled:
> >> - In case of Kernel virtio-net, it could be used to size the SKBs 
> >>at the expected MTU. If possible, we could disable Rx mergeable 
> >>buffers.
> >> - In case of virtio PMD, if the MTU advised by host is lower than 
> >>the pre-allocated mbuf size for the receive queue, then we should 
> >>not need mergeable buffers.
> >
> >Thanks for the explanation!
> >
> >I see. So, the point is to avoid using mergeable buffers while it is 
> >enabled.
> >
> >>Does that sound reasonnable?
> >
> >Yeah, maybe. Just don't know how well it may work in real life. Have 
> >you got any rought data so far?
> 
> The PoC is not done yet, only Qemu part is implemented.
> But what we noticed is that for small packets, we have a 50% 
> degradation when rx mergeable buffers are on when running PVP 
> use-case.
> 
> Main part of the degradation is due an additional cache-miss in 
> virtio-pmd receive path, because we fetch the header to get the number 
> of buffer.
> 
> When sending only small packets and removing this access, we recover 
> 25% of the degradation.
> 
> The 25% remaining part may be reduced significantly with Zhihong series.
> 
> Hope it answer your questions.

Yes, it does and thanks for the info.

	--yliu
diff mbox

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 07d6449..da16ad4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -92,6 +92,7 @@  static void virtio_mac_addr_add(struct rte_eth_dev *dev,
 static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index);
 static void virtio_mac_addr_set(struct rte_eth_dev *dev,
 				struct ether_addr *mac_addr);
+static int  virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
 
 static int virtio_dev_queue_stats_mapping_set(
 	__rte_unused struct rte_eth_dev *eth_dev,
@@ -652,6 +653,16 @@  virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
 		PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
 }
 
+static int 
+virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) 
+{
+   struct virtio_hw *hw = dev->data->dev_private;
+   if (mtu < VIRTIO_MIN_RX_BUFSIZE || mtu > VIRTIO_MAX_RX_PKTLEN) {
+	   PMD_INIT_LOG(ERR,"Mtu should be between 64 and 9728."
+          return -EINVAL;
+   }
+   return 0;
+}
+
 /*
  * dev_ops for virtio, bare necessities for basic operation
  */
@@ -664,6 +675,7 @@  static const struct eth_dev_ops virtio_eth_dev_ops = {
 	.promiscuous_disable     = virtio_dev_promiscuous_disable,
 	.allmulticast_enable     = virtio_dev_allmulticast_enable,
 	.allmulticast_disable    = virtio_dev_allmulticast_disable,
+	.mtu_set                 = virtio_mtu_set,
 
 	.dev_infos_get           = virtio_dev_info_get,
 	.stats_get               = virtio_dev_stats_get,