| Message ID | CAOMZO5B7Yx7tRy8Q5wi6ngcD0FDJzKXHOEGPXyUo52+f3oUHEg@mail.gmail.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Jul 02, 2013 at 06:01:12PM -0300, Fabio Estevam wrote: > On Tue, Jul 2, 2013 at 4:59 PM, Thomas Zeitlhofer > <thomas.zeitlhofer@nt.tuwien.ac.at> wrote: > > Commit "tuntap: set SOCK_ZEROCOPY flag during open" introduces a > > regression which is observed with live migration of qemu/kvm based > > virtual machines that are connected to an openvswitch bridge. > > > > Reverting this commit (b26c93c46a3dec25ed236d4ba6107eb4ed5d9401 in > > v3.9.8 and accordingly 19a6afb23e5d323e1245baa4e62755492b2f1200 in > > v3.10) fixes the following problem: > > Should the sock_set_flag stay in tun_set_iff as it was prior to 54f968d6efd? > > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -1652,6 +1652,7 @@ static int tun_set_iff(struct net *net, struct > file *file, struct ifreq *ifr) > tun->txflt.count = 0; > tun->vnet_hdr_sz = sizeof(struct virtio_net_hdr); > > + sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); > tun->filter_attached = false; > tun->sndbuf = tfile->socket.sk->sk_sndbuf; > > @@ -2159,8 +2160,6 @@ static int tun_chr_open(struct inode *inode, > struct file * file) > set_bit(SOCK_EXTERNALLY_ALLOCATED, &tfile->socket.flags); > INIT_LIST_HEAD(&tfile->next); > > - sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); > - > return 0; > } I guess no, as this also leads to a kernel panic (tested against v3.10). -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/03/2013 06:06 AM, Thomas Zeitlhofer wrote: > On Tue, Jul 02, 2013 at 06:01:12PM -0300, Fabio Estevam wrote: >> On Tue, Jul 2, 2013 at 4:59 PM, Thomas Zeitlhofer >> <thomas.zeitlhofer@nt.tuwien.ac.at> wrote: >>> Commit "tuntap: set SOCK_ZEROCOPY flag during open" introduces a >>> regression which is observed with live migration of qemu/kvm based >>> virtual machines that are connected to an openvswitch bridge. >>> >>> Reverting this commit (b26c93c46a3dec25ed236d4ba6107eb4ed5d9401 in >>> v3.9.8 and accordingly 19a6afb23e5d323e1245baa4e62755492b2f1200 in >>> v3.10) fixes the following problem: >> Should the sock_set_flag stay in tun_set_iff as it was prior to 54f968d6efd? >> >> --- a/drivers/net/tun.c >> +++ b/drivers/net/tun.c >> @@ -1652,6 +1652,7 @@ static int tun_set_iff(struct net *net, struct >> file *file, struct ifreq *ifr) >> tun->txflt.count = 0; >> tun->vnet_hdr_sz = sizeof(struct virtio_net_hdr); >> >> + sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); >> tun->filter_attached = false; >> tun->sndbuf = tfile->socket.sk->sk_sndbuf; >> >> @@ -2159,8 +2160,6 @@ static int tun_chr_open(struct inode *inode, >> struct file * file) >> set_bit(SOCK_EXTERNALLY_ALLOCATED, &tfile->socket.flags); >> INIT_LIST_HEAD(&tfile->next); >> >> - sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); >> - >> return 0; >> } > I guess no, as this also leads to a kernel panic (tested against v3.10). Yes, commit "tuntap: set SOCK_ZEROCOPY flag during open" just re-enable the zerocopy capability of tuntap. I believe it just uncover other zerocopy bugs. Which regression did you see? Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Jason, On Wed, Jul 03, 2013 at 10:44:32AM +0800, Jason Wang wrote: > On 07/03/2013 06:06 AM, Thomas Zeitlhofer wrote: > > On Tue, Jul 02, 2013 at 06:01:12PM -0300, Fabio Estevam wrote: > >> On Tue, Jul 2, 2013 at 4:59 PM, Thomas Zeitlhofer > >> <thomas.zeitlhofer@nt.tuwien.ac.at> wrote: > >>> Commit "tuntap: set SOCK_ZEROCOPY flag during open" introduces a > >>> regression which is observed with live migration of qemu/kvm based > >>> virtual machines that are connected to an openvswitch bridge. > >>> > >>> Reverting this commit (b26c93c46a3dec25ed236d4ba6107eb4ed5d9401 in > >>> v3.9.8 and accordingly 19a6afb23e5d323e1245baa4e62755492b2f1200 in > >>> v3.10) fixes the following problem: > >> Should the sock_set_flag stay in tun_set_iff as it was prior to 54f968d6efd? > >> > >> --- a/drivers/net/tun.c > >> +++ b/drivers/net/tun.c > >> @@ -1652,6 +1652,7 @@ static int tun_set_iff(struct net *net, struct > >> file *file, struct ifreq *ifr) > >> tun->txflt.count = 0; > >> tun->vnet_hdr_sz = sizeof(struct virtio_net_hdr); > >> > >> + sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); > >> tun->filter_attached = false; > >> tun->sndbuf = tfile->socket.sk->sk_sndbuf; > >> > >> @@ -2159,8 +2160,6 @@ static int tun_chr_open(struct inode *inode, > >> struct file * file) > >> set_bit(SOCK_EXTERNALLY_ALLOCATED, &tfile->socket.flags); > >> INIT_LIST_HEAD(&tfile->next); > >> > >> - sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); > >> - > >> return 0; > >> } > > I guess no, as this also leads to a kernel panic (tested against v3.10). > > Yes, commit "tuntap: set SOCK_ZEROCOPY flag during open" just re-enable > the zerocopy capability of tuntap. I believe it just uncover other > zerocopy bugs. > > Which regression did you see? a kernel panic on the host machine. The details are in the first message of this thread: http://lkml.org/lkml/2013/7/2/499 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/03/2013 02:11 PM, Thomas Zeitlhofer wrote: > Hello Jason, > > On Wed, Jul 03, 2013 at 10:44:32AM +0800, Jason Wang wrote: >> On 07/03/2013 06:06 AM, Thomas Zeitlhofer wrote: >>> On Tue, Jul 02, 2013 at 06:01:12PM -0300, Fabio Estevam wrote: >>>> On Tue, Jul 2, 2013 at 4:59 PM, Thomas Zeitlhofer >>>> <thomas.zeitlhofer@nt.tuwien.ac.at> wrote: >>>>> Commit "tuntap: set SOCK_ZEROCOPY flag during open" introduces a >>>>> regression which is observed with live migration of qemu/kvm based >>>>> virtual machines that are connected to an openvswitch bridge. >>>>> >>>>> Reverting this commit (b26c93c46a3dec25ed236d4ba6107eb4ed5d9401 in >>>>> v3.9.8 and accordingly 19a6afb23e5d323e1245baa4e62755492b2f1200 in >>>>> v3.10) fixes the following problem: >>>> Should the sock_set_flag stay in tun_set_iff as it was prior to 54f968d6efd? >>>> >>>> --- a/drivers/net/tun.c >>>> +++ b/drivers/net/tun.c >>>> @@ -1652,6 +1652,7 @@ static int tun_set_iff(struct net *net, struct >>>> file *file, struct ifreq *ifr) >>>> tun->txflt.count = 0; >>>> tun->vnet_hdr_sz = sizeof(struct virtio_net_hdr); >>>> >>>> + sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); >>>> tun->filter_attached = false; >>>> tun->sndbuf = tfile->socket.sk->sk_sndbuf; >>>> >>>> @@ -2159,8 +2160,6 @@ static int tun_chr_open(struct inode *inode, >>>> struct file * file) >>>> set_bit(SOCK_EXTERNALLY_ALLOCATED, &tfile->socket.flags); >>>> INIT_LIST_HEAD(&tfile->next); >>>> >>>> - sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); >>>> - >>>> return 0; >>>> } >>> I guess no, as this also leads to a kernel panic (tested against v3.10). >> Yes, commit "tuntap: set SOCK_ZEROCOPY flag during open" just re-enable >> the zerocopy capability of tuntap. I believe it just uncover other >> zerocopy bugs. >> >> Which regression did you see? > a kernel panic on the host machine. The details are in the first message > of this thread: http://lkml.org/lkml/2013/7/2/499 Could you please have a try with the patch in https://lkml.org/lkml/2013/6/25/304. Michael, please resubmit a patch which can applies cleanly. Thanks > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 05, 2013 at 11:43:30AM +0800, Jason Wang wrote: > On 07/03/2013 02:11 PM, Thomas Zeitlhofer wrote: > > Hello Jason, > > > > On Wed, Jul 03, 2013 at 10:44:32AM +0800, Jason Wang wrote: [...] > >> Which regression did you see? > > a kernel panic on the host machine. The details are in the first message > > of this thread: http://lkml.org/lkml/2013/7/2/499 > > Could you please have a try with the patch in > https://lkml.org/lkml/2013/6/25/304. I can confirm that this patch fixes the problem. Tried it against v3.10 and there are no issues any more when live migrating virtual machines off the host machine that runs v3.10 + this patch. Thanks, Thomas -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1652,6 +1652,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) tun->txflt.count = 0; tun->vnet_hdr_sz = sizeof(struct virtio_net_hdr); + sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); tun->filter_attached = false; tun->sndbuf = tfile->socket.sk->sk_sndbuf;