Re: [PATCH RFC v4 net-next 1/5] virtio_net: enable tx interrupt
On 2014/12/1 18:17, Jason Wang wrote:
On newer hosts that support delayed tx interrupts,
we probably don't have much to gain from orphaning
packets early.
Note: this might degrade performance for
hosts without event idx support.
Should be addressed by the next patch.
Cc: Rusty Russell
Cc: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Jason Wang
---
drivers/net/virtio_net.c | 132 +++
1 file changed, 88 insertions(+), 44 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..f68114e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
{
struct skb_vnet_hdr *hdr;
@@ -912,7 +951,9 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff
*skb)
sg_set_buf(sq->sg, hdr, hdr_len);
num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
}
- return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC);
+
+ return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb,
+ GFP_ATOMIC);
}
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -924,8 +965,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct
net_device *dev)
struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
bool kick = !skb->xmit_more;
- /* Free up any pending old buffers before queueing new ones. */
- free_old_xmit_skbs(sq);
I think there is no need to remove free_old_xmit_skbs here.
you could add free_old_xmit_skbs in tx_irq's napi func.
also could do this in start_xmit if you handle the race well.
I have done the same thing in ixgbe driver(free skb in ndo_start_xmit
and both in tx_irq's poll func), and it seems work well:)
I think there would be no so much interrupts in this way, also tx
interrupt coalesce is not needed.
+ virtqueue_disable_cb(sq->vq);
/* Try to transmit */
err = xmit_skb(sq, skb);
@@ -941,27 +981,19 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct
net_device *dev)
return NETDEV_TX_OK;
}
- /* Don't wait up for transmitted skbs to be freed. */
- skb_orphan(skb);
- nf_reset(skb);
-
/* Apparently nice girls don't return TX_BUSY; stop the queue
* before it gets out of hand. Naturally, this wastes entries. */
- if (sq->vq->num_free < 2+MAX_SKB_FRAGS) {
+ if (sq->vq->num_free < 2+MAX_SKB_FRAGS)
netif_stop_subqueue(dev, qnum);
- if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
- /* More just got used, free them then recheck. */
- free_old_xmit_skbs(sq);
- if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
- netif_start_subqueue(dev, qnum);
- virtqueue_disable_cb(sq->vq);
- }
- }
- }
if (kick || netif_xmit_stopped(txq))
virtqueue_kick(sq->vq);
+ if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
+ virtqueue_disable_cb(sq->vq);
+ napi_schedule(&sq->napi);
+ }
+
return NETDEV_TX_OK;
}
@@ -1138,8 +1170,10 @@ static int virtnet_close(struct net_device *dev)
/* Make sure refill_work doesn't re-enable napi! */
cancel_delayed_work_sync(&vi->refill);
- for (i = 0; i < vi->max_queue_pairs; i++)
+ for (i = 0; i < vi->max_queue_pairs; i++) {
napi_disable(&vi->rq[i].napi);
+ napi_disable(&vi->sq[i].napi);
+ }
return 0;
}
@@ -1452,8 +1486,10 @@ static void virtnet_free_queues(struct virtnet_info *vi)
{
int i;
- for (i = 0; i < vi->max_queue_pairs; i++)
+ for (i = 0; i < vi->max_queue_pairs; i++) {
netif_napi_del(&vi->rq[i].napi);
+ netif_napi_del(&vi->sq[i].napi);
+ }
kfree(vi->rq);
kfree(vi->sq);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 08/10] tun: Re-uanble UFO support.
On Thu, Dec 18, 2014 at 11:12 PM, Vlad Yasevich
wrote:
On 12/18/2014 12:51 AM, Jason Wang wrote:
- Original Message -
Now that UFO is split into v4 and v6 parts, we can bring
back v4 support without any trouble.
Continue to handle legacy applications by selecting the
IPv6 fragment id but do not change the gso type. Thist
makes sure that two legacy VMs may still communicate.
Based on original work from Ben Hutchings.
Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
CC: Ben Hutchings
Signed-off-by: Vladislav Yasevich
---
drivers/net/tun.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 9dd3746..8c32fca 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -175,7 +175,7 @@ struct tun_struct {
struct net_device *dev;
netdev_features_t set_features;
#define TUN_USER_FEATURES
(NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
-NETIF_F_TSO6)
+NETIF_F_TSO6|NETIF_F_UFO)
int vnet_hdr_sz;
int sndbuf;
@@ -1152,20 +1152,15 @@ static ssize_t tun_get_user(struct
tun_struct *tun,
struct tun_file *tfile,
skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
break;
case VIRTIO_NET_HDR_GSO_UDP:
- {
- static bool warned;
-
- if (!warned) {
- warned = true;
- netdev_warn(tun->dev,
- "%s: using disabled UFO feature; please fix this
program\n",
- current->comm);
- }
skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
- if (skb->protocol == htons(ETH_P_IPV6))
+ if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
This probably means UDPv6 with vlan does not work well, looks like
another
independent fixe and also for stable.
Ok. I can split this out.
+ /* This allows legacy application to work.
+ * Do not change the gso_type as it may
+ * not be upderstood by legacy applications.
+ */
Isn't this an issue that we use SKB_GSO_UDP for a UDPv6 packet?
Especially
consider we want to fix UFOv6 in the future? We probably can use
SKB_GSO_UDP6 here and try to fix it in tun_put_user() if a legacy
userspace
is detected.
There is a slight problem with this. This will force fragmentation
of IPv6
traffic between VMs since UFO6 wouldn't be enabled on a destination
device.
So, suddenly older VMs will see a performance regression for large
UDPv6 traffic.
With this code, a legacy VM will continue to receive large UDPv6
traffic.
New VMs will have an updated virtio-net which will reset the gso type
on input.
I get the point and that's why you still want ipv6 offload helpers to
accept SKB_GSO_UDP. So SKB_GSO_UDP was still ambigious since it can be a
UDP6 packet in fact.
I'm not sure, but probably we could just leave the regression of legacy
guest as is and fix current kernel. The sooner we eliminate the
ambigious
the better for future development.
ipv6_proxy_select_ident(skb);
Question still for vlan, is network header correctly set here?
Looks like
ipv6_proxy_select_ident() depends on correct network header to work.
+
Yes, you are right... I wonder how that worked. I'll re-test.
Thanks
-vlad
}
break;
- }
default:
tun->dev->stats.rx_frame_errors++;
kfree_skb(skb);
@@ -1273,6 +1268,8 @@ static ssize_t tun_put_user(struct
tun_struct *tun,
gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+ else if (sinfo->gso_type & SKB_GSO_UDP)
+ gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
else {
pr_err("unexpected GSO type: "
"0x%x, gso_size %d, hdr_len %d\n",
@@ -1780,6 +1777,11 @@ static int set_offload(struct tun_struct
*tun,
unsigned long arg)
features |= NETIF_F_TSO6;
arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
}
+
+ if (arg & TUN_F_UFO) {
+ features |= NETIF_F_UFO;
+ arg &= ~TUN_F_UFO;
+ }
}
/* This gives the user a way to test for new features in future
by
--
1.9.3
--
To unsubscribe from this list: send the li
Re: [PATCH 0/6] virtio 1.0 fixups, tweaks
"Michael S. Tsirkin" writes: > Fixes a couple of minor compliance issues in new virtio 1.0 code. > Plus, adds a couple of minor cleanups - not bugfixes, > but seem safe enough for 3.19. OK, I've applied these. Thanks, Rusty. > Michael S. Tsirkin (6): > virtio: set VIRTIO_CONFIG_S_FEATURES_OK on restore > virtio_config: fix virtio_cread_bytes > virtio_pci_common.h: drop VIRTIO_PCI_NO_LEGACY > virtio_pci: move probe to common file > virtio_pci: add VIRTIO_PCI_NO_LEGACY > virtio: core support for config generation > > drivers/virtio/virtio_pci_common.h | 7 +++ > include/linux/virtio_config.h | 29 - > include/uapi/linux/virtio_pci.h| 15 ++- > drivers/virtio/virtio.c| 37 +++-- > drivers/virtio/virtio_pci_common.c | 34 +- > drivers/virtio/virtio_pci_legacy.c | 24 ++-- > 6 files changed, 99 insertions(+), 47 deletions(-) > > -- > MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/6] tools/virtio: more stubs
"Michael S. Tsirkin" writes: > As usual, add more stubs to fix test build after main > codebase changes. This doesn't apply, since: > diff --git a/tools/virtio/linux/virtio_config.h > b/tools/virtio/linux/virtio_config.h > index dafe1c9..806d683 100644 > --- a/tools/virtio/linux/virtio_config.h > +++ b/tools/virtio/linux/virtio_config.h > @@ -1,8 +1,7 @@ > #include > #include > +#include > > -#define VIRTIO_TRANSPORT_F_START 28 > -#define VIRTIO_TRANSPORT_F_END 32 > /* Version in master has no headers in this file. What's this based on? Thanks, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/3] fix up vringh/mic sparse errors
"Michael S. Tsirkin" writes: > This fixes remaining sparse warnings in vringh and mic by using > virtio 1.0 compliant wrappers. > > This also needs by get_user patches to avoid getting warnings > from these calls. > > Tested by running vringh_test. > > Rusty, I prefer fixing all these warnings for 3.19, any objections? OK, I've queued these too. Cheers, Rusty. > Michael S. Tsirkin (3): > vringh: 64 bit features > vringh: initial virtio 1.0 support > mic/host: initial virtio 1.0 support > > include/linux/vringh.h | 37 ++- > drivers/misc/mic/host/mic_debugfs.c | 18 -- > drivers/vhost/vringh.c | 125 > ++-- > 3 files changed, 123 insertions(+), 57 deletions(-) > > -- > MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 01/10] core: Split out UFO6 support
On Thu, Dec 18, 2014 at 07:35:46PM +0200, Michael S. Tsirkin wrote: > > > We should either generate our own ID, > > > like we always did, or make sure we don't accept > > > these packets. > > > Second option is almost sure to break userspace, > > > so it seems we must do the first one. > > > > > > > Right. This was missing from packet sockets. I can fix it. > > > > -vlad > > Also, this can't be a patch on top, since we don't > want bisect to give us configurations which > can BUG(). So how doing this in stages: 1. add helper that checks skb GSO type. If it is SKB_GSO_UDP, check for IPv6, and generate the fragment ID. Call this helper in - virtio net rx path - tun rx path (get user) - macvtap tx patch (get user) - packet socket tx patch (get user) 2. Put back UFO flag everywhere: virtio,tun,macvtap,packet, since we can handle IPv6 now even if it's suboptimal. Above two seem like smalling patches, appropriate for stable. Next, work on a new feature to pass fragment ID in virtio header: 3. split UFO/UFO6 as you did here, but add code in above 4 places to convert UDP to UDP6, additionally, add code in - virtio net tx path - tun tx path (get user) - macvtap rx patch (put user) - packet socket rx patch (put user) to convert UDP6 to UDP. step 3 needs to be bisect-clean, somehow. 4. add new field in header, new feature bit for virtio net to gate it, new ioctls to tun,macvtap,packet socket. These two are more like optimizations, so not stable material. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 01/10] core: Split out UFO6 support
On Thu, Dec 18, 2014 at 10:01:35AM -0500, Vlad Yasevich wrote:
> On 12/18/2014 02:54 AM, Michael S. Tsirkin wrote:
> > Cc Dave, pls remember to do this next time otherwise
> > your patches won't get merged :)
> >
> > On Wed, Dec 17, 2014 at 06:31:50PM -0500, Vlad Yasevich wrote:
> >> On 12/17/2014 05:45 PM, Michael S. Tsirkin wrote:
> >>> On Wed, Dec 17, 2014 at 01:20:46PM -0500, Vladislav Yasevich wrote:
> Split IPv6 support for UFO into its own feature similiar to TSO.
> This will later allow us to re-enable UFO support for virtio-net
> devices.
>
> Signed-off-by: Vladislav Yasevich
> ---
> include/linux/netdev_features.h | 7 +--
> include/linux/netdevice.h | 1 +
> include/linux/skbuff.h | 1 +
> net/core/dev.c | 35 +++
> net/core/ethtool.c | 2 +-
> 5 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/netdev_features.h
> b/include/linux/netdev_features.h
> index dcfdecb..a078945 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -48,8 +48,9 @@ enum {
> NETIF_F_GSO_UDP_TUNNEL_BIT, /* ... UDP TUNNEL with TSO */
> NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT,/* ... UDP TUNNEL with TSO &
> CSUM */
> NETIF_F_GSO_MPLS_BIT, /* ... MPLS segmentation */
> +NETIF_F_UFO6_BIT, /* ... UDPv6 fragmentation */
> /**/NETIF_F_GSO_LAST = /* last bit, see GSO_MASK */
> -NETIF_F_GSO_MPLS_BIT,
> +NETIF_F_UFO6_BIT,
>
> NETIF_F_FCOE_CRC_BIT, /* FCoE CRC32 */
> NETIF_F_SCTP_CSUM_BIT, /* SCTP checksum offload */
> @@ -109,6 +110,7 @@ enum {
> #define NETIF_F_TSO_ECN __NETIF_F(TSO_ECN)
> #define NETIF_F_TSO __NETIF_F(TSO)
> #define NETIF_F_UFO __NETIF_F(UFO)
> +#define NETIF_F_UFO6__NETIF_F(UFO6)
> #define NETIF_F_VLAN_CHALLENGED __NETIF_F(VLAN_CHALLENGED)
> #define NETIF_F_RXFCS __NETIF_F(RXFCS)
> #define NETIF_F_RXALL __NETIF_F(RXALL)
> @@ -141,7 +143,7 @@ enum {
>
> /* List of features with software fallbacks. */
> #define NETIF_F_GSO_SOFTWARE(NETIF_F_TSO | NETIF_F_TSO_ECN | \
> - NETIF_F_TSO6 | NETIF_F_UFO)
> + NETIF_F_TSO6 | NETIF_F_UFO |
> NETIF_F_UFO6)
>
> #define NETIF_F_GEN_CSUMNETIF_F_HW_CSUM
> #define NETIF_F_V4_CSUM (NETIF_F_GEN_CSUM | NETIF_F_IP_CSUM)
> @@ -149,6 +151,7 @@ enum {
> #define NETIF_F_ALL_CSUM(NETIF_F_V4_CSUM | NETIF_F_V6_CSUM)
>
> #define NETIF_F_ALL_TSO (NETIF_F_TSO | NETIF_F_TSO6 |
> NETIF_F_TSO_ECN)
> +#define NETIF_F_ALL_UFO (NETIF_F_UFO | NETIF_F_UFO6)
>
> #define NETIF_F_ALL_FCOE(NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
> NETIF_F_FSO)
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 74fd5d3..86af10a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3559,6 +3559,7 @@ static inline bool net_gso_ok(netdev_features_t
> features, int gso_type)
> /* check flags correspondence */
> BUILD_BUG_ON(SKB_GSO_TCPV4 != (NETIF_F_TSO >>
> NETIF_F_GSO_SHIFT));
> BUILD_BUG_ON(SKB_GSO_UDP != (NETIF_F_UFO >>
> NETIF_F_GSO_SHIFT));
> +BUILD_BUG_ON(SKB_GSO_UDP6!= (NETIF_F_UFO6 >>
> NETIF_F_GSO_SHIFT));
> BUILD_BUG_ON(SKB_GSO_DODGY != (NETIF_F_GSO_ROBUST >>
> NETIF_F_GSO_SHIFT));
> BUILD_BUG_ON(SKB_GSO_TCP_ECN != (NETIF_F_TSO_ECN >>
> NETIF_F_GSO_SHIFT));
> BUILD_BUG_ON(SKB_GSO_TCPV6 != (NETIF_F_TSO6 >>
> NETIF_F_GSO_SHIFT));
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6c8b6f6..8538b67 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -372,6 +372,7 @@ enum {
>
> SKB_GSO_MPLS = 1 << 12,
>
> +SKB_GSO_UDP6 = 1 << 13
> };
>
> #if BITS_PER_LONG > 32
> >>>
> >>> So this implies anything getting GSO packets e.g.
> >>> from userspace now needs to check IP version to
> >>> set GSO type correctly.
> >>>
> >>> I think you missed some places that do this, e.g. af_packet
> >>> sockets.
> >>>
> >>
> >> I looked at af_packet sockets and they set this only in the event
> >> vnet header has been used with a GSO type. In this case, the user
> >> already knows the the type.
> >
> > Imagine you are receiving a packet:
> >
> > if (vnet_hdr.gso_type !
Re: [PATCH RFC v6 17/20] virtio-net: enable virtio 1.0
On Tue, 16 Dec 2014 15:10:04 +0200 "Michael S. Tsirkin" wrote: > On Thu, Dec 11, 2014 at 02:25:19PM +0100, Cornelia Huck wrote: > > virtio-net (non-vhost) now should have everything in place to support > > virtio 1.0: let's enable the feature bit for it. > > > > Note that VIRTIO_F_VERSION_1 is technically a transport feature; once > > every device is ready for virtio 1.0, we can move setting this > > feature bit out of the individual devices. > > > > Signed-off-by: Cornelia Huck > > So to use this with e.g. tun, you need to make tun device LE. > I posted a kernel patch 1418732988-3535-1-git-send-email-...@redhat.com > with TUNSETVNETLE/TUNGETVNETLE ioctls to support it. > > But you still need to call them in qemu, and disable virtio-1.0 > if not there. I had planned on playing with this a bit, but didn't come around to doing it. I'm afraid it will have to wait until next year. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 09/10] macvtap: Re-enable UFO support
On 12/18/2014 02:55 AM, Michael S. Tsirkin wrote:
> On Wed, Dec 17, 2014 at 09:43:51PM -0500, Vlad Yasevich wrote:
>> On 12/17/2014 05:41 PM, Michael S. Tsirkin wrote:
>>> On Wed, Dec 17, 2014 at 01:20:54PM -0500, Vladislav Yasevich wrote:
Now that UFO is split into v4 and v6 parts, we can bring
back v4 support. Continue to handle legacy applications
by selecting the ipv6 fagment id but do not change the
gso type. This allows 2 legacy VMs to continue to communicate.
Based on original work from Ben Hutchings.
Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
CC: Ben Hutchings
Signed-off-by: Vladislav Yasevich
---
drivers/net/macvtap.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 880cc09..75febd4 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -66,7 +66,7 @@ static struct cdev macvtap_cdev;
static const struct proto_ops macvtap_socket_ops;
#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
-NETIF_F_TSO6)
+NETIF_F_TSO6 | NETIF_F_UFO)
#define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
#define TAP_FEATURES (NETIF_F_GSO | NETIF_F_SG)
@@ -570,11 +570,14 @@ static int macvtap_skb_from_vnet_hdr(struct sk_buff
*skb,
gso_type = SKB_GSO_TCPV6;
break;
case VIRTIO_NET_HDR_GSO_UDP:
- pr_warn_once("macvtap: %s: using disabled UFO feature;
please fix this program\n",
- current->comm);
gso_type = SKB_GSO_UDP;
- if (skb->protocol == htons(ETH_P_IPV6))
+ if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
+ /* This is to support legacy appliacations.
+ * Do not change the gso_type as legacy apps
+ * may not know about the new type.
+ */
ipv6_proxy_select_ident(skb);
+ }
break;
default:
return -EINVAL;
@@ -619,6 +622,8 @@ static void macvtap_skb_to_vnet_hdr(const struct
sk_buff *skb,
vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+ else if (sinfo->gso_type & SKB_GSO_UDP)
+ vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP;
else
BUG();
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
@@ -955,6 +960,9 @@ static int set_offload(struct macvtap_queue *q,
unsigned long arg)
if (arg & TUN_F_TSO6)
feature_mask |= NETIF_F_TSO6;
}
+
+ if (arg & TUN_F_UFO)
+ feature_mask |= NETIF_F_UFO;
}
/* tun/tap driver inverts the usage for TSO offloads, where
@@ -965,7 +973,7 @@ static int set_offload(struct macvtap_queue *q,
unsigned long arg)
* When user space turns off TSO, we turn off GSO/LRO so that
* user-space will not receive TSO frames.
*/
- if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6))
+ if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO))
features |= RX_OFFLOADS;
else
features &= ~RX_OFFLOADS;
>>>
>>> By the way this logic is completely broken, even without your patch,
>>> isn't it?
>>>
>>> It says: enable LRO+GRO if any of NETIF_F_TSO | NETIF_F_TSO6 |
>>> NETIF_F_UFO set.
>>>
>>> So what happens if I enable TSO only?
>>> LRO gets enabled so I can still get TSO6 packets.
>>>
>>>
>>> This really should be:
>>>
>>> if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO) ==
>>> (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO)
>>>
>>>
>>> fixing this probably should be a separate patch before your
>>> series, and Cc stable.
>>
>> Actually, all this LRO/GRO feature manipulation on the macvtap device is
>> rather pointless as those features aren't really checked at any point
>> on the macvtap receive path. GRO calls are done from napi context on
>> the lowest device, so by the time a packet hits macvtap, GRO/LRO has
>> already been done.
>>
>> So it doesn't really matter that we disable them incorrectly. I
>> can send a separate patch to clean this up.
>
> Hmm so if userspace says it can't handle TSO packets,
> it might get them anyway?
No. It will get individual segments because in macvtap_handle_frame
we use user specified features to decide if segmentation needs
to be perfo
Re: [PATCH 08/10] tun: Re-uanble UFO support.
On 12/18/2014 12:51 AM, Jason Wang wrote:
>
>
> - Original Message -
>> Now that UFO is split into v4 and v6 parts, we can bring
>> back v4 support without any trouble.
>>
>> Continue to handle legacy applications by selecting the
>> IPv6 fragment id but do not change the gso type. Thist
>> makes sure that two legacy VMs may still communicate.
>>
>> Based on original work from Ben Hutchings.
>>
>> Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
>> CC: Ben Hutchings
>> Signed-off-by: Vladislav Yasevich
>> ---
>> drivers/net/tun.c | 26 ++
>> 1 file changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 9dd3746..8c32fca 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -175,7 +175,7 @@ struct tun_struct {
>> struct net_device *dev;
>> netdev_features_t set_features;
>> #define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
>> - NETIF_F_TSO6)
>> + NETIF_F_TSO6|NETIF_F_UFO)
>>
>> int vnet_hdr_sz;
>> int sndbuf;
>> @@ -1152,20 +1152,15 @@ static ssize_t tun_get_user(struct tun_struct *tun,
>> struct tun_file *tfile,
>> skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>> break;
>> case VIRTIO_NET_HDR_GSO_UDP:
>> -{
>> -static bool warned;
>> -
>> -if (!warned) {
>> -warned = true;
>> -netdev_warn(tun->dev,
>> -"%s: using disabled UFO feature;
>> please fix this program\n",
>> -current->comm);
>> -}
>> skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
>> -if (skb->protocol == htons(ETH_P_IPV6))
>> +if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
>
> This probably means UDPv6 with vlan does not work well, looks like another
> independent fixe and also for stable.
Ok. I can split this out.
>> +/* This allows legacy application to work.
>> + * Do not change the gso_type as it may
>> + * not be upderstood by legacy applications.
>> + */
>
> Isn't this an issue that we use SKB_GSO_UDP for a UDPv6 packet? Especially
> consider we want to fix UFOv6 in the future? We probably can use
> SKB_GSO_UDP6 here and try to fix it in tun_put_user() if a legacy userspace
> is detected.
There is a slight problem with this. This will force fragmentation of IPv6
traffic between VMs since UFO6 wouldn't be enabled on a destination device.
So, suddenly older VMs will see a performance regression for large UDPv6
traffic.
With this code, a legacy VM will continue to receive large UDPv6 traffic.
New VMs will have an updated virtio-net which will reset the gso type on input.
>> ipv6_proxy_select_ident(skb);
>
> Question still for vlan, is network header correctly set here? Looks like
> ipv6_proxy_select_ident() depends on correct network header to work.
>> +
Yes, you are right... I wonder how that worked. I'll re-test.
Thanks
-vlad
}
>> break;
>> -}
>> default:
>> tun->dev->stats.rx_frame_errors++;
>> kfree_skb(skb);
>> @@ -1273,6 +1268,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>> gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
>> else if (sinfo->gso_type & SKB_GSO_TCPV6)
>> gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
>> +else if (sinfo->gso_type & SKB_GSO_UDP)
>> +gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
>> else {
>> pr_err("unexpected GSO type: "
>> "0x%x, gso_size %d, hdr_len %d\n",
>> @@ -1780,6 +1777,11 @@ static int set_offload(struct tun_struct *tun,
>> unsigned long arg)
>> features |= NETIF_F_TSO6;
>> arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
>> }
>> +
>> +if (arg & TUN_F_UFO) {
>> +features |= NETIF_F_UFO;
>> +arg &= ~TUN_F_UFO;
>> +}
>> }
>>
>> /* This gives the user a way to test for new features in future by
>> --
>> 1.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfound
Re: [PATCH 01/10] core: Split out UFO6 support
On 12/18/2014 02:54 AM, Michael S. Tsirkin wrote:
> Cc Dave, pls remember to do this next time otherwise
> your patches won't get merged :)
>
> On Wed, Dec 17, 2014 at 06:31:50PM -0500, Vlad Yasevich wrote:
>> On 12/17/2014 05:45 PM, Michael S. Tsirkin wrote:
>>> On Wed, Dec 17, 2014 at 01:20:46PM -0500, Vladislav Yasevich wrote:
Split IPv6 support for UFO into its own feature similiar to TSO.
This will later allow us to re-enable UFO support for virtio-net
devices.
Signed-off-by: Vladislav Yasevich
---
include/linux/netdev_features.h | 7 +--
include/linux/netdevice.h | 1 +
include/linux/skbuff.h | 1 +
net/core/dev.c | 35 +++
net/core/ethtool.c | 2 +-
5 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/include/linux/netdev_features.h
b/include/linux/netdev_features.h
index dcfdecb..a078945 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -48,8 +48,9 @@ enum {
NETIF_F_GSO_UDP_TUNNEL_BIT, /* ... UDP TUNNEL with TSO */
NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT,/* ... UDP TUNNEL with TSO & CSUM */
NETIF_F_GSO_MPLS_BIT, /* ... MPLS segmentation */
+ NETIF_F_UFO6_BIT, /* ... UDPv6 fragmentation */
/**/NETIF_F_GSO_LAST = /* last bit, see GSO_MASK */
- NETIF_F_GSO_MPLS_BIT,
+ NETIF_F_UFO6_BIT,
NETIF_F_FCOE_CRC_BIT, /* FCoE CRC32 */
NETIF_F_SCTP_CSUM_BIT, /* SCTP checksum offload */
@@ -109,6 +110,7 @@ enum {
#define NETIF_F_TSO_ECN __NETIF_F(TSO_ECN)
#define NETIF_F_TSO __NETIF_F(TSO)
#define NETIF_F_UFO __NETIF_F(UFO)
+#define NETIF_F_UFO6 __NETIF_F(UFO6)
#define NETIF_F_VLAN_CHALLENGED __NETIF_F(VLAN_CHALLENGED)
#define NETIF_F_RXFCS __NETIF_F(RXFCS)
#define NETIF_F_RXALL __NETIF_F(RXALL)
@@ -141,7 +143,7 @@ enum {
/* List of features with software fallbacks. */
#define NETIF_F_GSO_SOFTWARE (NETIF_F_TSO | NETIF_F_TSO_ECN | \
- NETIF_F_TSO6 | NETIF_F_UFO)
+ NETIF_F_TSO6 | NETIF_F_UFO | NETIF_F_UFO6)
#define NETIF_F_GEN_CSUM NETIF_F_HW_CSUM
#define NETIF_F_V4_CSUM (NETIF_F_GEN_CSUM | NETIF_F_IP_CSUM)
@@ -149,6 +151,7 @@ enum {
#define NETIF_F_ALL_CSUM (NETIF_F_V4_CSUM | NETIF_F_V6_CSUM)
#define NETIF_F_ALL_TSO (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
+#define NETIF_F_ALL_UFO (NETIF_F_UFO | NETIF_F_UFO6)
#define NETIF_F_ALL_FCOE (NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
NETIF_F_FSO)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 74fd5d3..86af10a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3559,6 +3559,7 @@ static inline bool net_gso_ok(netdev_features_t
features, int gso_type)
/* check flags correspondence */
BUILD_BUG_ON(SKB_GSO_TCPV4 != (NETIF_F_TSO >> NETIF_F_GSO_SHIFT));
BUILD_BUG_ON(SKB_GSO_UDP != (NETIF_F_UFO >> NETIF_F_GSO_SHIFT));
+ BUILD_BUG_ON(SKB_GSO_UDP6!= (NETIF_F_UFO6 >> NETIF_F_GSO_SHIFT));
BUILD_BUG_ON(SKB_GSO_DODGY != (NETIF_F_GSO_ROBUST >>
NETIF_F_GSO_SHIFT));
BUILD_BUG_ON(SKB_GSO_TCP_ECN != (NETIF_F_TSO_ECN >> NETIF_F_GSO_SHIFT));
BUILD_BUG_ON(SKB_GSO_TCPV6 != (NETIF_F_TSO6 >> NETIF_F_GSO_SHIFT));
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6c8b6f6..8538b67 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -372,6 +372,7 @@ enum {
SKB_GSO_MPLS = 1 << 12,
+ SKB_GSO_UDP6 = 1 << 13
};
#if BITS_PER_LONG > 32
>>>
>>> So this implies anything getting GSO packets e.g.
>>> from userspace now needs to check IP version to
>>> set GSO type correctly.
>>>
>>> I think you missed some places that do this, e.g. af_packet
>>> sockets.
>>>
>>
>> I looked at af_packet sockets and they set this only in the event
>> vnet header has been used with a GSO type. In this case, the user
>> already knows the the type.
>
> Imagine you are receiving a packet:
>
> if (vnet_hdr.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> switch (vnet_hdr.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> case VIRTIO_NET_HDR_GSO_TCPV4:
> gso_type = SKB_GSO_TCPV4;
> break;
> case VIRTIO_NET_HDR_GSO_TCPV6:
> gso_type = SKB_GSO_TCPV6;
> break;
> case VIRTIO
[PULL] vhost: cleanups and fixes
The following changes since commit f01a2a811ae04124fc9382925038fcbbd2f0b7c8: virtio_ccw: finalize_features error handling (2014-12-09 21:42:06 +0200) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus for you to fetch changes up to 5ff16110c637726111662c1df41afd9df7ef36bd: virtio_pci: restore module attributes (2014-12-17 00:59:40 +0200) vhost/virtio: virtio 1.0 related fixes Most importantly, this fixes using virtio_pci as a module. Further, the big virtio 1.0 conversion missed a couple of places. This fixes them up. This isn't 100% sparse-clean yet because on many architectures get_user triggers sparse warnings when used with __bitwise tag (when same tag is on both pointer and value read). I posted a patchset to fix it up by adding __force on all arches that don't already have it (many do), when that's merged these warnings will go away. Cc: Rusty Russell Signed-off-by: Michael S. Tsirkin Herbert Xu (1): virtio_pci: restore module attributes Michael S. Tsirkin (15): virtio: set VIRTIO_CONFIG_S_FEATURES_OK on restore virtio_config: fix virtio_cread_bytes virtio_pci_common.h: drop VIRTIO_PCI_NO_LEGACY virtio_pci: move probe to common file virtio_pci: add VIRTIO_PCI_NO_LEGACY virtio: core support for config generation tools/virtio: more stubs tools/virtio: fix vringh test tools/virtio: 64 bit features tools/virtio: enable -Werror tools/virtio: add virtio 1.0 in virtio_test tools/virtio: add virtio 1.0 in vringh_test vringh: 64 bit features vringh: update for virtio 1.0 APIs mic/host: fix up virtio 1.0 APIs drivers/virtio/virtio_pci_common.h | 7 +- include/linux/virtio_config.h | 29 +++- include/linux/vringh.h | 37 +- include/uapi/linux/virtio_pci.h| 15 ++-- tools/virtio/linux/virtio.h| 1 + tools/virtio/linux/virtio_byteorder.h | 8 +++ tools/virtio/linux/virtio_config.h | 70 +- tools/virtio/uapi/linux/virtio_types.h | 1 + drivers/misc/mic/host/mic_debugfs.c| 18 +++-- drivers/vhost/vringh.c | 125 - drivers/virtio/virtio.c| 37 ++ drivers/virtio/virtio_pci_common.c | 39 +- drivers/virtio/virtio_pci_legacy.c | 24 +-- tools/virtio/virtio_test.c | 15 +++- tools/virtio/vringh_test.c | 5 +- tools/virtio/Makefile | 2 +- 16 files changed, 324 insertions(+), 109 deletions(-) create mode 100644 tools/virtio/linux/virtio_byteorder.h create mode 100644 tools/virtio/uapi/linux/virtio_types.h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
