Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1412370 > unrolled thread

[PATCH -next 2/2] virtio_net: Read the advised MTU

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH -next 2/2] virtio_net: Read the advised MTU Aaron Conole <aconole@redhat.com> - 2016-06-02 17:50 +0200
    Re: [PATCH -next 2/2] virtio_net: Read the advised MTU "Michael S. Tsirkin" <mst@redhat.com> - 2016-06-02 18:20 +0200
      Re: [PATCH -next 2/2] virtio_net: Read the advised MTU Aaron Conole <aconole@redhat.com> - 2016-06-02 19:20 +0200
    Re: [PATCH -next 2/2] virtio_net: Read the advised MTU Aaron Conole <aconole@redhat.com> - 2016-06-02 19:10 +0200
      Re: [PATCH -next 2/2] virtio_net: Read the advised MTU Rick Jones <rick.jones2@hpe.com> - 2016-06-02 20:20 +0200

#1412370 — [PATCH -next 2/2] virtio_net: Read the advised MTU

This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
exists, read the advised MTU and use it.

No proper error handling is provided for the case where a user changes the
negotiated MTU. A future commit will add proper error handling. Instead, a
warning is emitted if the guest changes the device MTU after previously
being given advice.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 drivers/net/virtio_net.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e0638e5..ef5ee01 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
 		vi->has_cvq = true;
 
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
+		dev->mtu = virtio_cread16(vdev,
+					  offsetof(struct virtio_net_config,
+						   mtu));
+	}
+
 	if (vi->any_header_sg)
 		dev->needed_headroom = vi->hdr_len;
 
@@ -2067,6 +2073,7 @@ static unsigned int features[] = {
 	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
 	VIRTIO_NET_F_CTRL_MAC_ADDR,
 	VIRTIO_F_ANY_LAYOUT,
+	VIRTIO_NET_F_MTU,
 };
 
 static struct virtio_driver virtio_net_driver = {
-- 
2.5.5

[toc] | [next] | [standalone]

#1412407

On Thu, Jun 02, 2016 at 11:43:31AM -0400, Aaron Conole wrote:
> This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
> exists, read the advised MTU and use it.
> 
> No proper error handling is provided for the case where a user changes the
> negotiated MTU. A future commit will add proper error handling. Instead, a
> warning is emitted if the guest changes the device MTU after previously
> being given advice.

I don't see a warning and I don't think it's needed.
Patch is ok commit log isn't.

> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  drivers/net/virtio_net.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index e0638e5..ef5ee01 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
>  		vi->has_cvq = true;
>  
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
> +		dev->mtu = virtio_cread16(vdev,
> +					  offsetof(struct virtio_net_config,
> +						   mtu));
> +	}
> +
>  	if (vi->any_header_sg)
>  		dev->needed_headroom = vi->hdr_len;
>  
> @@ -2067,6 +2073,7 @@ static unsigned int features[] = {
>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
>  	VIRTIO_NET_F_CTRL_MAC_ADDR,
>  	VIRTIO_F_ANY_LAYOUT,
> +	VIRTIO_NET_F_MTU,
>  };
>  
>  static struct virtio_driver virtio_net_driver = {
> -- 
> 2.5.5

[toc] | [prev] | [next] | [standalone]

#1412439

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Jun 02, 2016 at 11:43:31AM -0400, Aaron Conole wrote:
>> This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
>> exists, read the advised MTU and use it.
>> 
>> No proper error handling is provided for the case where a user changes the
>> negotiated MTU. A future commit will add proper error handling. Instead, a
>> warning is emitted if the guest changes the device MTU after previously
>> being given advice.
>
> I don't see a warning and I don't think it's needed.
> Patch is ok commit log isn't.

Okay, I'll fix it when I submit v2.

>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>  drivers/net/virtio_net.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>> 
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index e0638e5..ef5ee01 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
>>  		vi->has_cvq = true;
>>  
>> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
>> +		dev->mtu = virtio_cread16(vdev,
>> +					  offsetof(struct virtio_net_config,
>> +						   mtu));
>> +	}
>> +
>>  	if (vi->any_header_sg)
>>  		dev->needed_headroom = vi->hdr_len;
>>  
>> @@ -2067,6 +2073,7 @@ static unsigned int features[] = {
>>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
>>  	VIRTIO_NET_F_CTRL_MAC_ADDR,
>>  	VIRTIO_F_ANY_LAYOUT,
>> +	VIRTIO_NET_F_MTU,
>>  };
>>  
>>  static struct virtio_driver virtio_net_driver = {
>> -- 
>> 2.5.5

[toc] | [prev] | [next] | [standalone]

#1412437

Hi Rick,

In the future, please don't cut the list.

Rick Jones <rick.jones2@hpe.com> writes:

> On 06/02/2016 08:43 AM, Aaron Conole wrote:
>> This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
>> exists, read the advised MTU and use it.
>>
>> No proper error handling is provided for the case where a user changes the
>> negotiated MTU. A future commit will add proper error handling. Instead, a
>> warning is emitted if the guest changes the device MTU after previously
>> being given advice.
>
> One of the things I've been doing has been setting-up a cluster
> (OpenStack) with JumboFrames, and then setting MTUs on instance vNICs
> by hand to measure different MTU sizes.  It would be a shame if such a
> thing were not possible in the future.  Keeping a warning if shrinking
> the MTU would be good, leave the error (perhaps) to if an attempt is
> made to go beyond the advised value.

This was cut because it didn't make sense for such a warning to
be issued, but it seems like perhaps you may want such a feature?  I
agree with Michael, after thinking about it, that I don't know what sort
of use the warning would serve.  After all, if you're changing the MTU,
you must have wanted such a change to occur?

-Aaron

> happy benchmarking,
>
> rick jones

[toc] | [prev] | [next] | [standalone]

#1412478

On 06/02/2016 10:06 AM, Aaron Conole wrote:
> Rick Jones <rick.jones2@hpe.com> writes:
>> One of the things I've been doing has been setting-up a cluster
>> (OpenStack) with JumboFrames, and then setting MTUs on instance vNICs
>> by hand to measure different MTU sizes.  It would be a shame if such a
>> thing were not possible in the future.  Keeping a warning if shrinking
>> the MTU would be good, leave the error (perhaps) to if an attempt is
>> made to go beyond the advised value.
>
> This was cut because it didn't make sense for such a warning to
> be issued, but it seems like perhaps you may want such a feature?  I
> agree with Michael, after thinking about it, that I don't know what sort
> of use the warning would serve.  After all, if you're changing the MTU,
> you must have wanted such a change to occur?

I don't need a warning, was simply willing to live with one when 
shrinking the MTU.  Didn't want an error.

happy benchmarking,

rick jones

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web