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 feature MTU support virtio_net: Read the advised MTU drivers/net/virtio_net.c | 7 +++++++ include/uapi/linux/virtio_net.h | 2 ++ 2 files changed, 9 insertions(+) -- 2.5.5
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 at redhat.com>
---
include/uapi/linux/virtio_net.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index ec32293..751ff59 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -73,6 +73,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.5
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 at 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
Michael S. Tsirkin
2016-Jun-02 16:16 UTC
[PATCH -next 2/2] virtio_net: Read the advised MTU
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 at 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
Michael S. Tsirkin
2016-Jun-02 16:17 UTC
[PATCH -next 1/2] virtio: Start feature MTU support
On Thu, Jun 02, 2016 at 11:43:30AM -0400, Aaron Conole wrote:> 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.why split it out? Pls squash with the next patch.> Signed-off-by: Aaron Conole <aconole at redhat.com> > --- > include/uapi/linux/virtio_net.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h > index ec32293..751ff59 100644 > --- a/include/uapi/linux/virtio_net.h > +++ b/include/uapi/linux/virtio_net.h > @@ -73,6 +73,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.5
Hi Rick, In the future, please don't cut the list. Rick Jones <rick.jones2 at 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
kbuild test robot
2016-Jun-02 17:25 UTC
[PATCH -next 2/2] virtio_net: Read the advised MTU
Hi,
[auto build test ERROR on next-20160602]
url:
https://github.com/0day-ci/linux/commits/Aaron-Conole/virtio-net-Advised-MTU-feature/20160603-000714
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the
linux-review/Aaron-Conole/virtio-net-Advised-MTU-feature/20160603-000714 HEAD
d909da4df3c52f78b4f5fcccd89aea5e38722d10 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/net/virtio_net.c: In function
'virtnet_probe':>> drivers/net/virtio_net.c:1899:31: error: 'VIRTIO_NET_F_MTU'
undeclared (first use in this function)
if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
^~~~~~~~~~~~~~~~
drivers/net/virtio_net.c:1899:31: note: each undeclared identifier is
reported only once for each function it appears in
drivers/net/virtio_net.c: At top level:>> drivers/net/virtio_net.c:2076:2: error: 'VIRTIO_NET_F_MTU'
undeclared here (not in a function)
VIRTIO_NET_F_MTU,
^~~~~~~~~~~~~~~~
vim +/VIRTIO_NET_F_MTU +1899 drivers/net/virtio_net.c
1893 virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
1894 vi->any_header_sg = true;
1895
1896 if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
1897 vi->has_cvq = true;
1898 > 1899 if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
1900 dev->mtu = virtio_cread16(vdev,
1901 offsetof(struct virtio_net_config,
1902 mtu));
1903 }
1904
1905 if (vi->any_header_sg)
1906 dev->needed_headroom = vi->hdr_len;
1907
1908 /* Use single tx/rx queue pair as default */
1909 vi->curr_queue_pairs = 1;
1910 vi->max_queue_pairs = max_queue_pairs;
1911
1912 /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
1913 err = init_vqs(vi);
1914 if (err)
1915 goto free_stats;
1916
1917 #ifdef CONFIG_SYSFS
1918 if (vi->mergeable_rx_bufs)
1919 dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
1920 #endif
1921 netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
1922 netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
1923
1924 virtnet_init_settings(dev);
1925
1926 err = register_netdev(dev);
1927 if (err) {
1928 pr_debug("virtio_net: registering device failed\n");
1929 goto free_vqs;
1930 }
1931
1932 virtio_device_ready(vdev);
1933
1934 vi->nb.notifier_call = &virtnet_cpu_callback;
1935 err = register_hotcpu_notifier(&vi->nb);
1936 if (err) {
1937 pr_debug("virtio_net: registering cpu notifier failed\n");
1938 goto free_unregister_netdev;
1939 }
1940
1941 /* Assume link up if device can't report link status,
1942 otherwise get link status from config. */
1943 if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
1944 netif_carrier_off(dev);
1945 schedule_work(&vi->config_work);
1946 } else {
1947 vi->status = VIRTIO_NET_S_LINK_UP;
1948 netif_carrier_on(dev);
1949 }
1950
1951 pr_debug("virtnet: registered device %s with %d RX and TX
vq's\n",
1952 dev->name, max_queue_pairs);
1953
1954 return 0;
1955
1956 free_unregister_netdev:
1957 vi->vdev->config->reset(vdev);
1958
1959 unregister_netdev(dev);
1960 free_vqs:
1961 cancel_delayed_work_sync(&vi->refill);
1962 free_receive_page_frags(vi);
1963 virtnet_del_vqs(vi);
1964 free_stats:
1965 free_percpu(vi->stats);
1966 free:
1967 free_netdev(dev);
1968 return err;
1969 }
1970
1971 static void remove_vq_common(struct virtnet_info *vi)
1972 {
1973 vi->vdev->config->reset(vi->vdev);
1974
1975 /* Free unused buffers in both send and recv, if any. */
1976 free_unused_bufs(vi);
1977
1978 free_receive_bufs(vi);
1979
1980 free_receive_page_frags(vi);
1981
1982 virtnet_del_vqs(vi);
1983 }
1984
1985 static void virtnet_remove(struct virtio_device *vdev)
1986 {
1987 struct virtnet_info *vi = vdev->priv;
1988
1989 unregister_hotcpu_notifier(&vi->nb);
1990
1991 /* Make sure no work handler is accessing the device. */
1992 flush_work(&vi->config_work);
1993
1994 unregister_netdev(vi->dev);
1995
1996 remove_vq_common(vi);
1997
1998 free_percpu(vi->stats);
1999 free_netdev(vi->dev);
2000 }
2001
2002 #ifdef CONFIG_PM_SLEEP
2003 static int virtnet_freeze(struct virtio_device *vdev)
2004 {
2005 struct virtnet_info *vi = vdev->priv;
2006 int i;
2007
2008 unregister_hotcpu_notifier(&vi->nb);
2009
2010 /* Make sure no work handler is accessing the device */
2011 flush_work(&vi->config_work);
2012
2013 netif_device_detach(vi->dev);
2014 cancel_delayed_work_sync(&vi->refill);
2015
2016 if (netif_running(vi->dev)) {
2017 for (i = 0; i < vi->max_queue_pairs; i++)
2018 napi_disable(&vi->rq[i].napi);
2019 }
2020
2021 remove_vq_common(vi);
2022
2023 return 0;
2024 }
2025
2026 static int virtnet_restore(struct virtio_device *vdev)
2027 {
2028 struct virtnet_info *vi = vdev->priv;
2029 int err, i;
2030
2031 err = init_vqs(vi);
2032 if (err)
2033 return err;
2034
2035 virtio_device_ready(vdev);
2036
2037 if (netif_running(vi->dev)) {
2038 for (i = 0; i < vi->curr_queue_pairs; i++)
2039 if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
2040 schedule_delayed_work(&vi->refill, 0);
2041
2042 for (i = 0; i < vi->max_queue_pairs; i++)
2043 virtnet_napi_enable(&vi->rq[i]);
2044 }
2045
2046 netif_device_attach(vi->dev);
2047
2048 rtnl_lock();
2049 virtnet_set_queues(vi, vi->curr_queue_pairs);
2050 rtnl_unlock();
2051
2052 err = register_hotcpu_notifier(&vi->nb);
2053 if (err)
2054 return err;
2055
2056 return 0;
2057 }
2058 #endif
2059
2060 static struct virtio_device_id id_table[] = {
2061 { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
2062 { 0 },
2063 };
2064
2065 static unsigned int features[] = {
2066 VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
2067 VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
2068 VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
2069 VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
2070 VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
2071 VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
2072 VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
2073 VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
2074 VIRTIO_NET_F_CTRL_MAC_ADDR,
2075 VIRTIO_F_ANY_LAYOUT,> 2076 VIRTIO_NET_F_MTU,
2077 };
2078
2079 static struct virtio_driver virtio_net_driver = {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 55156 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20160603/17a37f8c/attachment-0001.obj>
On 06/02/2016 10:06 AM, Aaron Conole wrote:> Rick Jones <rick.jones2 at 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