| Message ID | 20161202204945.4331.2419.stgit@john-Precision-Tower-5810 |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: John Fastabend <john.fastabend@gmail.com> Date: Fri, 02 Dec 2016 12:49:45 -0800 > + 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)) { Hmmm, to me this reads as: if (X) { ... else if (X && ...) { I don't see how the second basic block can ever execute. If the virtio has the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature, we will execute only the first basic block. Maybe I misunderstand the logic for whatever reason.
On Fri, Dec 02, 2016 at 12:49:45PM -0800, John Fastabend wrote: > 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 | 45 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 44 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index a21d93a..d814e7cb 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)) { No need for VIRTIO_F_VERSION_1 here. > + 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) > @@ -1815,6 +1851,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 */ > @@ -2057,7 +2099,8 @@ static int virtnet_restore(struct virtio_device *vdev) > 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_MTU > + VIRTIO_NET_F_MTU, \ > + VIRTIO_NET_F_CTRL_GUEST_OFFLOADS > > static unsigned int features[] = { > VIRTNET_FEATURES,
On 16-12-03 09:36 PM, Michael S. Tsirkin wrote: > On Fri, Dec 02, 2016 at 12:49:45PM -0800, John Fastabend wrote: >> 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 | 45 ++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 44 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >> index a21d93a..d814e7cb 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)) { > > No need for VIRTIO_F_VERSION_1 here. > Yep Dave pointed it out as well. Also I found a bug on driver unload path where XDP tx queues are not cleaned up correctly so will need a v5 for that. Thanks, John
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index a21d93a..d814e7cb 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) @@ -1815,6 +1851,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 */ @@ -2057,7 +2099,8 @@ static int virtnet_restore(struct virtio_device *vdev) 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_MTU + VIRTIO_NET_F_MTU, \ + VIRTIO_NET_F_CTRL_GUEST_OFFLOADS static unsigned int features[] = { VIRTNET_FEATURES,
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 | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-)