[RFC -next 0/2] virtio-net: Advised MTU feature

All of lore.kernel.org
 help / color / mirror / Atom feed

* [RFC -next 0/2] virtio-net: Advised MTU feature
@ 2016-03-10 14:28 Aaron Conole
  2016-03-10 14:28 ` [RFC -next 1/2] virtio: Start the advised MTU feature support Aaron Conole
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Aaron Conole @ 2016-03-10 14:28 UTC (permalink / raw)
  To: netdev, Michael S. Tsirkin, virtualization, linux-kernel

The following series adds the ability for a hypervisor to set an MTU on the
guest during feature negotiation phase. This is useful for VM orchestration
when, for instance, tunneling is involved and the MTU of the various systems
should be homogenous.

The first patch adds the feature bit as described in the proposed VFIO spec
addition found at
https://lists.oasis-open.org/archives/virtio-dev/201603/msg00001.html

The second patch adds a user of the bit, and a warning when the guest changes
the MTU from the hypervisor advised MTU. Future patches may add more thorough
error handling.

Aaron Conole (2):
  virtio: Start the advised MTU feature support
  virtio_net: Read and use the advised MTU

 drivers/net/virtio_net.c        | 15 ++++++++++++++-
 include/uapi/linux/virtio_net.h |  3 +++
 2 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.5.0

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

* [RFC -next 1/2] virtio: Start the advised MTU feature support
  2016-03-10 14:28 [RFC -next 0/2] virtio-net: Advised MTU feature Aaron Conole
@ 2016-03-10 14:28 ` Aaron Conole
  2016-03-10 14:28 ` [RFC -next 2/2] virtio_net: Read and use the advised MTU Aaron Conole
  2016-03-10 14:28 ` Aaron Conole
  2 siblings, 0 replies; 11+ messages in thread
From: Aaron Conole @ 2016-03-10 14:28 UTC (permalink / raw)
  To: netdev, Michael S. Tsirkin, virtualization, linux-kernel

This commit adds the feature bit and associated mtu device entry for the
virtio network device. Future commits will make use of these bits to support
negotiated MTU.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 include/uapi/linux/virtio_net.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index ec32293..41a6a01 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -55,6 +55,7 @@
 #define VIRTIO_NET_F_MQ	22	/* Device supports Receive Flow
 					 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
+#define VIRTIO_NET_F_MTU 25	/* Device supports Default MTU Negotiation */
 
 #ifndef VIRTIO_NET_NO_LEGACY
 #define VIRTIO_NET_F_GSO	6	/* Host handles pkts w/ any GSO type */
