RE: [PATCH 1/5] net: virtio dynamically disable/enable LRO

RE: [PATCH 1/5] net: virtio dynamically disable/enable LRO

[Caleb Raitto]

On Fri, Nov 18, 2016 at 11:00 AM, John Fastabend
<john.fastabend@gmail.com> wrote:
>
> From: John Fastabend <john.fastabend@gmail.com>
> Date: Fri, Nov 18, 2016 at 11:00 AM
> Subject: [PATCH 1/5] net: virtio dynamically disable/enable LRO
> To: <tgraf@suug.ch>, <shm@cumulusnetworks.com>, <alexei.starovoitov@gmail.com>, <daniel@iogearbox.net>, <davem@davemloft.net>
> Cc: <john.r.fastabend@intel.com>, <netdev@vger.kernel.org>, <bblanco@plumgrid.com>, <john.fastabend@gmail.com>, <brouer@redhat.com>
>
> This adds support for dynamically setting the LRO feature flag. The
> message to control guest features in the backend uses the
> CTRL_GUEST_OFFLOADS msg type.
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>  drivers/net/virtio_net.c |   43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 2cafd12..0758cae 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1419,6 +1419,41 @@ static void virtnet_init_settings(struct net_device *dev)

Hi,

I work on Google Compute Engine, and I noticed that you had this
unmerged patch that allows controlling offloads using ethtool for
virtionet.

We are interested in merging this patch as it will be useful in
debugging customer issues related to offloads and MTU.

If work remains to get the patch merged, I'm willing to help out.

Thanks,
-Caleb

Re: [PATCH 1/5] net: virtio dynamically disable/enable LRO

[Michael S. Tsirkin]

On Wed, Oct 18, 2017 at 07:18:31PM -0700, Caleb Raitto wrote:
> On Fri, Nov 18, 2016 at 11:00 AM, John Fastabend
> <john.fastabend@gmail.com> wrote:
> >
> > From: John Fastabend <john.fastabend@gmail.com>
> > Date: Fri, Nov 18, 2016 at 11:00 AM
> > Subject: [PATCH 1/5] net: virtio dynamically disable/enable LRO
> > To: <tgraf@suug.ch>, <shm@cumulusnetworks.com>, <alexei.starovoitov@gmail.com>, <daniel@iogearbox.net>, <davem@davemloft.net>
> > Cc: <john.r.fastabend@intel.com>, <netdev@vger.kernel.org>, <bblanco@plumgrid.com>, <john.fastabend@gmail.com>, <brouer@redhat.com>
> >
> > This adds support for dynamically setting the LRO feature flag. The
> > message to control guest features in the backend uses the
> > CTRL_GUEST_OFFLOADS msg type.
> > Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> > ---
> >  drivers/net/virtio_net.c |   43 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 2cafd12..0758cae 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1419,6 +1419,41 @@ static void virtnet_init_settings(struct net_device *dev)
> 
> Hi,
> 
> I work on Google Compute Engine, and I noticed that you had this
> unmerged patch that allows controlling offloads using ethtool for
> virtionet.
> 
> We are interested in merging this patch as it will be useful in
> debugging customer issues related to offloads and MTU.
> 
> If work remains to get the patch merged, I'm willing to help out.
> 
> Thanks,
> -Caleb

This patch needs to be rebased on top of latest patches by Jason.
In particular, there's code to disable/enable offloads automatically.
That either needs to go away if user configured them manually,
or the conflict resolved.

-- 
MST

[PATCH 0/5] XDP for virtio_net

[John Fastabend]

This implements virtio_net for the mergeable buffers and big_packet
modes. I tested this with vhost_net running on qemu and did not see
any issues.

There are some restrictions for XDP to be enabled (see patch 3) for
more details.

  1. LRO must be off
  2. MTU must be less than PAGE_SIZE
  3. queues must be available to dedicate to XDP
  4. num_bufs received in mergeable buffers must be 1
  5. big_packet mode must have all data on single page

Please review any comments/feedback welcome as always.

Thanks,
John
---

John Fastabend (4):
      net: virtio dynamically disable/enable LRO
      net: xdp: add invalid buffer warning
      virtio_net: add dedicated XDP transmit queues
      virtio_net: add XDP_TX support

Shrijeet Mukherjee (1):
      virtio_net: Add XDP support


 drivers/net/virtio_net.c |  264 +++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/filter.h   |    1 
 net/core/filter.c        |    6 +
 3 files changed, 267 insertions(+), 4 deletions(-)

[PATCH 1/5] net: virtio dynamically disable/enable LRO

[John Fastabend]

This adds support for dynamically setting the LRO feature flag. The
message to control guest features in the backend uses the
CTRL_GUEST_OFFLOADS msg type.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/virtio_net.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 2cafd12..0758cae 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1419,6 +1419,41 @@ static void virtnet_init_settings(struct net_device *dev)
 	.set_settings = virtnet_set_settings,
 };
 
+static int virtnet_set_features(struct net_device *netdev,
+				netdev_features_t features)
+{
+	struct virtnet_info *vi = netdev_priv(netdev);
+	struct virtio_device *vdev = vi->vdev;
+	struct scatterlist sg;
+	u64 offloads = 0;
+
+	if (features & NETIF_F_LRO)
+		offloads |= (1 << VIRTIO_NET_F_GUEST_TSO4) |
+			    (1 << VIRTIO_NET_F_GUEST_TSO6);
+
+	if (features & NETIF_F_RXCSUM)
+		offloads |= (1 << VIRTIO_NET_F_GUEST_CSUM);
+
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) {
+		sg_init_one(&sg, &offloads, sizeof(uint64_t));
+		if (!virtnet_send_command(vi,
+					  VIRTIO_NET_CTRL_GUEST_OFFLOADS,
+					  VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET,
+					  &sg)) {
+			dev_warn(&netdev->dev,
+				 "Failed to set guest offloads by virtnet command.\n");
+			return -EINVAL;
+		}
+	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) &&
+		   !virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+		dev_warn(&netdev->dev,
+			 "No support for setting offloads pre version_1.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static const struct net_device_ops virtnet_netdev = {
 	.ndo_open            = virtnet_open,
 	.ndo_stop   	     = virtnet_close,
@@ -1435,6 +1470,7 @@ static void virtnet_init_settings(struct net_device *dev)
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	.ndo_busy_poll		= virtnet_busy_poll,
 #endif
+	.ndo_set_features	= virtnet_set_features,
 };
 
 static void virtnet_config_changed_work(struct work_struct *work)
@@ -1810,6 +1846,12 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
 		dev->features |= NETIF_F_RXCSUM;
 
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) &&
+	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6)) {
+		dev->features |= NETIF_F_LRO;
+		dev->hw_features |= NETIF_F_LRO;
+	}
+
 	dev->vlan_features = dev->features;
 
 	/* MTU range: 68 - 65535 */
@@ -2049,6 +2091,7 @@ static int virtnet_restore(struct virtio_device *vdev)
 	VIRTIO_NET_F_CTRL_MAC_ADDR,
 	VIRTIO_F_ANY_LAYOUT,
 	VIRTIO_NET_F_MTU,
+	VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
 };
 
 static struct virtio_driver virtio_net_driver = {