@@ -73,6 +74,8 @@ struct virtio_net_config {
 	 * Legal values are between 1 and 0x8000
 	 */
 	__u16 max_virtqueue_pairs;
+	/* Default maximum transmit unit advice */
+	__u16 mtu;
 } __attribute__((packed));
 
 /*
-- 
2.5.0

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

* [RFC -next 2/2] virtio_net: Read and use the advised MTU
  2016-03-10 14:28 [RFC -next 0/2] virtio-net: Advised MTU feature Aaron Conole
  2016-03-10 14:28 ` [RFC -next 1/2] virtio: Start the advised MTU feature support Aaron Conole
@ 2016-03-10 14:28 ` Aaron Conole
  2016-03-10 14:28 ` Aaron Conole
  2 siblings, 0 replies; 11+ messages in thread
From: Aaron Conole @ 2016-03-10 14:28 UTC (permalink / raw)
  To: netdev, Michael S. Tsirkin, virtualization, linux-kernel

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 | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 767ab11..7175563 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -146,6 +146,7 @@ struct virtnet_info {
 	virtio_net_ctrl_ack ctrl_status;
 	u8 ctrl_promisc;
 	u8 ctrl_allmulti;
+	bool negotiated_mtu;
 };
 
 struct padded_vnet_hdr {
@@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
 
 static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
 {
+	struct virtnet_info *vi = netdev_priv(dev);
 	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
 		return -EINVAL;
+	if (vi->negotiated_mtu == true) {
+		pr_warn("changing mtu from negotiated mtu.");
+	}
 	dev->mtu = new_mtu;
 	return 0;
 }
@@ -1836,6 +1841,13 @@ 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)) {
+		vi->negotiated_mtu = true;
+		dev->mtu = virtio_cread16(vdev,
+					  offsetof(struct virtio_net_config,
+						   mtu));
+	}
+
 	if (vi->any_header_sg)
 		dev->needed_headroom = vi->hdr_len;
 
@@ -2017,8 +2029,9 @@ static unsigned int features[] = {
 	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
 	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
 	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
-	VIRTIO_NET_F_CTRL_MAC_ADDR,
+	VIRTIO_NET_F_CTRL_MAC_ADDR, 
 	VIRTIO_F_ANY_LAYOUT,
+	VIRTIO_NET_F_MTU,
 };
 
 static struct virtio_driver virtio_net_driver = {
-- 
2.5.0

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

* [RFC -next 2/2] virtio_net: Read and use the advised MTU
  2016-03-10 14:28 [RFC -next 0/2] virtio-net: Advised MTU feature Aaron Conole
  2016-03-10 14:28 ` [RFC -next 1/2] virtio: Start the advised MTU feature support Aaron Conole
  2016-03-10 14:28 ` [RFC -next 2/2] virtio_net: Read and use the advised MTU Aaron Conole
@ 2016-03-10 14:28 ` Aaron Conole
  2016-03-10 14:57   ` Paolo Abeni
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Aaron Conole @ 2016-03-10 14:28 UTC (permalink / raw)
  To: netdev, Michael S. Tsirkin, virtualization, linux-kernel

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 | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 767ab11..7175563 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -146,6 +146,7 @@ struct virtnet_info {
 	virtio_net_ctrl_ack ctrl_status;
 	u8 ctrl_promisc;
 	u8 ctrl_allmulti;
+	bool negotiated_mtu;
 };
 
 struct padded_vnet_hdr {
@@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
 
 static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
 {
+	struct virtnet_info *vi = netdev_priv(dev);
 	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
 		return -EINVAL;
+	if (vi->negotiated_mtu == true) {
+		pr_warn("changing mtu from negotiated mtu.");
+	}
 	dev->mtu = new_mtu;
 	return 0;
 }
@@ -1836,6 +1841,13 @@ 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)) {
+		vi->negotiated_mtu = true;
+		dev->mtu = virtio_cread16(vdev,
+					  offsetof(struct virtio_net_config,
+						   mtu));
+	}
+
 	if (vi->any_header_sg)
 		dev->needed_headroom = vi->hdr_len;
 
@@ -2017,8 +2029,9 @@ static unsigned int features[] = {
 	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
 	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
 	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
-	VIRTIO_NET_F_CTRL_MAC_ADDR,
+	VIRTIO_NET_F_CTRL_MAC_ADDR, 
 	VIRTIO_F_ANY_LAYOUT,
+	VIRTIO_NET_F_MTU,
 };
 
 static struct virtio_driver virtio_net_driver = {
-- 
2.5.0

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

* Re: [RFC -next 2/2] virtio_net: Read and use the advised MTU
  2016-03-10 14:28 ` Aaron Conole
@ 2016-03-10 14:57   ` Paolo Abeni
  2016-03-10 14:57   ` Paolo Abeni
  2016-03-10 18:56   ` Sergei Shtylyov
  2 siblings, 0 replies; 11+ messages in thread
From: Paolo Abeni @ 2016-03-10 14:57 UTC (permalink / raw)
  To: Aaron Conole; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin

On Thu, 2016-03-10 at 09:28 -0500, 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.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  drivers/net/virtio_net.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 767ab11..7175563 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -146,6 +146,7 @@ struct virtnet_info {
>  	virtio_net_ctrl_ack ctrl_status;
>  	u8 ctrl_promisc;
>  	u8 ctrl_allmulti;
> +	bool negotiated_mtu;
>  };
>  
>  struct padded_vnet_hdr {
> @@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>  
>  static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>  {
> +	struct virtnet_info *vi = netdev_priv(dev);
>  	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
>  		return -EINVAL;
> +	if (vi->negotiated_mtu == true) {

why don't:

if ((vi->negotiated_mtu == true) && (dev->mtu != new_mtu))

?

> +		pr_warn("changing mtu from negotiated mtu.");
> +	}
>  	dev->mtu = new_mtu;
>  	return 0;
>  }
> @@ -1836,6 +1841,13 @@ 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)) {
> +		vi->negotiated_mtu = true;
> +		dev->mtu = virtio_cread16(vdev,
> +					  offsetof(struct virtio_net_config,
> +						   mtu));
> +	}
> +
>  	if (vi->any_header_sg)
>  		dev->needed_headroom = vi->hdr_len;
>  
> @@ -2017,8 +2029,9 @@ static unsigned int features[] = {
>  	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
>  	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> -	VIRTIO_NET_F_CTRL_MAC_ADDR,
> +	VIRTIO_NET_F_CTRL_MAC_ADDR, 

Here a trailing white space slipped-in.

Otherwise LGTM.

Paolo

>  	VIRTIO_F_ANY_LAYOUT,
> +	VIRTIO_NET_F_MTU,
>  };
>  
>  static struct virtio_driver virtio_net_driver = {

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

* Re: [RFC -next 2/2] virtio_net: Read and use the advised MTU
  2016-03-10 14:28 ` Aaron Conole
  2016-03-10 14:57   ` Paolo Abeni
@ 2016-03-10 14:57   ` Paolo Abeni
  2016-03-15 20:52       ` Aaron Conole
  2016-03-10 18:56   ` Sergei Shtylyov
  2 siblings, 1 reply; 11+ messages in thread
From: Paolo Abeni @ 2016-03-10 14:57 UTC (permalink / raw)
  To: Aaron Conole; +Cc: netdev, Michael S. Tsirkin, virtualization, linux-kernel

On Thu, 2016-03-10 at 09:28 -0500, 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.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  drivers/net/virtio_net.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 767ab11..7175563 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -146,6 +146,7 @@ struct virtnet_info {
>  	virtio_net_ctrl_ack ctrl_status;
>  	u8 ctrl_promisc;
>  	u8 ctrl_allmulti;
> +	bool negotiated_mtu;
>  };
>  
>  struct padded_vnet_hdr {
> @@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>  
>  static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>  {
> +	struct virtnet_info *vi = netdev_priv(dev);
>  	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
>  		return -EINVAL;
> +	if (vi->negotiated_mtu == true) {

why don't:

if ((vi->negotiated_mtu == true) && (dev->mtu != new_mtu))

?

> +		pr_warn("changing mtu from negotiated mtu.");
> +	}
>  	dev->mtu = new_mtu;
>  	return 0;
>  }
> @@ -1836,6 +1841,13 @@ 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)) {
> +		vi->negotiated_mtu = true;
> +		dev->mtu = virtio_cread16(vdev,
> +					  offsetof(struct virtio_net_config,
> +						   mtu));
> +	}
> +
>  	if (vi->any_header_sg)
>  		dev->needed_headroom = vi->hdr_len;
>  
> @@ -2017,8 +2029,9 @@ static unsigned int features[] = {
>  	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
>  	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> -	VIRTIO_NET_F_CTRL_MAC_ADDR,
> +	VIRTIO_NET_F_CTRL_MAC_ADDR, 

Here a trailing white space slipped-in.

Otherwise LGTM.

Paolo

>  	VIRTIO_F_ANY_LAYOUT,
> +	VIRTIO_NET_F_MTU,
>  };
>  
>  static struct virtio_driver virtio_net_driver = {

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

* Re: [RFC -next 2/2] virtio_net: Read and use the advised MTU
  2016-03-10 14:57   ` Paolo Abeni
@ 2016-03-15 20:52       ` Aaron Conole
  0 siblings, 0 replies; 11+ messages in thread
From: Aaron Conole @ 2016-03-15 20:52 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin

Paolo Abeni <pabeni@redhat.com> writes:

> On Thu, 2016-03-10 at 09:28 -0500, 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.
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>  drivers/net/virtio_net.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 767ab11..7175563 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -146,6 +146,7 @@ struct virtnet_info {
>>  	virtio_net_ctrl_ack ctrl_status;
>>  	u8 ctrl_promisc;
>>  	u8 ctrl_allmulti;
>> +	bool negotiated_mtu;
>>  };
>>  
>>  struct padded_vnet_hdr {
>> @@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>>  
>>  static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>>  {
>> +	struct virtnet_info *vi = netdev_priv(dev);
>>  	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
>>  		return -EINVAL;
>> +	if (vi->negotiated_mtu == true) {
>
> why don't:
>
> if ((vi->negotiated_mtu == true) && (dev->mtu != new_mtu))
>
> ?

Okay, I'll put this test in.

>> +		pr_warn("changing mtu from negotiated mtu.");
>> +	}
>>  	dev->mtu = new_mtu;
>>  	return 0;
>>  }
>> @@ -1836,6 +1841,13 @@ 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)) {
>> +		vi->negotiated_mtu = true;
>> +		dev->mtu = virtio_cread16(vdev,
>> +					  offsetof(struct virtio_net_config,
>> +						   mtu));
>> +	}
>> +
>>  	if (vi->any_header_sg)
>>  		dev->needed_headroom = vi->hdr_len;
>>  
>> @@ -2017,8 +2029,9 @@ static unsigned int features[] = {
>>  	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
>>  	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
>>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
>> -	VIRTIO_NET_F_CTRL_MAC_ADDR,
>> +	VIRTIO_NET_F_CTRL_MAC_ADDR, 
>
> Here a trailing white space slipped-in.
>
> Otherwise LGTM.
>
> Paolo

D'oh! Okay, v2 will have this fixed.

>>  	VIRTIO_F_ANY_LAYOUT,
>> +	VIRTIO_NET_F_MTU,
>>  };
>>  
>>  static struct virtio_driver virtio_net_driver = {

Thanks so much for the review, Paolo!

-Aaron

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

* Re: [RFC -next 2/2] virtio_net: Read and use the advised MTU
@ 2016-03-15 20:52       ` Aaron Conole
  0 siblings, 0 replies; 11+ messages in thread
From: Aaron Conole @ 2016-03-15 20:52 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, Michael S. Tsirkin, virtualization, linux-kernel

Paolo Abeni <pabeni@redhat.com> writes:

> On Thu, 2016-03-10 at 09:28 -0500, 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.
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>  drivers/net/virtio_net.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 767ab11..7175563 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -146,6 +146,7 @@ struct virtnet_info {
>>  	virtio_net_ctrl_ack ctrl_status;
>>  	u8 ctrl_promisc;
>>  	u8 ctrl_allmulti;
>> +	bool negotiated_mtu;
>>  };
>>  
>>  struct padded_vnet_hdr {
>> @@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>>  
>>  static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>>  {
>> +	struct virtnet_info *vi = netdev_priv(dev);
>>  	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
>>  		return -EINVAL;
>> +	if (vi->negotiated_mtu == true) {
>
> why don't:
>
> if ((vi->negotiated_mtu == true) && (dev->mtu != new_mtu))
>
> ?

Okay, I'll put this test in.

>> +		pr_warn("changing mtu from negotiated mtu.");
>> +	}
>>  	dev->mtu = new_mtu;
>>  	return 0;
>>  }
>> @@ -1836,6 +1841,13 @@ 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)) {
>> +		vi->negotiated_mtu = true;
>> +		dev->mtu = virtio_cread16(vdev,
>> +					  offsetof(struct virtio_net_config,
>> +						   mtu));
>> +	}
>> +
>>  	if (vi->any_header_sg)
>>  		dev->needed_headroom = vi->hdr_len;
>>  
>> @@ -2017,8 +2029,9 @@ static unsigned int features[] = {
>>  	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
>>  	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
>>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
>> -	VIRTIO_NET_F_CTRL_MAC_ADDR,
>> +	VIRTIO_NET_F_CTRL_MAC_ADDR, 
>
> Here a trailing white space slipped-in.
>
> Otherwise LGTM.
>
> Paolo

D'oh! Okay, v2 will have this fixed.

>>  	VIRTIO_F_ANY_LAYOUT,
>> +	VIRTIO_NET_F_MTU,
>>  };
>>  
>>  static struct virtio_driver virtio_net_driver = {

Thanks so much for the review, Paolo!

-Aaron

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

* Re: [RFC -next 2/2] virtio_net: Read and use the advised MTU
  2016-03-10 14:28 ` Aaron Conole
  2016-03-10 14:57   ` Paolo Abeni
  2016-03-10 14:57   ` Paolo Abeni
@ 2016-03-10 18:56   ` Sergei Shtylyov
  2016-03-15 20:53       ` Aaron Conole
  2 siblings, 1 reply; 11+ messages in thread
From: Sergei Shtylyov @ 2016-03-10 18:56 UTC (permalink / raw)
  To: Aaron Conole, netdev, Michael S. Tsirkin, virtualization,
	linux-kernel

Hello.

On 03/10/2016 05:28 PM, 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.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>   drivers/net/virtio_net.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 767ab11..7175563 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
[...]
> @@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>
>   static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>   {
> +	struct virtnet_info *vi = netdev_priv(dev);
>   	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
>   		return -EINVAL;
> +	if (vi->negotiated_mtu == true) {
> +		pr_warn("changing mtu from negotiated mtu.");
> +	}

    {} not needed, see Documentation/CodingStyle.

[...]

MBR, Sergei

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

* Re: [RFC -next 2/2] virtio_net: Read and use the advised MTU
  2016-03-10 18:56   ` Sergei Shtylyov
@ 2016-03-15 20:53       ` Aaron Conole
  0 siblings, 0 replies; 11+ messages in thread
From: Aaron Conole @ 2016-03-15 20:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin

Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes:

> Hello.

Hi Sergei,

> On 03/10/2016 05:28 PM, 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.
>>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>   drivers/net/virtio_net.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 767ab11..7175563 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
> [...]
>> @@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>>
>>   static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>>   {
>> +	struct virtnet_info *vi = netdev_priv(dev);
>>   	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
>>   		return -EINVAL;
>> +	if (vi->negotiated_mtu == true) {
>> +		pr_warn("changing mtu from negotiated mtu.");
>> +	}
>
>    {} not needed, see Documentation/CodingStyle.

Okay, I'll make sure to fix this with v2.

> [...]
>
> MBR, Sergei

Thanks so much for the review!

-Aaron

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

* Re: [RFC -next 2/2] virtio_net: Read and use the advised MTU
@ 2016-03-15 20:53       ` Aaron Conole
  0 siblings, 0 replies; 11+ messages in thread
From: Aaron Conole @ 2016-03-15 20:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev, Michael S. Tsirkin, virtualization, linux-kernel

Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes:

> Hello.

Hi Sergei,

> On 03/10/2016 05:28 PM, 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.
>>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>   drivers/net/virtio_net.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 767ab11..7175563 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
> [...]
>> @@ -1390,8 +1391,12 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>>
>>   static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>>   {
>> +	struct virtnet_info *vi = netdev_priv(dev);
>>   	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
>>   		return -EINVAL;
>> +	if (vi->negotiated_mtu == true) {
>> +		pr_warn("changing mtu from negotiated mtu.");
>> +	}
>
>    {} not needed, see Documentation/CodingStyle.

Okay, I'll make sure to fix this with v2.

> [...]
>
> MBR, Sergei

Thanks so much for the review!

-Aaron

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

end of thread, other threads:[~2016-03-15 20:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-10 14:28 [RFC -next 0/2] virtio-net: Advised MTU feature Aaron Conole
2016-03-10 14:28 ` [RFC -next 1/2] virtio: Start the advised MTU feature support Aaron Conole
2016-03-10 14:28 ` [RFC -next 2/2] virtio_net: Read and use the advised MTU Aaron Conole
2016-03-10 14:28 ` Aaron Conole
2016-03-10 14:57   ` Paolo Abeni
2016-03-10 14:57   ` Paolo Abeni
2016-03-15 20:52     ` Aaron Conole
2016-03-15 20:52       ` Aaron Conole
2016-03-10 18:56   ` Sergei Shtylyov
2016-03-15 20:53     ` Aaron Conole
2016-03-15 20:53       ` Aaron Conole

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.